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@xxxxxxxxxxxxxxx>
>>>> Cc: Jose Angel Pastrana <japp0005@xxxxxxxxxxxx>
>>>> Signed-off-by: Nikita Kravets <teackot@xxxxxxxxx>
>>>> ---
>>>> 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@xxxxxxxxx/
>
> 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
