All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] platform/x86: msi-ec: add and fix EC configs
@ 2023-10-06 17:53 Nikita Kravets
  2023-10-06 17:53 ` [PATCH v2 1/3] platform/x86: msi-ec: Fix the 3rd config Nikita Kravets
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Nikita Kravets @ 2023-10-06 17:53 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: Hans de Goede, Ilpo Järvinen, Nikita Kravets, Aakash Singh,
	Jose Angel Pastrana

Hi!

This patch series adds EC configs for more firmware (patch 3),
fixes a couple of issues with an existing configuration (patch 1),
and renames a struct field for consistency with downstream (patch 2)
in the msi-ec driver.

Cc: Aakash Singh <mail@singhaakash.dev>
Cc: Jose Angel Pastrana <japp0005@red.ujaen.es>
---
Changes in v2:
- Add a Fixes tag
- Split the first patch into two


Nikita Kravets (3):
  platform/x86: msi-ec: Fix the 3rd config
  platform/x86: msi-ec: rename fn_super_swap
  platform/x86: msi-ec: Add more EC configs

 drivers/platform/x86/msi-ec.c | 486 +++++++++++++++++++++++++++++++++-
 drivers/platform/x86/msi-ec.h |   4 +-
 2 files changed, 478 insertions(+), 12 deletions(-)

-- 
2.42.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/3] platform/x86: msi-ec: Fix the 3rd config
  2023-10-06 17:53 [PATCH v2 0/3] platform/x86: msi-ec: add and fix EC configs Nikita Kravets
@ 2023-10-06 17:53 ` Nikita Kravets
  2023-10-09 11:27   ` Ilpo Järvinen
  2023-10-11  9:22   ` Hans de Goede
  2023-10-06 17:53 ` [PATCH v2 2/3] platform/x86: msi-ec: rename fn_super_swap Nikita Kravets
  2023-10-06 17:53 ` [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs Nikita Kravets
  2 siblings, 2 replies; 12+ messages in thread
From: Nikita Kravets @ 2023-10-06 17:53 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: Hans de Goede, Ilpo Järvinen, Nikita Kravets, Aakash Singh,
	Jose Angel Pastrana

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.

Fixes: 392cacf2aa10 ("platform/x86: Add new msi-ec 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 | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
index f26a3121092f..492eb383ee7a 100644
--- a/drivers/platform/x86/msi-ec.c
+++ b/drivers/platform/x86/msi-ec.c
@@ -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,
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/3] platform/x86: msi-ec: rename fn_super_swap
  2023-10-06 17:53 [PATCH v2 0/3] platform/x86: msi-ec: add and fix EC configs Nikita Kravets
  2023-10-06 17:53 ` [PATCH v2 1/3] platform/x86: msi-ec: Fix the 3rd config Nikita Kravets
@ 2023-10-06 17:53 ` Nikita Kravets
  2023-10-09 11:39   ` Ilpo Järvinen
  2023-10-06 17:53 ` [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs Nikita Kravets
  2 siblings, 1 reply; 12+ messages in thread
From: Nikita Kravets @ 2023-10-06 17:53 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: Hans de Goede, Ilpo Järvinen, Nikita Kravets, Aakash Singh,
	Jose Angel Pastrana

This patch renames fn_super_swap to fn_win_swap for consistency
with the downstream version of the driver. Renaming the field to
fn_super_swap in the downstream driver would require modifying several
branches that are yet to be merged into the main branch, so I decided
to do it here instead.

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 | 16 ++++++++--------
 drivers/platform/x86/msi-ec.h |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
index 492eb383ee7a..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,
 	},
@@ -293,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,
 	},
@@ -371,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,
 	},
@@ -450,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,
 	},
@@ -528,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,
 	},
@@ -608,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;
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs
  2023-10-06 17:53 [PATCH v2 0/3] platform/x86: msi-ec: add and fix EC configs Nikita Kravets
  2023-10-06 17:53 ` [PATCH v2 1/3] platform/x86: msi-ec: Fix the 3rd config Nikita Kravets
  2023-10-06 17:53 ` [PATCH v2 2/3] platform/x86: msi-ec: rename fn_super_swap Nikita Kravets
@ 2023-10-06 17:53 ` Nikita Kravets
  2023-10-09 11:40   ` Ilpo Järvinen
  2 siblings, 1 reply; 12+ messages in thread
From: Nikita Kravets @ 2023-10-06 17:53 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: Hans de Goede, Ilpo Järvinen, Nikita Kravets, Aakash Singh,
	Jose Angel Pastrana

This patch adds configurations for new EC firmware from 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 | 467 ++++++++++++++++++++++++++++++++++
 1 file changed, 467 insertions(+)

diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
index 3074aee878c1..f19504dbf164 100644
--- a/drivers/platform/x86/msi-ec.c
+++ b/drivers/platform/x86/msi-ec.c
@@ -667,6 +667,467 @@ static struct msi_ec_conf CONF7 __initdata = {
 	},
 };
 
+static const char * const ALLOWED_FW_8[] __initconst = {
+	"14F1EMS1.115",
+	NULL
+};
+
+static struct msi_ec_conf CONF8 __initdata = {
+	.allowed_fw = ALLOWED_FW_8,
+	.charge_control = {
+		.address      = 0xd7,
+		.offset_start = 0x8a,
+		.offset_end   = 0x80,
+		.range_min    = 0x8a,
+		.range_max    = 0xe4,
+	},
+	.webcam = {
+		.address       = 0x2e,
+		.block_address = MSI_EC_ADDR_UNSUPP,
+		.bit           = 1,
+	},
+	.fn_win_swap = {
+		.address = 0xe8,
+		.bit     = 4,
+	},
+	.cooler_boost = {
+		.address = 0x98,
+		.bit     = 7,
+	},
+	.shift_mode = {
+		.address = 0xd2,
+		.modes = {
+			{ SM_ECO_NAME,     0xc2 },
+			{ SM_COMFORT_NAME, 0xc1 },
+			{ SM_SPORT_NAME,   0xc0 },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.super_battery = {
+		.address = 0xeb,
+		.mask    = 0x0f,
+	},
+	.fan_mode = {
+		.address = 0xd4,
+		.modes = {
+			{ FM_AUTO_NAME,     0x0d },
+			{ FM_SILENT_NAME,   0x1d },
+			{ FM_BASIC_NAME,    0x4d },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.cpu = {
+		.rt_temp_address       = 0x68,
+		.rt_fan_speed_address  = 0x71,
+		.rt_fan_speed_base_min = 0x19,
+		.rt_fan_speed_base_max = 0x37,
+		.bs_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
+		.bs_fan_speed_base_min = 0x00,
+		.bs_fan_speed_base_max = 0x0f,
+	},
+	.gpu = {
+		.rt_temp_address      = MSI_EC_ADDR_UNKNOWN,
+		.rt_fan_speed_address = MSI_EC_ADDR_UNKNOWN,
+	},
+	.leds = {
+		.micmute_led_address = MSI_EC_ADDR_UNSUPP,
+		.mute_led_address    = 0x2d,
+		.bit                 = 1,
+	},
+	.kbd_bl = {
+		.bl_mode_address  = MSI_EC_ADDR_UNKNOWN, // ?
+		.bl_modes         = { 0x00, 0x08 }, // ?
+		.max_mode         = 1, // ?
+		.bl_state_address = MSI_EC_ADDR_UNSUPP, // not functional
+		.state_base_value = 0x80,
+		.max_state        = 3,
+	},
+};
+
+static const char * const ALLOWED_FW_9[] __initconst = {
+	"14JKEMS1.104",
+	NULL
+};
+
+static struct msi_ec_conf CONF9 __initdata = {
+	.allowed_fw = ALLOWED_FW_9,
+	.charge_control = {
+		.address      = 0xef,
+		.offset_start = 0x8a,
+		.offset_end   = 0x80,
+		.range_min    = 0x8a,
+		.range_max    = 0xe4,
+	},
+	.webcam = {
+		.address       = 0x2e,
+		.block_address = 0x2f,
+		.bit           = 1,
+	},
+	.fn_win_swap = {
+		.address = 0xbf,
+		.bit     = 4,
+	},
+	.cooler_boost = {
+		.address = 0x98,
+		.bit     = 7,
+	},
+	.shift_mode = {
+		.address = 0xf2,
+		.modes = {
+			{ SM_ECO_NAME,     0xc2 },
+			{ SM_COMFORT_NAME, 0xc1 },
+			{ SM_SPORT_NAME,   0xc0 },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.super_battery = {
+		.address = MSI_EC_ADDR_UNSUPP, // unsupported or enabled by ECO shift
+		.mask    = 0x0f,
+	},
+	.fan_mode = {
+		.address = 0xf4,
+		.modes = {
+			{ FM_AUTO_NAME,     0x0d },
+			{ FM_SILENT_NAME,   0x1d },
+			{ FM_ADVANCED_NAME, 0x8d },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.cpu = {
+		.rt_temp_address       = 0x68,
+		.rt_fan_speed_address  = 0x71,
+		.rt_fan_speed_base_min = 0x00,
+		.rt_fan_speed_base_max = 0x96,
+		.bs_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
+		.bs_fan_speed_base_min = 0x00,
+		.bs_fan_speed_base_max = 0x0f,
+	},
+	.gpu = {
+		.rt_temp_address      = MSI_EC_ADDR_UNSUPP,
+		.rt_fan_speed_address = MSI_EC_ADDR_UNSUPP,
+	},
+	.leds = {
+		.micmute_led_address = 0x2b,
+		.mute_led_address    = 0x2c,
+		.bit                 = 2,
+	},
+	.kbd_bl = {
+		.bl_mode_address  = MSI_EC_ADDR_UNSUPP, // not presented in MSI app
+		.bl_modes         = { 0x00, 0x08 },
+		.max_mode         = 1,
+		.bl_state_address = 0xf3,
+		.state_base_value = 0x80,
+		.max_state        = 3,
+	},
+};
+
+static const char * const ALLOWED_FW_10[] __initconst = {
+	"1582EMS1.107", // GF66 11UC
+	NULL
+};
+
+static struct msi_ec_conf CONF10 __initdata = {
+	.allowed_fw = ALLOWED_FW_10,
+	.charge_control = {
+		.address      = 0xd7,
+		.offset_start = 0x8a,
+		.offset_end   = 0x80,
+		.range_min    = 0x8a,
+		.range_max    = 0xe4,
+	},
+	.webcam = {
+		.address       = 0x2e,
+		.block_address = 0x2f,
+		.bit           = 1,
+	},
+	.fn_win_swap = {
+		.address = MSI_EC_ADDR_UNSUPP,
+		.bit     = 4,
+	},
+	.cooler_boost = {
+		.address = 0x98,
+		.bit     = 7,
+	},
+	.shift_mode = {
+		.address = 0xd2,
+		.modes = {
+			{ SM_ECO_NAME,     0xc2 },
+			{ SM_COMFORT_NAME, 0xc1 },
+			{ SM_SPORT_NAME,   0xc0 },
+			{ SM_TURBO_NAME,   0xc4 },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.super_battery = {
+		.address = 0xe5,
+		.mask    = 0x0f,
+	},
+	.fan_mode = {
+		.address = 0xd4,
+		.modes = {
+			{ FM_AUTO_NAME,     0x0d },
+			{ FM_SILENT_NAME,   0x1d },
+			{ FM_ADVANCED_NAME, 0x8d },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.cpu = {
+		.rt_temp_address       = 0x68,
+		.rt_fan_speed_address  = 0x71, // ?
+		.rt_fan_speed_base_min = 0x19,
+		.rt_fan_speed_base_max = 0x37,
+		.bs_fan_speed_address  = MSI_EC_ADDR_UNKNOWN, // ?
+		.bs_fan_speed_base_min = 0x00,
+		.bs_fan_speed_base_max = 0x0f,
+	},
+	.gpu = {
+		.rt_temp_address      = 0x80,
+		.rt_fan_speed_address = 0x89,
+	},
+	.leds = {
+		.micmute_led_address = 0x2c,
+		.mute_led_address    = 0x2d,
+		.bit                 = 1,
+	},
+	.kbd_bl = {
+		.bl_mode_address  = 0x2c,
+		.bl_modes         = { 0x00, 0x08 },
+		.max_mode         = 1,
+		.bl_state_address = 0xd3,
+		.state_base_value = 0x80,
+		.max_state        = 3,
+	},
+};
+
+static const char * const ALLOWED_FW_11[] __initconst = {
+	"16S6EMS1.111", // Prestige 15 a11scx
+	"1552EMS1.115", // Modern 15 a11m
+	NULL
+};
+
+static struct msi_ec_conf CONF11 __initdata = {
+	.allowed_fw = ALLOWED_FW_11,
+	.charge_control = {
+		.address      = 0xd7,
+		.offset_start = 0x8a,
+		.offset_end   = 0x80,
+		.range_min    = 0x8a,
+		.range_max    = 0xe4,
+	},
+	.webcam = {
+		.address       = 0x2e,
+		.block_address = MSI_EC_ADDR_UNKNOWN,
+		.bit           = 1,
+	},
+	.fn_win_swap = {
+		.address = 0xe8,
+		.bit     = 4,
+	},
+	.cooler_boost = {
+		.address = 0x98,
+		.bit     = 7,
+	},
+	.shift_mode = {
+		.address = 0xd2,
+		.modes = {
+			{ SM_ECO_NAME,     0xc2 },
+			{ SM_COMFORT_NAME, 0xc1 },
+			{ SM_SPORT_NAME,   0xc0 },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.super_battery = {
+		.address = 0xeb,
+		.mask = 0x0f,
+	},
+	.fan_mode = {
+		.address = 0xd4,
+		.modes = {
+			{ FM_AUTO_NAME,     0x0d },
+			{ FM_SILENT_NAME,   0x1d },
+			{ FM_ADVANCED_NAME, 0x4d },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.cpu = {
+		.rt_temp_address       = 0x68,
+		.rt_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
+		.bs_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
+	},
+	.gpu = {
+		.rt_temp_address      = MSI_EC_ADDR_UNSUPP,
+		.rt_fan_speed_address = MSI_EC_ADDR_UNSUPP,
+	},
+	.leds = {
+		.micmute_led_address = 0x2c,
+		.mute_led_address    = 0x2d,
+		.bit                 = 1,
+	},
+	.kbd_bl = {
+		.bl_mode_address  = MSI_EC_ADDR_UNKNOWN,
+		.bl_modes         = {}, // ?
+		.max_mode         = 1, // ?
+		.bl_state_address = 0xd3,
+		.state_base_value = 0x80,
+		.max_state        = 3,
+	},
+};
+
+static const char * const ALLOWED_FW_12[] __initconst = {
+	"16R6EMS1.104", // GF63 Thin 11UC
+	NULL
+};
+
+static struct msi_ec_conf CONF12 __initdata = {
+	.allowed_fw = ALLOWED_FW_12,
+	.charge_control = {
+		.address      = 0xd7,
+		.offset_start = 0x8a,
+		.offset_end   = 0x80,
+		.range_min    = 0x8a,
+		.range_max    = 0xe4,
+	},
+	.webcam = {
+		.address       = 0x2e,
+		.block_address = 0x2f,
+		.bit           = 1,
+	},
+	.fn_win_swap = {
+		.address = 0xe8,
+		.bit     = 4,
+	},
+	.cooler_boost = {
+		.address = 0x98,
+		.bit     = 7,
+	},
+	.shift_mode = {
+		.address = 0xd2,
+		.modes = {
+			{ SM_ECO_NAME,     0xc2 },
+			{ SM_COMFORT_NAME, 0xc1 },
+			{ SM_SPORT_NAME,   0xc0 },
+			{ SM_TURBO_NAME,   0xc4 },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.super_battery = {
+		.address = MSI_EC_ADDR_UNSUPP, // 0xeb
+		.mask    = 0x0f, // 00, 0f
+	},
+	.fan_mode = {
+		.address = 0xd4,
+		.modes = {
+			{ FM_AUTO_NAME,     0x0d },
+			{ FM_SILENT_NAME,   0x1d },
+			{ FM_ADVANCED_NAME, 0x8d },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.cpu = {
+		.rt_temp_address       = 0x68,
+		.rt_fan_speed_address  = 0x71,
+		.rt_fan_speed_base_min = 0x19,
+		.rt_fan_speed_base_max = 0x37,
+		.bs_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
+		.bs_fan_speed_base_min = 0x00,
+		.bs_fan_speed_base_max = 0x0f,
+	},
+	.gpu = {
+		.rt_temp_address      = MSI_EC_ADDR_UNSUPP,
+		.rt_fan_speed_address = 0x89,
+	},
+	.leds = {
+		.micmute_led_address = MSI_EC_ADDR_UNSUPP,
+		.mute_led_address    = 0x2d,
+		.bit                 = 1,
+	},
+	.kbd_bl = {
+		.bl_mode_address  = MSI_EC_ADDR_UNKNOWN,
+		.bl_modes         = { 0x00, 0x08 },
+		.max_mode         = 1,
+		.bl_state_address = 0xd3,
+		.state_base_value = 0x80,
+		.max_state        = 3,
+	},
+};
+
+static const char * const ALLOWED_FW_13[] __initconst = {
+	"1594EMS1.109", // MSI Prestige 16 Studio A13VE
+	NULL
+};
+
+static struct msi_ec_conf CONF13 __initdata = {
+	.allowed_fw = ALLOWED_FW_13,
+	.charge_control = {
+		.address      = 0xd7,
+		.offset_start = 0x8a,
+		.offset_end   = 0x80,
+		.range_min    = 0x8a,
+		.range_max    = 0xe4,
+	},
+	.webcam = {
+		.address       = 0x2e,
+		.block_address = 0x2f,
+		.bit           = 1,
+	},
+	.fn_win_swap = {
+		.address = 0xe8,
+		.bit     = 4, // 0x00-0x10
+	},
+	.cooler_boost = {
+		.address = 0x98,
+		.bit     = 7,
+	},
+	.shift_mode = {
+		.address = 0xd2,
+		.modes = {
+			{ SM_ECO_NAME,     0xc2 }, // super battery
+			{ SM_COMFORT_NAME, 0xc1 }, // balanced
+			{ SM_TURBO_NAME,   0xc4 }, // extreme
+			MSI_EC_MODE_NULL
+		},
+	},
+	.super_battery = {
+		.address = MSI_EC_ADDR_UNSUPP,
+		.mask    = 0x0f, // 00, 0f
+	},
+	.fan_mode = {
+		.address = 0xd4,
+		.modes = {
+			{ FM_AUTO_NAME,     0x0d },
+			{ FM_SILENT_NAME,   0x1d },
+			{ FM_ADVANCED_NAME, 0x8d },
+			MSI_EC_MODE_NULL
+		},
+	},
+	.cpu = {
+		.rt_temp_address       = 0x68,
+		.rt_fan_speed_address  = 0x71, // 0x0-0x96
+		.rt_fan_speed_base_min = 0x00,
+		.rt_fan_speed_base_max = 0x96,
+		.bs_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
+		.bs_fan_speed_base_min = 0x00,
+		.bs_fan_speed_base_max = 0x0f,
+	},
+	.gpu = {
+		.rt_temp_address      = 0x80,
+		.rt_fan_speed_address = 0x89,
+	},
+	.leds = {
+		.micmute_led_address = 0x2c,
+		.mute_led_address    = 0x2d,
+		.bit                 = 1,
+	},
+	.kbd_bl = {
+		.bl_mode_address  = 0x2c, // KB auto turn off
+		.bl_modes         = { 0x00, 0x08 }, // always on; off after 10 sec
+		.max_mode         = 1,
+		.bl_state_address = 0xd3,
+		.state_base_value = 0x80,
+		.max_state        = 3,
+	},
+};
+
 static struct msi_ec_conf *CONFIGS[] __initdata = {
 	&CONF0,
 	&CONF1,
@@ -676,6 +1137,12 @@ static struct msi_ec_conf *CONFIGS[] __initdata = {
 	&CONF5,
 	&CONF6,
 	&CONF7,
+	&CONF8,
+	&CONF9,
+	&CONF10,
+	&CONF11,
+	&CONF12,
+	&CONF13,
 	NULL
 };
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] platform/x86: msi-ec: Fix the 3rd config
  2023-10-06 17:53 ` [PATCH v2 1/3] platform/x86: msi-ec: Fix the 3rd config Nikita Kravets
@ 2023-10-09 11:27   ` Ilpo Järvinen
  2023-10-11  9:22   ` Hans de Goede
  1 sibling, 0 replies; 12+ messages in thread
From: Ilpo Järvinen @ 2023-10-09 11:27 UTC (permalink / raw)
  To: Nikita Kravets
  Cc: platform-driver-x86, Hans de Goede, Aakash Singh, Jose Angel Pastrana

[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

On Fri, 6 Oct 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.
> 
> Fixes: 392cacf2aa10 ("platform/x86: Add new msi-ec 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 | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
> index f26a3121092f..492eb383ee7a 100644
> --- a/drivers/platform/x86/msi-ec.c
> +++ b/drivers/platform/x86/msi-ec.c
> @@ -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,

Thanks.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] platform/x86: msi-ec: rename fn_super_swap
  2023-10-06 17:53 ` [PATCH v2 2/3] platform/x86: msi-ec: rename fn_super_swap Nikita Kravets
@ 2023-10-09 11:39   ` Ilpo Järvinen
  0 siblings, 0 replies; 12+ messages in thread
From: Ilpo Järvinen @ 2023-10-09 11:39 UTC (permalink / raw)
  To: Nikita Kravets
  Cc: platform-driver-x86, Hans de Goede, Aakash Singh, Jose Angel Pastrana

On Fri, 6 Oct 2023, Nikita Kravets wrote:

> This patch renames fn_super_swap to fn_win_swap for consistency
> with the downstream version of the driver. Renaming the field to
> fn_super_swap in the downstream driver would require modifying several
> branches that are yet to be merged into the main branch, so I decided
> to do it here instead.
> 
> Cc: Aakash Singh <mail@singhaakash.dev>
> Cc: Jose Angel Pastrana <japp0005@red.ujaen.es>
> Signed-off-by: Nikita Kravets <teackot@gmail.com>

Applied this particular patch (2/3) to my local review-ilpo branch.

Hans will take care of applying the fix in 1/3.


-- 
 i.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs
  2023-10-06 17:53 ` [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs Nikita Kravets
@ 2023-10-09 11:40   ` Ilpo Järvinen
  2023-10-09 11:57     ` Hans de Goede
  0 siblings, 1 reply; 12+ messages in thread
From: Ilpo Järvinen @ 2023-10-09 11:40 UTC (permalink / raw)
  To: Nikita Kravets
  Cc: platform-driver-x86, Hans de Goede, Aakash Singh, Jose Angel Pastrana

On Fri, 6 Oct 2023, Nikita Kravets wrote:

> This patch adds configurations for new EC firmware from 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 | 467 ++++++++++++++++++++++++++++++++++
>  1 file changed, 467 insertions(+)
> 
> diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
> index 3074aee878c1..f19504dbf164 100644
> --- a/drivers/platform/x86/msi-ec.c
> +++ b/drivers/platform/x86/msi-ec.c
> @@ -667,6 +667,467 @@ static struct msi_ec_conf CONF7 __initdata = {
>  	},
>  };
>  
> +static const char * const ALLOWED_FW_8[] __initconst = {
> +	"14F1EMS1.115",
> +	NULL
> +};
> +
> +static struct msi_ec_conf CONF8 __initdata = {
> +	.allowed_fw = ALLOWED_FW_8,
> +	.charge_control = {
> +		.address      = 0xd7,
> +		.offset_start = 0x8a,
> +		.offset_end   = 0x80,
> +		.range_min    = 0x8a,
> +		.range_max    = 0xe4,
> +	},
> +	.webcam = {
> +		.address       = 0x2e,
> +		.block_address = MSI_EC_ADDR_UNSUPP,
> +		.bit           = 1,
> +	},
> +	.fn_win_swap = {
> +		.address = 0xe8,
> +		.bit     = 4,
> +	},
> +	.cooler_boost = {
> +		.address = 0x98,
> +		.bit     = 7,
> +	},
> +	.shift_mode = {
> +		.address = 0xd2,
> +		.modes = {
> +			{ SM_ECO_NAME,     0xc2 },
> +			{ SM_COMFORT_NAME, 0xc1 },
> +			{ SM_SPORT_NAME,   0xc0 },
> +			MSI_EC_MODE_NULL
> +		},
> +	},
> +	.super_battery = {
> +		.address = 0xeb,
> +		.mask    = 0x0f,
> +	},
> +	.fan_mode = {
> +		.address = 0xd4,
> +		.modes = {
> +			{ FM_AUTO_NAME,     0x0d },
> +			{ FM_SILENT_NAME,   0x1d },
> +			{ FM_BASIC_NAME,    0x4d },
> +			MSI_EC_MODE_NULL
> +		},
> +	},
> +	.cpu = {
> +		.rt_temp_address       = 0x68,
> +		.rt_fan_speed_address  = 0x71,
> +		.rt_fan_speed_base_min = 0x19,
> +		.rt_fan_speed_base_max = 0x37,
> +		.bs_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
> +		.bs_fan_speed_base_min = 0x00,
> +		.bs_fan_speed_base_max = 0x0f,
> +	},
> +	.gpu = {
> +		.rt_temp_address      = MSI_EC_ADDR_UNKNOWN,
> +		.rt_fan_speed_address = MSI_EC_ADDR_UNKNOWN,
> +	},
> +	.leds = {
> +		.micmute_led_address = MSI_EC_ADDR_UNSUPP,
> +		.mute_led_address    = 0x2d,
> +		.bit                 = 1,
> +	},
> +	.kbd_bl = {
> +		.bl_mode_address  = MSI_EC_ADDR_UNKNOWN, // ?
> +		.bl_modes         = { 0x00, 0x08 }, // ?
> +		.max_mode         = 1, // ?
> +		.bl_state_address = MSI_EC_ADDR_UNSUPP, // not functional

I only too patch 2/3 becase there seems to be some configuration option 
which causes // comments to trigger warning (that can be made errors 
with another config option) so please use only /* */ comments.

-- 
 i.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs
  2023-10-09 11:40   ` Ilpo Järvinen
@ 2023-10-09 11:57     ` Hans de Goede
  2023-10-09 12:34       ` Ilpo Järvinen
  0 siblings, 1 reply; 12+ messages in thread
From: Hans de Goede @ 2023-10-09 11:57 UTC (permalink / raw)
  To: Ilpo Järvinen, Nikita Kravets
  Cc: platform-driver-x86, Aakash Singh, Jose Angel Pastrana

Hi Ilpo,

On 10/9/23 13:40, Ilpo Järvinen wrote:
> On Fri, 6 Oct 2023, Nikita Kravets wrote:
> 
>> This patch adds configurations for new EC firmware from 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 | 467 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 467 insertions(+)
>>
>> diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
>> index 3074aee878c1..f19504dbf164 100644
>> --- a/drivers/platform/x86/msi-ec.c
>> +++ b/drivers/platform/x86/msi-ec.c
>> @@ -667,6 +667,467 @@ static struct msi_ec_conf CONF7 __initdata = {
>>  	},
>>  };
>>  
>> +static const char * const ALLOWED_FW_8[] __initconst = {
>> +	"14F1EMS1.115",
>> +	NULL
>> +};
>> +
>> +static struct msi_ec_conf CONF8 __initdata = {
>> +	.allowed_fw = ALLOWED_FW_8,
>> +	.charge_control = {
>> +		.address      = 0xd7,
>> +		.offset_start = 0x8a,
>> +		.offset_end   = 0x80,
>> +		.range_min    = 0x8a,
>> +		.range_max    = 0xe4,
>> +	},
>> +	.webcam = {
>> +		.address       = 0x2e,
>> +		.block_address = MSI_EC_ADDR_UNSUPP,
>> +		.bit           = 1,
>> +	},
>> +	.fn_win_swap = {
>> +		.address = 0xe8,
>> +		.bit     = 4,
>> +	},
>> +	.cooler_boost = {
>> +		.address = 0x98,
>> +		.bit     = 7,
>> +	},
>> +	.shift_mode = {
>> +		.address = 0xd2,
>> +		.modes = {
>> +			{ SM_ECO_NAME,     0xc2 },
>> +			{ SM_COMFORT_NAME, 0xc1 },
>> +			{ SM_SPORT_NAME,   0xc0 },
>> +			MSI_EC_MODE_NULL
>> +		},
>> +	},
>> +	.super_battery = {
>> +		.address = 0xeb,
>> +		.mask    = 0x0f,
>> +	},
>> +	.fan_mode = {
>> +		.address = 0xd4,
>> +		.modes = {
>> +			{ FM_AUTO_NAME,     0x0d },
>> +			{ FM_SILENT_NAME,   0x1d },
>> +			{ FM_BASIC_NAME,    0x4d },
>> +			MSI_EC_MODE_NULL
>> +		},
>> +	},
>> +	.cpu = {
>> +		.rt_temp_address       = 0x68,
>> +		.rt_fan_speed_address  = 0x71,
>> +		.rt_fan_speed_base_min = 0x19,
>> +		.rt_fan_speed_base_max = 0x37,
>> +		.bs_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
>> +		.bs_fan_speed_base_min = 0x00,
>> +		.bs_fan_speed_base_max = 0x0f,
>> +	},
>> +	.gpu = {
>> +		.rt_temp_address      = MSI_EC_ADDR_UNKNOWN,
>> +		.rt_fan_speed_address = MSI_EC_ADDR_UNKNOWN,
>> +	},
>> +	.leds = {
>> +		.micmute_led_address = MSI_EC_ADDR_UNSUPP,
>> +		.mute_led_address    = 0x2d,
>> +		.bit                 = 1,
>> +	},
>> +	.kbd_bl = {
>> +		.bl_mode_address  = MSI_EC_ADDR_UNKNOWN, // ?
>> +		.bl_modes         = { 0x00, 0x08 }, // ?
>> +		.max_mode         = 1, // ?
>> +		.bl_state_address = MSI_EC_ADDR_UNSUPP, // not functional
> 
> I only too patch 2/3 becase there seems to be some configuration option 
> which causes // comments to trigger warning (that can be made errors 
> with another config option) so please use only /* */ comments.

Hmm, that is very weird all the:

// SPDX-License-Identifier ...

comments at the top of many of our .c files are c++ style comments.

Regards,

Hans



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs
  2023-10-09 11:57     ` Hans de Goede
@ 2023-10-09 12:34       ` Ilpo Järvinen
  2023-10-09 12:57         ` Hans de Goede
  0 siblings, 1 reply; 12+ messages in thread
From: Ilpo Järvinen @ 2023-10-09 12:34 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Nikita Kravets, platform-driver-x86, Aakash Singh, Jose Angel Pastrana

[-- Attachment #1: Type: text/plain, Size: 3868 bytes --]

On Mon, 9 Oct 2023, Hans de Goede wrote:
> On 10/9/23 13:40, Ilpo Järvinen wrote:
> > On Fri, 6 Oct 2023, Nikita Kravets wrote:
> > 
> >> This patch adds configurations for new EC firmware from 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 | 467 ++++++++++++++++++++++++++++++++++
> >>  1 file changed, 467 insertions(+)
> >>
> >> diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
> >> index 3074aee878c1..f19504dbf164 100644
> >> --- a/drivers/platform/x86/msi-ec.c
> >> +++ b/drivers/platform/x86/msi-ec.c
> >> @@ -667,6 +667,467 @@ static struct msi_ec_conf CONF7 __initdata = {
> >>  	},
> >>  };
> >>  
> >> +static const char * const ALLOWED_FW_8[] __initconst = {
> >> +	"14F1EMS1.115",
> >> +	NULL
> >> +};
> >> +
> >> +static struct msi_ec_conf CONF8 __initdata = {
> >> +	.allowed_fw = ALLOWED_FW_8,
> >> +	.charge_control = {
> >> +		.address      = 0xd7,
> >> +		.offset_start = 0x8a,
> >> +		.offset_end   = 0x80,
> >> +		.range_min    = 0x8a,
> >> +		.range_max    = 0xe4,
> >> +	},
> >> +	.webcam = {
> >> +		.address       = 0x2e,
> >> +		.block_address = MSI_EC_ADDR_UNSUPP,
> >> +		.bit           = 1,
> >> +	},
> >> +	.fn_win_swap = {
> >> +		.address = 0xe8,
> >> +		.bit     = 4,
> >> +	},
> >> +	.cooler_boost = {
> >> +		.address = 0x98,
> >> +		.bit     = 7,
> >> +	},
> >> +	.shift_mode = {
> >> +		.address = 0xd2,
> >> +		.modes = {
> >> +			{ SM_ECO_NAME,     0xc2 },
> >> +			{ SM_COMFORT_NAME, 0xc1 },
> >> +			{ SM_SPORT_NAME,   0xc0 },
> >> +			MSI_EC_MODE_NULL
> >> +		},
> >> +	},
> >> +	.super_battery = {
> >> +		.address = 0xeb,
> >> +		.mask    = 0x0f,
> >> +	},
> >> +	.fan_mode = {
> >> +		.address = 0xd4,
> >> +		.modes = {
> >> +			{ FM_AUTO_NAME,     0x0d },
> >> +			{ FM_SILENT_NAME,   0x1d },
> >> +			{ FM_BASIC_NAME,    0x4d },
> >> +			MSI_EC_MODE_NULL
> >> +		},
> >> +	},
> >> +	.cpu = {
> >> +		.rt_temp_address       = 0x68,
> >> +		.rt_fan_speed_address  = 0x71,
> >> +		.rt_fan_speed_base_min = 0x19,
> >> +		.rt_fan_speed_base_max = 0x37,
> >> +		.bs_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
> >> +		.bs_fan_speed_base_min = 0x00,
> >> +		.bs_fan_speed_base_max = 0x0f,
> >> +	},
> >> +	.gpu = {
> >> +		.rt_temp_address      = MSI_EC_ADDR_UNKNOWN,
> >> +		.rt_fan_speed_address = MSI_EC_ADDR_UNKNOWN,
> >> +	},
> >> +	.leds = {
> >> +		.micmute_led_address = MSI_EC_ADDR_UNSUPP,
> >> +		.mute_led_address    = 0x2d,
> >> +		.bit                 = 1,
> >> +	},
> >> +	.kbd_bl = {
> >> +		.bl_mode_address  = MSI_EC_ADDR_UNKNOWN, // ?
> >> +		.bl_modes         = { 0x00, 0x08 }, // ?
> >> +		.max_mode         = 1, // ?
> >> +		.bl_state_address = MSI_EC_ADDR_UNSUPP, // not functional
> > 
> > I only too patch 2/3 becase there seems to be some configuration option 
> > which causes // comments to trigger warning (that can be made errors 
> > with another config option) so please use only /* */ comments.
> 
> Hmm, that is very weird all the:
> 
> // SPDX-License-Identifier ...
> 
> comments at the top of many of our .c files are c++ style comments.

I know there are those already which is why I didn't think there would 
have been any problem with them until I got burned.

If // comments are okay, what's the explanation for this then:

  https://lore.kernel.org/oe-kbuild-all/202309270535.g9nOUvFb-lkp@intel.com/

It's from randconfig build so it's a bit hard to know from the report 
itself which CONFIG combination exactly triggers the issue.

I can think of two potential ones:
  a) Only problems for changed lines are reported by LKP
  b) Header files have different rules than .c files (uapi ones in 
     particular, I'd guess, if that's the case)

Any ideas?


-- 
 i.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs
  2023-10-09 12:34       ` Ilpo Järvinen
@ 2023-10-09 12:57         ` Hans de Goede
  2023-10-09 13:10           ` Ilpo Järvinen
  0 siblings, 1 reply; 12+ messages in thread
From: Hans de Goede @ 2023-10-09 12:57 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Nikita Kravets, platform-driver-x86, Aakash Singh, Jose Angel Pastrana

Hi,

On 10/9/23 14:34, Ilpo Järvinen wrote:
> On Mon, 9 Oct 2023, Hans de Goede wrote:
>> On 10/9/23 13:40, Ilpo Järvinen wrote:
>>> On Fri, 6 Oct 2023, Nikita Kravets wrote:
>>>
>>>> This patch adds configurations for new EC firmware from 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 | 467 ++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 467 insertions(+)
>>>>
>>>> diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
>>>> index 3074aee878c1..f19504dbf164 100644
>>>> --- a/drivers/platform/x86/msi-ec.c
>>>> +++ b/drivers/platform/x86/msi-ec.c
>>>> @@ -667,6 +667,467 @@ static struct msi_ec_conf CONF7 __initdata = {
>>>>  	},
>>>>  };
>>>>  
>>>> +static const char * const ALLOWED_FW_8[] __initconst = {
>>>> +	"14F1EMS1.115",
>>>> +	NULL
>>>> +};
>>>> +
>>>> +static struct msi_ec_conf CONF8 __initdata = {
>>>> +	.allowed_fw = ALLOWED_FW_8,
>>>> +	.charge_control = {
>>>> +		.address      = 0xd7,
>>>> +		.offset_start = 0x8a,
>>>> +		.offset_end   = 0x80,
>>>> +		.range_min    = 0x8a,
>>>> +		.range_max    = 0xe4,
>>>> +	},
>>>> +	.webcam = {
>>>> +		.address       = 0x2e,
>>>> +		.block_address = MSI_EC_ADDR_UNSUPP,
>>>> +		.bit           = 1,
>>>> +	},
>>>> +	.fn_win_swap = {
>>>> +		.address = 0xe8,
>>>> +		.bit     = 4,
>>>> +	},
>>>> +	.cooler_boost = {
>>>> +		.address = 0x98,
>>>> +		.bit     = 7,
>>>> +	},
>>>> +	.shift_mode = {
>>>> +		.address = 0xd2,
>>>> +		.modes = {
>>>> +			{ SM_ECO_NAME,     0xc2 },
>>>> +			{ SM_COMFORT_NAME, 0xc1 },
>>>> +			{ SM_SPORT_NAME,   0xc0 },
>>>> +			MSI_EC_MODE_NULL
>>>> +		},
>>>> +	},
>>>> +	.super_battery = {
>>>> +		.address = 0xeb,
>>>> +		.mask    = 0x0f,
>>>> +	},
>>>> +	.fan_mode = {
>>>> +		.address = 0xd4,
>>>> +		.modes = {
>>>> +			{ FM_AUTO_NAME,     0x0d },
>>>> +			{ FM_SILENT_NAME,   0x1d },
>>>> +			{ FM_BASIC_NAME,    0x4d },
>>>> +			MSI_EC_MODE_NULL
>>>> +		},
>>>> +	},
>>>> +	.cpu = {
>>>> +		.rt_temp_address       = 0x68,
>>>> +		.rt_fan_speed_address  = 0x71,
>>>> +		.rt_fan_speed_base_min = 0x19,
>>>> +		.rt_fan_speed_base_max = 0x37,
>>>> +		.bs_fan_speed_address  = MSI_EC_ADDR_UNSUPP,
>>>> +		.bs_fan_speed_base_min = 0x00,
>>>> +		.bs_fan_speed_base_max = 0x0f,
>>>> +	},
>>>> +	.gpu = {
>>>> +		.rt_temp_address      = MSI_EC_ADDR_UNKNOWN,
>>>> +		.rt_fan_speed_address = MSI_EC_ADDR_UNKNOWN,
>>>> +	},
>>>> +	.leds = {
>>>> +		.micmute_led_address = MSI_EC_ADDR_UNSUPP,
>>>> +		.mute_led_address    = 0x2d,
>>>> +		.bit                 = 1,
>>>> +	},
>>>> +	.kbd_bl = {
>>>> +		.bl_mode_address  = MSI_EC_ADDR_UNKNOWN, // ?
>>>> +		.bl_modes         = { 0x00, 0x08 }, // ?
>>>> +		.max_mode         = 1, // ?
>>>> +		.bl_state_address = MSI_EC_ADDR_UNSUPP, // not functional
>>>
>>> I only too patch 2/3 becase there seems to be some configuration option 
>>> which causes // comments to trigger warning (that can be made errors 
>>> with another config option) so please use only /* */ comments.
>>
>> Hmm, that is very weird all the:
>>
>> // SPDX-License-Identifier ...
>>
>> comments at the top of many of our .c files are c++ style comments.
> 
> I know there are those already which is why I didn't think there would 
> have been any problem with them until I got burned.
> 
> If // comments are okay, what's the explanation for this then:
> 
>   https://lore.kernel.org/oe-kbuild-all/202309270535.g9nOUvFb-lkp@intel.com/
> 
> It's from randconfig build so it's a bit hard to know from the report 
> itself which CONFIG combination exactly triggers the issue.
> 
> I can think of two potential ones:
>   a) Only problems for changed lines are reported by LKP
>   b) Header files have different rules than .c files (uapi ones in 
>      particular, I'd guess, if that's the case)

Yes I think that the error you point at is caused by the file in question
being a uapi header. It makes some sense to avoid C++ style comments there
to e.g. avoid problems when userspace code is build with -ansi .

So I think that uapi headers are the exception to the rule that
c++ style comments are ok.

Regards,

Hans



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs
  2023-10-09 12:57         ` Hans de Goede
@ 2023-10-09 13:10           ` Ilpo Järvinen
  0 siblings, 0 replies; 12+ messages in thread
From: Ilpo Järvinen @ 2023-10-09 13:10 UTC (permalink / raw)
  To: Hans de Goede, Nikita Kravets
  Cc: platform-driver-x86, Aakash Singh, Jose Angel Pastrana

[-- Attachment #1: Type: text/plain, Size: 2105 bytes --]

On Mon, 9 Oct 2023, Hans de Goede wrote:
> On 10/9/23 14:34, Ilpo Järvinen wrote:
> > On Mon, 9 Oct 2023, Hans de Goede wrote:
> >> On 10/9/23 13:40, Ilpo Järvinen wrote:
> >>> On Fri, 6 Oct 2023, Nikita Kravets wrote:
> >>>
> >>>> This patch adds configurations for new EC firmware from 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>

> >>>> +		.bl_state_address = MSI_EC_ADDR_UNSUPP, // not functional
> >>>
> >>> I only too patch 2/3 becase there seems to be some configuration option 
> >>> which causes // comments to trigger warning (that can be made errors 
> >>> with another config option) so please use only /* */ comments.
> >>
> >> Hmm, that is very weird all the:
> >>
> >> // SPDX-License-Identifier ...
> >>
> >> comments at the top of many of our .c files are c++ style comments.
> > 
> > I know there are those already which is why I didn't think there would 
> > have been any problem with them until I got burned.
> > 
> > If // comments are okay, what's the explanation for this then:
> > 
> >   https://lore.kernel.org/oe-kbuild-all/202309270535.g9nOUvFb-lkp@intel.com/
> > 
> > It's from randconfig build so it's a bit hard to know from the report 
> > itself which CONFIG combination exactly triggers the issue.
> > 
> > I can think of two potential ones:
> >   a) Only problems for changed lines are reported by LKP
> >   b) Header files have different rules than .c files (uapi ones in 
> >      particular, I'd guess, if that's the case)
> 
> Yes I think that the error you point at is caused by the file in question
> being a uapi header. It makes some sense to avoid C++ style comments there
> to e.g. avoid problems when userspace code is build with -ansi .
> 
> So I think that uapi headers are the exception to the rule that
> c++ style comments are ok.

Okay, if that's the case, I took this patch into review-ilpo now.

Thank you Nikita for the patch (2 and 3 are now in the review-ilpo 
branch).

-- 
 i.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] platform/x86: msi-ec: Fix the 3rd config
  2023-10-06 17:53 ` [PATCH v2 1/3] platform/x86: msi-ec: Fix the 3rd config Nikita Kravets
  2023-10-09 11:27   ` Ilpo Järvinen
@ 2023-10-11  9:22   ` Hans de Goede
  1 sibling, 0 replies; 12+ messages in thread
From: Hans de Goede @ 2023-10-11  9:22 UTC (permalink / raw)
  To: Nikita Kravets, platform-driver-x86
  Cc: Ilpo Järvinen, Aakash Singh, Jose Angel Pastrana

Hi,

On 10/6/23 19:53, 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.
> 
> Fixes: 392cacf2aa10 ("platform/x86: Add new msi-ec driver")
> Cc: Aakash Singh <mail@singhaakash.dev>
> Cc: Jose Angel Pastrana <japp0005@red.ujaen.es>
> Signed-off-by: Nikita Kravets <teackot@gmail.com>

Thank you for your patch/series, I've applied this patch
(series) to the pdx86 fixes branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes

Note it will show up in the pdx86 fixes branch once I've pushed
my local branch there, which might take a while.

I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.

Regards,

Hans




> ---
>  drivers/platform/x86/msi-ec.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
> index f26a3121092f..492eb383ee7a 100644
> --- a/drivers/platform/x86/msi-ec.c
> +++ b/drivers/platform/x86/msi-ec.c
> @@ -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,


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-10-11  9:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-06 17:53 [PATCH v2 0/3] platform/x86: msi-ec: add and fix EC configs Nikita Kravets
2023-10-06 17:53 ` [PATCH v2 1/3] platform/x86: msi-ec: Fix the 3rd config Nikita Kravets
2023-10-09 11:27   ` Ilpo Järvinen
2023-10-11  9:22   ` Hans de Goede
2023-10-06 17:53 ` [PATCH v2 2/3] platform/x86: msi-ec: rename fn_super_swap Nikita Kravets
2023-10-09 11:39   ` Ilpo Järvinen
2023-10-06 17:53 ` [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs Nikita Kravets
2023-10-09 11:40   ` Ilpo Järvinen
2023-10-09 11:57     ` Hans de Goede
2023-10-09 12:34       ` Ilpo Järvinen
2023-10-09 12:57         ` Hans de Goede
2023-10-09 13:10           ` Ilpo Järvinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.