Thread (16 messages) 16 messages, 4 authors, 2013-06-07

[PATCH v2 5/8] pinctrl: add pinctrl driver for Rockchip SoCs

From: heiko@sntech.de (Heiko Stübner)
Date: 2013-06-07 23:14:10
Also in: linux-devicetree, linux-mmc, lkml
Subsystem: arm/rockchip soc support, pin control subsystem, the rest · Maintainers: Heiko Stuebner, Linus Walleij, Linus Torvalds

Am Freitag, 7. Juni 2013, 14:53:51 schrieb Linus Walleij:
On Thu, Jun 6, 2013 at 9:11 PM, Heiko St?bner [off-list ref] wrote:
quoted
+       for (i = 0, j = 0; i < size; i += 4, j++) {
+               unsigned long pinconf;
+
+               num = be32_to_cpu(*list++);
+               bank = bank_num_to_bank(info, num);
+               if (IS_ERR(bank))
+                       return PTR_ERR(bank);
+
+               pin = be32_to_cpu(*list++);
+               grp->pins[j] = bank->pin_base + pin;
+               grp->func[j] = be32_to_cpu(*list++);
+
+               pinconf = be32_to_cpu(*list++);
+               switch(pinconf) {
+               case RK_PINCTRL_NONE:
+                       bias = PIN_CONFIG_BIAS_DISABLE;
+                       break;
+               case RK_PINCTRL_PULL_PIN_DEFAULT:
+                       bias = PIN_CONFIG_BIAS_PULL_PIN_DEFAULT;
+                       break;
+               case RK_PINCTRL_PULL_UP:
+                       bias = PIN_CONFIG_BIAS_PULL_UP;
+                       break;
+               case RK_PINCTRL_PULL_DOWN:
+                       bias = PIN_CONFIG_BIAS_PULL_DOWN;
+                       break;
+               }
+
+               grp->configs[j] = pinconf_to_config_packed(bias, 0);
+       }
I would like this to be added to
drivers/pinctrl/pinconf-generic.c
as a utility function, with the header in
drivers/pinctrl/pinconf.h

Also in a separate patch.
quoted
+++ b/include/dt-bindings/pinctrl/rockchip.h
(...)
quoted
+#define RK_PINCTRL_NONE                        0
+#define RK_PINCTRL_PULL_PIN_DEFAULT    (1 << 0)
+#define RK_PINCTRL_PULL_UP             (1 << 1)
+#define RK_PINCTRL_PULL_DOWN           (1 << 2)
So I'd like these moved to /include/dt-bindings/pinctrl/pinconfig.h
and used as a separete include. Drop the RK_* prefix as it
will be universal and use a PINCONFIG_* prefix instead
of PINCTRL_*.

I think that is the route we need to take.

Bonus if you implement more config options from
pinconf-generic.h but otherwise me and others will have
to do it.
Gah, damn me for always wanting to get bonus points ;-),

I did play around a bit with the idea you described above and came up
with the stuff a bit below. This is in no way meant to be finished, I
also only was able to compile test it yet, but it looks like it might
work when I test it tomorrow.

So if possible I would just like to get a "looks halfway sane, continue"
or "completely wrong track" please :-) :


constants in dt-bindings/pinctrl/pinconfig.h:
-----------------------------
#define PINCONFIG_NONE			0
#define PINCONFIG_BIAS_DISABLE		(1 << 0)
#define PINCONFIG_BIAS_HIGH_IMPEDANCE	(1 << 1)
#define PINCONFIG_BIAS_BUS_HOLD		(1 << 2)
#define PINCONFIG_BIAS_PULL_PIN_DEFAULT	(1 << 3)
#define PINCONFIG_BIAS_PULL_UP		(1 << 4)
#define PINCONFIG_BIAS_PULL_DOWN	(1 << 5)
#define PINCONFIG_DRIVE_PUSH_PULL	(1 << 6)
#define PINCONFIG_DRIVE_OPEN_DRAIN	(1 << 7)
#define PINCONFIG_DRIVE_OPEN_SOURCE	(1 << 8)
#define PINCONFIG_INPUT_SCHMITT_ENABLE	(1 << 9)
#define PINCONFIG_INPUT_SCHMITT_DISABLE	(1 << 10)
#define PINCONFIG_LOW_POWER_MODE	(1 << 11)
#define PINCONFIG_OUTPUT_LOW		(1 << 12)
#define PINCONFIG_OUTPUT_HIGH		(1 << 13)


and code including how the rockchip driver could look like:
---------
diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 9a6812b..35c40be 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -139,3 +139,61 @@ void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
 }
 EXPORT_SYMBOL_GPL(pinconf_generic_dump_config);
 #endif
+
+/*
+ * Maps the devicetree config bits to actual pinconf values.
+ * The array indizes match the bits set in dt-bindings/pinctrl/pinconf.h
+ * and the array should contain an entry for each bit defined there
+ */
+static unsigned long conf_map[] = {
+	PIN_CONF_PACKED(PIN_CONFIG_BIAS_DISABLE, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_BIAS_HIGH_IMPEDANCE, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_BIAS_BUS_HOLD, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_PIN_DEFAULT, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_UP, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_DOWN, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_DRIVE_PUSH_PULL, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_DRIVE_OPEN_DRAIN, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_DRIVE_OPEN_SOURCE, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1),
+	PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_LOW_POWER_MODE, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_OUTPUT, 0),
+	PIN_CONF_PACKED(PIN_CONFIG_OUTPUT, 1),
+};
+
+/*
+ * Parse a pinconf bitmap from a devicetree entry into individual pin configs.
+ * @pinconf: the bitmap containing config bits
+ * @configs: after the function returns contains a pointer to an array of
+ *	     pin configs
+ * @nconfigs: number of entries of configs
+ */
+int pinconf_generic_parse_dt_bitmap(unsigned long pinconf, 
+				    unsigned long *configs,
+				    unsigned int *nconfigs)
+{
+	int bit;
+	int i;
+
+	*nconfigs = hweight_long(pinconf);
+	configs = kzalloc(*nconfigs * sizeof(unsigned long), GFP_KERNEL);
+
+	i = 0;
+	while (pinconf && i < *nconfigs) {
+		bit = __ffs(pinconf);
+		pinconf &= ~BIT(i);
+
+		if (bit > ARRAY_SIZE(conf_map)) {
+			pr_err("%s: unknown bit %d\n", __func__, bit);
+			kfree(configs);
+			return -EINVAL;
+		}
+
+		configs[i] = conf_map[bit];
+
+		i++;
+	}
+
+	return 0;
+}
diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index 92c7267..0fab53a 100644
--- a/drivers/pinctrl/pinconf.h
+++ b/drivers/pinctrl/pinconf.h
@@ -123,3 +123,9 @@ static inline void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
 	return;
 }
 #endif
+
+#ifdef CONFIG_GENERIC_PINCONF
+int pinconf_generic_parse_dt_bitmap(unsigned long pinconf, 
+				    unsigned long *configs,
+				    unsigned int *nconfigs);
+#endif
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index b77e241..88398df 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -40,6 +40,7 @@
 #include <dt-bindings/pinctrl/rockchip.h>
 
 #include "core.h"
+#include "pinconf.h"
 
 /* GPIO control registers */
 #define GPIO_SWPORT_DR		0x00
@@ -639,7 +640,8 @@ static int rockchip_pinctrl_parse_groups(struct device_node *np,
 	int num;
 	int i, j;
 	int pin;
-	int bias;
+	int ret;
+	int nconfigs;
 
 	dev_dbg(info->dev, "group(%d): %s\n", index, np->name);
 
@@ -683,22 +685,16 @@ static int rockchip_pinctrl_parse_groups(struct device_node *np,
 		grp->func[j] = be32_to_cpu(*list++);
 
 		pinconf = be32_to_cpu(*list++);
-		switch(pinconf) {
-		case RK_PINCTRL_NONE:
-			bias = PIN_CONFIG_BIAS_DISABLE;
-			break;
-		case RK_PINCTRL_PULL_PIN_DEFAULT:
-			bias = PIN_CONFIG_BIAS_PULL_PIN_DEFAULT;
-			break;
-		case RK_PINCTRL_PULL_UP:
-			bias = PIN_CONFIG_BIAS_PULL_UP;
-			break;
-		case RK_PINCTRL_PULL_DOWN:
-			bias = PIN_CONFIG_BIAS_PULL_DOWN;
-			break;
-		}
 
-		grp->configs[j] = pinconf_to_config_packed(bias, 0);
+		ret = pinconf_generic_parse_dt_bitmap(pinconf, 
+				    &grp->configs[j], &nconfigs);
+		if (ret)
+			return ret;
+
+		if (nconfigs != 1) {
+			pr_err("unexpected number of config entries\n");
+			return -EINVAL;
+		}
 	}
 
 	return 0;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help