| Message ID | 53F456F3.3000309@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed 20 Aug 01:06 PDT 2014, Srinivas Kandagatla wrote: > Hi Bjorn, > Hi Srinivas, Thanks for the testing. I'm reworking the driver to incorporate yours, Linus' and Ivans feedback. > Two things which I noticed while trying out this driver to drive a reset > line. > > 1> gpio numbering for pinconf vs gpio are not consistent, they differ by > an offset of 1. > After scratching my head regarding this I now see that there's a off by one in most of the gpio functions. I've rewroked this to make more sense now. The gpio numbers has to start at 1, as all documentation is based on that. [...] > > 2> Looking back at v3.4 kernel, for gpio modes, BIT(0) of bank 0 is set > to enable gpio mode. without this bit driver does not work for output pins. > Thanks, I missed that. Unfortunately, setting that bit results in input not working - the interrupt bits are never set for gpios that have that bit set. I'm trying to figure out why this is the case before sending out the new version... Regards, Bjorn -- 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
On Wed, Aug 20, 2014 at 2:28 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote: > On Wed 20 Aug 01:06 PDT 2014, Srinivas Kandagatla wrote: >> 2> Looking back at v3.4 kernel, for gpio modes, BIT(0) of bank 0 is set >> to enable gpio mode. without this bit driver does not work for output pins. >> > > Thanks, I missed that. > > Unfortunately, setting that bit results in input not working - the interrupt > bits are never set for gpios that have that bit set. I'm trying to figure out > why this is the case before sending out the new version... With help from Andy Gross this is now corrected as well, turned out that BIT(0) in bank0 controls if the "direction" is considered. As I was tricked by the multiple levels of indirection in the codeaurora version I got these wrong. Will send out an updated version shortly. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, On 20/08/14 23:13, Bjorn Andersson wrote: > On Wed, Aug 20, 2014 at 2:28 PM, Bjorn Andersson > <bjorn.andersson@sonymobile.com> wrote: >> On Wed 20 Aug 01:06 PDT 2014, Srinivas Kandagatla wrote: >>> 2> Looking back at v3.4 kernel, for gpio modes, BIT(0) of bank 0 is set >>> to enable gpio mode. without this bit driver does not work for output pins. >>> >> >> Thanks, I missed that. >> >> Unfortunately, setting that bit results in input not working - the interrupt >> bits are never set for gpios that have that bit set. I'm trying to figure out >> why this is the case before sending out the new version... > > With help from Andy Gross this is now corrected as well, turned out > that BIT(0) in bank0 controls if the "direction" is considered. As I > was tricked by the multiple levels of indirection in the codeaurora > version I got these wrong. > > Will send out an updated version shortly. Did you get a chance to spin a new version of this patcheset? --srini > > Regards, > Bjorn > -- 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
On Mon 03 Nov 00:48 PST 2014, Srinivas Kandagatla wrote: > Hi Bjorn, > > On 20/08/14 23:13, Bjorn Andersson wrote: > > On Wed, Aug 20, 2014 at 2:28 PM, Bjorn Andersson > > <bjorn.andersson@sonymobile.com> wrote: > >> On Wed 20 Aug 01:06 PDT 2014, Srinivas Kandagatla wrote: > >>> 2> Looking back at v3.4 kernel, for gpio modes, BIT(0) of bank 0 is set > >>> to enable gpio mode. without this bit driver does not work for output pins. > >>> > >> > >> Thanks, I missed that. > >> > >> Unfortunately, setting that bit results in input not working - the interrupt > >> bits are never set for gpios that have that bit set. I'm trying to figure out > >> why this is the case before sending out the new version... > > > > With help from Andy Gross this is now corrected as well, turned out > > that BIT(0) in bank0 controls if the "direction" is considered. As I > > was tricked by the multiple levels of indirection in the codeaurora > > version I got these wrong. > > > > Will send out an updated version shortly. > Did you get a chance to spin a new version of this patcheset? > Hi Srini, Unfortunately it ended up on my todo list waiting for a discussion on irq_read_line(). As this seems to fall in place I will try to get this updated and sent out. Sorry for the delays. Regards, Bjorn -- 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
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c b/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c index 27e5ec9..2633ea1 100644 --- a/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c +++ b/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c @@ -49,6 +49,7 @@ #define SSBI_REG_ADDR_GPIO_BASE 0x150 #define SSBI_REG_ADDR_GPIO(n) (SSBI_REG_ADDR_GPIO_BASE + n) +#define PM8XXX_GPIO_MODE_ENABLE BIT(0) #define PM8XXX_GPIO_WRITE BIT(7) #define PM8XXX_MAX_GPIOS 44 @@ -493,7 +494,8 @@ static int pm8xxx_gpio_config_set(struct pinctrl_dev *pctldev, } if (banks & BIT(0)) - pm8xxx_gpio_write(pctrl, offset, 0, pin->power_source << 1); + pm8xxx_gpio_write(pctrl, offset, 0, pin->power_source << 1 | + PM8XXX_GPIO_MODE_ENABLE); if (banks & BIT(1)) {
Hi Bjorn, Two things which I noticed while trying out this driver to drive a reset line. 1> gpio numbering for pinconf vs gpio are not consistent, they differ by an offset of 1. For example to control GPIO43 I had to do something like this in pinconf: wlan_default_gpios: wlan-gpios { pios { pins = "gpio43"; function = "normal"; bias-disable; power-source = <PM8921_GPIO_S4>; }; }; for same pin for gpio I had to do: reset-gpio = <&pm8921_gpio 42 GPIO_ACTIVE_LOW>; This offset by 1 is bit confusing and can easily lead to errors which are hard to find. I think this should be easy to fix.. 2> Looking back at v3.4 kernel, for gpio modes, BIT(0) of bank 0 is set to enable gpio mode. without this bit driver does not work for output pins. here is the patch which fixes this: From 5a5c5171a3371cf38ccd157922ea140bfd0253d5 Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Date: Wed, 20 Aug 2014 07:18:03 +0100 Subject: [PATCH] pinctrl:qcom:ssbi: Enable gpio mode Looking back at 3.4 kernel driver, it looks like the gpio enable bit is missing in this driver. This patch is just a hack to get the wlan reset line working. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) val = pin->direction << 2;