Re: [PATCH v3 1/5] platform: Add Surface platform directory

[Hans de Goede]

Hi,

On 10/8/20 5:18 PM, Maximilian Luz wrote:
> On 10/8/20 4:52 PM, Andy Shevchenko wrote:
> > On Thu, Oct 8, 2020 at 5:35 PM Maximilian Luz <luzmaximilian@xxxxxxxxx> wrote:
> > > It may make sense to split the Microsoft Surface hardware platform
> > > drivers out to a separate subdirectory, since some of it may be shared
> > > between ARM and x86 in the future (regarding devices like the Surface
> > > Pro X).
> > >
> > > Further, newer Surface devices will require additional platform drivers
> > > for fundamental support (mostly regarding their embedded controller),
> > > which may also warrant this split from a size perspective.
> > >
> > > This commit introduces a new platform/surface subdirectory for the
> > > Surface device family, with subsequent commits moving existing Surface
> > > drivers over from platform/x86.
> > >
> > > A new MAINTAINERS entry is added for this directory. Patches to files in
> > > this directory will be taken up by the platform-drivers-x86 team (i.e.
> > > Hans de Goede and Mark Gross) after they have been reviewed by
> > > Maximilian Luz.
> >
> > Thanks for the patch, my minor comments below.
> >
> > ...
> >
> > > +MICROSOFT SURFACE PLATFORM DRIVERS
> >
> > (1)
> >
> > > +M:     Hans de Goede <hdegoede@xxxxxxxxxx>
> > > +M:     Mark Gross <mgross@xxxxxxxxxxxxxxx>
> > > +M:     Maximilian Luz <luzmaximilian@xxxxxxxxx>
> > > +L:     platform-driver-x86@xxxxxxxxxxxxxxx
> > > +S:     Maintained
> >
> > > +T:     git git://git.infradead.org/linux-platform-drivers-x86.git
> >
> > It's now on kernel.org.
> > git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git
>
> Thank you, will update this. FYI: The entry was mostly copied from the
> X86 PLATFORM DRIVERS entry, so it should probably be updated there, too.

That is a good point I've pushed a commit fixing this to the for-next
branch of: git://git.infradead.org/linux-platform-drivers-x86.git

Please base the next version of this patch-set on top of that.

Regards,

Hans



> > > +F:     drivers/platform/surface/
> >
> > > @@ -9,3 +9,4 @@ obj-$(CONFIG_MIPS)              += mips/
> > >   obj-$(CONFIG_OLPC_EC)          += olpc/
> > >   obj-$(CONFIG_GOLDFISH)         += goldfish/
> > >   obj-$(CONFIG_CHROME_PLATFORMS) += chrome/
> > > +obj-$(CONFIG_SURFACE_PLATFORM) += surface/
> >
> > (2)
> >
> > > +menuconfig SURFACE_PLATFORM
> >
> > (3a)
> >
> > > +if SURFACE_PLATFORM
> >
> > (3b)
> >
> > > +endif # SURFACE_PLATFORM
> >
> > (3c)
> >
> > I think in (1), (2) and (3) it makes sense to mimic Chrome, i.e. use
> > plural: PLATFORMS.
>
> I agree with (2) and (3), but I'm not so sure about (1). For Chrome, the
> entry is CHROME HARDWARE PLATFORM SUPPORT, so should I change it to
> MICROSOFT SURFACE HARDWARE PLATFORM SUPPORT?
>
> Thanks,
> Max