Re: [platforms: PATCH v2 5/5] Marvell/Drivers: Drop 'PciEmulation' naming

[Marcin Wojtas <mw@semihalf.com>]

Hi Leif,

2017-12-08 15:08 GMT+01:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Dec 08, 2017 at 01:11:27PM +0100, Marcin Wojtas wrote:
>> 2017-12-08 9:40 GMT+01:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> > On 7 December 2017 at 23:08, Marcin Wojtas <mw@semihalf.com> wrote:
>> >> Hi Ard,
>> >>
>> >> 2017-12-07 20:49 GMT+01:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> >>> Hi Marcin,
>> >>>
>> >>> On 7 December 2017 at 19:20, Marcin Wojtas <mw@semihalf.com> wrote:
>> >>>> 'PciEmulation' is not a proper naming for a driver, which
>> >>>> uses NonDiscoverableDeviceRegistrationLib for handling
>> >>>> the devices attached via internal bus of the SoC.
>> >>>> Rename it to MvNonDiscoverableDxe and move under
>> >>>> Silicon/Marvell/Drivers directory.
>> >>>>
>> >>>
>> >>> Thank you!
>> >>>
>> >>>> Contributed-under: TianoCore Contribution Agreement 1.1
>> >>>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> >>>> ---
>> >>>>  Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf                                                             |  4 +--
>> >>>>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc                                                              |  4 +--
>> >>>>  Silicon/Marvell/{PciEmulation/PciEmulation.c => Drivers/NonDiscoverableDevices/MvNonDiscoverableDxe.c}     | 26 ++++++++++----------
>> >>>>  Silicon/Marvell/{PciEmulation/PciEmulation.inf => Drivers/NonDiscoverableDevices/MvNonDiscoverableDxe.inf} |  8 +++---
>> >>>>  4 files changed, 21 insertions(+), 21 deletions(-)
>> >>>>  rename Silicon/Marvell/{PciEmulation/PciEmulation.c => Drivers/NonDiscoverableDevices/MvNonDiscoverableDxe.c} (84%)
>> >>>>  rename Silicon/Marvell/{PciEmulation/PciEmulation.inf => Drivers/NonDiscoverableDevices/MvNonDiscoverableDxe.inf} (91%)
>> >>>>
>> >>>
>> >>> Could we please have
>> >>>
>> >>> Drivers/MvNonDiscoverableDxe/MvNonDiscoverableDxe.inf
>> >>>
>> >>> instead?
>> >>>
>> >>
>> >> My intention was to keep a generic naming at the first level inside
>> >> the Silicon/Drivers/Marvell:
>> >> I2c  Net SdMmc  Spi
>> >>
>> >> I can change it in a way you want, but to my taste
>> >> MvNonDiscoverableDxe doesn't fully fit the company above. Do you see
>> >> any possible compromise? :)
>> >>
>> >
>> > Well, if you want 'generic' at the level below
>> > Silicon/Drivers/Marvell, you will still need
>> > MvNonDiscoverableDxe/MvNonDiscoverableDxe.inf below that.
>> >
>> > So
>> >
>> > Drivers/NonDiscoverable/MvNonDiscoverableDxe/MvNonDiscoverableDxe.inf
>> >
>> > perhaps?
>>
>> This is ok to me.
>
> As we're bikeshedding...
>
> What would be the benefit of the above?
>
> I could get Drivers/NonDiscoverable/780xxNonDiscoverableDxe, but
> surely if you're preparing for Armada 37xx (which I would be very
> happy to see), having something called MvNonDiscoverableDxe is not
> going to be very identifying.

Well, I think we will most likely reuse this driver as is - on A37xx there is
generic AHCI/XHCI and even same Xenon driver. So, are you fine with
what I initially added in this patch
(Drivers/NonDiscoverableDevices/MvNonDiscoverableDxe.c, etc.),
or you wish to use Drivers/MvNonDiscoverableDxe/MvNonDiscoverableDxe.c, etc?

Best regards,
Marcin

>
> I don't mind a few extra levels of directories in order to clear up
> usage, but we do need to actually make use of it then.
>