Messages in this thread | | | Subject | Re: [RFC PATCH 0/9] Add support for Microsoft Surface System Aggregator Module | From | Maximilian Luz <> | Date | Fri, 25 Sep 2020 17:41:42 +0200 |
| |
On 9/25/20 4:58 PM, Andy Shevchenko wrote: > On Thu, Sep 24, 2020 at 10:17 PM Maximilian Luz <luzmaximilian@gmail.com> wrote: >> On 9/24/20 10:30 AM, Andy Shevchenko wrote: >>> On Wed, Sep 23, 2020 at 6:32 PM Arnd Bergmann <arnd@arndb.de> wrote: >>>> On Wed, Sep 23, 2020 at 5:15 PM Maximilian Luz <luzmaximilian@gmail.com> wrote: > > ... > >>>> I think this should go to drivers/platform/x86 or drivers/platform/surface/ >>>> along with other laptop vendor specific code rather than drivers/misc/. >>> >>> +1 here. drivers/platform/surface is a good place to start. >>> And you may begin with moving a few Surface drivers out of PDx86 to >>> the new folder. >> >> Perfect, thanks! I'll draft up a patch series over the weekend. >> >> A couple questions regarding structure and maintenance: >> >> - Should I CC the platform-driver-x86 list on future submissions to >> drivers/platform/surface? I.e. is this something you would want to >> review if it doesn't touch the drivers/platform/x86 directory? > > Include PDx86 mailing list to the list of that. Current SURFACE* > drivers have per driver record in MAINTAINERS IIRC. So, update them as > well if needed.
Will do.
>> - How would you want the layout to be, specifically regarding to the >> surface-aggregator stuff? My suggestion would be simply: >> >> drivers/platform/surface/ > >> surface_aggregator/ > > Don't repeat parts of the path, the aggregator is enough as a folder > name, but the driver of course should be in its own namespace > ('surface').
Okay.
>> Kconfig >> Makefile >> core.c >> controller.c >> ... (all core stuff built into the surface_aggregator module) >> Kconfig >> Makefile > >> surface_aggregator_debugfs.c > > (Not sure why it's not a part of aggregator folder)
I kind of thought of the aggregator folder to contain only files that build the core module. surface_aggregator_debugfs is intended as separate module, to be loaded when needed. So I'd consider it a client driver to the aggregator in the same way that surface_acpi_notify is.
Let me know if you still want me to move this into the aggregator folder though. Personally, I just feel that that might lead to a bit of confusion, specifically the idea that it's built into the core when it's not.
>> surface_acpi_notify.c >> surface_*.c (any other surface platform driver as well >> as drivers dependent on surface_aggregator) >> >> - Regarding future things like HID transport driver, battery/AC driver: >> Submit them to drivers/platform/surface or to their respective >> subsystem directories? > > Respective subsystem _if_ it is a subsystem related driver and not > kinda board file. Use common sense and existing examples.
Right, thank you!
Regards, Max
| |