Re: [PATCH v3 3/7] pwm: sun4i: Add an optional probe for bus clock

[ClÃment PÃron]

Hi Uwe,

On Tue, 5 Nov 2019 at 14:57, Uwe Kleine-KÃnig
<u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
>
> On Tue, Nov 05, 2019 at 02:14:52PM +0100, ClÃment PÃron wrote:
> > From: Jernej Skrabec <jernej.skrabec@xxxxxxxx>
> >
> > 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@xxxxxxxx>
> > Signed-off-by: ClÃment PÃron <peron.clem@xxxxxxxxx>
> > ---
> >  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/  |