linux-kernel - Re: [PATCH] pinctrl: qcom: Add egpio feature support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 21 Sep 2021 16:09:04 +0530
From:   Rajendra Nayak <rnayak@...eaurora.org>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     agross@...nel.org, linus.walleij@...aro.org,
        linux-arm-msm@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Prasad Sodagudi <psodagud@...eaurora.org>
Subject: Re: [PATCH] pinctrl: qcom: Add egpio feature support



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@...eaurora.org>
>>
>> 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.
  
> 
> 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?

> 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.
These egpio pins have regulator mux functions, some for apps, some for the
island processor, they might not always be used as gpios.
  
> PS. When I spoke with Prasad about this a couple of years ago, I think
> we talked about representing this as a pinconf property, but it seems to
> make more sense to me now that it would be a pinmux state.
> 
> Regards,
> Bjorn
> 
>> Signed-off-by: Prasad Sodagudi <psodagud@...eaurora.org>
>> [rnayak: rewrite commit log, minor rebase]
>> Signed-off-by: Rajendra Nayak <rnayak@...eaurora.org>
>> ---
>>   drivers/pinctrl/qcom/pinctrl-msm.c    | 4 ++++
>>   drivers/pinctrl/qcom/pinctrl-msm.h    | 2 ++
>>   drivers/pinctrl/qcom/pinctrl-sc7280.c | 2 ++
>>   drivers/pinctrl/qcom/pinctrl-sm8150.c | 2 ++
>>   drivers/pinctrl/qcom/pinctrl-sm8250.c | 2 ++
>>   drivers/pinctrl/qcom/pinctrl-sm8350.c | 2 ++
>>   6 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
>> index 8476a8a..f4a2343 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
>> @@ -220,6 +220,10 @@ static int msm_pinmux_set_mux(struct pinctrl_dev *pctldev,
>>   	val = msm_readl_ctl(pctrl, g);
>>   	val &= ~mask;
>>   	val |= i << g->mux_bit;
>> +	/* Check if egpio present and enable that feature */
>> +	if (val & BIT(g->egpio_present))
>> +		val |= BIT(g->egpio_enable);
>> +
>>   	msm_writel_ctl(val, pctrl, g);
>>   
>>   	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
>> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.h b/drivers/pinctrl/qcom/pinctrl-msm.h
>> index e31a516..3635b31 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-msm.h
>> +++ b/drivers/pinctrl/qcom/pinctrl-msm.h
>> @@ -77,6 +77,8 @@ struct msm_pingroup {
>>   	unsigned drv_bit:5;
>>   
>>   	unsigned od_bit:5;
>> +	unsigned egpio_enable:5;
>> +	unsigned egpio_present:5;
>>   	unsigned oe_bit:5;
>>   	unsigned in_bit:5;
>>   	unsigned out_bit:5;
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280.c b/drivers/pinctrl/qcom/pinctrl-sc7280.c
>> index afddf6d..607d459 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-sc7280.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280.c
>> @@ -43,6 +43,8 @@
>>   		.mux_bit = 2,			\
>>   		.pull_bit = 0,			\
>>   		.drv_bit = 6,			\
>> +		.egpio_enable = 12,		\
>> +		.egpio_present = 11,		\
>>   		.oe_bit = 9,			\
>>   		.in_bit = 0,			\
>>   		.out_bit = 1,			\
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sm8150.c b/drivers/pinctrl/qcom/pinctrl-sm8150.c
>> index 7359bae..63a625a 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-sm8150.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-sm8150.c
>> @@ -56,6 +56,8 @@ enum {
>>   		.mux_bit = 2,			\
>>   		.pull_bit = 0,			\
>>   		.drv_bit = 6,			\
>> +		.egpio_enable = 12,		\
>> +		.egpio_present = 11,		\
>>   		.oe_bit = 9,			\
>>   		.in_bit = 0,			\
>>   		.out_bit = 1,			\
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sm8250.c b/drivers/pinctrl/qcom/pinctrl-sm8250.c
>> index af144e7..ad4fd94 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-sm8250.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-sm8250.c
>> @@ -57,6 +57,8 @@ enum {
>>   		.mux_bit = 2,				\
>>   		.pull_bit = 0,				\
>>   		.drv_bit = 6,				\
>> +		.egpio_enable = 12,			\
>> +		.egpio_present = 11,			\
>>   		.oe_bit = 9,				\
>>   		.in_bit = 0,				\
>>   		.out_bit = 1,				\
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sm8350.c b/drivers/pinctrl/qcom/pinctrl-sm8350.c
>> index 4d8f863..bb436dc 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-sm8350.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-sm8350.c
>> @@ -46,6 +46,8 @@
>>   		.mux_bit = 2,			\
>>   		.pull_bit = 0,			\
>>   		.drv_bit = 6,			\
>> +		.egpio_enable = 12,		\
>> +		.egpio_present = 11,		\
>>   		.oe_bit = 9,			\
>>   		.in_bit = 0,			\
>>   		.out_bit = 1,			\
>> -- 
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation
>>

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