RE: [PATCH 2/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.

> Subject: Re: [PATCH 2/3] serial: 8250_em: Use dev_err_probe()
> 
> Hi Biju,
> 
> On Thu, Feb 9, 2023 at 2:26 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > This patch simplify 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().
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/drivers/tty/serial/8250/8250_em.c
> > +++ b/drivers/tty/serial/8250/8250_em.c
> > @@ -13,7 +13,6 @@
> >  #include <linux/serial_reg.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/clk.h>
> > -#include <linux/slab.h>
> >
> >  #include "8250.h"
> >
> > @@ -79,6 +78,7 @@ static void serial8250_em_serial_dl_write(struct
> > uart_8250_port *up, int value)  static int serial8250_em_probe(struct
> > platform_device *pdev)  {
> >         struct serial8250_em_priv *priv;
> > +       struct device *dev = &pdev->dev;
> >         struct uart_8250_port up;
> >         struct resource *regs;
> >         int irq, ret;
> > @@ -88,27 +88,23 @@ static int serial8250_em_probe(struct platform_device
> *pdev)
> >                 return irq;
> >
> >         regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -       if (!regs) {
> > -               dev_err(&pdev->dev, "missing registers\n");
> > -               return -EINVAL;
> > -       }
> > +       if (!regs)
> > +               return dev_err_probe(dev, -EINVAL, "missing
> > + registers\n");
> >
> > -       priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >         if (!priv)
> >                 return -ENOMEM;
> >
> >         priv->sclk = devm_clk_get(&pdev->dev, "sclk");
> 
> One missed opportunity to use "dev".

Agreed.

> And to use the new devm_clk_get_enabled() ;-)

OK, will do as it will simplify clk handling in probe()

Cheers,
Biju

> 
> > -       if (IS_ERR(priv->sclk)) {
> > -               dev_err(&pdev->dev, "unable to get clock\n");
> > -               return PTR_ERR(priv->sclk);
> > -       }
> > +       if (IS_ERR(priv->sclk))
> > +               return dev_err_probe(dev, PTR_ERR(priv->sclk), "unable
> > + to get clock\n");
> >
> >         memset(&up, 0, sizeof(up));
> >         up.port.mapbase = regs->start;
> >         up.port.irq = irq;
> >         up.port.type = PORT_UNKNOWN;
> >         up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_IOREMAP;
> > -       up.port.dev = &pdev->dev;
> > +       up.port.dev = dev;
> >         up.port.private_data = priv;
> >
> >         clk_prepare_enable(priv->sclk); @@ -122,9 +118,8 @@ static int
> > serial8250_em_probe(struct platform_device *pdev)
> >
> >         ret = serial8250_register_8250_port(&up);
> >         if (ret < 0) {
> > -               dev_err(&pdev->dev, "unable to register 8250 port\n");
> >                 clk_disable_unprepare(priv->sclk);
> > -               return ret;
> > +               return dev_err_probe(dev, ret, "unable to register
> > + 8250 port\n");
> >         }
> >
> >         priv->line = ret;
> 
> As the patch is correct:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> 
> 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]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux