Re: [PATCH v1] hwmon: drivetemp: support to be a platform driver for thermal_of

[Guenter Roeck <linux@roeck-us.net>]

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
> 
>