Re: [PATCH V1] firmware: arm_scmi: Optimize the iteration of scmi_requested_devices

[Cristian Marussi <cristian.marussi@arm.com>]

On Tue, Jan 07, 2025 at 01:20:04PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> scmi_requested_devices is organized in IDR based link lists, so only
> need to search the link lists when there is a match protocol_id.

Hi,

while the original implementation of this loop was awful (my bad), I think
this optiomization indeed NOW changes the logic of the checks....

...you can have number of proto_id/name pairs describing requested devices
and the idea of the logic in this loop was to catch ANY duplicate name across
ANY of the protocol list as stated in the function Dox:

---
 * 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;      
 * 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.       
---

The 'awfulness' of the original loop was that it was trying to do 2
things at once:

- loop all the lists in the IDR for ALL protocols looking for a
  duplicate name anywhere, and FAIL if ANY found

  ... WHILE at the same time:

- save the head of the protocol list_head matching the requested
  id_table->protocol, if any already existing, so as not to have to scan

So, indeed the original loop was meant to be an optimization, even
though probably unreadable....

... having said that, I cannot really recall WHY I decided to avoid ANY
kind of duplicate naming given that teh match happen by proto_id/name...

....maybe just for clarity ? not sure...even what the effects could be
of having such duplicated names across protocols...

Thanks,
Cristian