Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
MIME-Version: 1.0
References: <20231110102054.1393570-1-joychakr@google.com> <CACRpkdZ9RHcHh4o5g62ywK0eQHpLZuGUF0Ud6jogk9Sfqe4krA@mail.gmail.com>
 <ZVQkLqDB3KtOlIpK@surfacebook.localdomain> <CAOSNQF3QeFd857RCJE8wfJ=__-K7Bi4vfMeTVP-+O+LJ7y9SmQ@mail.gmail.com>
 <CACRpkdZ+UMOqatH4oOusdaX1ieeH2TtpC7VbX1wf+tzGDSfR3A@mail.gmail.com>
In-Reply-To: <CACRpkdZ+UMOqatH4oOusdaX1ieeH2TtpC7VbX1wf+tzGDSfR3A@mail.gmail.com>
From:   Joy Chakraborty <joychakr@google.com>
Date:   Fri, 1 Dec 2023 11:38:36 +0530
Message-ID: <CAOSNQF1cwCa9hPNQjN3g5b+nMs2qx+H8057aypH+GHaVn29NQw@mail.gmail.com>
Subject: Re: [RFC PATCH] PM: runtime: Apply pinctrl settings if defined
To:     Linus Walleij <linus.walleij@linaro.org>
Cc:     andy.shevchenko@gmail.com,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Pavel Machek <pavel@ucw.cz>, Len Brown <len.brown@intel.com>,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-gpio@vger.kernel.org, manugautam@google.com,
        aniketmaurya@google.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Nov 17, 2023 at 1:30=E2=80=AFAM Linus Walleij <linus.walleij@linaro=
.org> wrote:
>
> On Thu, Nov 16, 2023 at 4:34=E2=80=AFPM Joy Chakraborty <joychakr@google.=
com> wrote:
>
> > I tried to place the calls to set the pinctrl states after driver/user
> > callback  based on my understanding of runtime code so that existing
> > users do get a chance to set the state with any special sequence that
> > needs to be performed post which doing another call to set the state
> > would be ignored in the pinctrl framework.
>
> This makes sense. (And also is in the original commit.)
>
> I think you should actually over-document this by also mentioning
> this in the kerneldoc above each of the *_try_* callbacks so
> users simply can't miss this point.
>
> > But this only would be possible with the assumption that even in any
> > special sequences executed by users they set nothing but "default"
> > state in runtime_resume, "idle" state in runtime_idle and "'sleep"
> > state in their runtime suspend callbacks.
> > And like Andy mentions about "->prepare callback", if there are
> > drivers that are setting pinctrl state "default", "sleep" or "idle"
> > from any callback but
> > ...
> > int (*runtime_suspend)(struct device *dev);
> > int (*runtime_resume)(struct device *dev);
> > int (*runtime_idle)(struct device *dev);
> > ...
> > it could indeed be a problem.
> > I'll dig into users of pinctrl_select_sleep/default/idle and see if
> > there are such cases or if it could be done in some other way.
>
> It's worth a check but I doubt much will turn up. The "idle" and
> "sleep" states are simply not used much in the kernel.
>

Right, I will look into this.

> Your users will likely be the first.
>
> So which hardware target will use this?
> It's immensely useful to have a good example to point at:
> that device use "defaul", "sleep", "idle" the idiomatic way.

I see some upstreaming activity on gs101 SOC.
I think gs101 and follow on SOCs could use this, I will find that out
and get back.

>
> Yours,
> Linus Walleij

Thanks a lot for your feedback
Joy


Joy