RE: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering support

[Mario.Limonciello]

> -----Original Message-----
> From: platform-driver-x86-owner@xxxxxxxxxxxxxxx [mailto:platform-driver-x86-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Pali RohÃr
> Sent: Friday, January 5, 2018 5:13 AM
> To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx>
> Cc: dvhart@xxxxxxxxxxxxx; Andy Shevchenko <andy.shevchenko@xxxxxxxxx>;
> LKML <linux-kernel@xxxxxxxxxxxxxxx>; platform-driver-x86@xxxxxxxxxxxxxxx; Andy
> Lutomirski <luto@xxxxxxxxxx>; quasisec@xxxxxxxxxx; rjw@xxxxxxxxxxxxx;
> mjg59@xxxxxxxxxx; hch@xxxxxx; Greg KH <greg@xxxxxxxxx>; Alan Cox
> <gnomes@xxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering support
> 
> I know that this patch is already applied and merged, but I spotted this
> problem:
> 
> On Thursday 19 October 2017 12:50:15 Mario Limonciello wrote:
> > +/* calls that are explicitly blacklisted */
> > +static struct smbios_call call_blacklist[] = {
> > +	{0x0000, 01, 07}, /* manufacturing use */
> > +	{0x0000, 06, 05}, /* manufacturing use */
> > +	{0x0000, 11, 03}, /* write once */
> > +	{0x0000, 11, 07}, /* write once */
> 
> Numbers prefixed by zero means that they are in octal notation, right?
Is that how the kernel interprets an integer prefix by zero?

I prefixed by zero for readability, they're supposed to be decimal.

> This can lead to misunderstanding, confusion or problems in future...
> 
> Can we have all numbers either in hexadecimal or decimal notation?

Could you elaborate more why this is problematic the way it is?
Are you meaning you would rather see this?
	{0x0000, 1, 7}, /* manufacturing use */
	{0x0000, 6, 5}, /* manufacturing use */
	{0x0000, 11, 3}, /* write once */
	{0x0000, 11, 7}, /* write once */

That seems less readable to me but should interpret the same way.

Perhaps it would be better if you submit a patch with what is clearer to
you.

> 
> > +	{0x0000, 11, 11}, /* write once */
> > +	{0x0000, 19, -1}, /* diagnostics */
> > +	/* handled by kernel: dell-laptop */
> > +	{0x0000, CLASS_INFO, SELECT_RFKILL},
> > +	{0x0000, CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT},
> > +};
> 
> --
> Pali RohÃr
> pali.rohar@xxxxxxxxx