linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf
@ 2016-11-23  3:29 David Lechner
  2016-11-23  3:29 ` [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd David Lechner
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: David Lechner @ 2016-11-23  3:29 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Mark Rutland, Sekhar Nori, Kevin Hilman
  Cc: David Lechner, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel, Axel Haslam, Alexandre Bailon,
	Bartosz Gołaszewski

This series adds a new driver and DT bindings for TI DA850/OMAP-L138/AM18x
pinconf (bias pullup/pulldown).

The motivation for this series is LEGO MINDSTORMS EV3 support. It needs most,
if not all, internal pullup/down resistors disabled in order to work correctly.

David Lechner (3):
  devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
  pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf
  ARM: dts: da850: Add node for pullup/pulldown pinconf

 .../devicetree/bindings/pinctrl/ti,da850-pupd.txt  |  55 ++++++
 arch/arm/boot/dts/da850.dtsi                       |   5 +
 drivers/pinctrl/Kconfig                            |   9 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/pinctrl-da850-pupd.c               | 210 +++++++++++++++++++++
 5 files changed, 280 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt
 create mode 100644 drivers/pinctrl/pinctrl-da850-pupd.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
  2016-11-23  3:29 [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf David Lechner
@ 2016-11-23  3:29 ` David Lechner
  2016-11-23 11:01   ` Sekhar Nori
                     ` (2 more replies)
  2016-11-23  3:29 ` [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf David Lechner
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 15+ messages in thread
From: David Lechner @ 2016-11-23  3:29 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Mark Rutland, Sekhar Nori, Kevin Hilman
  Cc: David Lechner, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel, Axel Haslam, Alexandre Bailon,
	Bartosz Gołaszewski

Device-tree bindings for TI DA8XX/OMAP-L138/AM18XX pullup/pulldown
pinconf controller.

Signed-off-by: David Lechner <david@lechnology.com>
---
 .../devicetree/bindings/pinctrl/ti,da850-pupd.txt  | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt

diff --git a/Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt b/Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt
new file mode 100644
index 0000000..7f29805
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt
@@ -0,0 +1,55 @@
+* Pin configuration for TI DA850/OMAP-L138/AM18x
+
+These SoCs have a separate controller for setting bias (internal pullup/down).
+Bias can only be selected for groups rather than individual pins.
+
+Required Properties:
+
+  - compatible: Must be "ti,da850-pupd"
+  - reg: Base address and length of the memory resource used by the pullup/down
+    controller hardware module.
+
+The controller node also acts as a container for pin group configuration nodes.
+The names of these groups are ignored.
+
+Pin Group Node Properties:
+
+- groups: An array of strings, each string containing the name of a pin group.
+          Valid names are "cp0".."cp31".
+
+The pin configuration parameters use the generic pinconf bindings defined in
+pinctrl-bindings.txt in this directory. The supported parameters are
+bias-disable, bias-pull-up, bias-pull-down.
+
+
+Example
+-------
+
+In common dtsi file:
+
+	pinconf: pin-controller@22c00c {
+		compatible = "ti,da850-pupd";
+		reg = <0x22c00c 0x8>;
+	};
+
+In board-specific file:
+
+	&pinconf {
+		pinctrl-0 = <&pinconf_bias_groups>;
+		pinctrl-names = "default";
+
+		pinconf_bias_groups: bias-groups {
+			pull-up {
+				groups = "cp30", "cp31";
+				bias-pull-up;
+			};
+			pull-down {
+				groups = "cp29", "cp28";
+				bias-pull-down;
+			};
+			disable {
+				groups = "cp27", "cp26";
+				bias-disable;
+			};
+		};
+	};
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf
  2016-11-23  3:29 [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf David Lechner
  2016-11-23  3:29 ` [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd David Lechner
@ 2016-11-23  3:29 ` David Lechner
  2016-11-23 11:04   ` Sekhar Nori
  2016-11-24 14:05   ` Linus Walleij
  2016-11-23  3:29 ` [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf David Lechner
  2016-11-23 11:17 ` [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf Sekhar Nori
  3 siblings, 2 replies; 15+ messages in thread
From: David Lechner @ 2016-11-23  3:29 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Mark Rutland, Sekhar Nori, Kevin Hilman
  Cc: David Lechner, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel, Axel Haslam, Alexandre Bailon,
	Bartosz Gołaszewski

This adds a new driver for pinconf on TI DA8XX/OMAP-L138/AM18XX. These
SoCs have a separate controller for controlling pullup/pulldown groups.

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/pinctrl/Kconfig              |   9 ++
 drivers/pinctrl/Makefile             |   1 +
 drivers/pinctrl/pinctrl-da850-pupd.c | 210 +++++++++++++++++++++++++++++++++++
 3 files changed, 220 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-da850-pupd.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 5f40ad6..54044a8 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -93,6 +93,15 @@ config PINCTRL_AMD
 	  Requires ACPI/FDT device enumeration code to set up a platform
 	  device.
 
+config PINCTRL_DA850_PUPD
+	tristate "TI DA850/OMAP-L138/AM18XX pullup/pulldown groups"
+	depends on OF && (ARCH_DAVINCI_DA850 || COMPILE_TEST)
+	select PINCONF
+	select GENERIC_PINCONF
+	help
+	  Driver for TI DA850/OMAP-L138/AM18XX pinconf. Used to control
+	  pullup/pulldown pin groups.
+
 config PINCTRL_DIGICOLOR
 	bool
 	depends on OF && (ARCH_DIGICOLOR || COMPILE_TEST)
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 3b8e6f7..25d50a8 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_PINCTRL_BF60x)	+= pinctrl-adi2-bf60x.o
 obj-$(CONFIG_PINCTRL_AT91)	+= pinctrl-at91.o
 obj-$(CONFIG_PINCTRL_AT91PIO4)	+= pinctrl-at91-pio4.o
 obj-$(CONFIG_PINCTRL_AMD)	+= pinctrl-amd.o
+obj-$(CONFIG_PINCTRL_DA850_PUPD) += pinctrl-da850-pupd.o
 obj-$(CONFIG_PINCTRL_DIGICOLOR)	+= pinctrl-digicolor.o
 obj-$(CONFIG_PINCTRL_FALCON)	+= pinctrl-falcon.o
 obj-$(CONFIG_PINCTRL_MAX77620)	+= pinctrl-max77620.o
diff --git a/drivers/pinctrl/pinctrl-da850-pupd.c b/drivers/pinctrl/pinctrl-da850-pupd.c
new file mode 100644
index 0000000..0446df7
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-da850-pupd.c
@@ -0,0 +1,210 @@
+/*
+ * Pinconf driver for TI DA850/OMAP-L138/AM18XX pullup/pulldown groups
+ *
+ * Copyright (C) 2016  David Lechner
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+
+#define DA850_PUPD_ENA		0x00
+#define DA850_PUPD_SEL		0x04
+
+struct da850_pupd_data {
+	void __iomem *base;
+	struct pinctrl_desc desc;
+	struct pinctrl_dev *pinctrl;
+};
+
+static const char * const da850_pupd_group_names[] = {
+	"cp0", "cp1", "cp2", "cp3", "cp4", "cp5", "cp6", "cp7",
+	"cp8", "cp9", "cp10", "cp11", "cp12", "cp13", "cp14", "cp15",
+	"cp16", "cp17", "cp18", "cp19", "cp20", "cp21", "cp22", "cp23",
+	"cp24", "cp25", "cp26", "cp27", "cp28", "cp29", "cp30", "cp31",
+};
+
+static int da850_pupd_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(da850_pupd_group_names);
+}
+
+static const char *da850_pupd_get_get_group_name(struct pinctrl_dev *pctldev,
+						 unsigned int selector)
+{
+	return da850_pupd_group_names[selector];
+}
+
+static int da850_pupd_get_get_group_pins(struct pinctrl_dev *pctldev,
+					 unsigned int selector,
+					 const unsigned int **pins,
+					 unsigned int *num_pins)
+{
+	*num_pins = 0;
+
+	return 0;
+}
+
+static const struct pinctrl_ops da850_pupd_pctlops = {
+	.get_groups_count	= da850_pupd_get_groups_count,
+	.get_group_name		= da850_pupd_get_get_group_name,
+	.get_group_pins		= da850_pupd_get_get_group_pins,
+	.dt_node_to_map		= pinconf_generic_dt_node_to_map_group,
+	.dt_free_map		= pinconf_generic_dt_free_map,
+};
+
+static int da850_pupd_pin_config_group_get(struct pinctrl_dev *pctldev,
+					   unsigned int selector,
+					   unsigned long *config)
+{
+	struct da850_pupd_data *data = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	u32 val;
+	u16 arg;
+
+	val = readl(data->base + DA850_PUPD_ENA);
+	arg = !!(~val & BIT(selector));
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		if (arg) {
+			/* bias is disabled */
+			arg = 0;
+			break;
+		}
+		val = readl(data->base + DA850_PUPD_SEL);
+		if (param == PIN_CONFIG_BIAS_PULL_DOWN)
+			val = ~val;
+		arg = !!(val & BIT(selector));
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*config = pinconf_to_config_packed(param, arg);
+
+	return 0;
+}
+
+static int da850_pupd_pin_config_group_set(struct pinctrl_dev *pctldev,
+					   unsigned int selector,
+					   unsigned long *configs,
+					   unsigned int num_configs)
+{
+	struct da850_pupd_data *data = pinctrl_dev_get_drvdata(pctldev);
+	u32 ena, sel;
+	enum pin_config_param param;
+	u16 arg;
+	int i;
+
+	ena = readl(data->base + DA850_PUPD_ENA);
+	sel = readl(data->base + DA850_PUPD_SEL);
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+		arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_DISABLE:
+			ena &= ~BIT(selector);
+			break;
+		case PIN_CONFIG_BIAS_PULL_UP:
+			ena |= BIT(selector);
+			sel |= BIT(selector);
+			break;
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+			ena |= BIT(selector);
+			sel &= ~BIT(selector);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	writel(sel, data->base + DA850_PUPD_SEL);
+	writel(ena, data->base + DA850_PUPD_ENA);
+
+	return 0;
+}
+
+static const struct pinconf_ops da850_pupd_confops = {
+	.is_generic		= true,
+	.pin_config_group_get	= da850_pupd_pin_config_group_get,
+	.pin_config_group_set	= da850_pupd_pin_config_group_set,
+};
+
+static int da850_pupd_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct da850_pupd_data *data;
+	struct resource *res;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(data->base)) {
+		dev_err(dev, "Could not map resource\n");
+		return PTR_ERR(data->base);
+	}
+
+	data->desc.name = dev_name(dev);
+	data->desc.pctlops = &da850_pupd_pctlops;
+	data->desc.confops = &da850_pupd_confops;
+	data->desc.owner = THIS_MODULE;
+
+	data->pinctrl = devm_pinctrl_register(dev, &data->desc, data);
+	if (IS_ERR(data->pinctrl)) {
+		dev_err(dev, "Failed to register pinctrl\n");
+		return PTR_ERR(data->pinctrl);
+	}
+
+	platform_set_drvdata(pdev, data);
+
+	return 0;
+}
+
+static int da850_pupd_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct of_device_id da850_pupd_of_match[] = {
+	{ .compatible = "ti,da850-pupd" },
+	{ }
+};
+
+static struct platform_driver da850_pupd_driver = {
+	.driver	= {
+		.name		= "ti-da850-pupd",
+		.of_match_table	= da850_pupd_of_match,
+	},
+	.probe	= da850_pupd_probe,
+	.remove	= da850_pupd_remove,
+};
+module_platform_driver(da850_pupd_driver);
+
+MODULE_AUTHOR("David Lechner <david@lechnology.com>");
+MODULE_DESCRIPTION("TI DA850/OMAP-L138/AM18XX pullup/pulldown configuration");
+MODULE_LICENSE("GPL");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf
  2016-11-23  3:29 [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf David Lechner
  2016-11-23  3:29 ` [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd David Lechner
  2016-11-23  3:29 ` [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf David Lechner
@ 2016-11-23  3:29 ` David Lechner
  2016-11-23 11:12   ` Sekhar Nori
  2016-11-23 11:17 ` [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf Sekhar Nori
  3 siblings, 1 reply; 15+ messages in thread
From: David Lechner @ 2016-11-23  3:29 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Mark Rutland, Sekhar Nori, Kevin Hilman
  Cc: David Lechner, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel, Axel Haslam, Alexandre Bailon,
	Bartosz Gołaszewski

This SoC has a separate pin controller for configuring pullup/pulldown
bias on groups of pins.

Signed-off-by: David Lechner <david@lechnology.com>
---
 arch/arm/boot/dts/da850.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 8945815..1c0224c 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -210,6 +210,11 @@
 			};
 
 		};
+		pinconf: pin-controller@22c00c {
+			compatible = "ti,da850-pupd";
+			reg = <0x22c00c 0x8>;
+			status = "disabled";
+		};
 		prictrl: priority-controller@14110 {
 			compatible = "ti,da850-mstpri";
 			reg = <0x14110 0x0c>;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
  2016-11-23  3:29 ` [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd David Lechner
@ 2016-11-23 11:01   ` Sekhar Nori
  2016-11-24 14:04   ` Linus Walleij
  2016-11-28 21:54   ` Rob Herring
  2 siblings, 0 replies; 15+ messages in thread
From: Sekhar Nori @ 2016-11-23 11:01 UTC (permalink / raw)
  To: David Lechner, Linus Walleij, Rob Herring, Mark Rutland, Kevin Hilman
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski

On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
> Device-tree bindings for TI DA8XX/OMAP-L138/AM18XX pullup/pulldown

s/DA8XX/DA850. It looks like this support is absent from DA830.

> pinconf controller.
> 
> Signed-off-by: David Lechner <david@lechnology.com>

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf
  2016-11-23  3:29 ` [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf David Lechner
@ 2016-11-23 11:04   ` Sekhar Nori
  2016-11-23 16:21     ` David Lechner
  2016-11-24 14:05   ` Linus Walleij
  1 sibling, 1 reply; 15+ messages in thread
From: Sekhar Nori @ 2016-11-23 11:04 UTC (permalink / raw)
  To: David Lechner, Linus Walleij, Rob Herring, Mark Rutland, Kevin Hilman
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski

On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
> This adds a new driver for pinconf on TI DA8XX/OMAP-L138/AM18XX. These

s/DA8XX/DA850/

> SoCs have a separate controller for controlling pullup/pulldown groups.
> 
> Signed-off-by: David Lechner <david@lechnology.com>

> +static const char *da850_pupd_get_get_group_name(struct pinctrl_dev *pctldev,
> +						 unsigned int selector)
> +{
> +	return da850_pupd_group_names[selector];
> +}
> +
> +static int da850_pupd_get_get_group_pins(struct pinctrl_dev *pctldev,
> +					 unsigned int selector,
> +					 const unsigned int **pins,
> +					 unsigned int *num_pins)
> +{
> +	*num_pins = 0;
> +
> +	return 0;
> +}

usage of get_get_ in the function names above is odd.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf
  2016-11-23  3:29 ` [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf David Lechner
@ 2016-11-23 11:12   ` Sekhar Nori
  2016-11-23 16:24     ` David Lechner
  0 siblings, 1 reply; 15+ messages in thread
From: Sekhar Nori @ 2016-11-23 11:12 UTC (permalink / raw)
  To: David Lechner, Linus Walleij, Rob Herring, Mark Rutland, Kevin Hilman
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski

On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
> This SoC has a separate pin controller for configuring pullup/pulldown
> bias on groups of pins.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  arch/arm/boot/dts/da850.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 8945815..1c0224c 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -210,6 +210,11 @@
>  			};
>  
>  		};
> +		pinconf: pin-controller@22c00c {
> +			compatible = "ti,da850-pupd";
> +			reg = <0x22c00c 0x8>;
> +			status = "disabled";
> +		};

Can you please place this below the i2c1 node. I am trying to keep the
nodes sorted by unit address. I know thats broken in many places today,
but lets add the new ones where they should eventually end up.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf
  2016-11-23  3:29 [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf David Lechner
                   ` (2 preceding siblings ...)
  2016-11-23  3:29 ` [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf David Lechner
@ 2016-11-23 11:17 ` Sekhar Nori
  3 siblings, 0 replies; 15+ messages in thread
From: Sekhar Nori @ 2016-11-23 11:17 UTC (permalink / raw)
  To: David Lechner, Linus Walleij, Rob Herring, Mark Rutland, Kevin Hilman
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski

On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
> This series adds a new driver and DT bindings for TI DA850/OMAP-L138/AM18x
> pinconf (bias pullup/pulldown).
> 
> The motivation for this series is LEGO MINDSTORMS EV3 support. It needs most,
> if not all, internal pullup/down resistors disabled in order to work correctly.

This looks really neat to my non-pinconf trained eyes. I have sent some
minor comments. But apart from that:

Reviewed-by: Sekhar Nori <nsekhar@ti.com>

3/3 should go through my tree. If the driver patches are going to get
queued for v4.10, I can queue the DTS portion through my tree.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf
  2016-11-23 11:04   ` Sekhar Nori
@ 2016-11-23 16:21     ` David Lechner
  0 siblings, 0 replies; 15+ messages in thread
From: David Lechner @ 2016-11-23 16:21 UTC (permalink / raw)
  To: Sekhar Nori, Linus Walleij, Rob Herring, Mark Rutland, Kevin Hilman
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski

On 11/23/2016 05:04 AM, Sekhar Nori wrote:
> On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
>> This adds a new driver for pinconf on TI DA8XX/OMAP-L138/AM18XX. These
>
> s/DA8XX/DA850/
>
>> SoCs have a separate controller for controlling pullup/pulldown groups.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>
>> +static const char *da850_pupd_get_get_group_name(struct pinctrl_dev *pctldev,
>> +						 unsigned int selector)
>> +{
>> +	return da850_pupd_group_names[selector];
>> +}
>> +
>> +static int da850_pupd_get_get_group_pins(struct pinctrl_dev *pctldev,
>> +					 unsigned int selector,
>> +					 const unsigned int **pins,
>> +					 unsigned int *num_pins)
>> +{
>> +	*num_pins = 0;
>> +
>> +	return 0;
>> +}
>
> usage of get_get_ in the function names above is odd.

Will fix (copy/paste error)

>
> Thanks,
> Sekhar
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf
  2016-11-23 11:12   ` Sekhar Nori
@ 2016-11-23 16:24     ` David Lechner
  2016-11-23 22:33       ` Kevin Hilman
  2016-11-24  5:54       ` Sekhar Nori
  0 siblings, 2 replies; 15+ messages in thread
From: David Lechner @ 2016-11-23 16:24 UTC (permalink / raw)
  To: Sekhar Nori, Linus Walleij, Rob Herring, Mark Rutland, Kevin Hilman
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski

On 11/23/2016 05:12 AM, Sekhar Nori wrote:
> On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
>> This SoC has a separate pin controller for configuring pullup/pulldown
>> bias on groups of pins.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>>  arch/arm/boot/dts/da850.dtsi | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 8945815..1c0224c 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -210,6 +210,11 @@
>>  			};
>>
>>  		};
>> +		pinconf: pin-controller@22c00c {
>> +			compatible = "ti,da850-pupd";
>> +			reg = <0x22c00c 0x8>;
>> +			status = "disabled";
>> +		};
>
> Can you please place this below the i2c1 node. I am trying to keep the
> nodes sorted by unit address. I know thats broken in many places today,
> but lets add the new ones where they should eventually end up.

I can do this, but it seems that the predominant sorting pattern here is 
to keep subsystems together (e.g. all i2c are together, all uart are 
together, etc.)

Would a separate patch to sort everything by unit address to get this 
cleaned up be acceptable?

>
> Thanks,
> Sekhar
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf
  2016-11-23 16:24     ` David Lechner
@ 2016-11-23 22:33       ` Kevin Hilman
  2016-11-24  5:54       ` Sekhar Nori
  1 sibling, 0 replies; 15+ messages in thread
From: Kevin Hilman @ 2016-11-23 22:33 UTC (permalink / raw)
  To: David Lechner
  Cc: Sekhar Nori, Linus Walleij, Rob Herring, Mark Rutland,
	linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski

David Lechner <david@lechnology.com> writes:

> On 11/23/2016 05:12 AM, Sekhar Nori wrote:
>> On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
>>> This SoC has a separate pin controller for configuring pullup/pulldown
>>> bias on groups of pins.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>>  arch/arm/boot/dts/da850.dtsi | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index 8945815..1c0224c 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -210,6 +210,11 @@
>>>  			};
>>>
>>>  		};
>>> +		pinconf: pin-controller@22c00c {
>>> +			compatible = "ti,da850-pupd";
>>> +			reg = <0x22c00c 0x8>;
>>> +			status = "disabled";
>>> +		};
>>
>> Can you please place this below the i2c1 node. I am trying to keep the
>> nodes sorted by unit address. I know thats broken in many places today,
>> but lets add the new ones where they should eventually end up.
>
> I can do this, but it seems that the predominant sorting pattern here
> is to keep subsystems together (e.g. all i2c are together, all uart
> are together, etc.)
>
> Would a separate patch to sort everything by unit address to get this
> cleaned up be acceptable?

No thanks. That kind of thing is the needless churn that gets us flamed.

Kevin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf
  2016-11-23 16:24     ` David Lechner
  2016-11-23 22:33       ` Kevin Hilman
@ 2016-11-24  5:54       ` Sekhar Nori
  1 sibling, 0 replies; 15+ messages in thread
From: Sekhar Nori @ 2016-11-24  5:54 UTC (permalink / raw)
  To: David Lechner, Linus Walleij, Rob Herring, Mark Rutland, Kevin Hilman
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski

On Wednesday 23 November 2016 09:54 PM, David Lechner wrote:
> On 11/23/2016 05:12 AM, Sekhar Nori wrote:
>> On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
>>> This SoC has a separate pin controller for configuring pullup/pulldown
>>> bias on groups of pins.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>>  arch/arm/boot/dts/da850.dtsi | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index 8945815..1c0224c 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -210,6 +210,11 @@
>>>              };
>>>
>>>          };
>>> +        pinconf: pin-controller@22c00c {
>>> +            compatible = "ti,da850-pupd";
>>> +            reg = <0x22c00c 0x8>;
>>> +            status = "disabled";
>>> +        };
>>
>> Can you please place this below the i2c1 node. I am trying to keep the
>> nodes sorted by unit address. I know thats broken in many places today,
>> but lets add the new ones where they should eventually end up.
> 
> I can do this, but it seems that the predominant sorting pattern here is
> to keep subsystems together (e.g. all i2c are together, all uart are
> together, etc.)

Yeah, but that quickly gives away as there are many singleton devices
and everyone tries to add theirs at the end of the list resulting in
merge conflicts.

> Would a separate patch to sort everything by unit address to get this
> cleaned up be acceptable?

Agree with Kevin that it would be churn. If done, it should be last
thing that gets done in a merge window. I would not attempt it in
relatively busy merge windows like this one.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
  2016-11-23  3:29 ` [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd David Lechner
  2016-11-23 11:01   ` Sekhar Nori
@ 2016-11-24 14:04   ` Linus Walleij
  2016-11-28 21:54   ` Rob Herring
  2 siblings, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2016-11-24 14:04 UTC (permalink / raw)
  To: David Lechner
  Cc: Rob Herring, Mark Rutland, Sekhar Nori, Kevin Hilman, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel, Axel Haslam,
	Alexandre Bailon, Bartosz Gołaszewski

On Wed, Nov 23, 2016 at 4:29 AM, David Lechner <david@lechnology.com> wrote:

> Device-tree bindings for TI DA8XX/OMAP-L138/AM18XX pullup/pulldown
> pinconf controller.
>
> Signed-off-by: David Lechner <david@lechnology.com>

Looks good to me.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf
  2016-11-23  3:29 ` [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf David Lechner
  2016-11-23 11:04   ` Sekhar Nori
@ 2016-11-24 14:05   ` Linus Walleij
  1 sibling, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2016-11-24 14:05 UTC (permalink / raw)
  To: David Lechner
  Cc: Rob Herring, Mark Rutland, Sekhar Nori, Kevin Hilman, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel, Axel Haslam,
	Alexandre Bailon, Bartosz Gołaszewski

On Wed, Nov 23, 2016 at 4:29 AM, David Lechner <david@lechnology.com> wrote:

> This adds a new driver for pinconf on TI DA8XX/OMAP-L138/AM18XX. These
> SoCs have a separate controller for controlling pullup/pulldown groups.
>
> Signed-off-by: David Lechner <david@lechnology.com>

Nice and clean driver, resend with the minor fixes pointed out
by Sekhar and I'll merge it.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
  2016-11-23  3:29 ` [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd David Lechner
  2016-11-23 11:01   ` Sekhar Nori
  2016-11-24 14:04   ` Linus Walleij
@ 2016-11-28 21:54   ` Rob Herring
  2 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2016-11-28 21:54 UTC (permalink / raw)
  To: David Lechner
  Cc: Linus Walleij, Mark Rutland, Sekhar Nori, Kevin Hilman,
	linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski

On Tue, Nov 22, 2016 at 09:29:25PM -0600, David Lechner wrote:
> Device-tree bindings for TI DA8XX/OMAP-L138/AM18XX pullup/pulldown
> pinconf controller.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  .../devicetree/bindings/pinctrl/ti,da850-pupd.txt  | 55 ++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-11-28 21:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-23  3:29 [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf David Lechner
2016-11-23  3:29 ` [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd David Lechner
2016-11-23 11:01   ` Sekhar Nori
2016-11-24 14:04   ` Linus Walleij
2016-11-28 21:54   ` Rob Herring
2016-11-23  3:29 ` [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf David Lechner
2016-11-23 11:04   ` Sekhar Nori
2016-11-23 16:21     ` David Lechner
2016-11-24 14:05   ` Linus Walleij
2016-11-23  3:29 ` [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf David Lechner
2016-11-23 11:12   ` Sekhar Nori
2016-11-23 16:24     ` David Lechner
2016-11-23 22:33       ` Kevin Hilman
2016-11-24  5:54       ` Sekhar Nori
2016-11-23 11:17 ` [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf Sekhar Nori

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).