On Wed, 27 Mar 2024, Carlos Ferreira wrote: > On 3/27/24 1:05 PM, Ilpo Järvinen wrote: > > > On Tue, 26 Mar 2024, Carlos Ferreira wrote: > > > >> Hi, i have changed some of the code. How does it look now? > >> > >> Signed-off-by: Carlos Ferreira <carlosmiguelferreira.2003@xxxxxxxxx> > >> --- > > > > First of all, you need to make a proper submission with versioning, that > > is: > > > > - Put version into the subject: PATCH v2 > > - Don't put extra stuff into changelog like the above question, if you > > need to ask something, put your question underneath the first --- line. > > - List the changes you made underneath the first --- line (see ML > > archives for examples about formatting) > > Should i submit the v2 patch with the current state or with the changes > suggested such as the use of the leds API? Please send v3 once you've made the changes, there's no need to send patches which we know to become obsolete. > >> drivers/platform/x86/hp/hp-wmi.c | 251 +++++++++++++++++++++++++++++-- > >> 1 file changed, 241 insertions(+), 10 deletions(-) > >> > >> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c > >> index e53660422..8108ca7e9 100644 > >> --- a/drivers/platform/x86/hp/hp-wmi.c > >> +++ b/drivers/platform/x86/hp/hp-wmi.c > >> @@ -27,6 +27,7 @@ > >> #include <linux/rfkill.h> > >> #include <linux/string.h> > >> #include <linux/dmi.h> > >> +#include <linux/bitfield.h> > > > > Try to put it earlier, these should eventually be in alphabetic order > > (again, ordered by a separate patch, not this one). > > You mean organizing all the imports like this? > #include <linux/acpi.h> > #include <linux/bitfield.h> > #include <linux/dmi.h> > #include <linux/hwmon.h> > #include <linux/init.h> > #include <linux/input.h> > #include <linux/input/sparse-keymap.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/platform_profile.h> > #include <linux/rfkill.h> > #include <linux/slab.h> > #include <linux/string.h> > #include <linux/types.h> Yes. If adding to a non-sorted list, the include should be placed before any entries that are alphabetically after it but providing another patch to sort them is of course even better than that. -- i.