Thread (18 messages) 18 messages, 5 authors, 2017-09-22

[PATCH 3/3] pinctrl: add mt2712 pinctrl driver

From: Yingjoe Chen <hidden>
Date: 2017-08-01 09:14:13
Also in: linux-devicetree, linux-gpio, linux-mediatek, lkml


Hi Zhiyong,



On Mon, 2017-07-31 at 16:22 +0800, Zhiyong Tao wrote:
<...>
3)Add "spec_dir_set" and "spec_dir_get" in "mtk_pinctrl_devdata".
4)Change "spec_dir_set" and add "spec_dir_get" in "pinctrl-mt2701.c"
  and "pinctrl-mtk-common.c".
I think these deserve another patch.
Please also explain why we need this.

5)Change "port_mask" from "7" to "6" for EINT.
I'm assuming this is a bug fix for mt2701?
If yes, this should be a separate patch.
6)Remove generic pull config condition in "mtk_pconf_set_pull_select".
7)Change "arg" to "MTK_PUPD_SET_R1R0_00" of "mtk_pconf_set_pull_select".
Why we need to change arg?

Signed-off-by: Zhiyong Tao <redacted>
---
 drivers/pinctrl/mediatek/Kconfig              |    8 +
 drivers/pinctrl/mediatek/Makefile             |    1 +
 drivers/pinctrl/mediatek/pinctrl-mt2701.c     |   21 +-
 drivers/pinctrl/mediatek/pinctrl-mt2712.c     |  670 +++++++++
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c |   16 +-
 drivers/pinctrl/mediatek/pinctrl-mtk-common.h |   44 +-
 drivers/pinctrl/mediatek/pinctrl-mtk-mt2712.h | 1858 +++++++++++++++++++++++++
 7 files changed, 2586 insertions(+), 32 deletions(-)
 create mode 100644 drivers/pinctrl/mediatek/pinctrl-mt2712.c
 create mode 100644 drivers/pinctrl/mediatek/pinctrl-mtk-mt2712.h
<...>
quoted hunk ↗ jump to hunk
diff --git a/drivers/pinctrl/mediatek/pinctrl-mt2701.c b/drivers/pinctrl/mediatek/pinctrl-mt2701.c
index f86f3b3..4a43f5c 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mt2701.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mt2701.c
@@ -503,10 +503,26 @@ static void mt2701_spec_pinmux_set(struct regmap *reg, unsigned int pin,
 	regmap_update_bits(reg, mt2701_spec_pinmux[i].offset, mask, value);
 }
 
-static void mt2701_spec_dir_set(unsigned int *reg_addr, unsigned int pin)
+static int mt2701_spec_dir_set(struct mtk_pinctrl *pctl,
+				unsigned int *reg_addr,
+				unsigned int pin,
+				bool input)
 {
 	if (pin > 175)
 		*reg_addr += 0x10;
+
+	return 0;
+}
+
+static int mt2701_spec_dir_get(struct mtk_pinctrl *pctl,
+				unsigned int *reg_addr,
+				unsigned int pin,
+				bool input)
incorrect prototype?
quoted hunk ↗ jump to hunk
+{
+	if (pin > 175)
+		*reg_addr += 0x10;
+
+	return 0;
 }
 
 static const struct mtk_pinctrl_devdata mt2701_pinctrl_data = {
@@ -520,6 +536,7 @@ static void mt2701_spec_dir_set(unsigned int *reg_addr, unsigned int pin)
 	.spec_ies_smt_set = mt2701_ies_smt_set,
 	.spec_pinmux_set = mt2701_spec_pinmux_set,
 	.spec_dir_set = mt2701_spec_dir_set,
+	.spec_dir_get = mt2701_spec_dir_get,
 	.dir_offset = 0x0000,
 	.pullen_offset = 0x0150,
 	.pullsel_offset = 0x0280,
@@ -551,7 +568,7 @@ static void mt2701_spec_dir_set(unsigned int *reg_addr, unsigned int pin)
 		.dbnc_ctrl = 0x500,
 		.dbnc_set  = 0x600,
 		.dbnc_clr  = 0x700,
-		.port_mask = 6,
+		.port_mask = 7,
 		.ports     = 6,
 	},
 	.ap_num = 169,
diff --git a/drivers/pinctrl/mediatek/pinctrl-mt2712.c b/drivers/pinctrl/mediatek/pinctrl-mt2712.c
new file mode 100644
index 0000000..c933b75
--- /dev/null
+++ b/drivers/pinctrl/mediatek/pinctrl-mt2712.c
<...>
+
+static int mt2712_spec_dir_set(struct mtk_pinctrl *pctl,
+				unsigned int *reg_addr,
+				unsigned int pin,
+				bool input)
+{
+	u32 reg_val;
+
+	if (pin == 16) {
+		regmap_read(pctl->regmap2, 0x940, &reg_val);
+		reg_val |= BIT(15);
+		if (input == true)
+			reg_val &= ~BIT(14);
+		else
+			reg_val |= BIT(14);
+		regmap_write(pctl->regmap2, 0x940, reg_val);
+	}
+
+	if (pin == 17) {
+		regmap_read(pctl->regmap2, 0x940, &reg_val);
+		reg_val |= BIT(7);
+		if (input == true)
+			reg_val &= ~BIT(6);
+		else
+			reg_val |= BIT(6);
+		regmap_write(pctl->regmap2, 0x940, reg_val);
+	}
+
+	return 0;
+}
Does this means pin 16, 17 is in different/special reg/bit location?
I didn't see spec_dir_get in your patch, does this means they are in
standard location or you just forgot it?

The original idea of spec_dir_set is to get the register offset for the
pin, so both set_direction and get_direction are using the same
extension function. Instead of adding a new spec_dir_get, can we just
extend the function to also include bit location?



<...>
quoted hunk ↗ jump to hunk
diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
index 3cf384f..aeec87e 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
@@ -84,7 +84,7 @@ static int mtk_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
 	bit = BIT(offset & 0xf);
 
 	if (pctl->devdata->spec_dir_set)
-		pctl->devdata->spec_dir_set(&reg_addr, offset);
+		pctl->devdata->spec_dir_set(pctl, &reg_addr, offset, input);
 
 	if (input)
 		/* Different SoC has different alignment offset. */
@@ -307,13 +307,6 @@ static int mtk_pconf_set_pull_select(struct mtk_pinctrl *pctl,
 			return 0;
 	}
 
-	/* For generic pull config, default arg value should be 0 or 1. */
-	if (arg != 0 && arg != 1) {
-		dev_err(pctl->dev, "invalid pull-up argument %d on pin %d .\n",
-			arg, pin);
-		return -EINVAL;
-	}
-

Why we need to remove this?
quoted hunk ↗ jump to hunk
 	bit = BIT(pin & 0xf);
 	if (enable)
 		reg_pullen = SET_ADDR(mtk_get_port(pctl, pin) +
@@ -343,7 +336,8 @@ static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev,
 
 	switch (param) {
 	case PIN_CONFIG_BIAS_DISABLE:
-		ret = mtk_pconf_set_pull_select(pctl, pin, false, false, arg);
+		ret = mtk_pconf_set_pull_select(pctl, pin, false, false,
+						MTK_PUPD_SET_R1R0_00);
Why we need to change this?

Joe.C
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help