| Message ID | 20230209132630.194947-3-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add Identification and 64 bit fifo support to Renesas RZ/V2M 16750 UART | expand |
Hi Biju, On Thu, Feb 9, 2023 at 2:26 PM Biju Das <biju.das.jz@bp.renesas.com> 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@bp.renesas.com> 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". And to use the new devm_clk_get_enabled() ;-) > - 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@glider.be> 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
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@bp.renesas.com> 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@bp.renesas.com> > > 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@glider.be> > > 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
diff --git a/drivers/tty/serial/8250/8250_em.c b/drivers/tty/serial/8250/8250_em.c index f8e99995eee9..3a45aa066d3d 100644 --- 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"); - 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;
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@bp.renesas.com> --- drivers/tty/serial/8250/8250_em.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-)