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-fran...@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