| Message ID | 20230929113149.587436-3-teackot@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86: msi-ec: add and fix EC configs | expand |
On Fri, 29 Sep 2023, Nikita Kravets wrote: > Fix the charge control address of CONF3 and remove an incorrect firmware > version which turned out to be a BIOS firmware and not an EC firmware. Should there be a Fixes tag? > This patch also renames fn_super_swap to fn_win_swap for consistency > with the downstream version of the driver. Please don't mix changes like this. Hans, what do you think about the rename in this patch? (To me "super" sounds the normal terminology in Linux world so it feels a step backwards.)
Hi Nikita, Ilpo, Nikita, great to see that you are back to contributing to the driver upstream. On 9/29/23 18:07, Ilpo Järvinen wrote: > > On Fri, 29 Sep 2023, Nikita Kravets wrote: > >> Fix the charge control address of CONF3 and remove an incorrect firmware >> version which turned out to be a BIOS firmware and not an EC firmware. > > Should there be a Fixes tag? > >> This patch also renames fn_super_swap to fn_win_swap for consistency >> with the downstream version of the driver. > > Please don't mix changes like this. > > Hans, what do you think about the rename in this patch? I agree with you that the config fixes and the rename should be 2 separate patches. > (To me "super" > sounds the normal terminology in Linux world so it feels a step > backwards.) win/super is used interchangeable in many places and only old Unix/X11 folks really know/expect the super name. So I'm fine with the rename. Also as Nikita mentions this keeps the code in sync with the out of tree driver, which currently has many more features then the mainline driver. The plan is to upstream those features one by one (or a few at a time). Given the situation with an out of tree driver + a mainline one for now keeping the per laptop model config in sync (which is the trickiest part to test) is a good idea IMHO. So ack from me for doing the rename. Regards, Hans
Hi! Thank you for the comments, I will send a modified patch series soon! Regards, Nikita
diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c index f26a3121092f..3074aee878c1 100644 --- a/drivers/platform/x86/msi-ec.c +++ b/drivers/platform/x86/msi-ec.c @@ -58,7 +58,7 @@ static struct msi_ec_conf CONF0 __initdata = { .block_address = 0x2f, .bit = 1, }, - .fn_super_swap = { + .fn_win_swap = { .address = 0xbf, .bit = 4, }, @@ -138,7 +138,7 @@ static struct msi_ec_conf CONF1 __initdata = { .block_address = 0x2f, .bit = 1, }, - .fn_super_swap = { + .fn_win_swap = { .address = 0xbf, .bit = 4, }, @@ -215,7 +215,7 @@ static struct msi_ec_conf CONF2 __initdata = { .block_address = 0x2f, .bit = 1, }, - .fn_super_swap = { + .fn_win_swap = { .address = 0xe8, .bit = 4, }, @@ -276,14 +276,13 @@ static struct msi_ec_conf CONF2 __initdata = { static const char * const ALLOWED_FW_3[] __initconst = { "1592EMS1.111", - "E1592IMS.10C", NULL }; static struct msi_ec_conf CONF3 __initdata = { .allowed_fw = ALLOWED_FW_3, .charge_control = { - .address = 0xef, + .address = 0xd7, .offset_start = 0x8a, .offset_end = 0x80, .range_min = 0x8a, @@ -294,7 +293,7 @@ static struct msi_ec_conf CONF3 __initdata = { .block_address = 0x2f, .bit = 1, }, - .fn_super_swap = { + .fn_win_swap = { .address = 0xe8, .bit = 4, }, @@ -372,7 +371,7 @@ static struct msi_ec_conf CONF4 __initdata = { .block_address = 0x2f, .bit = 1, }, - .fn_super_swap = { + .fn_win_swap = { .address = MSI_EC_ADDR_UNKNOWN, // supported, but unknown .bit = 4, }, @@ -451,7 +450,7 @@ static struct msi_ec_conf CONF5 __initdata = { .block_address = 0x2f, .bit = 1, }, - .fn_super_swap = { // todo: reverse + .fn_win_swap = { // todo: reverse .address = 0xbf, .bit = 4, }, @@ -529,7 +528,7 @@ static struct msi_ec_conf CONF6 __initdata = { .block_address = MSI_EC_ADDR_UNSUPP, .bit = 1, }, - .fn_super_swap = { + .fn_win_swap = { .address = 0xbf, // todo: reverse .bit = 4, }, @@ -609,7 +608,7 @@ static struct msi_ec_conf CONF7 __initdata = { .block_address = MSI_EC_ADDR_UNSUPP, .bit = 1, }, - .fn_super_swap = { + .fn_win_swap = { .address = 0xbf, // needs testing .bit = 4, }, diff --git a/drivers/platform/x86/msi-ec.h b/drivers/platform/x86/msi-ec.h index be3533dc9cc6..086351217505 100644 --- a/drivers/platform/x86/msi-ec.h +++ b/drivers/platform/x86/msi-ec.h @@ -40,7 +40,7 @@ struct msi_ec_webcam_conf { int bit; }; -struct msi_ec_fn_super_swap_conf { +struct msi_ec_fn_win_swap_conf { int address; int bit; }; @@ -108,7 +108,7 @@ struct msi_ec_conf { struct msi_ec_charge_control_conf charge_control; struct msi_ec_webcam_conf webcam; - struct msi_ec_fn_super_swap_conf fn_super_swap; + struct msi_ec_fn_win_swap_conf fn_win_swap; struct msi_ec_cooler_boost_conf cooler_boost; struct msi_ec_shift_mode_conf shift_mode; struct msi_ec_super_battery_conf super_battery;
Fix the charge control address of CONF3 and remove an incorrect firmware version which turned out to be a BIOS firmware and not an EC firmware. This patch also renames fn_super_swap to fn_win_swap for consistency with the downstream version of the driver. Cc: Aakash Singh <mail@singhaakash.dev> Cc: Jose Angel Pastrana <japp0005@red.ujaen.es> Signed-off-by: Nikita Kravets <teackot@gmail.com> --- drivers/platform/x86/msi-ec.c | 19 +++++++++---------- drivers/platform/x86/msi-ec.h | 4 ++-- 2 files changed, 11 insertions(+), 12 deletions(-)