Re: [PATCH v2 1/2] pinctrl: qcom: Add egpio feature support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@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.

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@xxxxxxxxxxxxxx>
Signed-off-by: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx>
---
  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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux