linux-kernel - Re: [PATCH v1] hwmon: drivetemp: support to be a platform driver for thermal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2b1e7e29-daa6-4f24-9fad-7ebe8b87a5fe@roeck-us.net>
Date:   Thu, 16 Mar 2023 11:17:32 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Phinex Hung <phinex@...ltek.com>
Cc:     "jdelvare@...e.com" <jdelvare@...e.com>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] hwmon: drivetemp: support to be a platform driver for
 thermal_of

On Thu, Mar 16, 2023 at 02:17:34PM +0000, Phinex Hung wrote:
> 
> 
> On 3/16/23 22:48, Guenter Roeck wrote:
> 
> 
> >That needs to be in the hwmon core. We can not change the device pointer
> >passed to hwmon_device_register_with_info() because that determines the
> >lifetime of the hwmon device.
> 
> 
> >Guenter
> 
> Do you mean something like below?
> 

Yes, except of course for the bugs (see below). That is much less
than perfect, of course, since we'd really want the device node
for the drive, not the controller, but it might be the best we can do.

> Or is it reasonable that we just match a specific compatible string and assign the device node to the original dev->parent used in drivetemp_add function ?
> 

We can't add anything to the parent device node since we don't own it.
Also, I don't know if devicetree maintainers would accept the concept
of "virtual" device nodes (and I don't know how device nodes for drives
would or should look like either).

> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index 33edb5c02f7d..a76beeada33e 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -757,6 +757,7 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
>         struct hwmon_device *hwdev;
>         const char *label;
>         struct device *hdev;
> +       struct device *tedv = dev;

			tdev =

>         int i, err, id;
> 
>         /* Complain about invalid characters in hwmon name attribute */
> @@ -826,7 +827,9 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
>         hwdev->name = name;
>         hdev->class = &hwmon_class;
>         hdev->parent = dev;
> -       hdev->of_node = dev ? dev->of_node : NULL;
> +       while(!tdev->of_node)

	  while (tdev && !tdev->of_node)

> +               tdev = tdev->parent;
> +       hdev->of_node = tdev ? tdev->of_node : NULL;
>         hwdev->chip = chip;
>         dev_set_drvdata(hdev, drvdata);
> @@ -838,7 +841,7 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
> 
>         INIT_LIST_HEAD(&hwdev->tzdata);
> 
> -       if (dev && dev->of_node && chip && chip->ops->read &&
> +       if (tdev && tdev->of_node && chip && chip->ops->read &&

This could probably be simplified to
	  if (hdev->of_node && chip && ..

>             chip->info[0]->type == hwmon_chip &&
>             (chip->info[0]->config[0] & HWMON_C_REGISTER_TZ)) {
>                 err = hwmon_thermal_register_sensors(hdev);
> 
> Regards,
> Phinex
> 
>