Hi Nishanth

On 07/09/23 20:35, Nishanth Menon wrote:
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.

Hmm okay I will modify this and the earlier commit message to give a clearer picture.


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 <[email protected]>
---
  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.


Understood, will change that.

+       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?


This is a valid point, I am in the process of making sure there's no other consequences to getting the revision this late for platforms with other boot devices, if this gets an all clear, I will push it here for the next version, else I will explain why I couldn't.

what if ti_sci_cmd_get_revision fails? there is no error handling here.

Yes that would be a consequence since ti_sci_probe, the logic it was written; used reading the version as an "all okay return 0" for the TIFS/SYSFW binary.

But now after moving, ti_sci_get_handle_from_sysfw happens after k3_sysfw_loader so we're not checking binary sanity early on in the case of combined boot

I'm hesitant that we are checking version way later after k3_sysfw_loader for combined platforms and if that is something we want? But there is an advantage(?) that it will run every time someone tries to get a handle for doing anything with TISCI and that would be a kind of check (?) if the binary is good? Correct me if I'm wrong in thinking that.

+
        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



--
Thanking You
Neha Malcom Francis

Reply via email to