Re: [PATCH] pinctrl: qcom: Add egpio feature support

From: Rajendra Nayak
Date: Fri Oct 01 2021 - 01:34:39 EST



On 9/30/2021 8:25 PM, Bjorn Andersson wrote:
On Thu 30 Sep 02:46 PDT 2021, Rajendra Nayak wrote:



On 9/21/2021 9:56 PM, Bjorn Andersson wrote:
On Tue 21 Sep 03:39 PDT 2021, Rajendra Nayak wrote:



On 9/20/2021 6:14 AM, Bjorn Andersson wrote:
On Fri 17 Sep 01:37 CDT 2021, Rajendra Nayak wrote:

From: Prasad Sodagudi <psodagud@xxxxxxxxxxxxxx>

egpio is a scheme which allows special power Island Domain IOs
(LPASS,SSC) to be reused as regular chip GPIOs by muxing regular
TLMM functions with Island Domain functions.
With this scheme, an IO can be controlled both by the cpu running
linux and the Island processor. This provides great flexibility to
re-purpose the Island IOs for regular TLMM usecases.

2 new bits are added to ctl_reg, egpio_present is a read only bit
which shows if egpio feature is available or not on a given gpio.
egpio_enable is the read/write bit and only effective if egpio_present
is 1. Once its set, the Island IO is controlled from Chip TLMM.
egpio_enable when set to 0 means the GPIO is used as Island Domain IO.

The support exists on most recent qcom SoCs, and we add support
for sm8150/sm8250/sm8350 and sc7280 as part of this patch.


I was under the impression that this feature would allow you to
repurpose pins for use either by the remote island or by apps.

thats right, you can repurpose the pins for usage by apps by setting
the egpio_enable to 1, when set to 0 its owned by the island processor.

Good.


But if I understand your proposal, you check to see if the pin is
"egpio capable" for a pin and if so just sets the bit - muxing it to
apps (or the island?).

Right, so if there is a request for a egpio-capable pin, the driver
flips the ownership. Are you suggesting having some kind of checks to determine
who should own it?


I see, I missed that nuance. So Linux will steal any pins that are
mentioned in DT. But that would mean that you're relying on someone else
to ensure that this bit is cleared for the other pins and you would not
be able to explicitly flip the state back to island mode in runtime.

I would prefer that this was more explicit.

It seems reasonable that this would be another pinmux state for these
pins, rather than just flipping them all in one or the other direction.

hmm, I don't understand. This is not a pinmux state, its a switch to decide
the ownership.

But does it mux the pin to the island, or does it state that the island
is now in charge of the associated TLMM registers?

The island processor does not access the APPS TLMM register space, it has its
own TLMM register space that it configures. APPS TLMM registers control its
mux/conf settings and Island TLMM registers controls its mux/conf. So essentially
there are 2 sets of registers to control the same pin.
This bit is more like a top level mux which decides what register settings
take affect.


"One mux to rule them all" :)

When we switch this mux towards the Island TLMM, do we need to configure
the APPS TLMM in a particular way, or does the state of that not matter?

No APPS TLMM settings should be needed, the state of that does not matter.

Would it be reasonable to say that when muxed towards the island the
apps should always be in gpio mux with some predetermined properties, to
save power?

No, the the register settings in APPS TLMM are nop/dont care when egpio_enable is 0.

To reiterate, as proposed, mentioning a egpio-capable pin in the apps
DTS will cause it to be muxed to the APSS TLMM. But I'm not convinced
that we don't have scenarios where one might want to dynamically mux the
pin between island and apss tlmm.

My suggestion is that even that it's two independent muxes controlled in
the apps tlmm, we'd express them in the same pinmux, i.e. we'd have
something like:

some-local-state {
pins = "gpio1";
function = "gpio";
output-high;
};

so this would set the function to gpio in the APPS TLMM and set the egpio_enable = 1?
which was also what the original $SUBJECT patch did.

some-remote-state {
pins = "gpio1";
function = "island"; /* or just egpio... ? */
};

Here we add a new function to the pin and that's used to set the egpio_enable to 0?

One case I imaging where this could be useful is to allow Linux to
configure a known state of pins when the island isn't running, from the
remoteproc driver and then flip it over to island mode before booting
the remote.

So we save power during boot up until the island processor comes up?
So fwik when we boot linux its either configured to boot the island processor
or not. Are you talking about some scenario where the island processor comes
up on demand and goes down when not used?

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation