Skip to contentSkip to conversation

pinctrl: add DT support in PXA series

PATCH New Haojian Zhuang 4 participants 23 messages Jul 28, 2012 – Aug 13, 2012

Subsystems (5)
Device tree bindings Arm (32-bit) Arm sub-architectures Pin control +1 more subsystems
lore
Share & tools
lore

Haojian Zhuang

author
#0 lore raw

cover pinctrl: add DT support in PXA series

1. Add DT support in pinctrl driver of pxa series.
2. Add pinconf support in pinctrl driver of pxa series.
3. Select PINCTRL by default in PXA series boards with DT support.

author
#1 lore raw

PATCH 1/6 pinctrl: append new pins and groups in pxa910

Signed-off-by Haojian Zhuang <[email protected]>
---
 drivers/pinctrl/pinctrl-pxa910.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/pinctrl/pinctrl-pxa910.c +5 -1
index c72ab4b..4164a6b 100644
@@ -307,6 +307,7 @@ static struct pinctrl_pin_desc pxa910_pads[] = {
 	PINCTRL_PIN(ND_RDY0, "ND_RDY0"),
 	PINCTRL_PIN(SM_ADV, "SM_ADV"),
 	PINCTRL_PIN(ND_RDY1, "ND_RDY1"),
+	PINCTRL_PIN(SM_ADVMUX, "SM_ADVMUX"),
 	PINCTRL_PIN(SM_RDY, "SM_RDY"),
 	PINCTRL_PIN(MMC1_DAT7, "MMC1_DAT7"),
 	PINCTRL_PIN(MMC1_DAT6, "MMC1_DAT6"),

@@ -504,6 +505,8 @@ static const unsigned p910_mmc1_pin1[] = {MMC1_DAT7, MMC1_DAT6, MMC1_DAT5,
 	MMC1_CLK, MMC1_CD, GPIO99};
 static const unsigned p910_mmc2_pin1[] = {GPIO33, GPIO34, GPIO35, GPIO36,
 	GPIO37, GPIO38, GPIO39, GPIO40, GPIO41, GPIO42};
+static const unsigned p910_mmc2_pin2[] = {GPIO37, GPIO38, GPIO39, GPIO40,
+	GPIO41, GPIO42};
 static const unsigned p910_mmc3_pin1[] = {GPIO33, GPIO34, GPIO35, GPIO36,
 	GPIO49, GPIO50};
 static const unsigned p910_mmc3_pin2[] = {ND_IO7, ND_IO6, ND_IO5, ND_IO4,

@@ -674,6 +677,7 @@ static struct pxa3xx_pin_group pxa910_grps[] = {
 	GRP_910("usim2 3p2", USIM2, p910_usim2_pin2),
 	GRP_910("mmc1 12p", MMC1, p910_mmc1_pin1),
 	GRP_910("mmc2 10p", MMC2, p910_mmc2_pin1),
+	GRP_910("mmc2 6p", MMC2, p910_mmc2_pin2),
 	GRP_910("mmc3 6p", MMC3, p910_mmc3_pin1),
 	GRP_910("mmc3 10p", MMC3, p910_mmc3_pin2),
 	GRP_910("uart0 4p", UART0, p910_uart0_pin1),

@@ -808,7 +812,7 @@ static struct pxa3xx_pin_group pxa910_grps[] = {
 
 static const char * const p910_usim2_grps[] = {"usim2 3p1", "usim2 3p2"};
 static const char * const p910_mmc1_grps[] = {"mmc1 12p"};
-static const char * const p910_mmc2_grps[] = {"mmc2 10p"};
+static const char * const p910_mmc2_grps[] = {"mmc2 10p", "mmc2 6p"};
 static const char * const p910_mmc3_grps[] = {"mmc3 6p", "mmc3 10p"};
 static const char * const p910_uart0_grps[] = {"uart0 4p"};
 static const char * const p910_uart1_grps[] = {"uart1 2p1", "uart1 2p2",
Signature · 1 line
1.7.9.5
author
#3 lore raw

PATCH 2/6 pinctrl: add pinconf support in pxa3xx

In reply to Haojian Zhuang
Signed-off-by Haojian Zhuang <[email protected]>
---
 drivers/pinctrl/pinctrl-pxa3xx.c |  189 ++++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-pxa3xx.h |   17 ++++
 2 files changed, 206 insertions(+)
drivers/pinctrl/pinctrl-pxa3xx.c +189 -0
index f14cd6b..cae74db 100644
@@ -16,6 +16,8 @@
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
 #include "pinctrl-pxa3xx.h"
 
 static struct pinctrl_gpio_range pxa3xx_pinctrl_gpio_range = {

@@ -168,6 +170,192 @@ static struct pinmux_ops pxa3xx_pmx_ops = {
 	.gpio_request_enable	= pxa3xx_pmx_request_gpio,
 };
 
+static int pxa3xx_pinconf_get(struct pinctrl_dev *pctrldev,
+			      unsigned pin, unsigned long *config)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	unsigned int data, mask;
+	int mfpr;
+
+	mfpr = info->mfp[pin].mfpr;
+	data = readl_relaxed(info->virt_base + mfpr);
+
+	*config = 0;
+	mask = PXA3XX_MFPR_PULL_SEL | PXA3XX_MFPR_PULL_UP;
+	if ((data & mask) == mask)
+		*config |= PXA3XX_PINCONF_PULL_UP;
+	mask = PXA3XX_MFPR_PULL_SEL | PXA3XX_MFPR_PULL_DOWN;
+	if ((data & mask) == mask)
+		*config |= PXA3XX_PINCONF_PULL_DOWN;
+	mask = info->ds_mask;
+	if (data & mask) {
+		*config |= PXA3XX_PINCONF_DRIVE_STRENGTH;
+		*config |= ((data & mask) >> info->ds_shift)
+				<< PXA3XX_PINCONF_DS_SHIFT;
+	}
+	mask = info->slp_mask;
+	if (data & mask) {
+		if (data & info->slp_input_low)
+			*config |= PXA3XX_PINCONF_LOWPOWER_PULL_DOWN;
+		if (data & info->slp_input_high)
+			*config |= PXA3XX_PINCONF_LOWPOWER_PULL_UP;
+		if (data & info->slp_output_low)
+			*config |= PXA3XX_PINCONF_LOWPOWER_DRIVE_LOW;
+		if (data & info->slp_output_high)
+			*config |= PXA3XX_PINCONF_LOWPOWER_DRIVE_HIGH;
+		if (data & info->slp_float)
+			*config |= PXA3XX_PINCONF_LOWPOWER_FLOAT;
+	} else
+		*config |= PXA3XX_PINCONF_LOWPOWER_ZERO;
+	return 0;
+}
+
+static int pxa3xx_pinconf_set(struct pinctrl_dev *pctrldev,
+			      unsigned pin, unsigned long config)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	unsigned int data;
+	int mfpr;
+
+	mfpr = info->mfp[pin].mfpr;
+	data = readl_relaxed(info->virt_base + mfpr);
+	switch (config & PXA3XX_PINCONF_MASK) {
+	case PXA3XX_PINCONF_PULL_DOWN:
+		data |= PXA3XX_MFPR_PULL_SEL | PXA3XX_MFPR_PULL_DOWN;
+		break;
+	case PXA3XX_PINCONF_PULL_UP:
+		data |= PXA3XX_MFPR_PULL_SEL | PXA3XX_MFPR_PULL_UP;
+		break;
+	case PXA3XX_PINCONF_DRIVE_STRENGTH:
+		data &= ~info->ds_mask;
+		data |= (config >> PXA3XX_PINCONF_DS_SHIFT) << info->ds_shift;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_DRIVE_HIGH:
+		data &= ~info->slp_mask;
+		data |= info->slp_output_high;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_DRIVE_LOW:
+		data &= ~info->slp_mask;
+		data |= info->slp_output_low;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_PULL_UP:
+		data &= ~info->slp_mask;
+		data |= info->slp_input_high;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_PULL_DOWN:
+		data &= ~info->slp_mask;
+		data |= info->slp_input_low;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_FLOAT:
+		data &= ~info->slp_mask;
+		data |= info->slp_float;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_ZERO:
+		data &= ~info->slp_mask;
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+	writel_relaxed(data, info->virt_base + mfpr);
+	return 0;
+}
+
+static int pxa3xx_pinconf_group_get(struct pinctrl_dev *pctrldev,
+				    unsigned group, unsigned long *config)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	const unsigned *pins = info->grps[group].pins;
+	int npins, i;
+	unsigned long conf, match = 0;
+
+	npins = info->grps[group].npins;
+	for (i = 0; i < npins; i++) {
+		pxa3xx_pinconf_get(pctrldev, pins[i], &conf);
+		if (!match)
+			match = conf;
+		else if (match != conf) {
+			*config = conf;
+			return -EINVAL;
+		}
+	}
+	*config = conf;
+	return 0;
+}
+
+static int pxa3xx_pinconf_group_set(struct pinctrl_dev *pctrldev,
+				    unsigned group, unsigned long config)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	const unsigned *pins = info->grps[group].pins;
+	int npins, i;
+
+	npins = info->grps[group].npins;
+	for (i = 0; i < npins; i++)
+		pxa3xx_pinconf_set(pctrldev, pins[i], config);
+	return 0;
+}
+
+static void pxa3xx_pinconf_dbg_show(struct pinctrl_dev *pctrldev,
+				    struct seq_file *s, unsigned offset)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	unsigned long config;
+	char buf[80];
+	int i = 0, mfpr;
+
+	pxa3xx_pinconf_get(pctrldev, offset, &config);
+	memset(buf, 0, 80);
+	if (config & PXA3XX_PINCONF_PULL_UP)
+		i += sprintf(buf + i, "PULL UP, ");
+	if (config & PXA3XX_PINCONF_PULL_DOWN)
+		i += sprintf(buf + i, "PULL DOWN, ");
+	if (config & PXA3XX_PINCONF_DRIVE_STRENGTH)
+		i += sprintf(buf + i, "DRIVE STRENGTH (%ld), ",
+			config >> PXA3XX_PINCONF_DS_SHIFT);
+	if (config & PXA3XX_PINCONF_LOWPOWER_PULL_UP)
+		i += sprintf(buf + i, "LP PULL UP, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_PULL_DOWN)
+		i += sprintf(buf + i, "LP PULL DOWN, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_DRIVE_HIGH)
+		i += sprintf(buf + i, "LP DRIVE HIGH, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_DRIVE_LOW)
+		i += sprintf(buf + i, "LP DRIVE LOW, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_FLOAT)
+		i += sprintf(buf + i, "LP FLOAT, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_ZERO)
+		i += sprintf(buf + i, "LP ZERO, ");
+	seq_printf(s, "%s\n", buf);
+
+	mfpr = info->mfp[offset].mfpr;
+	seq_printf(s, "reg[0x%x]:0x%x\n", info->phy_base + mfpr,
+		   readl_relaxed(info->virt_base + mfpr));
+}
+
+static void pxa3xx_pinconf_group_dbg_show(struct pinctrl_dev *pctrldev,
+					  struct seq_file *s, unsigned group)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	const unsigned *pins = info->grps[group].pins;
+	int ret;
+	unsigned long config;
+
+	ret = pxa3xx_pinconf_group_get(pctrldev, group, &config);
+	if (ret < 0) {
+		seq_printf(s, "group config is not consistent\n");
+		return;
+	}
+	pxa3xx_pinconf_dbg_show(pctrldev, s, pins[0]);
+}
+
+static struct pinconf_ops pxa3xx_pinconf_ops = {
+	.pin_config_get		= pxa3xx_pinconf_get,
+	.pin_config_set		= pxa3xx_pinconf_set,
+	.pin_config_group_get	= pxa3xx_pinconf_group_get,
+	.pin_config_group_set	= pxa3xx_pinconf_group_set,
+	.pin_config_dbg_show	= pxa3xx_pinconf_dbg_show,
+	.pin_config_group_dbg_show = pxa3xx_pinconf_group_dbg_show,
+};
+
 int pxa3xx_pinctrl_register(struct platform_device *pdev,
 			    struct pxa3xx_pinmux_info *info)
 {

@@ -182,6 +370,7 @@ int pxa3xx_pinctrl_register(struct platform_device *pdev,
 	desc->npins = info->num_pads;
 	desc->pctlops = &pxa3xx_pctrl_ops;
 	desc->pmxops = &pxa3xx_pmx_ops;
+	desc->confops = &pxa3xx_pinconf_ops;
 	info->dev = &pdev->dev;
 	pxa3xx_pinctrl_gpio_range.npins = info->num_gpio;
drivers/pinctrl/pinctrl-pxa3xx.h +17 -0
index 8135744..fdcf805 100644
@@ -12,6 +12,7 @@
  */
 
 #ifndef __PINCTRL_PXA3XX_H
+#define __PINCTRL_PXA3XX_H
 
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>

@@ -23,6 +24,22 @@
 #define PXA3xx_MAX_MUX		8
 #define MFPR_FUNC_MASK		0x7
 
+#define PXA3XX_PINCONF_PULL_UP			(1 << 0)
+#define PXA3XX_PINCONF_PULL_DOWN		(1 << 1)
+#define PXA3XX_PINCONF_DRIVE_STRENGTH		(1 << 2)
+#define PXA3XX_PINCONF_LOWPOWER_PULL_UP		(1 << 3)
+#define PXA3XX_PINCONF_LOWPOWER_PULL_DOWN	(1 << 4)
+#define PXA3XX_PINCONF_LOWPOWER_DRIVE_HIGH	(1 << 5)
+#define PXA3XX_PINCONF_LOWPOWER_DRIVE_LOW	(1 << 6)
+#define PXA3XX_PINCONF_LOWPOWER_FLOAT		(1 << 7)
+#define PXA3XX_PINCONF_LOWPOWER_ZERO		(1 << 8) /* lowpower bits: 0 */
+#define PXA3XX_PINCONF_MASK			0xffff
+#define PXA3XX_PINCONF_DS_SHIFT			16
+
+#define PXA3XX_MFPR_PULL_SEL			(1 << 15)
+#define PXA3XX_MFPR_PULL_UP			(1 << 14)
+#define PXA3XX_MFPR_PULL_DOWN			(1 << 13)
+
 enum pxa_cpu_type {
 	PINCTRL_INVALID = 0,
 	PINCTRL_PXA300,
Signature · 1 line
1.7.9.5
#4 lore raw

PATCH 2/6 pinctrl: add pinconf support in pxa3xx

Quoted 5 lines · Haojian Zhuang
Signed-off-by Haojian Zhuang <[email protected]>
---
 drivers/pinctrl/pinctrl-pxa3xx.c |  189 ++++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-pxa3xx.h |   17 ++++
 2 files changed, 206 insertions(+)

As I'm working on a PXA3xx platform currently that already fully boots from DT, I'd like to give this series a try.

But I wonder if this is supposed to be fully working yet, because that code won't be compiled for PXA3xx as long as PINCTRL isn't set for that platform (arch/arm/mach-pxa/Kconfig).

Daniel

Quoted 251 lines
drivers/pinctrl/pinctrl-pxa3xx.c +189 -0
index f14cd6b..cae74db 100644
@@ -16,6 +16,8 @@
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
 #include "pinctrl-pxa3xx.h"
 
 static struct pinctrl_gpio_range pxa3xx_pinctrl_gpio_range = {

@@ -168,6 +170,192 @@ static struct pinmux_ops pxa3xx_pmx_ops = {
 	.gpio_request_enable	= pxa3xx_pmx_request_gpio,
 };
 
+static int pxa3xx_pinconf_get(struct pinctrl_dev *pctrldev,
+			      unsigned pin, unsigned long *config)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	unsigned int data, mask;
+	int mfpr;
+
+	mfpr = info->mfp[pin].mfpr;
+	data = readl_relaxed(info->virt_base + mfpr);
+
+	*config = 0;
+	mask = PXA3XX_MFPR_PULL_SEL | PXA3XX_MFPR_PULL_UP;
+	if ((data & mask) == mask)
+		*config |= PXA3XX_PINCONF_PULL_UP;
+	mask = PXA3XX_MFPR_PULL_SEL | PXA3XX_MFPR_PULL_DOWN;
+	if ((data & mask) == mask)
+		*config |= PXA3XX_PINCONF_PULL_DOWN;
+	mask = info->ds_mask;
+	if (data & mask) {
+		*config |= PXA3XX_PINCONF_DRIVE_STRENGTH;
+		*config |= ((data & mask) >> info->ds_shift)
+				<< PXA3XX_PINCONF_DS_SHIFT;
+	}
+	mask = info->slp_mask;
+	if (data & mask) {
+		if (data & info->slp_input_low)
+			*config |= PXA3XX_PINCONF_LOWPOWER_PULL_DOWN;
+		if (data & info->slp_input_high)
+			*config |= PXA3XX_PINCONF_LOWPOWER_PULL_UP;
+		if (data & info->slp_output_low)
+			*config |= PXA3XX_PINCONF_LOWPOWER_DRIVE_LOW;
+		if (data & info->slp_output_high)
+			*config |= PXA3XX_PINCONF_LOWPOWER_DRIVE_HIGH;
+		if (data & info->slp_float)
+			*config |= PXA3XX_PINCONF_LOWPOWER_FLOAT;
+	} else
+		*config |= PXA3XX_PINCONF_LOWPOWER_ZERO;
+	return 0;
+}
+
+static int pxa3xx_pinconf_set(struct pinctrl_dev *pctrldev,
+			      unsigned pin, unsigned long config)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	unsigned int data;
+	int mfpr;
+
+	mfpr = info->mfp[pin].mfpr;
+	data = readl_relaxed(info->virt_base + mfpr);
+	switch (config & PXA3XX_PINCONF_MASK) {
+	case PXA3XX_PINCONF_PULL_DOWN:
+		data |= PXA3XX_MFPR_PULL_SEL | PXA3XX_MFPR_PULL_DOWN;
+		break;
+	case PXA3XX_PINCONF_PULL_UP:
+		data |= PXA3XX_MFPR_PULL_SEL | PXA3XX_MFPR_PULL_UP;
+		break;
+	case PXA3XX_PINCONF_DRIVE_STRENGTH:
+		data &= ~info->ds_mask;
+		data |= (config >> PXA3XX_PINCONF_DS_SHIFT) << info->ds_shift;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_DRIVE_HIGH:
+		data &= ~info->slp_mask;
+		data |= info->slp_output_high;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_DRIVE_LOW:
+		data &= ~info->slp_mask;
+		data |= info->slp_output_low;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_PULL_UP:
+		data &= ~info->slp_mask;
+		data |= info->slp_input_high;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_PULL_DOWN:
+		data &= ~info->slp_mask;
+		data |= info->slp_input_low;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_FLOAT:
+		data &= ~info->slp_mask;
+		data |= info->slp_float;
+		break;
+	case PXA3XX_PINCONF_LOWPOWER_ZERO:
+		data &= ~info->slp_mask;
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+	writel_relaxed(data, info->virt_base + mfpr);
+	return 0;
+}
+
+static int pxa3xx_pinconf_group_get(struct pinctrl_dev *pctrldev,
+				    unsigned group, unsigned long *config)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	const unsigned *pins = info->grps[group].pins;
+	int npins, i;
+	unsigned long conf, match = 0;
+
+	npins = info->grps[group].npins;
+	for (i = 0; i < npins; i++) {
+		pxa3xx_pinconf_get(pctrldev, pins[i], &conf);
+		if (!match)
+			match = conf;
+		else if (match != conf) {
+			*config = conf;
+			return -EINVAL;
+		}
+	}
+	*config = conf;
+	return 0;
+}
+
+static int pxa3xx_pinconf_group_set(struct pinctrl_dev *pctrldev,
+				    unsigned group, unsigned long config)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	const unsigned *pins = info->grps[group].pins;
+	int npins, i;
+
+	npins = info->grps[group].npins;
+	for (i = 0; i < npins; i++)
+		pxa3xx_pinconf_set(pctrldev, pins[i], config);
+	return 0;
+}
+
+static void pxa3xx_pinconf_dbg_show(struct pinctrl_dev *pctrldev,
+				    struct seq_file *s, unsigned offset)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	unsigned long config;
+	char buf[80];
+	int i = 0, mfpr;
+
+	pxa3xx_pinconf_get(pctrldev, offset, &config);
+	memset(buf, 0, 80);
+	if (config & PXA3XX_PINCONF_PULL_UP)
+		i += sprintf(buf + i, "PULL UP, ");
+	if (config & PXA3XX_PINCONF_PULL_DOWN)
+		i += sprintf(buf + i, "PULL DOWN, ");
+	if (config & PXA3XX_PINCONF_DRIVE_STRENGTH)
+		i += sprintf(buf + i, "DRIVE STRENGTH (%ld), ",
+			config >> PXA3XX_PINCONF_DS_SHIFT);
+	if (config & PXA3XX_PINCONF_LOWPOWER_PULL_UP)
+		i += sprintf(buf + i, "LP PULL UP, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_PULL_DOWN)
+		i += sprintf(buf + i, "LP PULL DOWN, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_DRIVE_HIGH)
+		i += sprintf(buf + i, "LP DRIVE HIGH, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_DRIVE_LOW)
+		i += sprintf(buf + i, "LP DRIVE LOW, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_FLOAT)
+		i += sprintf(buf + i, "LP FLOAT, ");
+	if (config & PXA3XX_PINCONF_LOWPOWER_ZERO)
+		i += sprintf(buf + i, "LP ZERO, ");
+	seq_printf(s, "%s\n", buf);
+
+	mfpr = info->mfp[offset].mfpr;
+	seq_printf(s, "reg[0x%x]:0x%x\n", info->phy_base + mfpr,
+		   readl_relaxed(info->virt_base + mfpr));
+}
+
+static void pxa3xx_pinconf_group_dbg_show(struct pinctrl_dev *pctrldev,
+					  struct seq_file *s, unsigned group)
+{
+	struct pxa3xx_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+	const unsigned *pins = info->grps[group].pins;
+	int ret;
+	unsigned long config;
+
+	ret = pxa3xx_pinconf_group_get(pctrldev, group, &config);
+	if (ret < 0) {
+		seq_printf(s, "group config is not consistent\n");
+		return;
+	}
+	pxa3xx_pinconf_dbg_show(pctrldev, s, pins[0]);
+}
+
+static struct pinconf_ops pxa3xx_pinconf_ops = {
+	.pin_config_get		= pxa3xx_pinconf_get,
+	.pin_config_set		= pxa3xx_pinconf_set,
+	.pin_config_group_get	= pxa3xx_pinconf_group_get,
+	.pin_config_group_set	= pxa3xx_pinconf_group_set,
+	.pin_config_dbg_show	= pxa3xx_pinconf_dbg_show,
+	.pin_config_group_dbg_show = pxa3xx_pinconf_group_dbg_show,
+};
+
 int pxa3xx_pinctrl_register(struct platform_device *pdev,
 			    struct pxa3xx_pinmux_info *info)
 {

@@ -182,6 +370,7 @@ int pxa3xx_pinctrl_register(struct platform_device *pdev,
 	desc->npins = info->num_pads;
 	desc->pctlops = &pxa3xx_pctrl_ops;
 	desc->pmxops = &pxa3xx_pmx_ops;
+	desc->confops = &pxa3xx_pinconf_ops;
 	info->dev = &pdev->dev;
 	pxa3xx_pinctrl_gpio_range.npins = info->num_gpio;
drivers/pinctrl/pinctrl-pxa3xx.h +17 -0
index 8135744..fdcf805 100644
@@ -12,6 +12,7 @@
  */
 
 #ifndef __PINCTRL_PXA3XX_H
+#define __PINCTRL_PXA3XX_H
 
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>

@@ -23,6 +24,22 @@
 #define PXA3xx_MAX_MUX		8
 #define MFPR_FUNC_MASK		0x7
 
+#define PXA3XX_PINCONF_PULL_UP			(1 << 0)
+#define PXA3XX_PINCONF_PULL_DOWN		(1 << 1)
+#define PXA3XX_PINCONF_DRIVE_STRENGTH		(1 << 2)
+#define PXA3XX_PINCONF_LOWPOWER_PULL_UP		(1 << 3)
+#define PXA3XX_PINCONF_LOWPOWER_PULL_DOWN	(1 << 4)
+#define PXA3XX_PINCONF_LOWPOWER_DRIVE_HIGH	(1 << 5)
+#define PXA3XX_PINCONF_LOWPOWER_DRIVE_LOW	(1 << 6)
+#define PXA3XX_PINCONF_LOWPOWER_FLOAT		(1 << 7)
+#define PXA3XX_PINCONF_LOWPOWER_ZERO		(1 << 8) /* lowpower bits: 0 */
+#define PXA3XX_PINCONF_MASK			0xffff
+#define PXA3XX_PINCONF_DS_SHIFT			16
+
+#define PXA3XX_MFPR_PULL_SEL			(1 << 15)
+#define PXA3XX_MFPR_PULL_UP			(1 << 14)
+#define PXA3XX_MFPR_PULL_DOWN			(1 << 13)
+
 enum pxa_cpu_type {
 	PINCTRL_INVALID = 0,
 	PINCTRL_PXA300,
author
#5 lore raw

PATCH 2/6 pinctrl: add pinconf support in pxa3xx

Quoted 10 lines · Daniel Mack
Quoted 5 lines · Haojian Zhuang
Signed-off-by Haojian Zhuang <[email protected]>
---
 drivers/pinctrl/pinctrl-pxa3xx.c |  189 ++++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-pxa3xx.h |   17 ++++
 2 files changed, 206 insertions(+)

As I'm working on a PXA3xx platform currently that already fully boots from DT, I'd like to give this series a try.

Quoted 8 lines

But I wonder if this is supposed to be fully working yet, because that code won't be compiled for PXA3xx as long as PINCTRL isn't set for that platform (arch/arm/mach-pxa/Kconfig).

Daniel

pinctrl-pxa3xx.c is the common code for those PXA silicons. But I didn't add the pxa300/pxa310/pxa320 pin support yet. Now I only support pxa168/pxa910/ mmp2 silicon.

Regards
Haojian

author
#7 lore raw

PATCH 3/6 pinctrl: support dt in pxa series

In reply to Haojian Zhuang

Add DT support in PXA168/PXA910/MMP2 pinctrl driver.

Signed-off-by Haojian Zhuang <[email protected]>
---
 drivers/pinctrl/pinctrl-mmp2.c   |   10 ++-
 drivers/pinctrl/pinctrl-pxa168.c |   10 ++-
 drivers/pinctrl/pinctrl-pxa3xx.c |  136 ++++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-pxa910.c |   10 ++-
 4 files changed, 163 insertions(+), 3 deletions(-)
drivers/pinctrl/pinctrl-mmp2.c +9 -1
index 2cfed55..25b9cd3 100644
@@ -14,6 +14,7 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/io.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include "pinctrl-pxa3xx.h"
 

@@ -696,10 +697,16 @@ static int __devexit mmp2_pinmux_remove(struct platform_device *pdev)
 	return pxa3xx_pinctrl_unregister(pdev);
 }
 
+static struct of_device_id mmp2_pinctrl_of_match[] __devinitdata = {
+	{ .compatible = "marvell,mmp2-pinmux", },
+	{ },
+};
+
 static struct platform_driver mmp2_pinmux_driver = {
 	.driver = {
 		.name	= "mmp2-pinmux",
 		.owner	= THIS_MODULE,
+		.of_match_table	= mmp2_pinctrl_of_match,
 	},
 	.probe	= mmp2_pinmux_probe,
 	.remove	= __devexit_p(mmp2_pinmux_remove),

@@ -718,5 +725,6 @@ static void __exit mmp2_pinmux_exit(void)
 module_exit(mmp2_pinmux_exit);
 
 MODULE_AUTHOR("Haojian Zhuang <[email protected]>");
-MODULE_DESCRIPTION("PXA3xx pin control driver");
+MODULE_DESCRIPTION("MMP2 pin control driver");
 MODULE_LICENSE("GPL v2");
+MODULE_DEVICE_TABLE(of, mmp2_pinctrl_of_match);
drivers/pinctrl/pinctrl-pxa168.c +9 -1
index c1997fa..959a13d 100644
@@ -14,6 +14,7 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/io.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include "pinctrl-pxa3xx.h"
 

@@ -625,10 +626,16 @@ static int __devexit pxa168_pinmux_remove(struct platform_device *pdev)
 	return pxa3xx_pinctrl_unregister(pdev);
 }
 
+static struct of_device_id pxa168_pinctrl_of_match[] __devinitdata = {
+	{ .compatible = "marvell,pxa168-pinmux", },
+	{ },
+};
+
 static struct platform_driver pxa168_pinmux_driver = {
 	.driver = {
 		.name	= "pxa168-pinmux",
 		.owner	= THIS_MODULE,
+		.of_match_table	= pxa168_pinctrl_of_match,
 	},
 	.probe	= pxa168_pinmux_probe,
 	.remove	= __devexit_p(pxa168_pinmux_remove),

@@ -647,5 +654,6 @@ static void __exit pxa168_pinmux_exit(void)
 module_exit(pxa168_pinmux_exit);
 
 MODULE_AUTHOR("Haojian Zhuang <[email protected]>");
-MODULE_DESCRIPTION("PXA3xx pin control driver");
+MODULE_DESCRIPTION("PXA168 pin control driver");
 MODULE_LICENSE("GPL v2");
+MODULE_DEVICE_TABLE(of, pxa168_pinctrl_of_match);
drivers/pinctrl/pinctrl-pxa3xx.c +136 -0

152 lines not on this page · raw message

drivers/pinctrl/pinctrl-pxa910.c +9 -1
index 4164a6b..ccda259 100644
@@ -14,6 +14,7 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/io.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include "pinctrl-pxa3xx.h"
 

@@ -985,10 +986,16 @@ static int __devexit pxa910_pinmux_remove(struct platform_device *pdev)
 	return pxa3xx_pinctrl_unregister(pdev);
 }
 
+static struct of_device_id pxa910_pinctrl_of_match[] __devinitdata = {
+	{ .compatible = "marvell,pxa910-pinmux", },
+	{ },
+};
+
 static struct platform_driver pxa910_pinmux_driver = {
 	.driver = {
 		.name	= "pxa910-pinmux",
 		.owner	= THIS_MODULE,
+		.of_match_table	= pxa910_pinctrl_of_match,
 	},
 	.probe	= pxa910_pinmux_probe,
 	.remove	= __devexit_p(pxa910_pinmux_remove),

@@ -1007,5 +1014,6 @@ static void __exit pxa910_pinmux_exit(void)
 module_exit(pxa910_pinmux_exit);
 
 MODULE_AUTHOR("Haojian Zhuang <[email protected]>");
-MODULE_DESCRIPTION("PXA3xx pin control driver");
+MODULE_DESCRIPTION("PXA910 pin control driver");
 MODULE_LICENSE("GPL v2");
+MODULE_DEVICE_TABLE(of, pxa910_pinctrl_of_match);
Signature · 1 line
1.7.9.5
author
#9 lore raw

PATCH 4/6 pinctrl: append new groups of mmp2

In reply to Haojian Zhuang
Signed-off-by Haojian Zhuang <[email protected]>
---
 drivers/pinctrl/pinctrl-mmp2.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/pinctrl/pinctrl-mmp2.c +6 -2
index 25b9cd3..2c56de7 100644
@@ -483,12 +483,14 @@ struct pxa3xx_mfp_pin mmp2_mfp[] = {
 static const unsigned mmp2_uart1_pin1[] = {GPIO29, GPIO30, GPIO31, GPIO32};
 static const unsigned mmp2_uart1_pin2[] = {GPIO45, GPIO46};
 static const unsigned mmp2_uart1_pin3[] = {GPIO140, GPIO141};
+static const unsigned mmp2_uart1_pin4[] = {GPIO29, GPIO30};
 static const unsigned mmp2_uart2_pin1[] = {GPIO37, GPIO38, GPIO39, GPIO40};
 static const unsigned mmp2_uart2_pin2[] = {GPIO43, GPIO44, GPIO45, GPIO46};
 static const unsigned mmp2_uart2_pin3[] = {GPIO47, GPIO48, GPIO49, GPIO50};
 static const unsigned mmp2_uart2_pin4[] = {GPIO74, GPIO75, GPIO76, GPIO77};
 static const unsigned mmp2_uart2_pin5[] = {GPIO55, GPIO56};
 static const unsigned mmp2_uart2_pin6[] = {GPIO140, GPIO141};
+static const unsigned mmp2_uart2_pin7[] = {GPIO47, GPIO48};
 static const unsigned mmp2_uart3_pin1[] = {GPIO37, GPIO38, GPIO39, GPIO40};
 static const unsigned mmp2_uart3_pin2[] = {GPIO43, GPIO44, GPIO45, GPIO46};
 static const unsigned mmp2_uart3_pin3[] = {GPIO51, GPIO52, GPIO53, GPIO54};

@@ -552,12 +554,14 @@ static struct pxa3xx_pin_group mmp2_grps[] = {
 	GRP_MMP2("uart1 4p1", UART1, mmp2_uart1_pin1),
 	GRP_MMP2("uart1 2p2", UART1, mmp2_uart1_pin2),
 	GRP_MMP2("uart1 2p3", UART1, mmp2_uart1_pin3),
+	GRP_MMP2("uart1 2p4", UART1, mmp2_uart1_pin4),
 	GRP_MMP2("uart2 4p1", UART2, mmp2_uart2_pin1),
 	GRP_MMP2("uart2 4p2", UART2, mmp2_uart2_pin2),
 	GRP_MMP2("uart2 4p3", UART2, mmp2_uart2_pin3),
 	GRP_MMP2("uart2 4p4", UART2, mmp2_uart2_pin4),
 	GRP_MMP2("uart2 2p5", UART2, mmp2_uart2_pin5),
 	GRP_MMP2("uart2 2p6", UART2, mmp2_uart2_pin6),
+	GRP_MMP2("uart2 2p7", UART2, mmp2_uart2_pin7),
 	GRP_MMP2("uart3 4p1", UART3, mmp2_uart3_pin1),
 	GRP_MMP2("uart3 4p2", UART3, mmp2_uart3_pin2),
 	GRP_MMP2("uart3 4p3", UART3, mmp2_uart3_pin3),

@@ -610,9 +614,9 @@ static struct pxa3xx_pin_group mmp2_grps[] = {
 };
 
 static const char * const mmp2_uart1_grps[] = {"uart1 4p1", "uart1 2p2",
-	"uart1 2p3"};
+	"uart1 2p3", "uart1 2p4"};
 static const char * const mmp2_uart2_grps[] = {"uart2 4p1", "uart2 4p2",
-	"uart2 4p3", "uart2 4p4", "uart2 4p5", "uart2 4p6"};
+	"uart2 4p3", "uart2 4p4", "uart2 2p5", "uart2 2p6", "uart2 2p7"};
 static const char * const mmp2_uart3_grps[] = {"uart3 4p1", "uart3 4p2",
 	"uart3 4p3", "uart3 4p4", "uart3 4p5", "uart3 2p6"};
 static const char * const mmp2_uart4_grps[] = {"uart4 4p1", "uart4 4p2",
Signature · 1 line
1.7.9.5
author
#11 lore raw

PATCH 5/6 ARM: mmp: select pinctrl for boards with DT

In reply to Haojian Zhuang
Signed-off-by Haojian Zhuang <[email protected]>
---
 arch/arm/mach-mmp/Kconfig |    2 ++
 1 file changed, 2 insertions(+)
arch/arm/mach-mmp/Kconfig +2 -0
index 7fddd01..c70c787 100644
@@ -89,6 +89,7 @@ config MACH_MMP_DT
 	select CPU_PXA168
 	select CPU_PXA910
 	select USE_OF
+	select PINCTRL
 	help
 	  Include support for Marvell MMP2 based platforms using
 	  the device tree. Needn't select any other machine while

@@ -99,6 +100,7 @@ config MACH_MMP2_DT
 	depends on !CPU_MOHAWK
 	select CPU_MMP2
 	select USE_OF
+	select PINCTRL
 	help
 	  Include support for Marvell MMP2 based platforms using
 	  the device tree.
Signature · 1 line
1.7.9.5
author
#13 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

In reply to Haojian Zhuang
Signed-off-by Haojian Zhuang <[email protected]>
---
 .../bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt       |   35 ++++++++++++++
 arch/arm/boot/dts/mmp2-brownstone.dts              |   51 ++++++++++++++++++++
 arch/arm/boot/dts/mmp2.dtsi                        |    5 ++
 arch/arm/boot/dts/pxa168.dtsi                      |    5 ++
 arch/arm/boot/dts/pxa910-dkb.dts                   |   47 ++++++++++++++++++
 arch/arm/boot/dts/pxa910.dtsi                      |    5 ++
 6 files changed, 148 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +35 -0
new file mode 100644
index 0000000..1f516e4
@@ -0,0 +1,35 @@
+Marvell Technology Group, PXA3XX pinmux controller
+
+Required properties:
+- compatible	: "marvell,pxa910-pinmux"
+		: "marvell,pxa168-pinmux"
+		: "marvell,mmp2-pinmux"
+- reg		: Address range of the pinctrl registers
+
+PXA3xx's pinmux nodes act as a container for an abitrary number of subnodes.
+Each of these subnodes represents muxing for a pin, a group, or a list of
+pins or groups.
+
+The name of each subnode is not important; all subnodes should be enumerated
+and processed purely based on their content.
+
+Required subnode-properties:
+- marvell,pins	: An array of strings. Each string contains the name of a pin
+		  or group.
+- marvell,function	: A string containing the name of the function to mux to the
+		  pin or group.
+- marvell,drive-strength : An integer value that means the drive strength of a
+                        pin.
+- marvell,pull-up	: The property means a pin is pull up for input.
+- marvell,pull-down : The property means a pin is pull down for input.
+- marvell,lowpower-pull-up : The property means a pin is pull up for input in
+			  sleep state.
+- marvell,lowpower-pull-down : The property means a pin is pull down for input
+			    in sleep state.
+- marvell,lowpower-drive-high : The property means a pin is driving high for
+			     output in sleep state.
+- marvell,lowpower-drive-low : The property means a pin is driving low for
+			    output in sleep state.
+- marvell,lowpower-float : The property means a pin is float in sleep state.
+- marvell,lowpower-zero : The property means a pin is not configured in sleep
+			state.
arch/arm/boot/dts/mmp2-brownstone.dts +51 -0

57 lines not on this page · raw message

arch/arm/boot/dts/mmp2.dtsi +5 -0
index 80f74e2..cb4ecaf 100644
@@ -120,6 +120,11 @@
 			reg = <0xd4000000 0x00200000>;
 			ranges;
 
+			pinmux at d401e000 {
+				compatible = "marvell,mmp2-pinmux";
+				reg = <0xd401e000 0x4000>;
+			};
+
 			timer0: timer at d4014000 {
 				compatible = "mrvl,mmp-timer";
 				reg = <0xd4014000 0x100>;
arch/arm/boot/dts/pxa168.dtsi +5 -0
index 31a7186..f15491d 100644
@@ -49,6 +49,11 @@
 			reg = <0xd4000000 0x00200000>;
 			ranges;
 
+			pinmux at d401e000 {
+				compatible = "marvell,pxa168-pinmux";
+				reg = <0xd401e000 0x4000>;
+			};
+
 			timer0: timer at d4014000 {
 				compatible = "mrvl,mmp-timer";
 				reg = <0xd4014000 0x100>;
arch/arm/boot/dts/pxa910-dkb.dts +47 -0

53 lines not on this page · raw message

arch/arm/boot/dts/pxa910.dtsi +5 -0
index aebf32d..187041d 100644
@@ -49,6 +49,11 @@
 			reg = <0xd4000000 0x00200000>;
 			ranges;
 
+			pinmux at d401e000 {
+				compatible = "marvell,pxa910-pinmux";
+				reg = <0xd401e000 0x4000>;
+			};
+
 			timer0: timer at d4014000 {
 				compatible = "mrvl,mmp-timer";
 				reg = <0xd4014000 0x100>;
Signature · 1 line
1.7.9.5
#14 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

Quoted 1 line · Haojian Zhuang
Signed-off-by Haojian Zhuang <[email protected]>

I have no clue as to whether these are good DT bindings or not, so leaving this up to the PXA maintainers :-)

You don't need my ACK for this part anyway.

Yours,
Linus Walleij

#15 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

In reply to Haojian Zhuang
Quoted 10 lines · Haojian Zhuang
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+Required subnode-properties:
+- marvell,pins : An array of strings. Each string contains the name of a pin
+                 or group.
+- marvell,function     : A string containing the name of the function to mux to the
+                 pin or group.
+- marvell,drive-strength : An integer value that means the drive strength of a
+                        pin.
+- marvell,pull-up      : The property means a pin is pull up for input.
+- marvell,pull-down : The property means a pin is pull down for input.
+- marvell,lowpower-pull-up : The property means a pin is pull up for input in

Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt line 35

Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+                         sleep state.
+- marvell,lowpower-pull-down : The property means a pin is pull down for input
+                           in sleep state.
+- marvell,lowpower-drive-high : The property means a pin is driving high for
+                            output in sleep state.
+- marvell,lowpower-drive-low : The property means a pin is driving low for
+                           output in sleep state.
+- marvell,lowpower-float : The property means a pin is float in sleep state.
+- marvell,lowpower-zero : The property means a pin is not configured in sleep
+                       state.

These (or most of them) look like they are not very Marvell specific. Should we try to standardize the way they are set across different bindings?

On a side-node, you probably don't want to make all of the above properties "required" because some of them are mutually exclusive.

Arnd

#16 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

Quoted 22 lines · Arnd Bergmann
Quoted 10 lines · Haojian Zhuang
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+Required subnode-properties:
+- marvell,pins : An array of strings. Each string contains the name of a pin
+                 or group.
+- marvell,function     : A string containing the name of the function to mux to the
+                 pin or group.
+- marvell,drive-strength : An integer value that means the drive strength of a
+                        pin.
+- marvell,pull-up      : The property means a pin is pull up for input.
+- marvell,pull-down : The property means a pin is pull down for input.
+- marvell,lowpower-pull-up : The property means a pin is pull up for input in

Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt line 35

Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+                         sleep state.
+- marvell,lowpower-pull-down : The property means a pin is pull down for input
+                           in sleep state.
+- marvell,lowpower-drive-high : The property means a pin is driving high for
+                            output in sleep state.
+- marvell,lowpower-drive-low : The property means a pin is driving low for
+                           output in sleep state.
+- marvell,lowpower-float : The property means a pin is float in sleep state.
+- marvell,lowpower-zero : The property means a pin is not configured in sleep
+                       state.
Quoted 2 lines

These (or most of them) look like they are not very Marvell specific. Should we try to standardize the way they are set across different bindings?

Yes please. That way we also have a higher chance that implementations on other platforms will be similar.

Daniel

author
#17 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

In reply to Arnd Bergmann
Quoted 22 lines · Arnd Bergmann
Quoted 10 lines · Haojian Zhuang
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+Required subnode-properties:
+- marvell,pins : An array of strings. Each string contains the name of a pin
+                 or group.
+- marvell,function     : A string containing the name of the function to mux to the
+                 pin or group.
+- marvell,drive-strength : An integer value that means the drive strength of a
+                        pin.
+- marvell,pull-up      : The property means a pin is pull up for input.
+- marvell,pull-down : The property means a pin is pull down for input.
+- marvell,lowpower-pull-up : The property means a pin is pull up for input in

Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt line 35

Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+                         sleep state.
+- marvell,lowpower-pull-down : The property means a pin is pull down for input
+                           in sleep state.
+- marvell,lowpower-drive-high : The property means a pin is driving high for
+                            output in sleep state.
+- marvell,lowpower-drive-low : The property means a pin is driving low for
+                           output in sleep state.
+- marvell,lowpower-float : The property means a pin is float in sleep state.
+- marvell,lowpower-zero : The property means a pin is not configured in sleep
+                       state.
Quoted 6 lines

These (or most of them) look like they are not very Marvell specific. Should we try to standardize the way they are set across different bindings?

On a side-node, you probably don't want to make all of the above properties "required" because some of them are mutually exclusive.

OK, I'll handle it.

#18 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

In reply to Arnd Bergmann
Quoted 22 lines · Arnd Bergmann
Quoted 10 lines · Haojian Zhuang
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+Required subnode-properties:
+- marvell,pins : An array of strings. Each string contains the name of a pin
+                 or group.
+- marvell,function     : A string containing the name of the function to mux to the
+                 pin or group.
+- marvell,drive-strength : An integer value that means the drive strength of a
+                        pin.
+- marvell,pull-up      : The property means a pin is pull up for input.
+- marvell,pull-down : The property means a pin is pull down for input.
+- marvell,lowpower-pull-up : The property means a pin is pull up for input in

Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt line 35

Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+                         sleep state.
+- marvell,lowpower-pull-down : The property means a pin is pull down for input
+                           in sleep state.
+- marvell,lowpower-drive-high : The property means a pin is driving high for
+                            output in sleep state.
+- marvell,lowpower-drive-low : The property means a pin is driving low for
+                           output in sleep state.
+- marvell,lowpower-float : The property means a pin is float in sleep state.
+- marvell,lowpower-zero : The property means a pin is not configured in sleep
+                       state.
Quoted 2 lines

These (or most of them) look like they are not very Marvell specific. Should we try to standardize the way they are set across different bindings?

If the device tree properties are to be standardized, the driver should be moved over to using generic pin config (drivers/pinctrl/pinconf-generic.c, include/linux/pinctrl/pinconf-generic.h) too, or it makes no sense.

So please augment the PXA driver to use these, then move the generic bindings to pinconf-generic.txt or something.

If pinconf-generic needs extending/augmenting in the process, just do it, it's supposed to be generic...

Yours,
Linus Walleij

#19 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

Quoted 30 lines · Linus Walleij
Quoted 22 lines · Arnd Bergmann
Quoted 10 lines · Haojian Zhuang
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+Required subnode-properties:
+- marvell,pins : An array of strings. Each string contains the name of a pin
+                 or group.
+- marvell,function     : A string containing the name of the function to mux to the
+                 pin or group.
+- marvell,drive-strength : An integer value that means the drive strength of a
+                        pin.
+- marvell,pull-up      : The property means a pin is pull up for input.
+- marvell,pull-down : The property means a pin is pull down for input.
+- marvell,lowpower-pull-up : The property means a pin is pull up for input in

Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt line 35

Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/mrvl,pxa3xx-pinctrl.txt new file +10 -0
+                         sleep state.
+- marvell,lowpower-pull-down : The property means a pin is pull down for input
+                           in sleep state.
+- marvell,lowpower-drive-high : The property means a pin is driving high for
+                            output in sleep state.
+- marvell,lowpower-drive-low : The property means a pin is driving low for
+                           output in sleep state.
+- marvell,lowpower-float : The property means a pin is float in sleep state.
+- marvell,lowpower-zero : The property means a pin is not configured in sleep
+                       state.
Quoted 2 lines

These (or most of them) look like they are not very Marvell specific. Should we try to standardize the way they are set across different bindings?

If the device tree properties are to be standardized, the driver should be moved over to using generic pin config (drivers/pinctrl/pinconf-generic.c, include/linux/pinctrl/pinconf-generic.h) too, or it makes no sense.

Quoted 5 lines

So please augment the PXA driver to use these, then move the generic bindings to pinconf-generic.txt or something.

If pinconf-generic needs extending/augmenting in the process, just do it, it's supposed to be generic...

I am looking at the generic pinctrl code right now and I wonder if the following approach would be ok:

- each driver that calls pinctrl_register() has its own dt_match_table entry, so the 'compatible' part of the bindings is specific to a pin controller chip.
- introduce a pinctrl_parse_of(struct of_node *) and call that after the driver has added all its pins. That function walks all the children of the given node and uses the callbacks provided by the driver to do the actual work.
- Unfortunately, we can't parse the node from pinctrl_register() as the gpio ranges are only added after that by the drivers.

If that sounds good, I can come up with a patch.

Thanks,
Daniel

#20 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

Quoted 1 line · Daniel Mack

I am looking at the generic pinctrl code right now

Sweet!

Quoted 5 lines

and I wonder if the following approach would be ok:

- each driver that calls pinctrl_register() has its own dt_match_table entry, so the 'compatible' part of the bindings is specific to a pin controller chip.

Yep.

Quoted 4 lines

- introduce a pinctrl_parse_of(struct of_node *) and call that after the driver has added all its pins. That function walks all the children of the given node and uses the callbacks provided by the driver to do the actual work.

Isn't that already how it works? Maybe I misunderstand, do you mean that you want to pass the DT node to pinctrl_register()?

Are you talking about using pinctrl hogs to set up the default configuration and muxing? That can already be done today, and has nothing to do with whether you use generic pin config or not.

This uses the .dt_node_to_map() and .dt_free_map() callbacks in the pinctrl_ops vtable to generate a map from the device tree, in any way you want.

So the infrastructure I think already exists, but whereas all current driver have their own config and mux (etc) parsing code, pinconf-generic could provide something that would be common for all driver using generic pin config.

Quoted 2 lines

- Unfortunately, we can't parse the node from pinctrl_register() as the gpio ranges are only added after that by the drivers.

The ranges are a *big* problem and we haven't come up with a proper DT binding for these. After discussion with Grant, he has proposed that GPIO chips should register their ranges using their local numberspace and always passing the struct gpio_chip, instead of pin controllers registering ranges. Documented here:
https://blueprints.launchpad.net/linux-linaro/+spec/pinctrl-gpiorange-makeover

Quoted 1 line

If that sounds good, I can come up with a patch.

I think you already have what you need, look closer at how e.g. the Tegra or i.MX driver does this. Or maybe I'm only getting it backwards?

Yours,
Linus Walleij

#21 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

Quoted 14 lines · Linus Walleij
Quoted 1 line · Daniel Mack

I am looking at the generic pinctrl code right now

Sweet!

Quoted 5 lines

and I wonder if the following approach would be ok:

- each driver that calls pinctrl_register() has its own dt_match_table entry, so the 'compatible' part of the bindings is specific to a pin controller chip.

Yep.

Quoted 7 lines
Quoted 4 lines

- introduce a pinctrl_parse_of(struct of_node *) and call that after the driver has added all its pins. That function walks all the children of the given node and uses the callbacks provided by the driver to do the actual work.

Isn't that already how it works? Maybe I misunderstand, do you mean that you want to pass the DT node to pinctrl_register()?

To parse the generic bits, yes. But I realized only now that we already have dt_to_map_one_config()

Quoted 8 lines

Are you talking about using pinctrl hogs to set up the default configuration and muxing? That can already be done today, and has nothing to do with whether you use generic pin config or not.

This uses the .dt_node_to_map() and .dt_free_map() callbacks in the pinctrl_ops vtable to generate a map from the device tree, in any way you want.

Yes, I see. And this is where duplication emerges, or, where consolidation would need to be done. If we want to standardize the way pins are set up in terms of bias, pullup/pulldown and all that, we would just need to map generic DT entries to PIN_CONFIG_* and call the pin controllers specific functions.

Quoted 4 lines

So the infrastructure I think already exists, but whereas all current driver have their own config and mux (etc) parsing code, pinconf-generic could provide something that would be common for all driver using generic pin config.

Exactly. And then the question is whether we want implicit behaviour. IOW - let dt_to_map_one_config() parse the generic bits and then call the driver specific augmentation code, if any.

Daniel

#22 lore raw

PATCH 6/6 ARM: dts: add pinctrl in PXA DT files

Quoted 10 lines · Daniel Mack
Quoted 4 lines · Linus Walleij

So the infrastructure I think already exists, but whereas all current driver have their own config and mux (etc) parsing code, pinconf-generic could provide something that would be common for all driver using generic pin config.

Exactly. And then the question is whether we want implicit behaviour. IOW - let dt_to_map_one_config() parse the generic bits and then call the driver specific augmentation code, if any.

Well if you make a patch I think we can see pretty clear from the code what the best way of doing this will be.

Yours,
Linus Walleij