RE: [PATCH v2 1/3] serial: 8250_em: Use dev_err_probe()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Friday, February 10, 2023 6:40 PM
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Jiri Slaby
> <jirislaby@xxxxxxxxxx>; linux-serial@xxxxxxxxxxxxxxx; Fabrizio Castro
> <fabrizio.castro.jz@xxxxxxxxxxx>; linux-renesas-soc@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 1/3] serial: 8250_em: Use dev_err_probe()
> 
> Hi Biju,
> 
> On Fri, Feb 10, 2023 at 4:41 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > This patch simplifies probe() function by using dev_err_probe()
> > instead of dev_err in probe().
> >
> > While at it, remove the unused header file slab.h and added a local
> > variable 'dev' to replace '&pdev->dev' in probe().
> >
> > Also replace devm_clk_get->devm_clk_get_enabled and updated the clk
> > handling in probe().
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > ---
> > v1->v2:
> >  * replaced devm_clk_get->devm_clk_get_enabled() and updated clk
> >    handling in probe().
> >  * Added Rb tag from Geert.
> 
> Thanks for the update!
> 
> > --- a/drivers/tty/serial/8250/8250_em.c
> > +++ b/drivers/tty/serial/8250/8250_em.c
> 
> > @@ -121,11 +116,8 @@ static int serial8250_em_probe(struct platform_device
> *pdev)
> >         up.dl_write = serial8250_em_serial_dl_write;
> >
> >         ret = serial8250_register_8250_port(&up);
> > -       if (ret < 0) {
> > -               dev_err(&pdev->dev, "unable to register 8250 port\n");
> > -               clk_disable_unprepare(priv->sclk);
> 
> You forgot to remove the call to clk_disable_unprepare() in
> serial8250_em_remove().
> After that, there is no longer a need to store the clock pointer in priv-
> >sclk.

OK, will fix this in next version.

Cheers,
Biju

> 
> > -               return ret;
> > -       }
> > +       if (ret < 0)
> > +               return dev_err_probe(dev, ret, "unable to register
> > + 8250 port\n");
> >
> >         priv->line = ret;
> >         platform_set_drvdata(pdev, priv);
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
>                                 -- Linus Torvalds




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux