Re: [PATCH 1/3] firmware: arm_scmi: Relax duplicate name constraint across protocol ids

[Dhruva Gole <d-gole@ti.com>]

Hi Sudeep,

On Jan 31, 2025 at 14:18:20 +0000, Sudeep Holla wrote:
> Currently in scmi_protocol_device_request(), no duplicate scmi device
> name is allowed across any protocol. However scmi_dev_match_id() first

Yeah this was indeed a bit too stringent.

> matches the protocol id and then the name. So, there is no strict
> requirement to keep this scmi device name unique across all the protocols.

Right.

> 
> Relax the constraint on the duplicate name across the protocols and
> inhibit only within the same protocol id.

Makes sense to me, it will indeed make things simpler.
Reviewed-by: Dhruva Gole <d-gole@ti.com>

> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_scmi/bus.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
> index a3386bf36de5..8acf33ccfd21 100644
> --- a/drivers/firmware/arm_scmi/bus.c
> +++ b/drivers/firmware/arm_scmi/bus.c
> @@ -42,7 +42,7 @@ static atomic_t scmi_syspower_registered = ATOMIC_INIT(0);
>   * This helper let an SCMI driver request specific devices identified by the
>   * @id_table to be created for each active SCMI instance.
>   *
> - * The requested device name MUST NOT be already existent for any protocol;
> + * The requested device name MUST NOT be already existent for this protocol;
>   * at first the freshly requested @id_table is annotated in the IDR table
>   * @scmi_requested_devices and then the requested device is advertised to any
>   * registered party via the @scmi_requested_devices_nh notification chain.
> @@ -52,7 +52,6 @@ static atomic_t scmi_syspower_registered = ATOMIC_INIT(0);
>  static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
>  {
>  	int ret = 0;
> -	unsigned int id = 0;
>  	struct list_head *head, *phead = NULL;
>  	struct scmi_requested_dev *rdev;
>  
> @@ -67,19 +66,13 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
>  	}
>  
>  	/*
> -	 * Search for the matching protocol rdev list and then search
> -	 * of any existent equally named device...fails if any duplicate found.
> +	 * Find the matching protocol rdev list and then search of any
> +	 * existent equally named device...fails if any duplicate found.
>  	 */
>  	mutex_lock(&scmi_requested_devices_mtx);
> -	idr_for_each_entry(&scmi_requested_devices, head, id) {
> -		if (!phead) {
> -			/* A list found registered in the IDR is never empty */
> -			rdev = list_first_entry(head, struct scmi_requested_dev,
> -						node);
> -			if (rdev->id_table->protocol_id ==
> -			    id_table->protocol_id)
> -				phead = head;
> -		}
> +	phead = idr_find(&scmi_requested_devices, id_table->protocol_id);
> +	if (phead) {
> +		head = phead;
>  		list_for_each_entry(rdev, head, node) {
>  			if (!strcmp(rdev->id_table->name, id_table->name)) {
>  				pr_err("Ignoring duplicate request [%d] %s\n",
> -- 
> 2.34.1
> 
> 

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated