[PATCH v2] firmware: arm_scmi: Delete the meaningless scmi_bus_id.
Cristian Marussi
cristian.marussi at arm.com
Mon Dec 16 00:50:26 PST 2024
On Mon, Dec 16, 2024 at 03:37:45PM +0800, guomin_chen at sina.com wrote:
> From: guomin chen <guomin_chen at sina.com>
>
> Currently, scmi_bus_id is only used to set scmi_dev.id,
> which in turn sets the SCMI device name. After removing
> scmi_bus_id, it is clearer and more meaningful to directly
> use the unique tuple [Parent name,device name, protocol] to
> set the SCMI device name.
>
Hi Guomin,
this same pTCH was NACKed(Rejected) a few days ago:
https://lore.kernel.org/arm-scmi/20241211134505.2218386-1-guomin_chen@sina.com/T/#u
...and you agreed that is not a simplification we can do (not to break
multuple instances...)...
..so why you are posting V2 now ?
Thanks,
Cristian
> Signed-off-by: guomin chen <guomin_chen at sina.com>
> ---
> drivers/firmware/arm_scmi/bus.c | 17 +++--------------
> drivers/firmware/arm_scmi/driver.c | 4 ++--
> 2 files changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
> index 157172a5f2b5..800e8ec9357c 100644
> --- a/drivers/firmware/arm_scmi/bus.c
> +++ b/drivers/firmware/arm_scmi/bus.c
> @@ -20,7 +20,6 @@
> BLOCKING_NOTIFIER_HEAD(scmi_requested_devices_nh);
> EXPORT_SYMBOL_GPL(scmi_requested_devices_nh);
>
> -static DEFINE_IDA(scmi_bus_id);
>
> static DEFINE_IDR(scmi_requested_devices);
> /* Protect access to scmi_requested_devices */
> @@ -341,7 +340,6 @@ static void __scmi_device_destroy(struct scmi_device *scmi_dev)
> if (scmi_dev->protocol_id == SCMI_PROTOCOL_SYSTEM)
> atomic_set(&scmi_syspower_registered, 0);
>
> - ida_free(&scmi_bus_id, scmi_dev->id);
> device_unregister(&scmi_dev->dev);
> }
>
> @@ -349,7 +347,7 @@ static struct scmi_device *
> __scmi_device_create(struct device_node *np, struct device *parent,
> int protocol, const char *name)
> {
> - int id, retval;
> + int retval;
> struct scmi_device *scmi_dev;
>
> /*
> @@ -387,20 +385,13 @@ __scmi_device_create(struct device_node *np, struct device *parent,
> return NULL;
> }
>
> - id = ida_alloc_min(&scmi_bus_id, 1, GFP_KERNEL);
> - if (id < 0) {
> - kfree_const(scmi_dev->name);
> - kfree(scmi_dev);
> - return NULL;
> - }
> -
> - scmi_dev->id = id;
> scmi_dev->protocol_id = protocol;
> scmi_dev->dev.parent = parent;
> device_set_node(&scmi_dev->dev, of_fwnode_handle(np));
> scmi_dev->dev.bus = &scmi_bus_type;
> scmi_dev->dev.release = scmi_device_release;
> - dev_set_name(&scmi_dev->dev, "scmi_dev.%d", id);
> + dev_set_name(&scmi_dev->dev, "scmi_dev.%s.%s.%d", dev_name(parent),
> + scmi_dev->name, protocol);
>
> retval = device_register(&scmi_dev->dev);
> if (retval)
> @@ -413,7 +404,6 @@ __scmi_device_create(struct device_node *np, struct device *parent,
> return scmi_dev;
> put_dev:
> put_device(&scmi_dev->dev);
> - ida_free(&scmi_bus_id, id);
> return NULL;
> }
>
> @@ -526,7 +516,6 @@ static void __exit scmi_bus_exit(void)
> */
> scmi_devices_unregister();
> bus_unregister(&scmi_bus_type);
> - ida_destroy(&scmi_bus_id);
> }
> module_exit(scmi_bus_exit);
>
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 1b5fb2c4ce86..bbf1f05f2be3 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -2641,8 +2641,8 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
> cinfo->max_msg_size = info->desc->max_msg_size;
>
> /* Create a unique name for this transport device */
> - snprintf(name, 32, "__scmi_transport_device_%s_%02X",
> - idx ? "rx" : "tx", prot_id);
> + snprintf(name, 32, "__scmi_transport_device_%s",
> + idx ? "rx" : "tx");
> /* Create a uniquely named, dedicated transport device for this chan */
> tdev = scmi_device_create(of_node, info->dev, prot_id, name);
> if (!tdev) {
> --
> 2.47.1
>
More information about the linux-arm-kernel
mailing list