[PATCH v2 2/6] pwm: pxa: add optional reset control

Guodong Xu posted 6 patches 9 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 2/6] pwm: pxa: add optional reset control
Posted by Guodong Xu 9 months, 3 weeks ago
Support optional reset control for the PWM PXA driver.

During the probe, it acquires the reset controller using
devm_reset_control_get_optional_exclusive_deasserted() to get and deassert
the reset controller to enable the PWM channel.

Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v2: No change

drivers/pwm/pwm-pxa.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index 430bd6a709e9..dd9c6af0f672 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -25,6 +25,7 @@
 #include <linux/io.h>
 #include <linux/pwm.h>
 #include <linux/of.h>
+#include <linux/reset.h>
 
 #include <asm/div64.h>
 
@@ -49,10 +50,10 @@ MODULE_DEVICE_TABLE(platform, pwm_id_table);
 #define PWMDCR_FD	(1 << 10)
 
 struct pxa_pwm_chip {
-	struct device	*dev;
-
-	struct clk	*clk;
-	void __iomem	*mmio_base;
+	struct device		*dev;
+	struct clk		*clk;
+	void __iomem		*mmio_base;
+	struct reset_control	*reset;
 };
 
 static inline struct pxa_pwm_chip *to_pxa_pwm_chip(struct pwm_chip *chip)
@@ -179,6 +180,11 @@ static int pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(pc->clk))
 		return PTR_ERR(pc->clk);
 
+	pc->reset = devm_reset_control_get_optional_exclusive_deasserted(
+			&pdev->dev, NULL);
+	if (IS_ERR(pc->reset))
+		return PTR_ERR(pc->reset);
+
 	chip->ops = &pxa_pwm_ops;
 
 	if (IS_ENABLED(CONFIG_OF))
-- 
2.43.0
Re: [PATCH v2 2/6] pwm: pxa: add optional reset control
Posted by Uwe Kleine-König 9 months, 3 weeks ago
Hello,

On Sun, Apr 20, 2025 at 03:02:47PM +0800, Guodong Xu wrote:
> @@ -49,10 +50,10 @@ MODULE_DEVICE_TABLE(platform, pwm_id_table);
>  #define PWMDCR_FD	(1 << 10)
>  
>  struct pxa_pwm_chip {
> -	struct device	*dev;
> -
> -	struct clk	*clk;
> -	void __iomem	*mmio_base;
> +	struct device		*dev;
> +	struct clk		*clk;
> +	void __iomem		*mmio_base;
> +	struct reset_control	*reset;

Changes like these are the reason I prefer to not align the member
names. Luckily reset is a write only variable and you can just drop this
hunk if you use a local variable for it in pwm_probe() below.

>  };
>  
>  static inline struct pxa_pwm_chip *to_pxa_pwm_chip(struct pwm_chip *chip)
> @@ -179,6 +180,11 @@ static int pwm_probe(struct platform_device *pdev)
>  	if (IS_ERR(pc->clk))
>  		return PTR_ERR(pc->clk);
>  
> +	pc->reset = devm_reset_control_get_optional_exclusive_deasserted(
> +			&pdev->dev, NULL);
> +	if (IS_ERR(pc->reset))
> +		return PTR_ERR(pc->reset);
> +
>  	chip->ops = &pxa_pwm_ops;
>  
>  	if (IS_ENABLED(CONFIG_OF))

Best regards
Uwe
Re: [PATCH v2 2/6] pwm: pxa: add optional reset control
Posted by Guodong Xu 9 months, 2 weeks ago
On Thu, Apr 24, 2025 at 3:30 PM Uwe Kleine-König <ukleinek@kernel.org> wrote:
>
> Hello,
>
> On Sun, Apr 20, 2025 at 03:02:47PM +0800, Guodong Xu wrote:
> > @@ -49,10 +50,10 @@ MODULE_DEVICE_TABLE(platform, pwm_id_table);
> >  #define PWMDCR_FD    (1 << 10)
> >
> >  struct pxa_pwm_chip {
> > -     struct device   *dev;
> > -
> > -     struct clk      *clk;
> > -     void __iomem    *mmio_base;
> > +     struct device           *dev;
> > +     struct clk              *clk;
> > +     void __iomem            *mmio_base;
> > +     struct reset_control    *reset;
>
> Changes like these are the reason I prefer to not align the member
> names. Luckily reset is a write only variable and you can just drop this
> hunk if you use a local variable for it in pwm_probe() below.
>

Thanks Uwe for the suggestion, I agree.
I will update it in v3 by dropping this hunk and using a local variable for
reset in pwm_probe() as you recommended.

BR,
Guodong

> >  };
> >
> >  static inline struct pxa_pwm_chip *to_pxa_pwm_chip(struct pwm_chip *chip)
> > @@ -179,6 +180,11 @@ static int pwm_probe(struct platform_device *pdev)
> >       if (IS_ERR(pc->clk))
> >               return PTR_ERR(pc->clk);
> >
> > +     pc->reset = devm_reset_control_get_optional_exclusive_deasserted(
> > +                     &pdev->dev, NULL);
> > +     if (IS_ERR(pc->reset))
> > +             return PTR_ERR(pc->reset);
> > +
> >       chip->ops = &pxa_pwm_ops;
> >
> >       if (IS_ENABLED(CONFIG_OF))
>
> Best regards
> Uwe