Re: [PATCH V1 2/3] pinctrl: qcom: spmi-gpio: Add dtest route for digital input
From: Bjorn Andersson <hidden>
Date: 2017-07-12 21:25:02
Also in:
linux-arm-msm, linux-gpio, lkml
On Mon 12 Jun 23:16 PDT 2017, fenglinw-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org wrote:
quoted hunk ↗ jump to hunk
From: Fenglin Wu <redacted> Add property "qcom,dtest-buffer" to specify which dtest rail to feed when the pin is configured as a digital input. Signed-off-by: Fenglin Wu <redacted> --- .../devicetree/bindings/pinctrl/qcom,pmic-gpio.txt | 15 ++++++++ drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 45 ++++++++++++++++++++++ include/dt-bindings/pinctrl/qcom,pmic-gpio.h | 6 +++ 3 files changed, 66 insertions(+)diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt index 1493c0a..521c783 100644 --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt@@ -195,6 +195,21 @@ to specify in a pin configuration subnode: Valid values are 0-3 corresponding to PMIC_GPIO_AOUT_ATESTx defined in <dt-bindings/pinctrl/qcom,pmic-gpio.h>. +- qcom,dtest-buffer: + Usage: optional + Value type: <u32> + Definition: Selects DTEST rail to route to GPIO when it's configured + as a digital input. + For LV/MV GPIO subtypes, the valid values are 0-3 + corresponding to PMIC_GPIO_DIN_DTESTx defined in + <dt-bindings/pinctrl/qcom,pmic-gpio.h>. Only one + DTEST rail can be selected at a time.
As with the analog lines, this is a natural number and I think we should encode it as such in the DeviceTree.
+ For 4CH/8CH GPIO subtypes, supported values are 1-15. + 4 DTEST rails are supported in total and more than 1 DTEST + rail can be selected simultaneously. Each bit of the + 4 LSBs represent one DTEST rail, such as [3:0] = 0101 + means both DTEST1 and DTEST3 are selected.
I'm not too keen on encoding this as a bitmask. I would much rather encode it as multiple values - although that complicates the implementation. Or can we just ignore it? (Is the lack of support in the newer chips a result of no-one using this?)
quoted hunk ↗ jump to hunk
+ Example: pm8921_gpio: gpio@150 {diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
[..]
quoted hunk ↗ jump to hunk
@@ -512,6 +526,13 @@ static int pmic_gpio_config_set(struct pinctrl_dev *pctldev, unsigned int pin, return -EINVAL; pad->atest = arg; break; + case PMIC_GPIO_CONF_DTEST_BUFFER: + if ((pad->lv_mv_type && arg > PMIC_GPIO_DIN_DTEST4) + || (!pad->lv_mv_type && arg > + PMIC_GPIO_DIG_IN_DTEST_SEL_MASK)) + return -EINVAL; + pad->dtest_buffer = arg; + break; default: return -EINVAL; }@@ -544,6 +565,17 @@ static int pmic_gpio_config_set(struct pinctrl_dev *pctldev, unsigned int pin, val = PMIC_GPIO_MODE_DIGITAL_OUTPUT; }
Remember that you're supposed to be able to have two different states defines, one with dtest-buffer and one without - and switching between them should enable _and_ disable the dtest buffer. So you need to detect the absence of qcom,dtest-buffer and you need to write the register in this case as well. So before looping over all the config parameters, set pad->dtest_buffer to "disabled" and when you get here it will either be disabled or have the specified value.
+ if (pad->dtest_buffer != INT_MAX) {
+ val = pad->dtest_buffer;
+ if (pad->lv_mv_type)
+ val |= PMIC_GPIO_LV_MV_DIG_IN_DTEST_EN;
+Instead of representing "disabled" as INT_MAX, you could keep track of it in the same representation as the hardware, i.e. 0 would be disabled. The additional effort would be in the lv_mv case that you need to mask in PMIC_GPIO_LV_MV_DIG_IN_DTEST_EN in the few places where you actually enable dtest buffering.
+ ret = pmic_gpio_write(state, pad, + PMIC_GPIO_REG_DIG_IN_CTL, val); + if (ret < 0) + return ret; + } +
[..]
quoted hunk ↗ jump to hunk
diff --git a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h index 85d8809..21738ed 100644 --- a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h +++ b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h@@ -99,6 +99,12 @@ #define PMIC_GPIO_AOUT_ATEST3 2 #define PMIC_GPIO_AOUT_ATEST4 3 +/* DTEST buffer for digital input mode */ +#define PMIC_GPIO_DIN_DTEST1 0 +#define PMIC_GPIO_DIN_DTEST2 1 +#define PMIC_GPIO_DIN_DTEST3 2 +#define PMIC_GPIO_DIN_DTEST4 3 +
Please drop these defines. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html