Messages in this thread | | | Subject | Re: [PATCH v2 1/2] pinctrl: qcom: Add egpio feature support | From | Rajendra Nayak <> | Date | Fri, 29 Oct 2021 15:28:44 +0530 |
| |
On 10/29/2021 4:31 AM, Bjorn Andersson wrote: > On Tue 26 Oct 05:07 PDT 2021, Rajendra Nayak wrote: > >> From: Prasad Sodagudi <psodagud@codeaurora.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. >> >> To support this we add a new function 'egpio' which can be used to >> set the egpio_enable to 0, for any other TLMM controlled functions >> we set the egpio_enable to 1. >> >> Signed-off-by: Prasad Sodagudi <psodagud@codeaurora.org> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> --- >> drivers/pinctrl/qcom/pinctrl-msm.c | 17 +++++++++++++++-- >> drivers/pinctrl/qcom/pinctrl-msm.h | 4 ++++ >> 2 files changed, 19 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c >> index 8476a8a..bfdba3a 100644 >> --- a/drivers/pinctrl/qcom/pinctrl-msm.c >> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c >> @@ -185,6 +185,7 @@ static int msm_pinmux_set_mux(struct pinctrl_dev *pctldev, >> unsigned int irq = irq_find_mapping(gc->irq.domain, group); >> struct irq_data *d = irq_get_irq_data(irq); >> unsigned int gpio_func = pctrl->soc->gpio_func; >> + unsigned int egpio_func = pctrl->soc->egpio_func; >> const struct msm_pingroup *g; >> unsigned long flags; >> u32 val, mask; >> @@ -218,8 +219,20 @@ static int msm_pinmux_set_mux(struct pinctrl_dev *pctldev, >> raw_spin_lock_irqsave(&pctrl->lock, flags); >> >> val = msm_readl_ctl(pctrl, g); >> - val &= ~mask; >> - val |= i << g->mux_bit; >> + >> + if (egpio_func && i == egpio_func) { >> + if (val & BIT(g->egpio_present)) >> + val &= ~BIT(g->egpio_enable); >> + else >> + return -EINVAL; > > You're returning here with pctrl->lock held and irqs disabled.
argh, right. I will fix that and repost. I wonder if I should just drop that error handling completely, we wouldn't end up here unless the platform driver wrongly populates a pin which does not support egpio with a egpio function.
> >> + } else { >> + val &= ~mask; >> + val |= i << g->mux_bit; >> + /* Check if egpio present and enable that feature */ > > I never remember if egpio_enable means apss or lpass, so I think this > comment would be better as: > > /* Claim ownership of pin if egpio capable */
:) makes sense
> >> + if (egpio_func && (val & BIT(g->egpio_present))) > > Can't you drop the parenthesis around the second expression?
yes, will do, thanks for the review.
-- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
| |