Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
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>
In-Reply-To: <CAOSNQF3QeFd857RCJE8wfJ=__-K7Bi4vfMeTVP-+O+LJ7y9SmQ@mail.gmail.com>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Thu, 16 Nov 2023 21:00:42 +0100
Message-ID: <CACRpkdZ+UMOqatH4oOusdaX1ieeH2TtpC7VbX1wf+tzGDSfR3A@mail.gmail.com>
Subject: Re: [RFC PATCH] PM: runtime: Apply pinctrl settings if defined
To:     Joy Chakraborty <joychakr@google.com>
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 Thu, Nov 16, 2023 at 4:34=E2=80=AFPM Joy Chakraborty <joychakr@google.co=
m> 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.

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.

Yours,
Linus Walleij