| Message ID | 20251009162445.701589-1-martyn.welch@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | pwm: rz-mtu3: Share parent device node to MTU3 PWM | expand |
Hello, [adding maintainers of drivers/of and Krzysztof to Cc:] On Thu, Oct 09, 2025 at 05:24:44PM +0100, Martyn Welch wrote: > The PWM currently functions, however if we try to utilise the pwn in a > device tree, for example as a pwm-backlight: > > lcd_bl: backlight { > compatible = "pwm-backlight"; > pwms = <&mtu3 3 833333>; > ... > > This fails: > > [ 15.603948] platform backlight: deferred probe pending: pwm-backlight: unable to request PWM > > The PWM driver forms part of the Renesas Multi-Function Timer Pulse Unit > 3. The PWM does not have a DT node of it's own. Share the DT node of the > parent MFD device, so that the PWM channels can be referenced via phandles. > > Co-developed-by: Sebastian Reichel <sebastian.reichel@collabora.com> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > Signed-off-by: Martyn Welch <martyn.welch@collabora.com> > --- > drivers/pwm/pwm-rz-mtu3.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/pwm/pwm-rz-mtu3.c b/drivers/pwm/pwm-rz-mtu3.c > index ab39bd37edafc..5825875fa0128 100644 > --- a/drivers/pwm/pwm-rz-mtu3.c > +++ b/drivers/pwm/pwm-rz-mtu3.c > @@ -523,6 +523,12 @@ static int rz_mtu3_pwm_probe(struct platform_device *pdev) > if (ret < 0) > return ret; > > + /* > + * There is only one DT node, get it from the parent MFD device, so > + * that the PWM channels can be referenced via phandles > + */ > + dev->of_node = dev->parent->of_node; > + I (very quickly) talked to Krzysztof about this. He said that of_node_get() should probably be used here. I wonder if device_add_of_node() is the right function to use (which uses of_node_get(), also handles fwnode and implements some safeguards). > chip->ops = &rz_mtu3_pwm_ops; > ret = devm_pwmchip_add(&pdev->dev, chip); > if (ret) Best regards Uwe
On 21/10/2025 12:19, Uwe Kleine-König wrote: >> diff --git a/drivers/pwm/pwm-rz-mtu3.c b/drivers/pwm/pwm-rz-mtu3.c >> index ab39bd37edafc..5825875fa0128 100644 >> --- a/drivers/pwm/pwm-rz-mtu3.c >> +++ b/drivers/pwm/pwm-rz-mtu3.c >> @@ -523,6 +523,12 @@ static int rz_mtu3_pwm_probe(struct platform_device *pdev) >> if (ret < 0) >> return ret; >> >> + /* >> + * There is only one DT node, get it from the parent MFD device, so >> + * that the PWM channels can be referenced via phandles >> + */ >> + dev->of_node = dev->parent->of_node; >> + > > I (very quickly) talked to Krzysztof about this. He said that > of_node_get() should probably be used here. I wonder if > device_add_of_node() is the right function to use (which uses > of_node_get(), also handles fwnode and implements some safeguards). I am not so sure about device_add_of_node(). You do not need to get_device(), because reference is already hold. Although setting dev->fwnode might make sense... But, not that important I think, works with me. Best regards, Krzysztof
Hello, On Tue, Oct 21, 2025 at 12:47:52PM +0200, Krzysztof Kozlowski wrote: > On 21/10/2025 12:19, Uwe Kleine-König wrote: > >> diff --git a/drivers/pwm/pwm-rz-mtu3.c b/drivers/pwm/pwm-rz-mtu3.c > >> index ab39bd37edafc..5825875fa0128 100644 > >> --- a/drivers/pwm/pwm-rz-mtu3.c > >> +++ b/drivers/pwm/pwm-rz-mtu3.c > >> @@ -523,6 +523,12 @@ static int rz_mtu3_pwm_probe(struct platform_device *pdev) > >> if (ret < 0) > >> return ret; > >> > >> + /* > >> + * There is only one DT node, get it from the parent MFD device, so > >> + * that the PWM channels can be referenced via phandles > >> + */ > >> + dev->of_node = dev->parent->of_node; > >> + > > > > I (very quickly) talked to Krzysztof about this. He said that > > of_node_get() should probably be used here. I wonder if > > device_add_of_node() is the right function to use (which uses > > of_node_get(), also handles fwnode and implements some safeguards). > > > I am not so sure about device_add_of_node(). You do not need to > get_device(), because reference is already hold. Although setting > dev->fwnode might make sense... But, not that important I think, works > with me. Note that device_add_of_node() only holds the get_device() reference temporarily as it calls put_device() before returning. So that's only to assert that the device doesn't disappear in-flight. That "locking" might not be needed here, but it also doesn't do any harm (apart from a minor runtime overhead). Best regards Uwe
diff --git a/drivers/pwm/pwm-rz-mtu3.c b/drivers/pwm/pwm-rz-mtu3.c index ab39bd37edafc..5825875fa0128 100644 --- a/drivers/pwm/pwm-rz-mtu3.c +++ b/drivers/pwm/pwm-rz-mtu3.c @@ -523,6 +523,12 @@ static int rz_mtu3_pwm_probe(struct platform_device *pdev) if (ret < 0) return ret; + /* + * There is only one DT node, get it from the parent MFD device, so + * that the PWM channels can be referenced via phandles + */ + dev->of_node = dev->parent->of_node; + chip->ops = &rz_mtu3_pwm_ops; ret = devm_pwmchip_add(&pdev->dev, chip); if (ret)
The PWM currently functions, however if we try to utilise the pwn in a device tree, for example as a pwm-backlight: lcd_bl: backlight { compatible = "pwm-backlight"; pwms = <&mtu3 3 833333>; ... This fails: [ 15.603948] platform backlight: deferred probe pending: pwm-backlight: unable to request PWM The PWM driver forms part of the Renesas Multi-Function Timer Pulse Unit 3. The PWM does not have a DT node of it's own. Share the DT node of the parent MFD device, so that the PWM channels can be referenced via phandles. Co-developed-by: Sebastian Reichel <sebastian.reichel@collabora.com> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> Signed-off-by: Martyn Welch <martyn.welch@collabora.com> --- drivers/pwm/pwm-rz-mtu3.c | 6 ++++++ 1 file changed, 6 insertions(+)