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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, Feb 10, 2023 at 03:41:38PM +0000, Biju Das 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().

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v1->v2:
>  * replaced devm_clk_get->devm_clk_get_enabled() and updated clk
>    handling in probe().
>  * Added Rb tag from Geert.
> ---
>  drivers/tty/serial/8250/8250_em.c | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_em.c b/drivers/tty/serial/8250/8250_em.c
> index f8e99995eee9..b78c74755735 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,30 +88,25 @@ 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);
> -	}
> +	priv->sclk = devm_clk_get_enabled(dev, "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);
>  	up.port.uartclk = clk_get_rate(priv->sclk);
>  
>  	up.port.iotype = UPIO_MEM32;
> @@ -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);
> -		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);
> -- 
> 2.25.1
> 
> 

-- 
With Best Regards,
Andy Shevchenko