Re: [PATCH] scsi: ufs: core: Register the RPMB bus before adding devices to it

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] scsi: ufs: core: bus device name collision in
  ufs_rpmb_probe()
--

commit d7d8b8a6fbcbb4705a5d1c464cb5ca946759c72a
Author: Alexey Charkov <alchark@flipper.net>

scsi: ufs: core: Register the RPMB bus before adding devices to it

This commit registers ufs_rpmb_bus_type during ufshcd-core initialization
and unregisters it on exit so that RPMB region devices can be added to the
bus.

> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index 7566733376480..591e7f2e14740 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c
> @@ -276,5 +276,15 @@ void ufs_rpmb_remove(struct ufs_hba *hba)
>  	dev_info(hba->dev, "All UFS RPMB devices unregistered\n");
>  }
>  
> +int __init ufs_rpmb_bus_init(void)
> +{
> +	return bus_register(&ufs_rpmb_bus_type);
> +}

[Severity: Medium]
With ufs_rpmb_bus_type registered, can ufs_rpmb_probe() fail with -EEXIST
on systems with more than one UFS host controller?

ufs_rpmb_probe() names each device using only the per-controller region
index:

drivers/ufs/core/ufs-rpmb.c:ufs_rpmb_probe() {
	...
		dev_set_name(&ufs_rpmb->dev, "ufs_rpmb%d", region);
	...
		ret = device_register(&ufs_rpmb->dev);
	...
}

Because bus_add_device() creates a symlink named dev_name(dev) in the
shared /sys/bus/ufs_rpmb/devices/ directory, the second controller's
"ufs_rpmb0" collides with the first controller's and leaves OP-TEE RPMB
unavailable on additional controllers.  Including hba->host->host_no in
the device name (as ufs_bsg_probe() does) would keep the names unique
across controllers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ufs-rpmb-bus-v1-1-b07100e53f6c@flipper.net?part=1