| Message ID | 20191105131456.32400-4-peron.clem@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for H6 PWM | expand |
On Tue, Nov 05, 2019 at 02:14:52PM +0100, Clément Péron wrote: > From: Jernej Skrabec <jernej.skrabec@siol.net> > > H6 PWM core needs bus clock to be enabled in order to work. > > Add an optional probe for it and a fallback for previous > bindings without name on module clock. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > Signed-off-by: Clément Péron <peron.clem@gmail.com> > --- > drivers/pwm/pwm-sun4i.c | 45 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 43 insertions(+), 2 deletions(-) > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c > index 9ba83769a478..54e19fa56a4e 100644 > --- a/drivers/pwm/pwm-sun4i.c > +++ b/drivers/pwm/pwm-sun4i.c > @@ -78,6 +78,7 @@ struct sun4i_pwm_data { > > struct sun4i_pwm_chip { > struct pwm_chip chip; > + struct clk *bus_clk; > struct clk *clk; > struct reset_control *rst; > void __iomem *base; > @@ -363,9 +364,35 @@ static int sun4i_pwm_probe(struct platform_device *pdev) > if (IS_ERR(pwm->base)) > return PTR_ERR(pwm->base); > > - pwm->clk = devm_clk_get(&pdev->dev, NULL); > - if (IS_ERR(pwm->clk)) > + /* Get all clocks and reset line */ > + pwm->clk = devm_clk_get_optional(&pdev->dev, "mod"); > + if (IS_ERR(pwm->clk)) { > + dev_err(&pdev->dev, "get clock failed %ld\n", > + PTR_ERR(pwm->clk)); Please only print this message if PTR_ERR(pwm->clk) != -EPROBE_DEFER. You might also want to make use of commit 57f5677e535b ("printf: add support for printing symbolic error names") and use dev_err(&pdev->dev, "get clock failed: %pe\n", pwm->clk); Other than that the patch is fine for me. Best regards Uwe
Hi Uwe, On Tue, 5 Nov 2019 at 14:57, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > On Tue, Nov 05, 2019 at 02:14:52PM +0100, Clément Péron wrote: > > From: Jernej Skrabec <jernej.skrabec@siol.net> > > > > H6 PWM core needs bus clock to be enabled in order to work. > > > > Add an optional probe for it and a fallback for previous > > bindings without name on module clock. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > > --- > > drivers/pwm/pwm-sun4i.c | 45 +++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 43 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c > > index 9ba83769a478..54e19fa56a4e 100644 > > --- a/drivers/pwm/pwm-sun4i.c > > +++ b/drivers/pwm/pwm-sun4i.c > > @@ -78,6 +78,7 @@ struct sun4i_pwm_data { > > > > struct sun4i_pwm_chip { > > struct pwm_chip chip; > > + struct clk *bus_clk; > > struct clk *clk; > > struct reset_control *rst; > > void __iomem *base; > > @@ -363,9 +364,35 @@ static int sun4i_pwm_probe(struct platform_device *pdev) > > if (IS_ERR(pwm->base)) > > return PTR_ERR(pwm->base); > > > > - pwm->clk = devm_clk_get(&pdev->dev, NULL); > > - if (IS_ERR(pwm->clk)) > > + /* Get all clocks and reset line */ > > + pwm->clk = devm_clk_get_optional(&pdev->dev, "mod"); > > + if (IS_ERR(pwm->clk)) { > > + dev_err(&pdev->dev, "get clock failed %ld\n", > > + PTR_ERR(pwm->clk)); > > Please only print this message if PTR_ERR(pwm->clk) != -EPROBE_DEFER. I didn't do it because the sunxi clock can't be compiled as module but indeed i can change it so it's more generic. > You might also want to make use of commit 57f5677e535b ("printf: add > support for printing symbolic error names") and use > > dev_err(&pdev->dev, "get clock failed: %pe\n", pwm->clk); Thanks I will change it, Regards, Clément > > Other than that the patch is fine for me. > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | http://www.pengutronix.de/ |
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 9ba83769a478..54e19fa56a4e 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -78,6 +78,7 @@ struct sun4i_pwm_data { struct sun4i_pwm_chip { struct pwm_chip chip; + struct clk *bus_clk; struct clk *clk; struct reset_control *rst; void __iomem *base; @@ -363,9 +364,35 @@ static int sun4i_pwm_probe(struct platform_device *pdev) if (IS_ERR(pwm->base)) return PTR_ERR(pwm->base); - pwm->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(pwm->clk)) + /* Get all clocks and reset line */ + pwm->clk = devm_clk_get_optional(&pdev->dev, "mod"); + if (IS_ERR(pwm->clk)) { + dev_err(&pdev->dev, "get clock failed %ld\n", + PTR_ERR(pwm->clk)); return PTR_ERR(pwm->clk); + } + + /* + * Fallback for old dtbs with a single clock and no name. + * If a parent has a clock-name called "mod" whereas the + * current node is unnamed the clock reference will be + * incorrectly obtained and will not go into this fallback. + */ + if (!pwm->clk) { + pwm->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(pwm->clk)) { + dev_err(&pdev->dev, "get clock failed %ld\n", + PTR_ERR(pwm->clk)); + return PTR_ERR(pwm->clk); + } + } + + pwm->bus_clk = devm_clk_get_optional(&pdev->dev, "bus"); + if (IS_ERR(pwm->bus_clk)) { + dev_err(&pdev->dev, "get bus_clock failed %ld\n", + PTR_ERR(pwm->bus_clk)); + return PTR_ERR(pwm->bus_clk); + } pwm->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL); if (IS_ERR(pwm->rst)) { @@ -382,6 +409,17 @@ static int sun4i_pwm_probe(struct platform_device *pdev) return ret; } + /* + * We're keeping the bus clock on for the sake of simplicity. + * Actually it only needs to be on for hardware register + * accesses. + */ + ret = clk_prepare_enable(pwm->bus_clk); + if (ret) { + dev_err(&pdev->dev, "Cannot prepare and enable bus_clk\n"); + goto err_bus; + } + pwm->chip.dev = &pdev->dev; pwm->chip.ops = &sun4i_pwm_ops; pwm->chip.base = -1; @@ -402,6 +440,8 @@ static int sun4i_pwm_probe(struct platform_device *pdev) return 0; err_pwm_add: + clk_disable_unprepare(pwm->bus_clk); +err_bus: reset_control_assert(pwm->rst); return ret; @@ -416,6 +456,7 @@ static int sun4i_pwm_remove(struct platform_device *pdev) if (ret) return ret; + clk_disable_unprepare(pwm->bus_clk); reset_control_assert(pwm->rst); return 0;