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

[Leif Lindholm <leif.lindholm@linaro.org>]

On Fri, Dec 08, 2017 at 03:13:54PM +0100, Marcin Wojtas wrote:
> 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?

I'm more on the Drivers/MvNonDiscoverableDxe/MvNonDiscoverableDxe.c
side of the fence in that case. NonDiscoverableDevices suggests there
will be multiple.

(That said, there is no need for the Mv prefix if it's under a Marvell
directory.)

Regards,

Leif