[PATCH 1/1] platform/x86: think-lmi: Convert container_of() macros to static inline

[PATCH 1/1] platform/x86: think-lmi: Convert container_of() macros to static inline

[Ilpo Järvinen]

The macros to_tlmi_pwd_setting() and to_tlmi_attr_setting() are fragile
because they expect the variable name to be 'kobj', otherwise the build
will fail because container_of()'s 3rd parameter (member) is taken from
the parameter given to the macro.

While at it, move them into a more logical place.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/platform/x86/think-lmi.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 9345316b45db..0f2264bb7577 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -175,9 +175,6 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support");
 #define TLMI_SMP_PWD BIT(6) /* System Management */
 #define TLMI_CERT    BIT(7) /* Certificate Based */
 
-#define to_tlmi_pwd_setting(kobj)  container_of(kobj, struct tlmi_pwd_setting, kobj)
-#define to_tlmi_attr_setting(kobj)  container_of(kobj, struct tlmi_attr_setting, kobj)
-
 static const struct tlmi_err_codes tlmi_errs[] = {
 	{"Success", 0},
 	{"Not Supported", -EOPNOTSUPP},
@@ -198,6 +195,16 @@ static struct think_lmi tlmi_priv;
 static const struct class *fw_attr_class;
 static DEFINE_MUTEX(tlmi_mutex);
 
+static inline struct tlmi_pwd_setting *to_tlmi_pwd_setting(struct kobject *kobj)
+{
+	return container_of(kobj, struct tlmi_pwd_setting, kobj);
+}
+
+static inline struct tlmi_attr_setting *to_tlmi_attr_setting(struct kobject *kobj)
+{
+	return container_of(kobj, struct tlmi_attr_setting, kobj);
+}
+
 /* Convert BIOS WMI error string to suitable error code */
 static int tlmi_errstr_to_err(const char *errstr)
 {
-- 
2.39.2

Re: [PATCH 1/1] platform/x86: think-lmi: Convert container_of() macros to static inline

[Mark Pearson]

Thanks Ilpo,

On Fri, Apr 12, 2024, at 9:09 AM, Ilpo Järvinen wrote:
> The macros to_tlmi_pwd_setting() and to_tlmi_attr_setting() are fragile
> because they expect the variable name to be 'kobj', otherwise the build
> will fail because container_of()'s 3rd parameter (member) is taken from
> the parameter given to the macro.
>
> While at it, move them into a more logical place.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/platform/x86/think-lmi.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/think-lmi.c 
> b/drivers/platform/x86/think-lmi.c
> index 9345316b45db..0f2264bb7577 100644
> --- a/drivers/platform/x86/think-lmi.c
> +++ b/drivers/platform/x86/think-lmi.c
> @@ -175,9 +175,6 @@ MODULE_PARM_DESC(debug_support, "Enable debug 
> command support");
>  #define TLMI_SMP_PWD BIT(6) /* System Management */
>  #define TLMI_CERT    BIT(7) /* Certificate Based */
> 
> -#define to_tlmi_pwd_setting(kobj)  container_of(kobj, struct 
> tlmi_pwd_setting, kobj)
> -#define to_tlmi_attr_setting(kobj)  container_of(kobj, struct 
> tlmi_attr_setting, kobj)
> -
>  static const struct tlmi_err_codes tlmi_errs[] = {
>  	{"Success", 0},
>  	{"Not Supported", -EOPNOTSUPP},
> @@ -198,6 +195,16 @@ static struct think_lmi tlmi_priv;
>  static const struct class *fw_attr_class;
>  static DEFINE_MUTEX(tlmi_mutex);
> 
> +static inline struct tlmi_pwd_setting *to_tlmi_pwd_setting(struct 
> kobject *kobj)
> +{
> +	return container_of(kobj, struct tlmi_pwd_setting, kobj);
> +}
> +
> +static inline struct tlmi_attr_setting *to_tlmi_attr_setting(struct 
> kobject *kobj)
> +{
> +	return container_of(kobj, struct tlmi_attr_setting, kobj);
> +}
> +
>  /* Convert BIOS WMI error string to suitable error code */
>  static int tlmi_errstr_to_err(const char *errstr)
>  {
> -- 
> 2.39.2

Looks good to me. Let me know if you want this tested on Lenovo HW and I'll do a build with this in - but it looks very uncontroversial :)

Reviewed-by Mark Pearson <mpearson-lenovo@squebbb.ca>

Re: [PATCH 1/1] platform/x86: think-lmi: Convert container_of() macros to static inline

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2531 bytes --]

On Fri, 12 Apr 2024, Mark Pearson wrote:

> Thanks Ilpo,
> 
> On Fri, Apr 12, 2024, at 9:09 AM, Ilpo Järvinen wrote:
> > The macros to_tlmi_pwd_setting() and to_tlmi_attr_setting() are fragile
> > because they expect the variable name to be 'kobj', otherwise the build
> > will fail because container_of()'s 3rd parameter (member) is taken from
> > the parameter given to the macro.
> >
> > While at it, move them into a more logical place.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >  drivers/platform/x86/think-lmi.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/think-lmi.c 
> > b/drivers/platform/x86/think-lmi.c
> > index 9345316b45db..0f2264bb7577 100644
> > --- a/drivers/platform/x86/think-lmi.c
> > +++ b/drivers/platform/x86/think-lmi.c
> > @@ -175,9 +175,6 @@ MODULE_PARM_DESC(debug_support, "Enable debug 
> > command support");
> >  #define TLMI_SMP_PWD BIT(6) /* System Management */
> >  #define TLMI_CERT    BIT(7) /* Certificate Based */
> > 
> > -#define to_tlmi_pwd_setting(kobj)  container_of(kobj, struct 
> > tlmi_pwd_setting, kobj)
> > -#define to_tlmi_attr_setting(kobj)  container_of(kobj, struct 
> > tlmi_attr_setting, kobj)
> > -
> >  static const struct tlmi_err_codes tlmi_errs[] = {
> >  	{"Success", 0},
> >  	{"Not Supported", -EOPNOTSUPP},
> > @@ -198,6 +195,16 @@ static struct think_lmi tlmi_priv;
> >  static const struct class *fw_attr_class;
> >  static DEFINE_MUTEX(tlmi_mutex);
> > 
> > +static inline struct tlmi_pwd_setting *to_tlmi_pwd_setting(struct 
> > kobject *kobj)
> > +{
> > +	return container_of(kobj, struct tlmi_pwd_setting, kobj);
> > +}
> > +
> > +static inline struct tlmi_attr_setting *to_tlmi_attr_setting(struct 
> > kobject *kobj)
> > +{
> > +	return container_of(kobj, struct tlmi_attr_setting, kobj);
> > +}
> > +
> >  /* Convert BIOS WMI error string to suitable error code */
> >  static int tlmi_errstr_to_err(const char *errstr)
> >  {
> > -- 
> > 2.39.2
> 
> Looks good to me. Let me know if you want this tested on Lenovo HW and 
> I'll do a build with this in - but it looks very uncontroversial :) 
> 
> Reviewed-by Mark Pearson <mpearson-lenovo@squebbb.ca>

Yes, pretty uncontroversial so probably not worth the effort to test on 
HW. The compile done by lkp is good enough to capture stupid mistakes 
(which it already passed here internally before I even sent these out).

-- 
 i.

Re: [PATCH 1/1] platform/x86: think-lmi: Convert container_of() macros to static inline

[Hans de Goede]

Hi,

On 4/12/24 3:09 PM, Ilpo Järvinen wrote:
> The macros to_tlmi_pwd_setting() and to_tlmi_attr_setting() are fragile
> because they expect the variable name to be 'kobj', otherwise the build
> will fail because container_of()'s 3rd parameter (member) is taken from
> the parameter given to the macro.
> 
> While at it, move them into a more logical place.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans



> ---
>  drivers/platform/x86/think-lmi.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
> index 9345316b45db..0f2264bb7577 100644
> --- a/drivers/platform/x86/think-lmi.c
> +++ b/drivers/platform/x86/think-lmi.c
> @@ -175,9 +175,6 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support");
>  #define TLMI_SMP_PWD BIT(6) /* System Management */
>  #define TLMI_CERT    BIT(7) /* Certificate Based */
>  
> -#define to_tlmi_pwd_setting(kobj)  container_of(kobj, struct tlmi_pwd_setting, kobj)
> -#define to_tlmi_attr_setting(kobj)  container_of(kobj, struct tlmi_attr_setting, kobj)
> -
>  static const struct tlmi_err_codes tlmi_errs[] = {
>  	{"Success", 0},
>  	{"Not Supported", -EOPNOTSUPP},
> @@ -198,6 +195,16 @@ static struct think_lmi tlmi_priv;
>  static const struct class *fw_attr_class;
>  static DEFINE_MUTEX(tlmi_mutex);
>  
> +static inline struct tlmi_pwd_setting *to_tlmi_pwd_setting(struct kobject *kobj)
> +{
> +	return container_of(kobj, struct tlmi_pwd_setting, kobj);
> +}
> +
> +static inline struct tlmi_attr_setting *to_tlmi_attr_setting(struct kobject *kobj)
> +{
> +	return container_of(kobj, struct tlmi_attr_setting, kobj);
> +}
> +
>  /* Convert BIOS WMI error string to suitable error code */
>  static int tlmi_errstr_to_err(const char *errstr)
>  {