[PATCH v2 4/7] drivers: firmware: ti_sci: Get SCI revision only if TIFS/SYSFW is up

[Nishanth Menon]

On 19:44-20230907, Neha Malcom Francis wrote:
> While setting up necessary dependent clocks and power domains, we end up
> probing the ti_sci driver before TIFS/SYSFW has been loaded in the case
> of legacy boot flow devices. This leads to panic when getting the SCI
> revision. Return the revision only if it is combined boot flow. Also

Not clear description of the problem.

> ensure that after TIFS/SYSFW comes up we run ti_sci_cmd_get_revision
> again so we print the revision right.
> 
> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
> ---
>  drivers/firmware/ti_sci.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 72f572d824..6a85d202f2 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -2703,6 +2703,9 @@ struct ti_sci_handle *ti_sci_get_handle_from_sysfw(struct udevice *sci_dev)
>  	if (!handle)
>  		return ERR_PTR(-EINVAL);
>  
> +	/* May have been skipped in case TIFS/SYSFW had come up later */
Very confusing comment.

> +	ti_sci_cmd_get_revision(handle);

should this be protected with if (IS_ENABLED(CONFIG_K3_DM_FW) ?

Since getting the handle is where the entire flow happens, why not do it
here always?

what if ti_sci_cmd_get_revision fails? there is no error handling here.
> +
>  	return handle;
>  }
>  
> @@ -2825,7 +2828,11 @@ static int ti_sci_probe(struct udevice *dev)
>  	list_add_tail(&info->list, &ti_sci_list);
>  	ti_sci_setup_ops(info);
>  
> -	ret = ti_sci_cmd_get_revision(&info->handle);
> +	/* Get SCI revision ONLY if we are real */
> +	if (!IS_ENABLED(CONFIG_K3_DM_FW))
> +		ret = ti_sci_cmd_get_revision(&info->handle);
> +	else
> +		ret = 0;
>  
>  	INIT_LIST_HEAD(&info->dev_list);
>  
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D