| Message ID | 20201110090602.2255-4-t-kristo@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Lokesh Vutla |
| Headers | show |
| Series | TI J7 SoC HSM Rearch support series | expand |
On 10/11/20 2:35 pm, Tero Kristo wrote: > With the sysfw rearch, sysfw PM calls are no longer available from SPL > level. To properly support this, remove the is_on checks and the reset > assertion from the R5 remoteproc driver as these are not supported. > Attempting to access unavailable services will cause the device to hang. > > Signed-off-by: Tero Kristo <t-kristo@ti.com> > --- > drivers/remoteproc/ti_k3_r5f_rproc.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/remoteproc/ti_k3_r5f_rproc.c b/drivers/remoteproc/ti_k3_r5f_rproc.c > index 9332a63d21..b2902e7fd3 100644 > --- a/drivers/remoteproc/ti_k3_r5f_rproc.c > +++ b/drivers/remoteproc/ti_k3_r5f_rproc.c > @@ -781,7 +781,9 @@ static int k3_r5f_probe(struct udevice *dev) > { > struct k3_r5f_cluster *cluster = dev_get_priv(dev->parent); > struct k3_r5f_core *core = dev_get_priv(dev); > +#ifdef CONFIG_CLK_TI_SCI Since we are checking for device state, can you make this guard under power domain instead of clock? > bool r_state; > +#endif > int ret; > > dev_dbg(dev, "%s\n", __func__); > @@ -804,6 +806,7 @@ static int k3_r5f_probe(struct udevice *dev) > return ret; > } > > +#ifdef CONFIG_CLK_TI_SCI Can you also add a comment here on why we are doing this? Thanks and regards, Lokesh > ret = core->tsp.sci->ops.dev_ops.is_on(core->tsp.sci, core->tsp.dev_id, > &r_state, &core->in_use); > if (ret) > @@ -817,6 +820,7 @@ static int k3_r5f_probe(struct udevice *dev) > > /* Make sure Local reset is asserted. Redundant? */ > reset_assert(&core->reset); > +#endif > > ret = k3_r5f_rproc_configure(core); > if (ret) { >
On 15/11/2020 12:29, Lokesh Vutla wrote: > > > On 10/11/20 2:35 pm, Tero Kristo wrote: >> With the sysfw rearch, sysfw PM calls are no longer available from SPL >> level. To properly support this, remove the is_on checks and the reset >> assertion from the R5 remoteproc driver as these are not supported. >> Attempting to access unavailable services will cause the device to hang. >> >> Signed-off-by: Tero Kristo <t-kristo@ti.com> >> --- >> drivers/remoteproc/ti_k3_r5f_rproc.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/remoteproc/ti_k3_r5f_rproc.c b/drivers/remoteproc/ti_k3_r5f_rproc.c >> index 9332a63d21..b2902e7fd3 100644 >> --- a/drivers/remoteproc/ti_k3_r5f_rproc.c >> +++ b/drivers/remoteproc/ti_k3_r5f_rproc.c >> @@ -781,7 +781,9 @@ static int k3_r5f_probe(struct udevice *dev) >> { >> struct k3_r5f_cluster *cluster = dev_get_priv(dev->parent); >> struct k3_r5f_core *core = dev_get_priv(dev); >> +#ifdef CONFIG_CLK_TI_SCI > > Since we are checking for device state, can you make this guard under power > domain instead of clock? Hmm right, that would work. Will change. > >> bool r_state; >> +#endif >> int ret; >> >> dev_dbg(dev, "%s\n", __func__); >> @@ -804,6 +806,7 @@ static int k3_r5f_probe(struct udevice *dev) >> return ret; >> } >> >> +#ifdef CONFIG_CLK_TI_SCI > > Can you also add a comment here on why we are doing this? I can add an inline comment for this. -Tero > > Thanks and regards, > Lokesh > >> ret = core->tsp.sci->ops.dev_ops.is_on(core->tsp.sci, core->tsp.dev_id, >> &r_state, &core->in_use); >> if (ret) >> @@ -817,6 +820,7 @@ static int k3_r5f_probe(struct udevice *dev) >> >> /* Make sure Local reset is asserted. Redundant? */ >> reset_assert(&core->reset); >> +#endif >> >> ret = k3_r5f_rproc_configure(core); >> if (ret) { >> -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff --git a/drivers/remoteproc/ti_k3_r5f_rproc.c b/drivers/remoteproc/ti_k3_r5f_rproc.c index 9332a63d21..b2902e7fd3 100644 --- a/drivers/remoteproc/ti_k3_r5f_rproc.c +++ b/drivers/remoteproc/ti_k3_r5f_rproc.c @@ -781,7 +781,9 @@ static int k3_r5f_probe(struct udevice *dev) { struct k3_r5f_cluster *cluster = dev_get_priv(dev->parent); struct k3_r5f_core *core = dev_get_priv(dev); +#ifdef CONFIG_CLK_TI_SCI bool r_state; +#endif int ret; dev_dbg(dev, "%s\n", __func__); @@ -804,6 +806,7 @@ static int k3_r5f_probe(struct udevice *dev) return ret; } +#ifdef CONFIG_CLK_TI_SCI ret = core->tsp.sci->ops.dev_ops.is_on(core->tsp.sci, core->tsp.dev_id, &r_state, &core->in_use); if (ret) @@ -817,6 +820,7 @@ static int k3_r5f_probe(struct udevice *dev) /* Make sure Local reset is asserted. Redundant? */ reset_assert(&core->reset); +#endif ret = k3_r5f_rproc_configure(core); if (ret) {
With the sysfw rearch, sysfw PM calls are no longer available from SPL level. To properly support this, remove the is_on checks and the reset assertion from the R5 remoteproc driver as these are not supported. Attempting to access unavailable services will cause the device to hang. Signed-off-by: Tero Kristo <t-kristo@ti.com> --- drivers/remoteproc/ti_k3_r5f_rproc.c | 4 ++++ 1 file changed, 4 insertions(+)