Re: [PATCH v2 3/3] platform/x86: msi-ec: Add more EC configs

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- 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.