Re: [PATCH 1/3] rockchip: block: simplify rkmtd driver

[Johan Jonker <jbx6244@gmail.com>]

On 10/18/24 03:30, Heinrich Schuchardt wrote:
> By using blk_create_devicef() instead of blk_create_devicef() the driver
> can be simplified and brought into line with other block device drivers.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  drivers/block/rkmtd.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/block/rkmtd.c b/drivers/block/rkmtd.c
> index c55f052e51b..f84cacd7ead 100644
> --- a/drivers/block/rkmtd.c
> +++ b/drivers/block/rkmtd.c
> @@ -794,36 +794,19 @@ int rkmtd_init_plat(struct udevice *dev)
>  	return 0;
>  }
>  
> -static void rkmtd_blk_kmalloc_release(struct udevice *dev, void *res)
> -{
> -	/* noop */
> -}
> -
>  static int rkmtd_bind(struct udevice *dev)
>  {
>  	struct rkmtd_dev *plat = dev_get_plat(dev);
> -	char dev_name[30], *str;
>  	struct blk_desc *desc;
>  	struct udevice *bdev;
>  	int ret;
>  
> -	snprintf(dev_name, sizeof(dev_name), "%s.%s", dev->name, "blk");
> -

> -	str = devres_alloc(rkmtd_blk_kmalloc_release, strlen(dev_name) + 1, GFP_KERNEL);

Hi Heinrich, Simon,

The function strdup() is not an exact replacement for the devres_alloc() function in relation to a device.
It is in use for memory leak detection / device resource management. 
Not sure what the status of that devres project currently is? Also tracking in general in relation to EFI and blk-class.

https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/blk-uclass.c?ref_type=heads#L739

https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/rkmtd.c?ref_type=heads#L812

Test for this driver are based on work done by Simon.
https://source.denx.de/u-boot/u-boot/-/blob/master/test/dm/host.c?ref_type=heads#L71

This rkmtd driver makes use of devm_kzalloc(). All the memory that rkmtd reserves is freed.
But if I remember well somehow I was not able to free all memory on unbind and was not able to find the source, so I didn't include that last test.
https://elixir.bootlin.com/u-boot/v2024.10/source/test/dm/rkmtd.c#L84

Johan







> -	if (unlikely(!str))
> -		return -ENOMEM;
> -
> -	strcpy(str, dev_name);
> -
> -	ret = blk_create_device(dev, "rkmtd_blk", str, UCLASS_RKMTD,
> -				-1, 512, LBA, &bdev);
> +	ret = blk_create_devicef(dev, "rkmtd_blk", "blk", UCLASS_RKMTD,
> +				 -1, 512, LBA, &bdev);
>  	if (ret) {
> -		free(str);
>  		return log_msg_ret("blk", ret);
>  	}
>  
> -	devres_add(dev, str);
> -
>  	desc = dev_get_uclass_plat(bdev);
>  	sprintf(desc->vendor, "0x%.4x", 0x2207);
>  	memcpy(desc->product, "RKMTD", sizeof("RKMTD"));