Re: [PATCH net-next 1/8] net: dsa: ocelot: use devres in ocelot_ext_probe()

[Colin Foster <colin.foster@in-advantage.com>]

Reviewed-by: Colin Foster <colin.foster@in-advantage.com>

On Thu, May 30, 2024 at 07:33:26PM +0300, Vladimir Oltean wrote:
> Russell King suggested that felix_vsc9959, seville_vsc9953 and
> ocelot_ext have a large portion of duplicated init and teardown code,
> which could be made common [1]. The teardown code could even be
> simplified away if we made use of devres, something which is used here
> and there in the felix driver, just not very consistently.
> 
> [1] https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/
> 
> Prepare the ground in the ocelot_ext driver, by allocating the data
> structures using devres and deleting the kfree() calls. This also
> deletes the "Failed to allocate ..." message, since memory allocation
> errors are extremely loud anyway, and it's hard to miss them.
> 
> Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/ocelot/ocelot_ext.c | 24 +++++-------------------
>  1 file changed, 5 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
> index a8927dc7aca4..c893f3ee238b 100644
> --- a/drivers/net/dsa/ocelot/ocelot_ext.c
> +++ b/drivers/net/dsa/ocelot/ocelot_ext.c
> @@ -71,7 +71,7 @@ static int ocelot_ext_probe(struct platform_device *pdev)
>  	struct felix *felix;
>  	int err;
>  
> -	felix = kzalloc(sizeof(*felix), GFP_KERNEL);
> +	felix = devm_kzalloc(dev, sizeof(*felix), GFP_KERNEL);
>  	if (!felix)
>  		return -ENOMEM;
>  
> @@ -84,12 +84,9 @@ static int ocelot_ext_probe(struct platform_device *pdev)
>  
>  	felix->info = &vsc7512_info;
>  
> -	ds = kzalloc(sizeof(*ds), GFP_KERNEL);
> -	if (!ds) {
> -		err = -ENOMEM;
> -		dev_err_probe(dev, err, "Failed to allocate DSA switch\n");
> -		goto err_free_felix;
> -	}
> +	ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
> +	if (!ds)
> +		return -ENOMEM;
>  
>  	ds->dev = dev;
>  	ds->num_ports = felix->info->num_ports;
> @@ -102,17 +99,9 @@ static int ocelot_ext_probe(struct platform_device *pdev)
>  	felix->tag_proto = DSA_TAG_PROTO_OCELOT;
>  
>  	err = dsa_register_switch(ds);
> -	if (err) {
> +	if (err)
>  		dev_err_probe(dev, err, "Failed to register DSA switch\n");
> -		goto err_free_ds;
> -	}
> -
> -	return 0;
>  
> -err_free_ds:
> -	kfree(ds);
> -err_free_felix:
> -	kfree(felix);
>  	return err;
>  }
>  
> @@ -124,9 +113,6 @@ static void ocelot_ext_remove(struct platform_device *pdev)
>  		return;
>  
>  	dsa_unregister_switch(felix->ds);
> -
> -	kfree(felix->ds);
> -	kfree(felix);
>  }
>  
>  static void ocelot_ext_shutdown(struct platform_device *pdev)
> -- 
> 2.34.1
>