Message ID | 20221020201033.12790-1-jorge.lopez2@hp.com (mailing list archive) |
---|---|
Headers | show |
Series | Introduction of HP-BIOSCFG driver | expand |
Hi Jorge, On 10/20/22 22:10, Jorge Lopez wrote: > Version 4 restructures the patches submitted in previous versions. > Earlier hp-bioscfg patches were squashed together before creating > the new split. > > Version 4.0 breaks down the changes as follows: > > 1. Moving existing HP drivers to a central location I have merged this patch, so you can drop this for version 5 of the patchset. > The driver files were broken down in 5 patches of 3 files each > with exception of patch 6/6 > > 2. Introduction of HP-BIOSCFG driver - Set 1 I've done a detailed review of this single patches. This has found quite a few things to improve. Note that many of the remarks and especially the remarks about enum-attributes.c also apply to the other files (to the other *-attributes.c files). Please prepare a version 5 taking all remarks into account for all files of the driver and then I will continue the review from there. One thing which I did already notice for the last patch in the series, please drop the "depends on DMI" from the Kconfig bits and drop "include/dmi.h", you are not using any DMI functions so these are not necessary. And please add a: L: platform-driver-x86@vger.kernel.org line to the MAINTAINERS entry. Regards, Hans > 3. HP BIOSCFG driver - set 2 > 4. HP BIOSCFG driver - set 3 > 5. HP BIOSCFG driver - set 4 > 6. HP BIOSCFG driver - remaining components > > -- > > > Jorge Lopez (6): > Moving existing HP drivers to a central location > Introduction of HP-BIOSCFG driver > HP BIOSCFG driver - set 2 > HP BIOSCFG driver - set 3 > HP BIOSCFG driver - set 4 > HP BIOSCFG driver - remaining components > > .../testing/sysfs-class-firmware-attributes | 181 ++- > MAINTAINERS | 15 +- > drivers/platform/x86/Kconfig | 80 +- > drivers/platform/x86/Makefile | 4 +- > drivers/platform/x86/hp/Kconfig | 81 ++ > drivers/platform/x86/hp/Makefile | 11 + > drivers/platform/x86/hp/hp-bioscfg/Makefile | 19 + > .../x86/hp/hp-bioscfg/biosattr-interface.c | 285 +++++ > drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 1064 +++++++++++++++++ > drivers/platform/x86/hp/hp-bioscfg/bioscfg.h | 671 +++++++++++ > .../x86/hp/hp-bioscfg/enum-attributes.c | 521 ++++++++ > .../x86/hp/hp-bioscfg/int-attributes.c | 478 ++++++++ > .../x86/hp/hp-bioscfg/ordered-attributes.c | 586 +++++++++ > .../x86/hp/hp-bioscfg/passwdattr-interface.c | 50 + > .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 647 ++++++++++ > .../x86/hp/hp-bioscfg/spmobj-attributes.c | 408 +++++++ > .../x86/hp/hp-bioscfg/string-attributes.c | 457 +++++++ > .../x86/hp/hp-bioscfg/sureadmin-attributes.c | 1014 ++++++++++++++++ > .../x86/hp/hp-bioscfg/surestart-attributes.c | 145 +++ > drivers/platform/x86/{ => hp}/hp-wmi.c | 0 > drivers/platform/x86/{ => hp}/hp_accel.c | 0 > drivers/platform/x86/{ => hp}/tc1100-wmi.c | 0 > 22 files changed, 6647 insertions(+), 70 deletions(-) > create mode 100644 drivers/platform/x86/hp/Kconfig > create mode 100644 drivers/platform/x86/hp/Makefile > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/Makefile > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/biosattr-interface.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/bioscfg.h > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/int-attributes.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/ordered-attributes.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdattr-interface.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/string-attributes.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/sureadmin-attributes.c > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c > rename drivers/platform/x86/{ => hp}/hp-wmi.c (100%) > rename drivers/platform/x86/{ => hp}/hp_accel.c (100%) > rename drivers/platform/x86/{ => hp}/tc1100-wmi.c (100%) > > -- > 2.34.1 >
Ok, I will do that. On Tue, Nov 8, 2022 at 8:59 AM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi Jorge, > > On 10/20/22 22:10, Jorge Lopez wrote: > > Version 4 restructures the patches submitted in previous versions. > > Earlier hp-bioscfg patches were squashed together before creating > > the new split. > > > > Version 4.0 breaks down the changes as follows: > > > > 1. Moving existing HP drivers to a central location > > I have merged this patch, so you can drop this for version 5 > of the patchset. > > > The driver files were broken down in 5 patches of 3 files each > > with exception of patch 6/6 > > > > 2. Introduction of HP-BIOSCFG driver - Set 1 > > I've done a detailed review of this single patches. This has > found quite a few things to improve. Note that many of the > remarks and especially the remarks about enum-attributes.c > also apply to the other files (to the other *-attributes.c > files). > > Please prepare a version 5 taking all remarks into account > for all files of the driver and then I will continue the > review from there. > > One thing which I did already notice for the last patch > in the series, please drop the "depends on DMI" from the > Kconfig bits and drop "include/dmi.h", you are not using > any DMI functions so these are not necessary. > > And please add a: > > L: platform-driver-x86@vger.kernel.org > > line to the MAINTAINERS entry. > > Regards, > > Hans > > > > > > > 3. HP BIOSCFG driver - set 2 > > 4. HP BIOSCFG driver - set 3 > > 5. HP BIOSCFG driver - set 4 > > 6. HP BIOSCFG driver - remaining components > > > > -- > > > > > > Jorge Lopez (6): > > Moving existing HP drivers to a central location > > Introduction of HP-BIOSCFG driver > > HP BIOSCFG driver - set 2 > > HP BIOSCFG driver - set 3 > > HP BIOSCFG driver - set 4 > > HP BIOSCFG driver - remaining components > > > > .../testing/sysfs-class-firmware-attributes | 181 ++- > > MAINTAINERS | 15 +- > > drivers/platform/x86/Kconfig | 80 +- > > drivers/platform/x86/Makefile | 4 +- > > drivers/platform/x86/hp/Kconfig | 81 ++ > > drivers/platform/x86/hp/Makefile | 11 + > > drivers/platform/x86/hp/hp-bioscfg/Makefile | 19 + > > .../x86/hp/hp-bioscfg/biosattr-interface.c | 285 +++++ > > drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 1064 +++++++++++++++++ > > drivers/platform/x86/hp/hp-bioscfg/bioscfg.h | 671 +++++++++++ > > .../x86/hp/hp-bioscfg/enum-attributes.c | 521 ++++++++ > > .../x86/hp/hp-bioscfg/int-attributes.c | 478 ++++++++ > > .../x86/hp/hp-bioscfg/ordered-attributes.c | 586 +++++++++ > > .../x86/hp/hp-bioscfg/passwdattr-interface.c | 50 + > > .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 647 ++++++++++ > > .../x86/hp/hp-bioscfg/spmobj-attributes.c | 408 +++++++ > > .../x86/hp/hp-bioscfg/string-attributes.c | 457 +++++++ > > .../x86/hp/hp-bioscfg/sureadmin-attributes.c | 1014 ++++++++++++++++ > > .../x86/hp/hp-bioscfg/surestart-attributes.c | 145 +++ > > drivers/platform/x86/{ => hp}/hp-wmi.c | 0 > > drivers/platform/x86/{ => hp}/hp_accel.c | 0 > > drivers/platform/x86/{ => hp}/tc1100-wmi.c | 0 > > 22 files changed, 6647 insertions(+), 70 deletions(-) > > create mode 100644 drivers/platform/x86/hp/Kconfig > > create mode 100644 drivers/platform/x86/hp/Makefile > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/Makefile > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/biosattr-interface.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/bioscfg.h > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/int-attributes.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/ordered-attributes.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdattr-interface.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/string-attributes.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/sureadmin-attributes.c > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c > > rename drivers/platform/x86/{ => hp}/hp-wmi.c (100%) > > rename drivers/platform/x86/{ => hp}/hp_accel.c (100%) > > rename drivers/platform/x86/{ => hp}/tc1100-wmi.c (100%) > > > > -- > > 2.34.1 > > >