[v2] pinctrl: rockchip: fix pull setting error for rk3399

Commit Message

Caesar Wang May 11, 2016, 3:39 a.m. UTC

From: David Wu <[e-mail]>

This patch fixes the pinctrl pull bias setting, since the pull up/down
setting is the contrary for gpio0(just the gpio0a and gpio0b) and
gpio2(just the gpio2c and gpio2d).

From the TRM said, the gpio0a pull polarity setting:
gpio0a_p
GPIO0A PE/PS programmation section, every
GPIO bit corresponding to 2bits[PS:PE]
2'b00: Z(Normal operation);
2'b11: weak 1(pull-up);
2'b01: weak 0(pull-down);
2'b10: Z(Normal operation);

Then, the other gpios setting as the following:
gpio1a_p (e.g.: gpio1, gpio2a, gpio2b, gpio3...)
GPIO1A PU/PD programmation section, every
GPIO bit corresponding to 2bits
2'b00: Z(Normal operation);
2'b01: weak 1(pull-up);
2'b10: weak 0(pull-down);
2'b11: Z(Normal operation);

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 has been set
in the dts.

Signed-off-by: David Wu <[e-mail]>
Signed-off-by: Caesar Wang <[e-mail]>
Cc: Linus Walleij <[e-mail]>
Cc: Heiko Stuebner <[e-mail]>
Cc: [e-mail]
---

Changes in v2:
- As Doug commnets on https://patchwork.kernel.org/patch/9056961/
- update the commit for gpio2.
- change the print error messgae.

 drivers/pinctrl/pinctrl-rockchip.c | 179 ++++++++++++++++++++++++++-----------
 1 file changed, 127 insertions(+), 52 deletions(-)

Comments

Doug Anderson May 11, 2016, 4 a.m. UTC | #1

Caesar,

On Tue, May 10, 2016 at 8:39 PM, Caesar Wang <[e-mail]> wrote:
> From: David Wu <[e-mail]>
>
> This patch fixes the pinctrl pull bias setting, since the pull up/down
> setting is the contrary for gpio0(just the gpio0a and gpio0b) and
> gpio2(just the gpio2c and gpio2d).
>
> From the TRM said, the gpio0a pull polarity setting:
> gpio0a_p
> GPIO0A PE/PS programmation section, every
> GPIO bit corresponding to 2bits[PS:PE]
> 2'b00: Z(Normal operation);
> 2'b11: weak 1(pull-up);
> 2'b01: weak 0(pull-down);
> 2'b10: Z(Normal operation);
>
> Then, the other gpios setting as the following:
> gpio1a_p (e.g.: gpio1, gpio2a, gpio2b, gpio3...)
> GPIO1A PU/PD programmation section, every
> GPIO bit corresponding to 2bits
> 2'b00: Z(Normal operation);
> 2'b01: weak 1(pull-up);
> 2'b10: weak 0(pull-down);
> 2'b11: Z(Normal operation);
>
> 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 has been set
> in the dts.
>
> Signed-off-by: David Wu <[e-mail]>
> Signed-off-by: Caesar Wang <[e-mail]>
> Cc: Linus Walleij <[e-mail]>
> Cc: Heiko Stuebner <[e-mail]>
> Cc: [e-mail]
> ---
>
> Changes in v2:
> - As Doug commnets on https://patchwork.kernel.org/patch/9056961/
> - update the commit for gpio2.
> - change the print error messgae.
>
>  drivers/pinctrl/pinctrl-rockchip.c | 179 ++++++++++++++++++++++++++-----------
>  1 file changed, 127 insertions(+), 52 deletions(-)

Reviewed-by: Douglas Anderson <[e-mail]>

Heiko Stübner May 11, 2016, 7:11 a.m. UTC | #2

Am Mittwoch, 11. Mai 2016, 11:39:28 schrieb Caesar Wang:
> From: David Wu <[e-mail]>
> 
> This patch fixes the pinctrl pull bias setting, since the pull up/down
> setting is the contrary for gpio0(just the gpio0a and gpio0b) and
> gpio2(just the gpio2c and gpio2d).
> 
> From the TRM said, the gpio0a pull polarity setting:
> gpio0a_p
> GPIO0A PE/PS programmation section, every
> GPIO bit corresponding to 2bits[PS:PE]
> 2'b00: Z(Normal operation);
> 2'b11: weak 1(pull-up);
> 2'b01: weak 0(pull-down);
> 2'b10: Z(Normal operation);

This could mention gpio2c and gpio2d as well, as the code now does.

Otherwise the patch looks good and I also compared with the TRM, so

Reviewed-by: Heiko Stuebner <[e-mail]>

Linus Walleij May 11, 2016, 8:44 a.m. UTC | #3

On Wed, May 11, 2016 at 5:39 AM, Caesar Wang <[e-mail]> wrote:

> From: David Wu <[e-mail]>
>
> This patch fixes the pinctrl pull bias setting, since the pull up/down
> setting is the contrary for gpio0(just the gpio0a and gpio0b) and
> gpio2(just the gpio2c and gpio2d).
>
> From the TRM said, the gpio0a pull polarity setting:
> gpio0a_p
> GPIO0A PE/PS programmation section, every
> GPIO bit corresponding to 2bits[PS:PE]
> 2'b00: Z(Normal operation);
> 2'b11: weak 1(pull-up);
> 2'b01: weak 0(pull-down);
> 2'b10: Z(Normal operation);
>
> Then, the other gpios setting as the following:
> gpio1a_p (e.g.: gpio1, gpio2a, gpio2b, gpio3...)
> GPIO1A PU/PD programmation section, every
> GPIO bit corresponding to 2bits
> 2'b00: Z(Normal operation);
> 2'b01: weak 1(pull-up);
> 2'b10: weak 0(pull-down);
> 2'b11: Z(Normal operation);
>
> 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 has been set
> in the dts.
>
> Signed-off-by: David Wu <[e-mail]>
> Signed-off-by: Caesar Wang <[e-mail]>
> Cc: Linus Walleij <[e-mail]>
> Cc: Heiko Stuebner <[e-mail]>
> Cc: [e-mail]
> ---
>
> Changes in v2:
> - As Doug commnets on https://patchwork.kernel.org/patch/9056961/
> - update the commit for gpio2.
> - change the print error messgae.

Patch applied with the Review tags.

Yours,
Linus Walleij

Patch

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 596b869..a91026e 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,28 @@  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, "unsupported 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 +2618,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 +2649,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,