On Tue, Aug 10, 2021 at 10:05 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi Kate, > > On 8/10/21 11:58 AM, Kate Hsuan wrote: > > All the intel platform specific drivers are moved to intel/. > > It makes more clear file structure to improve the readability. > > > > Kate Hsuan (20): > > Move Intel hid of pdx86 to intel directory to improve readability. > > Move Intel WMI driver of pdx86 to intel/ directory to improve > > readability. > > Move Intel bxtwc driver of pdx86 to intel/ directory to improve > > readability. > > Move Intel chtdc_ti driver of pdx86 to improve readability. > > Move MRFLD power button driver of pdx86 to intel directory to improve > > readability. > > Move Intel PMC core of pdx86 to intel/ directory to improve > > readability. > > Move Intel PMT driver of pdx86 to intel/ to improve readability. > > Move Intel P-Unit of pdx86 to intel/ directory to improve readability. > > Move Intel SCU IPC of pdx86 to intel directory to increase > > readability. > > Move Intel SoC telemetry driver to intel directory to improve > > readability. > > Move Intel IPS driver of pdx86 to improve readability. > > Move Intel RST driver of pdx86 to intel directory to improve > > readability. > > Move Intel smartconnect driver of pdx86 to intel/ directory to improve > > readability. > > Move Intel SST driver to intel/ directory to improve readability. > > Move Intel turbo max 3 driver to intel/ directory to improve > > readability. > > Move Intel uncore freq driver to intel/ directory to improve > > readability. > > Move Intel int0002 vgpio driver to intel/ directory to inprove > > readability. > > Move Intel thermal driver for menlow platform driver to intel/ > > directory to improve readability. > > Move OakTrail driver to the intel/ directory to improve readability. > > Move Intel virtual botton driver to intel/ directory to improve > > readability. > > Thank you for doing this. I have a couple of remarks which I would > like to see addressed for version 2 of this series: > > 1. The commit messages are currently all one line, so basically > only a Subject and they are missing a Body describing the change > in more detail (as already pointed out by Mika). > > > 2. Kernel patch subjects (the first line of the commit message) > should always be prefixed with the subsystem + component which they > are for, so instead of e.g. > > "Move Intel hid of pdx86 to intel directory to improve readability." > > you would use: > > "platform/x86: intel-hid: Move to intel sub-directory to improve readability." > > But that is a bit long; and normally the Subject line is just > a summary while the body explains things like the why, so this should > probably be shorted to: > > "platform/x86: intel-hid: Move to intel sub-directory" > > For the Subject, with the Body explaining what exactly is being changed > and why. > > > 3. You are using new sub-directories for all drivers which you > are moving, but for drivers which are currently just 1 c-file, this > means going from 1 c-file to 3 files (c-file + Kconfig + Makefile) > this seems a bit too much. I believe that it would be better for > the single file drivers (e.g. intel-hid.c, intel-vbtn.c) to be moved > directly under drivers/platform/x86/intel and for there Kconfig > and Makefile bits to be moved to the already existing Kconfig > and Makefile files there. > > > 4. As Andy already mentioned when moving a file like > "intel_scu_ipc.c" to drivers/platform/x86/intel/scu then the > whole path becomes: > > drivers/platform/x86/intel/scu/intel_scu_ipc.c > > Which is quite long / quite a lot to type and repeats > intel + scu twice, so it would be better to drop the intel_scu > prefix from the filenames in this case resulting in: > > drivers/platform/x86/intel/scu/ipc.c > > in this example's case. This requires some extrea work: > > 4.1 You will need to adjust private includes to the new > filenames > > 4.2 If you simply adjust say this Makefile line: > > obj-$(CONFIG_INTEL_SCU_PCI) += intel_scu_pcidrv.o > > to: > > obj-$(CONFIG_INTEL_SCU_PCI) += pcidrv.o > > a pcidrv.ko will get build, but that is a very generic name > and then we end up with module-name clashes which are not > allowed. > > So the Makefile needs to become a bit more complicated like this: > > intel_scu_pcidrv-y := pcidrv.o > obj-$(CONFIG_INTEL_SCU_PCI) += intel_scu_pcidrv.o > > See for example: > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/tree/drivers/platform/x86/intel/pmt/Makefile?h=for-next > > > 5. Some of the files which you are moving are mentioned in the > MAINTAINERS file. For each file which you are moving please check if > it is listed in the MAINTAINERS file, keeping wildcards in mind, > so search e.g. for intel_scu for the intel_scu_* move. > > And if the file is listed then update the MAINTAINERS entry to > point to the new location. > > Regards, > > Hans > Thanks Hans. I'll do that for my v2 patches.
