[PATCH V2] firmware: ti_sci: Scan all device instances when releasing exclusive devices

[Francis, Neha]

Hi Nishanth

On 4/7/2025 5:45 PM, Nishanth Menon wrote:
> When FIT image with multiple dtbs are involved for R5 boot process,
> R5 SPL starts off with the first instance of dtb to probe the
> eeprom, then once we have identified the type of board, invocation
> of setup_multi_dtb_fit will replace the gd->fdt_blob with the proper
> board dtb match. However, when we do this, two things happen:
> 
> a) Prior to the invocation of setup_multi_dtb_fit, as part of the eeprom
>    discovery process, i2c controller device is already probed and marked
>    as exclusive with the match of the very first tisci match (from the
>    original boot dtb). This list is stored in the info->dev_list of the
>    first probe.
> b) When the second dtb is loaded, tisci is probed again (since this is a
>    new node) and the new info->dev_list is empty.
> 
> At this stage, the exclusive devices such as i2c instances used to
> probe the board information is left in the old info->dev_list that is
> no longer used actively by the system using the replaced dtb.
> 
> As a result of this, the cleanup we intend to do with
> ti_sci_cmd_release_exclusive_devices is no longer complete and
> leaves the instances such as i2c for eeprom marked used as we scan just
> the new info->dev_list.
> 
> This creates a problem when Device Manager(DM) firmware starts up later
> on in the boot process and identifies that this instance of i2c is
> already marked active, so it assumes this can no longer be controlled
> by software and is marked internally as reserved and HLOS can no
> longer control these instances. This defeated the purpose of
> ti_sci_cmd_release_exclusive_devices.
> 
> NOTE: This scheme works just fine if the FIT has just a single dtb as
> the info->dev_list is upto date.
> 
> To fix this, let us make ti_sci_cmd_release_exclusive_devices scan the
> all registrations of tisci instances and cleanup all exclusive devices
> that have ever been registered.
> 
> As part of this, change the prototype of release_exclusive_devices to
> drop the handle since that has no further meaning now.
> 
> Though this issue was identified on AM64-sk, this can be present in
> other builds which use multi-fit-dtb for R5 SPL startup.
> 
> Fixes: 9566b777ae0a ("firmware: ti_sci: Add a command for releasing all exclusive devices")
> Signed-off-by: Nishanth Menon <nm at ti.com>
> ---
> Changes since V1:
> * commit message: s/intent/intend (review comment)
> * Added comment to indicate am64x-sk
> * No functional changes.
> 
> V1: https://lore.kernel.org/r/20250401205515.4078312-1-nm@ti.com
> 
> Retaining test log:
> 
> Test logs:
> ==========
> prior to this patch:
> https://gist.github.com/nmenon/8ce817ac991b6ff898734d15ae5ce623#file-before
> With this patch
> https://gist.github.com/nmenon/8ce817ac991b6ff898734d15ae5ce623#file-after
> Notice that 20000000.i2c is no longer deffered in Linux.
> 
> Test of platforms (on c17f03a7e93d  Merge tag 'net-20250314' of https://source.denx.de/u-boot/custodians/u-boot-net)
> 
> https://gist.github.com/nmenon/917e15b397e5249dd76664ea62e9e662
> 
> Though this was uncovered in automated check in upstream testing, this was also
> independently reported in:
> https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1459509/sk-am64b-deferred-probe-of-i2c-bus-using-sdk-09-and-10
> 
>  arch/arm/mach-k3/r5/common.c           |  2 +-
>  drivers/firmware/ti_sci.c              | 23 ++++++++++++++---------
>  include/linux/soc/ti/ti_sci_protocol.h |  2 +-
>  3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
> index 0f6c294f1eb2..fd188e7c90e4 100644
> --- a/arch/arm/mach-k3/r5/common.c
> +++ b/arch/arm/mach-k3/r5/common.c
> @@ -144,7 +144,7 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>  	int ret, size = 0, shut_cpu = 0;
>  
>  	/* Release all the exclusive devices held by SPL before starting ATF */
> -	ti_sci->ops.dev_ops.release_exclusive_devices(ti_sci);
> +	ti_sci->ops.dev_ops.release_exclusive_devices();
>  
>  	ret = rproc_init();
>  	if (ret)
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 190a1e3f5fc3..54d6689ce783 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -696,20 +696,25 @@ static int ti_sci_cmd_put_device(const struct ti_sci_handle *handle, u32 id)
>  				       MSG_DEVICE_SW_STATE_AUTO_OFF);
>  }
>  
> -static
> -int ti_sci_cmd_release_exclusive_devices(const struct ti_sci_handle *handle)
> +static int ti_sci_cmd_release_exclusive_devices(void)
>  {
>  	struct ti_sci_exclusive_dev *dev, *tmp;
>  	struct ti_sci_info *info;
>  	int i, cnt;
>  
> -	info = handle_to_ti_sci_info(handle);
> -
> -	list_for_each_entry_safe(dev, tmp, &info->dev_list, list) {
> -		cnt = dev->count;
> -		debug("%s: id = %d, cnt = %d\n", __func__, dev->id, cnt);
> -		for (i = 0; i < cnt; i++)
> -			ti_sci_cmd_put_device(handle, dev->id);
> +	/*
> +	 * Scan all ti_sci_list registrations, since with FIT images, we could
> +	 * have started with one device tree registration and switched over
> +	 * to a final version. This prevents exclusive devices identified
> +	 * during the first probe to be left orphan.
> +	 */
> +	list_for_each_entry(info, &ti_sci_list, list) {
> +		list_for_each_entry_safe(dev, tmp, &info->dev_list, list) {
> +			cnt = dev->count;
> +			debug("%s: id = %d, cnt = %d\n", __func__, dev->id, cnt);
> +			for (i = 0; i < cnt; i++)
> +				ti_sci_cmd_put_device(&info->handle, dev->id);
> +		}
>  	}
>  
>  	return 0;
> diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
> index 8e4c43cef311..aa4d105ee988 100644
> --- a/include/linux/soc/ti/ti_sci_protocol.h
> +++ b/include/linux/soc/ti/ti_sci_protocol.h
> @@ -143,7 +143,7 @@ struct ti_sci_dev_ops {
>  				 u32 reset_state);
>  	int (*get_device_resets)(const struct ti_sci_handle *handle, u32 id,
>  				 u32 *reset_state);
> -	int (*release_exclusive_devices)(const struct ti_sci_handle *handle);
> +	int (*release_exclusive_devices)(void);
>  };
>  
>  /**

Reviewed-by: Neha Malcom Francis <n-francis at ti.com>

-- 
Thanking You
Neha Malcom Francis