* [PATCH] pinctrl: rockchip: fix pull setting error for rk3399
@ 2016-05-10 11:14 Caesar Wang
2016-05-10 21:07 ` Doug Anderson
0 siblings, 1 reply; 3+ messages in thread
From: Caesar Wang @ 2016-05-10 11:14 UTC (permalink / raw)
To: Linus Walleij, Heiko Stuebner
Cc: dianders, briannorris, smbarber, linux-rockchip, David Wu,
Caesar Wang, linux-gpio, linux-arm-kernel, linux-kernel
From: David Wu <david.wu@rock-chips.com>
This patch fixes the pinctrl pull bias setting, since the pull up/down
setting is the contrary for gpio0.
>From the TRM said, the gpio0 pull polarity setting:
gpio0a_p (gpio0 )
GPIO0A PE/PS programmation section, every
GPIO bit corresponding to 2bits[PS:PE]
2'b00: Z(Noraml operaton);
2'b11: weak 1(pull-up);
2'b01: weak 0(pull-down);
2'b10: Z(Noraml operaton);
Then, the other gpios setting as the following:
gpio1a_p (gpio1, gpio2, gpio3...)
GPIO1A PU/PD programmation section, every
GPIO bit corresponding to 2bits
2'b00: Z(Noraml operaton);
2'b01: weak 1(pull-up);
2'b10: weak 0(pull-down);
2'b11: Repeater(Bus keeper)
For example,(rk3399evb board)
sdmmc_cd --->gpio0_a7
localhost / # io -r -4 0xff320040
ff320040: 00004d5f
In general,the value should be 0x0000cd5f since the pin set in the dts.
Signed-off-by: David Wu <david.wu@rock-chips.com>
Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: linux-gpio@vger.kernel.org
---
drivers/pinctrl/pinctrl-rockchip.c | 178 ++++++++++++++++++++++++++-----------
1 file changed, 126 insertions(+), 52 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 596b869..38be9c7 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -99,6 +99,15 @@ enum rockchip_pin_drv_type {
};
/**
+ * enum type index corresponding to rockchip_pull_list arrays index.
+ */
+enum rockchip_pin_pull_type {
+ PULL_TYPE_IO_DEFAULT = 0,
+ PULL_TYPE_IO_1V8_ONLY,
+ PULL_TYPE_MAX
+};
+
+/**
* @drv_type: drive strength variant using rockchip_perpin_drv_type
* @offset: if initialized to -1 it will be autocalculated, by specifying
* an initial offset value the relevant source offset can be reset
@@ -123,6 +132,7 @@ struct rockchip_drv {
* @bank_num: number of the bank, to account for holes
* @iomux: array describing the 4 iomux sources of the bank
* @drv: array describing the 4 drive strength sources of the bank
+ * @pull_type: array describing the 4 pull type sources of the bank
* @valid: are all necessary informations present
* @of_node: dt node of this bank
* @drvdata: common pinctrl basedata
@@ -143,6 +153,7 @@ struct rockchip_pin_bank {
u8 bank_num;
struct rockchip_iomux iomux[4];
struct rockchip_drv drv[4];
+ enum rockchip_pin_pull_type pull_type[4];
bool valid;
struct device_node *of_node;
struct rockchip_pinctrl *drvdata;
@@ -198,6 +209,31 @@ struct rockchip_pin_bank {
}, \
}
+#define PIN_BANK_DRV_FLAGS_PULL_FLAGS(id, pins, label, drv0, drv1, \
+ drv2, drv3, pull0, pull1, \
+ pull2, pull3) \
+ { \
+ .bank_num = id, \
+ .nr_pins = pins, \
+ .name = label, \
+ .iomux = { \
+ { .offset = -1 }, \
+ { .offset = -1 }, \
+ { .offset = -1 }, \
+ { .offset = -1 }, \
+ }, \
+ .drv = { \
+ { .drv_type = drv0, .offset = -1 }, \
+ { .drv_type = drv1, .offset = -1 }, \
+ { .drv_type = drv2, .offset = -1 }, \
+ { .drv_type = drv3, .offset = -1 }, \
+ }, \
+ .pull_type[0] = pull0, \
+ .pull_type[1] = pull1, \
+ .pull_type[2] = pull2, \
+ .pull_type[3] = pull3, \
+ }
+
#define PIN_BANK_IOMUX_DRV_FLAGS_OFFSET(id, pins, label, iom0, iom1, \
iom2, iom3, drv0, drv1, drv2, \
drv3, offset0, offset1, \
@@ -220,6 +256,34 @@ struct rockchip_pin_bank {
}, \
}
+#define PIN_BANK_IOMUX_FLAGS_DRV_FLAGS_OFFSET_PULL_FLAGS(id, pins, \
+ label, iom0, iom1, iom2, \
+ iom3, drv0, drv1, drv2, \
+ drv3, offset0, offset1, \
+ offset2, offset3, pull0, \
+ pull1, pull2, pull3) \
+ { \
+ .bank_num = id, \
+ .nr_pins = pins, \
+ .name = label, \
+ .iomux = { \
+ { .type = iom0, .offset = -1 }, \
+ { .type = iom1, .offset = -1 }, \
+ { .type = iom2, .offset = -1 }, \
+ { .type = iom3, .offset = -1 }, \
+ }, \
+ .drv = { \
+ { .drv_type = drv0, .offset = offset0 }, \
+ { .drv_type = drv1, .offset = offset1 }, \
+ { .drv_type = drv2, .offset = offset2 }, \
+ { .drv_type = drv3, .offset = offset3 }, \
+ }, \
+ .pull_type[0] = pull0, \
+ .pull_type[1] = pull1, \
+ .pull_type[2] = pull2, \
+ .pull_type[3] = pull3, \
+ }
+
/**
*/
struct rockchip_pin_ctrl {
@@ -1020,12 +1084,27 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
return ret;
}
+static int rockchip_pull_list[PULL_TYPE_MAX][4] = {
+ {
+ PIN_CONFIG_BIAS_DISABLE,
+ PIN_CONFIG_BIAS_PULL_UP,
+ PIN_CONFIG_BIAS_PULL_DOWN,
+ PIN_CONFIG_BIAS_BUS_HOLD
+ },
+ {
+ PIN_CONFIG_BIAS_DISABLE,
+ PIN_CONFIG_BIAS_PULL_DOWN,
+ PIN_CONFIG_BIAS_DISABLE,
+ PIN_CONFIG_BIAS_PULL_UP
+ },
+};
+
static int rockchip_get_pull(struct rockchip_pin_bank *bank, int pin_num)
{
struct rockchip_pinctrl *info = bank->drvdata;
struct rockchip_pin_ctrl *ctrl = info->ctrl;
struct regmap *regmap;
- int reg, ret;
+ int reg, ret, pull_type;
u8 bit;
u32 data;
@@ -1048,22 +1127,11 @@ static int rockchip_get_pull(struct rockchip_pin_bank *bank, int pin_num)
case RK3288:
case RK3368:
case RK3399:
+ pull_type = bank->pull_type[pin_num / 8];
data >>= bit;
data &= (1 << RK3188_PULL_BITS_PER_PIN) - 1;
- switch (data) {
- case 0:
- return PIN_CONFIG_BIAS_DISABLE;
- case 1:
- return PIN_CONFIG_BIAS_PULL_UP;
- case 2:
- return PIN_CONFIG_BIAS_PULL_DOWN;
- case 3:
- return PIN_CONFIG_BIAS_BUS_HOLD;
- }
-
- dev_err(info->dev, "unknown pull setting\n");
- return -EIO;
+ return rockchip_pull_list[pull_type][data];
default:
dev_err(info->dev, "unsupported pinctrl type\n");
return -EINVAL;
@@ -1076,7 +1144,7 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
struct rockchip_pinctrl *info = bank->drvdata;
struct rockchip_pin_ctrl *ctrl = info->ctrl;
struct regmap *regmap;
- int reg, ret;
+ int reg, ret, i, pull_type;
unsigned long flags;
u8 bit;
u32 data, rmask;
@@ -1105,30 +1173,27 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
case RK3288:
case RK3368:
case RK3399:
+ pull_type = bank->pull_type[pin_num / 8];
+ ret = -EINVAL;
+ for (i = 0; i < ARRAY_SIZE(rockchip_pull_list[pull_type]);
+ i++) {
+ if (rockchip_pull_list[pull_type][i] == pull) {
+ ret = i;
+ break;
+ }
+ }
+
+ if (ret < 0) {
+ dev_err(info->dev, "unknown pull setting %d\n", pull);
+ return ret;
+ }
+
spin_lock_irqsave(&bank->slock, flags);
/* enable the write to the equivalent lower bits */
data = ((1 << RK3188_PULL_BITS_PER_PIN) - 1) << (bit + 16);
rmask = data | (data >> 16);
-
- switch (pull) {
- case PIN_CONFIG_BIAS_DISABLE:
- break;
- case PIN_CONFIG_BIAS_PULL_UP:
- data |= (1 << bit);
- break;
- case PIN_CONFIG_BIAS_PULL_DOWN:
- data |= (2 << bit);
- break;
- case PIN_CONFIG_BIAS_BUS_HOLD:
- data |= (3 << bit);
- break;
- default:
- spin_unlock_irqrestore(&bank->slock, flags);
- dev_err(info->dev, "unsupported pull setting %d\n",
- pull);
- return -EINVAL;
- }
+ data |= (ret << bit);
ret = regmap_update_bits(regmap, reg, rmask, data);
@@ -2552,19 +2617,24 @@ static struct rockchip_pin_ctrl rk3368_pin_ctrl = {
};
static struct rockchip_pin_bank rk3399_pin_banks[] = {
- PIN_BANK_IOMUX_DRV_FLAGS_OFFSET(0, 32, "gpio0", IOMUX_SOURCE_PMU,
- IOMUX_SOURCE_PMU,
- IOMUX_SOURCE_PMU,
- IOMUX_SOURCE_PMU,
- DRV_TYPE_IO_1V8_ONLY,
- DRV_TYPE_IO_1V8_ONLY,
- DRV_TYPE_IO_DEFAULT,
- DRV_TYPE_IO_DEFAULT,
- 0x0,
- 0x8,
- -1,
- -1
- ),
+ PIN_BANK_IOMUX_FLAGS_DRV_FLAGS_OFFSET_PULL_FLAGS(0, 32, "gpio0",
+ IOMUX_SOURCE_PMU,
+ IOMUX_SOURCE_PMU,
+ IOMUX_SOURCE_PMU,
+ IOMUX_SOURCE_PMU,
+ DRV_TYPE_IO_1V8_ONLY,
+ DRV_TYPE_IO_1V8_ONLY,
+ DRV_TYPE_IO_DEFAULT,
+ DRV_TYPE_IO_DEFAULT,
+ 0x0,
+ 0x8,
+ -1,
+ -1,
+ PULL_TYPE_IO_1V8_ONLY,
+ PULL_TYPE_IO_1V8_ONLY,
+ PULL_TYPE_IO_DEFAULT,
+ PULL_TYPE_IO_DEFAULT
+ ),
PIN_BANK_IOMUX_DRV_FLAGS_OFFSET(1, 32, "gpio1", IOMUX_SOURCE_PMU,
IOMUX_SOURCE_PMU,
IOMUX_SOURCE_PMU,
@@ -2578,11 +2648,15 @@ static struct rockchip_pin_bank rk3399_pin_banks[] = {
0x30,
0x38
),
- PIN_BANK_DRV_FLAGS(2, 32, "gpio2", DRV_TYPE_IO_1V8_OR_3V0,
- DRV_TYPE_IO_1V8_OR_3V0,
- DRV_TYPE_IO_1V8_ONLY,
- DRV_TYPE_IO_1V8_ONLY
- ),
+ PIN_BANK_DRV_FLAGS_PULL_FLAGS(2, 32, "gpio2", DRV_TYPE_IO_1V8_OR_3V0,
+ DRV_TYPE_IO_1V8_OR_3V0,
+ DRV_TYPE_IO_1V8_ONLY,
+ DRV_TYPE_IO_1V8_ONLY,
+ PULL_TYPE_IO_DEFAULT,
+ PULL_TYPE_IO_DEFAULT,
+ PULL_TYPE_IO_1V8_ONLY,
+ PULL_TYPE_IO_1V8_ONLY
+ ),
PIN_BANK_DRV_FLAGS(3, 32, "gpio3", DRV_TYPE_IO_3V3_ONLY,
DRV_TYPE_IO_3V3_ONLY,
DRV_TYPE_IO_3V3_ONLY,
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] pinctrl: rockchip: fix pull setting error for rk3399
2016-05-10 11:14 [PATCH] pinctrl: rockchip: fix pull setting error for rk3399 Caesar Wang
@ 2016-05-10 21:07 ` Doug Anderson
[not found] ` <CAD=FV=VrE=9=sgqUXBMwmoQin8S+yPbAgnrO1VUgx5QafYWXhA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Doug Anderson @ 2016-05-10 21:07 UTC (permalink / raw)
To: Caesar Wang, David Wu
Cc: Linus Walleij, Heiko Stuebner, Brian Norris, Stephen Barber,
open list:ARM/Rockchip SoC..., linux-gpio@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Caesar / David,
On Tue, May 10, 2016 at 4:14 AM, Caesar Wang <wxt@rock-chips.com> wrote:
> From: David Wu <david.wu@rock-chips.com>
>
> This patch fixes the pinctrl pull bias setting, since the pull up/down
> setting is the contrary for gpio0.
Commit message only mentions gpio0, but gpio2 is also fixed in the
commit. Please mention gpio2 in the commit message.
> From the TRM said, the gpio0 pull polarity setting:
> gpio0a_p (gpio0 )
> GPIO0A PE/PS programmation section, every
> GPIO bit corresponding to 2bits[PS:PE]
> 2'b00: Z(Noraml operaton);
> 2'b11: weak 1(pull-up);
> 2'b01: weak 0(pull-down);
> 2'b10: Z(Noraml operaton);
Despite the fact that the typo (Noralm vs. Normal) is present in the
TRM, maybe we should fix it here?
> Then, the other gpios setting as the following:
> gpio1a_p (gpio1, gpio2, gpio3...)
> GPIO1A PU/PD programmation section, every
> GPIO bit corresponding to 2bits
> 2'b00: Z(Noraml operaton);
> 2'b01: weak 1(pull-up);
> 2'b10: weak 0(pull-down);
> 2'b11: Repeater(Bus keeper)
>
> For example,(rk3399evb board)
> sdmmc_cd --->gpio0_a7
> localhost / # io -r -4 0xff320040
> ff320040: 00004d5f
> In general,the value should be 0x0000cd5f since the pin set in the dts.
>
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: linux-gpio@vger.kernel.org
> ---
>
> drivers/pinctrl/pinctrl-rockchip.c | 178 ++++++++++++++++++++++++++-----------
> 1 file changed, 126 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 596b869..38be9c7 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -99,6 +99,15 @@ enum rockchip_pin_drv_type {
> };
>
> /**
> + * enum type index corresponding to rockchip_pull_list arrays index.
> + */
> +enum rockchip_pin_pull_type {
> + PULL_TYPE_IO_DEFAULT = 0,
> + PULL_TYPE_IO_1V8_ONLY,
> + PULL_TYPE_MAX
> +};
> +
> +/**
> * @drv_type: drive strength variant using rockchip_perpin_drv_type
> * @offset: if initialized to -1 it will be autocalculated, by specifying
> * an initial offset value the relevant source offset can be reset
> @@ -123,6 +132,7 @@ struct rockchip_drv {
> * @bank_num: number of the bank, to account for holes
> * @iomux: array describing the 4 iomux sources of the bank
> * @drv: array describing the 4 drive strength sources of the bank
> + * @pull_type: array describing the 4 pull type sources of the bank
> * @valid: are all necessary informations present
> * @of_node: dt node of this bank
> * @drvdata: common pinctrl basedata
> @@ -143,6 +153,7 @@ struct rockchip_pin_bank {
> u8 bank_num;
> struct rockchip_iomux iomux[4];
> struct rockchip_drv drv[4];
> + enum rockchip_pin_pull_type pull_type[4];
> bool valid;
> struct device_node *of_node;
> struct rockchip_pinctrl *drvdata;
> @@ -198,6 +209,31 @@ struct rockchip_pin_bank {
> }, \
> }
>
> +#define PIN_BANK_DRV_FLAGS_PULL_FLAGS(id, pins, label, drv0, drv1, \
> + drv2, drv3, pull0, pull1, \
> + pull2, pull3) \
> + { \
> + .bank_num = id, \
> + .nr_pins = pins, \
> + .name = label, \
> + .iomux = { \
> + { .offset = -1 }, \
> + { .offset = -1 }, \
> + { .offset = -1 }, \
> + { .offset = -1 }, \
> + }, \
> + .drv = { \
> + { .drv_type = drv0, .offset = -1 }, \
> + { .drv_type = drv1, .offset = -1 }, \
> + { .drv_type = drv2, .offset = -1 }, \
> + { .drv_type = drv3, .offset = -1 }, \
> + }, \
> + .pull_type[0] = pull0, \
> + .pull_type[1] = pull1, \
> + .pull_type[2] = pull2, \
> + .pull_type[3] = pull3, \
> + }
> +
> #define PIN_BANK_IOMUX_DRV_FLAGS_OFFSET(id, pins, label, iom0, iom1, \
> iom2, iom3, drv0, drv1, drv2, \
> drv3, offset0, offset1, \
> @@ -220,6 +256,34 @@ struct rockchip_pin_bank {
> }, \
> }
>
> +#define PIN_BANK_IOMUX_FLAGS_DRV_FLAGS_OFFSET_PULL_FLAGS(id, pins, \
> + label, iom0, iom1, iom2, \
> + iom3, drv0, drv1, drv2, \
> + drv3, offset0, offset1, \
> + offset2, offset3, pull0, \
> + pull1, pull2, pull3) \
> + { \
> + .bank_num = id, \
> + .nr_pins = pins, \
> + .name = label, \
> + .iomux = { \
> + { .type = iom0, .offset = -1 }, \
> + { .type = iom1, .offset = -1 }, \
> + { .type = iom2, .offset = -1 }, \
> + { .type = iom3, .offset = -1 }, \
> + }, \
> + .drv = { \
> + { .drv_type = drv0, .offset = offset0 }, \
> + { .drv_type = drv1, .offset = offset1 }, \
> + { .drv_type = drv2, .offset = offset2 }, \
> + { .drv_type = drv3, .offset = offset3 }, \
> + }, \
> + .pull_type[0] = pull0, \
> + .pull_type[1] = pull1, \
> + .pull_type[2] = pull2, \
> + .pull_type[3] = pull3, \
> + }
> +
> /**
> */
> struct rockchip_pin_ctrl {
> @@ -1020,12 +1084,27 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
> return ret;
> }
>
> +static int rockchip_pull_list[PULL_TYPE_MAX][4] = {
> + {
> + PIN_CONFIG_BIAS_DISABLE,
> + PIN_CONFIG_BIAS_PULL_UP,
> + PIN_CONFIG_BIAS_PULL_DOWN,
> + PIN_CONFIG_BIAS_BUS_HOLD
> + },
> + {
> + PIN_CONFIG_BIAS_DISABLE,
> + PIN_CONFIG_BIAS_PULL_DOWN,
> + PIN_CONFIG_BIAS_DISABLE,
> + PIN_CONFIG_BIAS_PULL_UP
> + },
> +};
> +
> static int rockchip_get_pull(struct rockchip_pin_bank *bank, int pin_num)
> {
> struct rockchip_pinctrl *info = bank->drvdata;
> struct rockchip_pin_ctrl *ctrl = info->ctrl;
> struct regmap *regmap;
> - int reg, ret;
> + int reg, ret, pull_type;
> u8 bit;
> u32 data;
>
> @@ -1048,22 +1127,11 @@ static int rockchip_get_pull(struct rockchip_pin_bank *bank, int pin_num)
> case RK3288:
> case RK3368:
> case RK3399:
> + pull_type = bank->pull_type[pin_num / 8];
> data >>= bit;
> data &= (1 << RK3188_PULL_BITS_PER_PIN) - 1;
>
> - switch (data) {
> - case 0:
> - return PIN_CONFIG_BIAS_DISABLE;
> - case 1:
> - return PIN_CONFIG_BIAS_PULL_UP;
> - case 2:
> - return PIN_CONFIG_BIAS_PULL_DOWN;
> - case 3:
> - return PIN_CONFIG_BIAS_BUS_HOLD;
> - }
> -
> - dev_err(info->dev, "unknown pull setting\n");
> - return -EIO;
> + return rockchip_pull_list[pull_type][data];
> default:
> dev_err(info->dev, "unsupported pinctrl type\n");
> return -EINVAL;
> @@ -1076,7 +1144,7 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
> struct rockchip_pinctrl *info = bank->drvdata;
> struct rockchip_pin_ctrl *ctrl = info->ctrl;
> struct regmap *regmap;
> - int reg, ret;
> + int reg, ret, i, pull_type;
> unsigned long flags;
> u8 bit;
> u32 data, rmask;
> @@ -1105,30 +1173,27 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
> case RK3288:
> case RK3368:
> case RK3399:
> + pull_type = bank->pull_type[pin_num / 8];
> + ret = -EINVAL;
> + for (i = 0; i < ARRAY_SIZE(rockchip_pull_list[pull_type]);
> + i++) {
> + if (rockchip_pull_list[pull_type][i] == pull) {
> + ret = i;
> + break;
> + }
> + }
> +
> + if (ret < 0) {
> + dev_err(info->dev, "unknown pull setting %d\n", pull);
nit: why change the error message? Old message was "unsupported"
instead of your new "unknown". "unsupported" was better IMHO.
> + return ret;
> + }
> +
> spin_lock_irqsave(&bank->slock, flags);
>
> /* enable the write to the equivalent lower bits */
> data = ((1 << RK3188_PULL_BITS_PER_PIN) - 1) << (bit + 16);
> rmask = data | (data >> 16);
> -
> - switch (pull) {
> - case PIN_CONFIG_BIAS_DISABLE:
> - break;
> - case PIN_CONFIG_BIAS_PULL_UP:
> - data |= (1 << bit);
> - break;
> - case PIN_CONFIG_BIAS_PULL_DOWN:
> - data |= (2 << bit);
> - break;
> - case PIN_CONFIG_BIAS_BUS_HOLD:
> - data |= (3 << bit);
> - break;
> - default:
> - spin_unlock_irqrestore(&bank->slock, flags);
> - dev_err(info->dev, "unsupported pull setting %d\n",
> - pull);
> - return -EINVAL;
> - }
> + data |= (ret << bit);
>
> ret = regmap_update_bits(regmap, reg, rmask, data);
>
> @@ -2552,19 +2617,24 @@ static struct rockchip_pin_ctrl rk3368_pin_ctrl = {
> };
>
> static struct rockchip_pin_bank rk3399_pin_banks[] = {
> - PIN_BANK_IOMUX_DRV_FLAGS_OFFSET(0, 32, "gpio0", IOMUX_SOURCE_PMU,
> - IOMUX_SOURCE_PMU,
> - IOMUX_SOURCE_PMU,
> - IOMUX_SOURCE_PMU,
> - DRV_TYPE_IO_1V8_ONLY,
> - DRV_TYPE_IO_1V8_ONLY,
> - DRV_TYPE_IO_DEFAULT,
> - DRV_TYPE_IO_DEFAULT,
> - 0x0,
> - 0x8,
> - -1,
> - -1
> - ),
> + PIN_BANK_IOMUX_FLAGS_DRV_FLAGS_OFFSET_PULL_FLAGS(0, 32, "gpio0",
> + IOMUX_SOURCE_PMU,
> + IOMUX_SOURCE_PMU,
> + IOMUX_SOURCE_PMU,
> + IOMUX_SOURCE_PMU,
> + DRV_TYPE_IO_1V8_ONLY,
> + DRV_TYPE_IO_1V8_ONLY,
> + DRV_TYPE_IO_DEFAULT,
> + DRV_TYPE_IO_DEFAULT,
> + 0x0,
> + 0x8,
> + -1,
> + -1,
> + PULL_TYPE_IO_1V8_ONLY,
> + PULL_TYPE_IO_1V8_ONLY,
> + PULL_TYPE_IO_DEFAULT,
> + PULL_TYPE_IO_DEFAULT
> + ),
> PIN_BANK_IOMUX_DRV_FLAGS_OFFSET(1, 32, "gpio1", IOMUX_SOURCE_PMU,
> IOMUX_SOURCE_PMU,
> IOMUX_SOURCE_PMU,
> @@ -2578,11 +2648,15 @@ static struct rockchip_pin_bank rk3399_pin_banks[] = {
> 0x30,
> 0x38
> ),
> - PIN_BANK_DRV_FLAGS(2, 32, "gpio2", DRV_TYPE_IO_1V8_OR_3V0,
> - DRV_TYPE_IO_1V8_OR_3V0,
> - DRV_TYPE_IO_1V8_ONLY,
> - DRV_TYPE_IO_1V8_ONLY
> - ),
> + PIN_BANK_DRV_FLAGS_PULL_FLAGS(2, 32, "gpio2", DRV_TYPE_IO_1V8_OR_3V0,
> + DRV_TYPE_IO_1V8_OR_3V0,
> + DRV_TYPE_IO_1V8_ONLY,
> + DRV_TYPE_IO_1V8_ONLY,
> + PULL_TYPE_IO_DEFAULT,
> + PULL_TYPE_IO_DEFAULT,
> + PULL_TYPE_IO_1V8_ONLY,
> + PULL_TYPE_IO_1V8_ONLY
Are you certain that gpio2 behaves the same way? The TRM I have says
this for GPIO2C and GPIO2D:
2'b00: pervious-state
2'b01: weak 0(pull-down);
2'b10: pervious-state
2'b11: weak 1(pull-up);
Assuming that "pervious-state" is a simple typo for "previous state"
that would imply that it was behaving as "bus hold" and _not_ "bias
disable".
Note: if it actually is a "bus hold" state then we'll have to figure
out how this would work with existing device trees. I'd imagine that
they are currently specifying "bias disable" and technically that
might not be possible?
-Doug
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] pinctrl: rockchip: fix pull setting error for rk3399
[not found] ` <CAD=FV=VrE=9=sgqUXBMwmoQin8S+yPbAgnrO1VUgx5QafYWXhA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-05-11 3:31 ` Caesar Wang
0 siblings, 0 replies; 3+ messages in thread
From: Caesar Wang @ 2016-05-11 3:31 UTC (permalink / raw)
To: Doug Anderson
Cc: Heiko Stuebner, open list:ARM/Rockchip SoC..., Linus Walleij,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Barber, David Wu, Brian Norris,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Caesar Wang
Doug,
于 2016年05月11日 05:07, Doug Anderson 写道:
> Caesar / David,
>
> On Tue, May 10, 2016 at 4:14 AM, Caesar Wang <wxt@rock-chips.com> wrote:
>> From: David Wu <david.wu@rock-chips.com>
>>
>> This patch fixes the pinctrl pull bias setting, since the pull up/down
>> setting is the contrary for gpio0.
> Commit message only mentions gpio0, but gpio2 is also fixed in the
> commit. Please mention gpio2 in the commit message.
Fix it in next version.
>
>> From the TRM said, the gpio0 pull polarity setting:
>> gpio0a_p (gpio0 )
>> GPIO0A PE/PS programmation section, every
>> GPIO bit corresponding to 2bits[PS:PE]
>> 2'b00: Z(Noraml operaton);
>> 2'b11: weak 1(pull-up);
>> 2'b01: weak 0(pull-down);
>> 2'b10: Z(Noraml operaton);
> Despite the fact that the typo (Noralm vs. Normal) is present in the
> TRM, maybe we should fix it here?
Yep, ditto.
> <cut>
>
> +
> + if (ret < 0) {
> + dev_err(info->dev, "unknown pull setting %d\n", pull);
> nit: why change the error message? Old message was "unsupported"
> instead of your new "unknown". "unsupported" was better IMHO.
Okay, sound resonable.
>
>
>> - PIN_BANK_DRV_FLAGS(2, 32, "gpio2", DRV_TYPE_IO_1V8_OR_3V0,
>> - DRV_TYPE_IO_1V8_OR_3V0,
>> - DRV_TYPE_IO_1V8_ONLY,
>> - DRV_TYPE_IO_1V8_ONLY
>> - ),
>> + PIN_BANK_DRV_FLAGS_PULL_FLAGS(2, 32, "gpio2", DRV_TYPE_IO_1V8_OR_3V0,
>> + DRV_TYPE_IO_1V8_OR_3V0,
>> + DRV_TYPE_IO_1V8_ONLY,
>> + DRV_TYPE_IO_1V8_ONLY,
>> + PULL_TYPE_IO_DEFAULT,
>> + PULL_TYPE_IO_DEFAULT,
>>
> Are you certain that gpio2 behaves the same way? The TRM I have says
> this for GPIO2C and GPIO2D:
+ PULL_TYPE_IO_1V8_ONLY,
+ PULL_TYPE_IO_1V8_ONLY
Yep, so this just set the gpio2c&gpio2d in this function.
>
> 2'b00: pervious-state
> 2'b01: weak 0(pull-down);
> 2'b10: pervious-state
> 2'b11: weak 1(pull-up);
>
> Assuming that "pervious-state" is a simple typo for "previous state"
> that would imply that it was behaving as "bus hold" and _not_ "bias
> disable".
I will say that TRM made the mistake since this is *not* exist.
Okay, fixes by the newer TRM.
>
> Note: if it actually is a "bus hold" state then we'll have to figure
> out how this would work with existing device trees. I'd imagine that
> they are currently specifying "bias disable" and technically that
> might not be possible?
>
>
>
> -Doug
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-05-11 3:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-10 11:14 [PATCH] pinctrl: rockchip: fix pull setting error for rk3399 Caesar Wang
2016-05-10 21:07 ` Doug Anderson
[not found] ` <CAD=FV=VrE=9=sgqUXBMwmoQin8S+yPbAgnrO1VUgx5QafYWXhA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-05-11 3:31 ` Caesar Wang
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).