Re: [PATCH] regulator: qcom-rpmh: Revert "regulator: qcom-rpmh: Use PROBE_FORCE_SYNCHRONOUS"

[Amit Pundir]

On Sun, 14 May 2023 at 18:11, Caleb Connolly <caleb.connolly@xxxxxxxxxx> wrote:
>
>
>
> On 13/05/2023 18:08, Amit Pundir wrote:
> > On Fri, 24 Mar 2023 at 19:05, Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:
> >>
> >> This reverts commit 58973046c1bf ("regulator: qcom-rpmh: Use
> >> PROBE_FORCE_SYNCHRONOUS"). Further digging into the problems that
> >> prompted the us to switch to synchronous probe showed that the root
> >> cause was a missing "rootwait" in the kernel command line
> >> arguments. Let's reinstate asynchronous probe.
> >
> > Hi, the asynchronous probe is broken on Dragonboard 845c (SDM845)
> > running AOSP (Android Open Source Project) with v6.4-rc1
> > https://bugs.linaro.org/show_bug.cgi?id=5975.
> > Can we please go back to synchronous probe.
> >
> > AOSP do not make use of rootwait, IIRC, but it is added by the
> > bootloader anyway. And the device fails to boot AOSP regardless of
> > "rootwait" bootarg being present or not.
>
> Could you try applying this diff to enable some log spam and let me know
> what you get? I'm keen to try and figure this one out. My mail client
> might crunch this a bit so I have pasted it here too
> https://p.calebs.dev/ab74b7@raw

These prints add just enough delay for the UFS probe to succeed that I
can't reproduce the failure anymore.

>
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index f93544f6d796..67859f1bdb28 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -653,11 +653,23 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, const
> struct tcs_request *msg)
>
>         spin_lock_irqsave(&drv->lock, flags);
>
> +       dev_info(drv->dev, "%s: %p tcs->type=%d state=%d, "
> +               "wait_for_compl=%d, num_cmds=%d\n",
> +               __func__, msg, tcs->type, msg->state,
> +               msg->wait_for_compl, msg->num_cmds);
> +       for (int i = 0; i < msg->num_cmds; i++)
> +               dev_info(drv->dev, "%s: %p cmd[%d] "
> +                       "addr=0x%x data=0x%x\n", __func__,
> +                       msg, i, msg->cmds[i].addr, msg->cmds[i].data);
> +
>         /* Wait forever for a free tcs. It better be there eventually! */
>         wait_event_lock_irq(drv->tcs_wait,
>                             (tcs_id = claim_tcs_for_req(drv, tcs, msg))
> >= 0,
>                             drv->lock);
>
> +       dev_info(drv->dev, "%s: %px GOT TCS! %d\n",
> +               __func__, msg, tcs_id);
> +
>         tcs->req[tcs_id - tcs->offset] = msg;
>         set_bit(tcs_id, drv->tcs_in_use);
>         if (msg->state == RPMH_ACTIVE_ONLY_STATE && tcs->type !=
> ACTIVE_TCS) {
>
> >
> > FWIW I do not see this regression on RB5 (QRB5165/SM8250) running the
> > same set of AOSP images.
> >
> > Regards,
> > Amit Pundir
> >
> >
> >
> >
> >>
> >> Fixes: 58973046c1bf ("regulator: qcom-rpmh: Use PROBE_FORCE_SYNCHRONOUS")
> >> Cc: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> >> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> >> ---
> >>
> >>  drivers/regulator/qcom-rpmh-regulator.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
> >> index 903032b2875f..4826d60e5d95 100644
> >> --- a/drivers/regulator/qcom-rpmh-regulator.c
> >> +++ b/drivers/regulator/qcom-rpmh-regulator.c
> >> @@ -1462,7 +1462,7 @@ MODULE_DEVICE_TABLE(of, rpmh_regulator_match_table);
> >>  static struct platform_driver rpmh_regulator_driver = {
> >>         .driver = {
> >>                 .name = "qcom-rpmh-regulator",
> >> -               .probe_type = PROBE_FORCE_SYNCHRONOUS,
> >> +               .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> >>                 .of_match_table = of_match_ptr(rpmh_regulator_match_table),
> >>         },
> >>         .probe = rpmh_regulator_probe,
> >> --
> >> 2.40.0.348.gf938b09366-goog
> >>
>
> --
> Kind Regards,
> Caleb (they/them)