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