* [PATCH V1 1/2] pinctrl: qcom: spmi-gpio: Read REG_EN_CTL to get initial enable state
[not found] <20171013061550.996-1-fenglinw@codeaurora.org>
@ 2017-10-13 6:15 ` fenglinw
2017-10-16 22:19 ` Bjorn Andersson
2017-10-13 6:15 ` [PATCH V1 2/2] pinctrl: qcom: spmi-gpio: Set is_enabled flag in set_mux() fenglinw
1 sibling, 1 reply; 6+ messages in thread
From: fenglinw @ 2017-10-13 6:15 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel, Bjorn Andersson, Linus Walleij,
linux-gpio
Cc: collinsd, aghayal, wruan, subbaram, kgunda, Fenglin Wu
From: Fenglin Wu <fenglinw@codeaurora.org>
Get initial value of is_enabled flag by reading REG_EN_CTL register
so that it can reflect the correct hardware enable state before
setting pin config.
Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index a0edaa8..0a1e173 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -106,6 +106,7 @@
/* PMIC_GPIO_REG_EN_CTL */
#define PMIC_GPIO_REG_MASTER_EN_SHIFT 7
+#define PMIC_GPIO_REG_MASTER_EN 0x80
#define PMIC_GPIO_PHYSICAL_OFFSET 1
@@ -914,8 +915,12 @@ static int pmic_gpio_populate(struct pmic_gpio_state *state,
pad->atest = (val & PMIC_GPIO_LV_MV_ANA_MUX_SEL_MASK) + 1;
}
- /* Pin could be disabled with PIN_CONFIG_BIAS_HIGH_IMPEDANCE */
- pad->is_enabled = true;
+ val = pmic_gpio_read(state, pad, PMIC_GPIO_REG_EN_CTL);
+ if (val < 0)
+ return val;
+
+ pad->is_enabled = val & PMIC_GPIO_REG_MASTER_EN;
+
return 0;
}
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V1 2/2] pinctrl: qcom: spmi-gpio: Set is_enabled flag in set_mux()
[not found] <20171013061550.996-1-fenglinw@codeaurora.org>
2017-10-13 6:15 ` [PATCH V1 1/2] pinctrl: qcom: spmi-gpio: Read REG_EN_CTL to get initial enable state fenglinw
@ 2017-10-13 6:15 ` fenglinw
2017-10-16 22:29 ` Bjorn Andersson
1 sibling, 1 reply; 6+ messages in thread
From: fenglinw @ 2017-10-13 6:15 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel, Bjorn Andersson, Linus Walleij,
linux-gpio
Cc: collinsd, aghayal, wruan, subbaram, kgunda, Fenglin Wu
From: Fenglin Wu <fenglinw@codeaurora.org>
The initial value of is_enabled flag is read out from hardware in
pmic_gpio_populate(), and it will be set in pmic_gpio_config_set() if
pinconf is defined. For any GPIOs disabled initially in hardware which
only have pinmux defined, they won't be enabled in pmic_gpio_set_mux()
calling. So set is_enabled flag in set_mux() to ensure the GPIO module
could be enabled in above case.
Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 0a1e173..219c934 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -312,6 +312,7 @@ static int pmic_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned function,
}
pad = pctldev->desc->pins[pin].drv_data;
+ pad->is_enabled = true;
/*
* Non-LV/MV subtypes only support 2 special functions,
* offsetting the dtestx function values by 2
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V1 1/2] pinctrl: qcom: spmi-gpio: Read REG_EN_CTL to get initial enable state
2017-10-13 6:15 ` [PATCH V1 1/2] pinctrl: qcom: spmi-gpio: Read REG_EN_CTL to get initial enable state fenglinw
@ 2017-10-16 22:19 ` Bjorn Andersson
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2017-10-16 22:19 UTC (permalink / raw)
To: fenglinw
Cc: linux-arm-msm, linux-kernel, Linus Walleij, linux-gpio, collinsd,
aghayal, wruan, subbaram, kgunda
On Thu 12 Oct 23:15 PDT 2017, fenglinw@codeaurora.org wrote:
> From: Fenglin Wu <fenglinw@codeaurora.org>
>
> Get initial value of is_enabled flag by reading REG_EN_CTL register
> so that it can reflect the correct hardware enable state before
> setting pin config.
>
> Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
> ---
> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index a0edaa8..0a1e173 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> @@ -106,6 +106,7 @@
>
> /* PMIC_GPIO_REG_EN_CTL */
> #define PMIC_GPIO_REG_MASTER_EN_SHIFT 7
> +#define PMIC_GPIO_REG_MASTER_EN 0x80
>
> #define PMIC_GPIO_PHYSICAL_OFFSET 1
>
> @@ -914,8 +915,12 @@ static int pmic_gpio_populate(struct pmic_gpio_state *state,
> pad->atest = (val & PMIC_GPIO_LV_MV_ANA_MUX_SEL_MASK) + 1;
> }
>
> - /* Pin could be disabled with PIN_CONFIG_BIAS_HIGH_IMPEDANCE */
> - pad->is_enabled = true;
> + val = pmic_gpio_read(state, pad, PMIC_GPIO_REG_EN_CTL);
> + if (val < 0)
> + return val;
> +
> + pad->is_enabled = val & PMIC_GPIO_REG_MASTER_EN;
Please be more explicit here: is_enable = !!(val & ...)
> +
other than that,
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V1 2/2] pinctrl: qcom: spmi-gpio: Set is_enabled flag in set_mux()
2017-10-13 6:15 ` [PATCH V1 2/2] pinctrl: qcom: spmi-gpio: Set is_enabled flag in set_mux() fenglinw
@ 2017-10-16 22:29 ` Bjorn Andersson
2017-10-17 4:36 ` Fenglin Wu
0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Andersson @ 2017-10-16 22:29 UTC (permalink / raw)
To: fenglinw
Cc: linux-arm-msm, linux-kernel, Linus Walleij, linux-gpio, collinsd,
aghayal, wruan, subbaram, kgunda
On Thu 12 Oct 23:15 PDT 2017, fenglinw@codeaurora.org wrote:
> From: Fenglin Wu <fenglinw@codeaurora.org>
>
> The initial value of is_enabled flag is read out from hardware in
> pmic_gpio_populate(), and it will be set in pmic_gpio_config_set() if
> pinconf is defined. For any GPIOs disabled initially in hardware which
> only have pinmux defined, they won't be enabled in pmic_gpio_set_mux()
> calling. So set is_enabled flag in set_mux() to ensure the GPIO module
> could be enabled in above case.
>
I'm still interested in knowing when it is valid to configure a pin with
only mux, no config. I.e. in what cases does setting a alternative
function make the pinconfig not count.
Regards,
Bjorn
> Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
> ---
> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index 0a1e173..219c934 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> @@ -312,6 +312,7 @@ static int pmic_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned function,
> }
>
> pad = pctldev->desc->pins[pin].drv_data;
> + pad->is_enabled = true;
> /*
> * Non-LV/MV subtypes only support 2 special functions,
> * offsetting the dtestx function values by 2
> --
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V1 2/2] pinctrl: qcom: spmi-gpio: Set is_enabled flag in set_mux()
2017-10-16 22:29 ` Bjorn Andersson
@ 2017-10-17 4:36 ` Fenglin Wu
2017-10-20 9:51 ` Linus Walleij
0 siblings, 1 reply; 6+ messages in thread
From: Fenglin Wu @ 2017-10-17 4:36 UTC (permalink / raw)
To: Bjorn Andersson
Cc: linux-arm-msm, linux-kernel, Linus Walleij, linux-gpio, collinsd,
aghayal, wruan, subbaram, kgunda
On 10/17/2017 6:29 AM, Bjorn Andersson wrote:
> On Thu 12 Oct 23:15 PDT 2017, fenglinw@codeaurora.org wrote:
>
>> From: Fenglin Wu <fenglinw@codeaurora.org>
>>
>> The initial value of is_enabled flag is read out from hardware in
>> pmic_gpio_populate(), and it will be set in pmic_gpio_config_set() if
>> pinconf is defined. For any GPIOs disabled initially in hardware which
>> only have pinmux defined, they won't be enabled in pmic_gpio_set_mux()
>> calling. So set is_enabled flag in set_mux() to ensure the GPIO module
>> could be enabled in above case.
>>
>
> I'm still interested in knowing when it is valid to configure a pin with
> only mux, no config. I.e. in what cases does setting a alternative
> function make the pinconfig not count.
I agreed that any pins should be configured with pinmux as well as
pinconf, but the driver doesn't prevent the case of only pinmux defined,
right? I am not sure if this is valid case but it would happen: The
hardware or the sw prior to linux kernel has the default setting of the
function and config for one GPIO but we need to keep it disabled until
the consumer request it, in this case, we just need to define the pinmux
and ignore the pinconf definition in its device node.
>
> Regards,
> Bjorn
>
>> Signed-off-by: Fenglin Wu <fenglinw@codeaurora.org>
>> ---
>> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> index 0a1e173..219c934 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> @@ -312,6 +312,7 @@ static int pmic_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned function,
>> }
>>
>> pad = pctldev->desc->pins[pin].drv_data;
>> + pad->is_enabled = true;
>> /*
>> * Non-LV/MV subtypes only support 2 special functions,
>> * offsetting the dtestx function values by 2
>> --
>> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project.
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V1 2/2] pinctrl: qcom: spmi-gpio: Set is_enabled flag in set_mux()
2017-10-17 4:36 ` Fenglin Wu
@ 2017-10-20 9:51 ` Linus Walleij
0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2017-10-20 9:51 UTC (permalink / raw)
To: Fenglin Wu
Cc: Bjorn Andersson, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
David Collins, aghayal, wruan, subbaram, kgunda
On Tue, Oct 17, 2017 at 6:36 AM, Fenglin Wu <fenglinw@codeaurora.org> wrote:
> I am not sure if this is valid case but it would happen: The
> hardware or the sw prior to linux kernel has the default setting of the
> function and config for one GPIO but we need to keep it disabled until
> the consumer request it, in this case, we just need to define the pinmux
> and ignore the pinconf definition in its device node.
This firmware-kernel partitioning of responsibilities makes me nervous
every time it happens. Who's in charge really? Who fixes bugs? Firmware?
Linux? Linux overriding firmware? Linux overriding firmware on special
firmware revisions? Linux overriding firmware on special firmware revisions
that cannot be detected and instead needs to be passed as cmdline
parmeters?
This kind of stuff gives me the creeps and just a general feeling of not
being in control having very little clue as to what is really going on in
the system.
But I guess that is essentially the working assumption for things like
ACPI and other behind-my-back firmware: don't worry be happy.
Anyways, it's up to Björn to establish what is best for the qcom pin control,
I'm just gonna accept whatever he ACKs. Just rambling.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-20 9:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20171013061550.996-1-fenglinw@codeaurora.org>
2017-10-13 6:15 ` [PATCH V1 1/2] pinctrl: qcom: spmi-gpio: Read REG_EN_CTL to get initial enable state fenglinw
2017-10-16 22:19 ` Bjorn Andersson
2017-10-13 6:15 ` [PATCH V1 2/2] pinctrl: qcom: spmi-gpio: Set is_enabled flag in set_mux() fenglinw
2017-10-16 22:29 ` Bjorn Andersson
2017-10-17 4:36 ` Fenglin Wu
2017-10-20 9:51 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).