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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
> > ---
> > 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 <[email protected]>
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.
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 <[email protected]>
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)
> {