On Monday 29 January 2018 16:22:07 Mario.Limonciello@dell.com wrote: > > -----Original Message----- > > From: Pali Rohár [mailto:pali.rohar@gmail.com] > > Sent: Saturday, January 27, 2018 8:51 AM > > To: Limonciello, Mario > > Cc: dvhart@infradead.org; andy.shevchenko@gmail.com; linux- > > kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; luto@kernel.org; > > quasisec@google.com; rjw@rjwysocki.net; mjg59@google.com; hch@lst.de; > > greg@kroah.com; gnomes@lxorguk.ukuu.org.uk > > Subject: Re: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering support > > > > On Friday 05 January 2018 14:48:39 Mario.Limonciello@dell.com wrote: > > > > > > > > > > -----Original Message----- > > > > From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86- > > > > owner@vger.kernel.org] On Behalf Of Pali Rohár > > > > Sent: Friday, January 5, 2018 8:44 AM > > > > To: Limonciello, Mario > > > > Cc: dvhart@infradead.org; andy.shevchenko@gmail.com; linux- > > > > kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; > > luto@kernel.org; > > > > quasisec@google.com; rjw@rjwysocki.net; mjg59@google.com; hch@lst.de; > > > > greg@kroah.com; gnomes@lxorguk.ukuu.org.uk > > > > Subject: Re: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering support > > > > > > > > On Friday 05 January 2018 14:32:54 Mario.Limonciello@dell.com wrote: > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver- > > x86- > > > > > > owner@vger.kernel.org] On Behalf Of Pali Rohár > > > > > > Sent: Friday, January 5, 2018 5:13 AM > > > > > > To: Limonciello, Mario > > > > > > Cc: dvhart@infradead.org; Andy Shevchenko > > ; > > > > > > LKML ; platform-driver- > > x86@vger.kernel.org; > > > > Andy > > > > > > Lutomirski ; quasisec@google.com; rjw@rjwysocki.net; > > > > > > mjg59@google.com; hch@lst.de; Greg KH ; Alan Cox > > > > > > > > > > > > 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? > > > > > > > > No, this is how C language define it. See e.g. C11 standard, section > > > > 6.4.4.1 Integer constants: > > > > > > > > decimal-constant: > > > > nonzero-digit > > > > decimal-constant digit > > > > > > > > octal-constant: > > > > 0 > > > > octal-constant octal-digit > > > > > > > > So in C decimal number cannot start with digit zero. > > > > > > > > I think the place where octal numbers are used are in permissions (0777) > > > > > > > > > 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? > > > > > > > > Currently it is not problem as 7 is same number in octal (07) and > > > > decimal (7). representation. But e.g. octal 077 is 63 in decimal. > > > > > > > > > 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 */ > > > > > > > > Yes, this is better. If you need to achieve alignment then use spaces. > > > > Really, not leading zeros. > > > > > > > > > That seems less readable to me but should interpret the same way. > > > > > > > > Example: > > > > > > > > {0x000, 077, 7}, > > > > {0x000, 007, 7}, > > > > > > > > is **not** same as > > > > > > > > {0x000, 77, 7}, > > > > {0x000, 7, 7}, > > > > > > > > As first number in first section is (decimal) 63, not (decimal) 77. > > > > > > > > > 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@gmail.com > > > > > > > > -- > > > > Pali Rohár > > > > pali.rohar@gmail.com > > > > > > Thanks very much for sharing. I wasn't aware of this. I'll send a patch. > > > > Hi! Do you have a patch for it? > > I used the wrong tag so it didn't automatically CC you. Sorry about that. > It's here: > > https://patchwork.kernel.org/patch/10146671/ Ok, looks good for me. -- Pali Rohár pali.rohar@gmail.com