regulator: core: Set the fwnode for regulator_dev

[PATCH] regulator: core: Set the fwnode for regulator_dev

Posted by Yoann Congal 1 year, 5 months ago

From: Yoann Congal <yoann.congal@smile.fr>

After commit 3fb16866b51d ("driver core: fw_devlink: Make cycle
detection more robust"), fw_devlink prints an error when consumer
devices don't have their fwnode set. This used to be ignored silently.

Set the fwnode in regulator_dev so fw_devlink can find them and properly
track their dependencies.

This fixes errors like this:
  stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link (0x180) with 2-0033

NB: This is similar to the commit a26cc2934331 ("drm/mipi-dsi: Set the
fwnode for mipi_dsi_device") applied to the regulator framework.

Cc: Saravana Kannan <saravanak@google.com>
Cc: stable@vger.kernel.org # 5.13.x
Fixes: 63c7c9e16c8e ("regulator: core: Get and put regulator of_node")
Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
---
 drivers/regulator/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 844e9587a880..f05f873021d2 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5656,7 +5656,7 @@ regulator_register(struct device *dev,
 		dangling_of_gpiod = true;
 	if (!init_data) {
 		init_data = config->init_data;
-		rdev->dev.of_node = of_node_get(config->of_node);
+		device_set_node(&rdev->dev, of_fwnode_handle(config->of_node));
 	}
 
 	ww_mutex_init(&rdev->mutex, &regulator_ww_class);

Re: [PATCH] regulator: core: Set the fwnode for regulator_dev

Posted by Saravana Kannan 1 year, 5 months ago

On Thu, Jul 11, 2024 at 2:25 PM Yoann Congal <yoann.congal@smile.fr> wrote:
>
> From: Yoann Congal <yoann.congal@smile.fr>
>
> After commit 3fb16866b51d ("driver core: fw_devlink: Make cycle
> detection more robust"), fw_devlink prints an error when consumer
> devices don't have their fwnode set. This used to be ignored silently.
>
> Set the fwnode in regulator_dev so fw_devlink can find them and properly
> track their dependencies.
>
> This fixes errors like this:
>   stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link (0x180) with 2-0033
>
> NB: This is similar to the commit a26cc2934331 ("drm/mipi-dsi: Set the
> fwnode for mipi_dsi_device") applied to the regulator framework.
>
> Cc: Saravana Kannan <saravanak@google.com>
> Cc: stable@vger.kernel.org # 5.13.x
> Fixes: 63c7c9e16c8e ("regulator: core: Get and put regulator of_node")
> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>

The change is valid but the problem is that managed device links don't
work correctly for "class" type devices. So, we can't merge this
change yet.

So, for now, we shouldn't land this. Are you not seeing probing issues
when you do this? Or significant changes/delays in probing?

-Saravana

> ---
>  drivers/regulator/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 844e9587a880..f05f873021d2 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -5656,7 +5656,7 @@ regulator_register(struct device *dev,
>                 dangling_of_gpiod = true;
>         if (!init_data) {
>                 init_data = config->init_data;
> -               rdev->dev.of_node = of_node_get(config->of_node);
> +               device_set_node(&rdev->dev, of_fwnode_handle(config->of_node));
>         }
>
>         ww_mutex_init(&rdev->mutex, &regulator_ww_class);

Re: [PATCH] regulator: core: Set the fwnode for regulator_dev

Posted by Yoann Congal 1 year, 5 months ago

Le 11/07/2024 à 23:29, Saravana Kannan a écrit :
> On Thu, Jul 11, 2024 at 2:25 PM Yoann Congal <yoann.congal@smile.fr> wrote:
>>
>> From: Yoann Congal <yoann.congal@smile.fr>
>>
>> After commit 3fb16866b51d ("driver core: fw_devlink: Make cycle
>> detection more robust"), fw_devlink prints an error when consumer
>> devices don't have their fwnode set. This used to be ignored silently.
>>
>> Set the fwnode in regulator_dev so fw_devlink can find them and properly
>> track their dependencies.
>>
>> This fixes errors like this:
>>   stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link (0x180) with 2-0033
>>
>> NB: This is similar to the commit a26cc2934331 ("drm/mipi-dsi: Set the
>> fwnode for mipi_dsi_device") applied to the regulator framework.
>>
>> Cc: Saravana Kannan <saravanak@google.com>
>> Cc: stable@vger.kernel.org # 5.13.x
>> Fixes: 63c7c9e16c8e ("regulator: core: Get and put regulator of_node")
>> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> 
> The change is valid but the problem is that managed device links don't
> work correctly for "class" type devices. So, we can't merge this
> change yet.

I was totally unaware of that.

> So, for now, we shouldn't land this. Are you not seeing probing issues
> when you do this? Or significant changes/delays in probing?

I just checked my logs : No probing issue nor noticeable delay.

Thanks for your answer, I'll drop this for now.

Regards,
-- 
Yoann Congal
Smile ECS - Tech Expert

Re: [PATCH] regulator: core: Set the fwnode for regulator_dev

Posted by Saravana Kannan 1 year, 5 months ago

On Thu, Jul 11, 2024 at 3:19 PM Yoann Congal <yoann.congal@smile.fr> wrote:
>
> Le 11/07/2024 à 23:29, Saravana Kannan a écrit :
> > On Thu, Jul 11, 2024 at 2:25 PM Yoann Congal <yoann.congal@smile.fr> wrote:
> >>
> >> From: Yoann Congal <yoann.congal@smile.fr>
> >>
> >> After commit 3fb16866b51d ("driver core: fw_devlink: Make cycle
> >> detection more robust"), fw_devlink prints an error when consumer
> >> devices don't have their fwnode set. This used to be ignored silently.
> >>
> >> Set the fwnode in regulator_dev so fw_devlink can find them and properly
> >> track their dependencies.
> >>
> >> This fixes errors like this:
> >>   stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link (0x180) with 2-0033
> >>
> >> NB: This is similar to the commit a26cc2934331 ("drm/mipi-dsi: Set the
> >> fwnode for mipi_dsi_device") applied to the regulator framework.
> >>
> >> Cc: Saravana Kannan <saravanak@google.com>
> >> Cc: stable@vger.kernel.org # 5.13.x
> >> Fixes: 63c7c9e16c8e ("regulator: core: Get and put regulator of_node")
> >> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> >
> > The change is valid but the problem is that managed device links don't
> > work correctly for "class" type devices. So, we can't merge this
> > change yet.
>
> I was totally unaware of that.

Yeah, it's not obvious. The TL;DR of the problem is that your consumer
will end up waiting for ever for the supplier to probe but a "class"
device never probes.

And the fix is non-trivial. So, it's been on my todo list for a bit.

> > So, for now, we shouldn't land this. Are you not seeing probing issues
> > when you do this? Or significant changes/delays in probing?
>
> I just checked my logs : No probing issue nor noticeable delay.

With your change, I'm guessing you see more links under /sys/class/devlink

Are all the consumers in these new links "class" type consumers too
(like other regulator devices, etc)?

> Thanks for your answer, I'll drop this for now.

Thanks.

-Saravana