[U-Boot] [PATCH v3] Add single register pin controller driver

[U-Boot] [PATCH v3] Add single register pin controller driver

[James Balean]

This patch adds a pin controller driver supporting devices using a single configuration register per pin.

Signed-off-by: Felix Brack <fb@ltec.ch>
[james at balean.com.au: changed .set_state_simple operation to .set_state]
Signed-off-by: James Balean <james@balean.com.au>
---
 drivers/pinctrl/Kconfig          |  10 +++
 drivers/pinctrl/Makefile         |   1 +
 drivers/pinctrl/pinctrl-single.c | 141 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 152 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-single.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index efcb4c0..a2b9212 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -175,6 +175,16 @@ config PIC32_PINCTRL
 	  by a device tree node which contains both GPIO defintion and pin control
 	  functions.
 
+config PINCTRL_SINGLE
+	bool "Single register pin-control and pin-multiplex driver"
+	depends on PINCTRL_FULL || SPL_PINCTRL_FULL
+	help
+	  This enables pinctrl driver for systems using a single register for
+	  pin configuration and multiplexing. TI's AM335X SoCs are examples of
+	  such systems.
+	  Depending on the platform make sure to also enable OF_TRANSLATE and
+	  eventually SPL_OF_TRANSLATE to get correct address translations.
+
 endif
 
 source "drivers/pinctrl/meson/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 512112a..f148f94 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_PIC32_PINCTRL)	+= pinctrl_pic32.o
 obj-$(CONFIG_PINCTRL_EXYNOS)	+= exynos/
 obj-$(CONFIG_PINCTRL_MESON)	+= meson/
 obj-$(CONFIG_PINCTRL_MVEBU)	+= mvebu/
+obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
new file mode 100644
index 0000000..a5b4d75
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -0,0 +1,141 @@
+/*
+ * Copyright (C) EETS GmbH, 2017, Felix Brack <f.brack@eets.ch>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm/device.h>
+#include <dm/pinctrl.h>
+#include <libfdt.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct single_pdata {
+	fdt_addr_t base;	/* first configuration register */
+	int offset;		/* index of last configuration register */
+	u32 mask;		/* configuration-value mask bits */
+	int width;		/* configuration register bit width */
+};
+
+struct single_fdt_pin_cfg {
+	fdt32_t reg;		/* configuration register offset */
+	fdt32_t val;		/* configuration register value */
+};
+
+/**
+ * single_configure_pins() - Configure pins based on FDT data
+ *
+ * @dev: Pointer to single pin configuration device which is the parent of
+ *       the pins node holding the pin configuration data.
+ * @pins: Pointer to the first element of an array of register/value pairs
+ *        of type 'struct single_fdt_pin_cfg'. Each such pair describes the
+ *        the pin to be configured and the value to be used for configuration.
+ *        This pointer points to a 'pinctrl-single,pins' property in the
+ *        device-tree.
+ * @size: Size of the 'pins' array in bytes.
+ *        The number of register/value pairs in the 'pins' array therefore
+ *        equals to 'size / sizeof(struct single_fdt_pin_cfg)'.
+ */
+static int single_configure_pins(struct udevice *dev,
+				 const struct single_fdt_pin_cfg *pins,
+				 int size)
+{
+	struct single_pdata *pdata = dev->platdata;
+	int count = size / sizeof(struct single_fdt_pin_cfg);
+	int n, reg;
+	u32 val;
+
+	for (n = 0; n < count; n++) {
+		reg = fdt32_to_cpu(pins->reg);
+		if ((reg < 0) || (reg > pdata->offset)) {
+			dev_dbg(dev, "  invalid register offset 0x%08x\n", reg);
+			pins++;
+			continue;
+		}
+		reg += pdata->base;
+		switch (pdata->width) {
+		case 32:
+			val = readl(reg) & ~pdata->mask;
+			val |= fdt32_to_cpu(pins->val) & pdata->mask;
+			writel(val, reg);
+			dev_dbg(dev, "  reg/val 0x%08x/0x%08x\n",
+				reg, val);
+			break;
+		default:
+			dev_warn(dev, "unsupported register width %i\n",
+				 pdata->width);
+		}
+		pins++;
+	}
+	return 0;
+}
+
+static int single_set_state(struct udevice *dev, struct udevice *config)
+{
+	const void *fdt = gd->fdt_blob;
+	const struct single_fdt_pin_cfg *prop;
+	int len;
+
+	prop = fdt_getprop(fdt, config->of_offset, "pinctrl-single,pins", &len);
+	if (prop) {
+		dev_dbg(dev, "configuring pins for %s\n", config->name);
+		if (len % sizeof(struct single_fdt_pin_cfg)) {
+			dev_dbg(dev, "  invalid pin configuration in fdt\n");
+			return -FDT_ERR_BADSTRUCTURE;
+		}
+		single_configure_pins(dev, prop, len);
+		len = 0;
+	}
+
+	return len;
+}
+
+static int single_ofdata_to_platdata(struct udevice *dev)
+{
+	fdt_addr_t addr;
+	u32 of_reg[2];
+	int res;
+	struct single_pdata *pdata = dev->platdata;
+
+	pdata->width = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+				      "pinctrl-single,register-width", 0);
+
+	res = fdtdec_get_int_array(gd->fdt_blob, dev->of_offset,
+				   "reg", of_reg, 2);
+	if (res)
+		return res;
+	pdata->offset = of_reg[1] - pdata->width / 8;
+
+	addr = dev_get_addr(dev);
+	if (addr == FDT_ADDR_T_NONE) {
+		dev_dbg(dev, "no valid base register address\n");
+		return -EINVAL;
+	}
+	pdata->base = addr;
+
+	pdata->mask = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+				     "pinctrl-single,function-mask",
+				     0xffffffff);
+	return 0;
+}
+
+const struct pinctrl_ops single_pinctrl_ops = {
+	.set_state = single_set_state,
+};
+
+static const struct udevice_id single_pinctrl_match[] = {
+	{ .compatible = "pinctrl-single" },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(single_pinctrl) = {
+	.name = "single-pinctrl",
+	.id = UCLASS_PINCTRL,
+	.of_match = single_pinctrl_match,
+	.ops = &single_pinctrl_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+	.platdata_auto_alloc_size = sizeof(struct single_pdata),
+	.ofdata_to_platdata = single_ofdata_to_platdata,
+};
-- 
2.7.4

[U-Boot] [PATCH v3] Add single register pin controller driver

[Lukasz Majewski]

On Thu,  9 Mar 2017 21:55:44 -0600
James Balean <james@balean.com.au> wrote:

> This patch adds a pin controller driver supporting devices using a
> single configuration register per pin.
> 
> Signed-off-by: Felix Brack <fb@ltec.ch>
> [james at balean.com.au: changed .set_state_simple operation
> to .set_state] Signed-off-by: James Balean <james@balean.com.au>

Reviewed-by: Lukasz Majewski <lukma@denx.de>

> ---
>  drivers/pinctrl/Kconfig          |  10 +++
>  drivers/pinctrl/Makefile         |   1 +
>  drivers/pinctrl/pinctrl-single.c | 141
> +++++++++++++++++++++++++++++++++++++++ 3 files changed, 152
> insertions(+) create mode 100644 drivers/pinctrl/pinctrl-single.c
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index efcb4c0..a2b9212 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -175,6 +175,16 @@ config PIC32_PINCTRL
>  	  by a device tree node which contains both GPIO defintion
> and pin control functions.
>  
> +config PINCTRL_SINGLE
> +	bool "Single register pin-control and pin-multiplex driver"
> +	depends on PINCTRL_FULL || SPL_PINCTRL_FULL
> +	help
> +	  This enables pinctrl driver for systems using a single
> register for
> +	  pin configuration and multiplexing. TI's AM335X SoCs are
> examples of
> +	  such systems.
> +	  Depending on the platform make sure to also enable
> OF_TRANSLATE and
> +	  eventually SPL_OF_TRANSLATE to get correct address
> translations. +
>  endif
>  
>  source "drivers/pinctrl/meson/Kconfig"
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index 512112a..f148f94 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_PIC32_PINCTRL)	+=
> pinctrl_pic32.o obj-$(CONFIG_PINCTRL_EXYNOS)	+= exynos/
>  obj-$(CONFIG_PINCTRL_MESON)	+= meson/
>  obj-$(CONFIG_PINCTRL_MVEBU)	+= mvebu/
> +obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
> diff --git a/drivers/pinctrl/pinctrl-single.c
> b/drivers/pinctrl/pinctrl-single.c new file mode 100644
> index 0000000..a5b4d75
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -0,0 +1,141 @@
> +/*
> + * Copyright (C) EETS GmbH, 2017, Felix Brack <f.brack@eets.ch>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm/device.h>
> +#include <dm/pinctrl.h>
> +#include <libfdt.h>
> +#include <asm/io.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct single_pdata {
> +	fdt_addr_t base;	/* first configuration register */
> +	int offset;		/* index of last configuration
> register */
> +	u32 mask;		/* configuration-value mask bits */
> +	int width;		/* configuration register bit
> width */ +};
> +
> +struct single_fdt_pin_cfg {
> +	fdt32_t reg;		/* configuration register offset
> */
> +	fdt32_t val;		/* configuration register value
> */ +};
> +
> +/**
> + * single_configure_pins() - Configure pins based on FDT data
> + *
> + * @dev: Pointer to single pin configuration device which is the
> parent of
> + *       the pins node holding the pin configuration data.
> + * @pins: Pointer to the first element of an array of register/value
> pairs
> + *        of type 'struct single_fdt_pin_cfg'. Each such pair
> describes the
> + *        the pin to be configured and the value to be used for
> configuration.
> + *        This pointer points to a 'pinctrl-single,pins' property in
> the
> + *        device-tree.
> + * @size: Size of the 'pins' array in bytes.
> + *        The number of register/value pairs in the 'pins' array
> therefore
> + *        equals to 'size / sizeof(struct single_fdt_pin_cfg)'.
> + */
> +static int single_configure_pins(struct udevice *dev,
> +				 const struct single_fdt_pin_cfg
> *pins,
> +				 int size)
> +{
> +	struct single_pdata *pdata = dev->platdata;
> +	int count = size / sizeof(struct single_fdt_pin_cfg);
> +	int n, reg;
> +	u32 val;
> +
> +	for (n = 0; n < count; n++) {
> +		reg = fdt32_to_cpu(pins->reg);
> +		if ((reg < 0) || (reg > pdata->offset)) {
> +			dev_dbg(dev, "  invalid register offset
> 0x%08x\n", reg);
> +			pins++;
> +			continue;
> +		}
> +		reg += pdata->base;
> +		switch (pdata->width) {
> +		case 32:
> +			val = readl(reg) & ~pdata->mask;
> +			val |= fdt32_to_cpu(pins->val) & pdata->mask;
> +			writel(val, reg);
> +			dev_dbg(dev, "  reg/val 0x%08x/0x%08x\n",
> +				reg, val);
> +			break;
> +		default:
> +			dev_warn(dev, "unsupported register width
> %i\n",
> +				 pdata->width);
> +		}
> +		pins++;
> +	}
> +	return 0;
> +}
> +
> +static int single_set_state(struct udevice *dev, struct udevice
> *config) +{
> +	const void *fdt = gd->fdt_blob;
> +	const struct single_fdt_pin_cfg *prop;
> +	int len;
> +
> +	prop = fdt_getprop(fdt, config->of_offset,
> "pinctrl-single,pins", &len);
> +	if (prop) {
> +		dev_dbg(dev, "configuring pins for %s\n",
> config->name);
> +		if (len % sizeof(struct single_fdt_pin_cfg)) {
> +			dev_dbg(dev, "  invalid pin configuration in
> fdt\n");
> +			return -FDT_ERR_BADSTRUCTURE;
> +		}
> +		single_configure_pins(dev, prop, len);
> +		len = 0;
> +	}
> +
> +	return len;
> +}
> +
> +static int single_ofdata_to_platdata(struct udevice *dev)
> +{
> +	fdt_addr_t addr;
> +	u32 of_reg[2];
> +	int res;
> +	struct single_pdata *pdata = dev->platdata;
> +
> +	pdata->width = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +
> "pinctrl-single,register-width", 0); +
> +	res = fdtdec_get_int_array(gd->fdt_blob, dev->of_offset,
> +				   "reg", of_reg, 2);
> +	if (res)
> +		return res;
> +	pdata->offset = of_reg[1] - pdata->width / 8;
> +
> +	addr = dev_get_addr(dev);
> +	if (addr == FDT_ADDR_T_NONE) {
> +		dev_dbg(dev, "no valid base register address\n");
> +		return -EINVAL;
> +	}
> +	pdata->base = addr;
> +
> +	pdata->mask = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +				     "pinctrl-single,function-mask",
> +				     0xffffffff);
> +	return 0;
> +}
> +
> +const struct pinctrl_ops single_pinctrl_ops = {
> +	.set_state = single_set_state,
> +};
> +
> +static const struct udevice_id single_pinctrl_match[] = {
> +	{ .compatible = "pinctrl-single" },
> +	{ /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(single_pinctrl) = {
> +	.name = "single-pinctrl",
> +	.id = UCLASS_PINCTRL,
> +	.of_match = single_pinctrl_match,
> +	.ops = &single_pinctrl_ops,
> +	.flags = DM_FLAG_PRE_RELOC,
> +	.platdata_auto_alloc_size = sizeof(struct single_pdata),
> +	.ofdata_to_platdata = single_ofdata_to_platdata,
> +};




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH v3] Add single register pin controller driver

[Felix Brack]

Hi,

On 10.03.2017 04:55, James Balean wrote:
> This patch adds a pin controller driver supporting devices using a single configuration register per pin.
> 
> Signed-off-by: Felix Brack <fb@ltec.ch>

Remove this "Signed-off-by" tag as I neither made nor tested these
modifications. Furthermore it might not be good idea to name your
modifications to my patch "v3 ..." as this too could make people belief
I'm the originator or co-author of your modifications. What if I fix a
bug in my v2 patch? Should I then increase from v2 to v4?
Don't just 'hijack' a patch. Better add your comments, modification
requests, bug reports and explanations to the v2 thread of this patch as
I already suggested to you.

Felix

> [james at balean.com.au: changed .set_state_simple operation to .set_state]
> Signed-off-by: James Balean <james@balean.com.au>
> ---
>  drivers/pinctrl/Kconfig          |  10 +++
>  drivers/pinctrl/Makefile         |   1 +
>  drivers/pinctrl/pinctrl-single.c | 141 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 152 insertions(+)
>  create mode 100644 drivers/pinctrl/pinctrl-single.c
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index efcb4c0..a2b9212 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -175,6 +175,16 @@ config PIC32_PINCTRL
>  	  by a device tree node which contains both GPIO defintion and pin control
>  	  functions.
>  
> +config PINCTRL_SINGLE
> +	bool "Single register pin-control and pin-multiplex driver"
> +	depends on PINCTRL_FULL || SPL_PINCTRL_FULL
> +	help
> +	  This enables pinctrl driver for systems using a single register for
> +	  pin configuration and multiplexing. TI's AM335X SoCs are examples of
> +	  such systems.
> +	  Depending on the platform make sure to also enable OF_TRANSLATE and
> +	  eventually SPL_OF_TRANSLATE to get correct address translations.
> +
>  endif
>  
>  source "drivers/pinctrl/meson/Kconfig"
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index 512112a..f148f94 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_PIC32_PINCTRL)	+= pinctrl_pic32.o
>  obj-$(CONFIG_PINCTRL_EXYNOS)	+= exynos/
>  obj-$(CONFIG_PINCTRL_MESON)	+= meson/
>  obj-$(CONFIG_PINCTRL_MVEBU)	+= mvebu/
> +obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> new file mode 100644
> index 0000000..a5b4d75
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -0,0 +1,141 @@
> +/*
> + * Copyright (C) EETS GmbH, 2017, Felix Brack <f.brack@eets.ch>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm/device.h>
> +#include <dm/pinctrl.h>
> +#include <libfdt.h>
> +#include <asm/io.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct single_pdata {
> +	fdt_addr_t base;	/* first configuration register */
> +	int offset;		/* index of last configuration register */
> +	u32 mask;		/* configuration-value mask bits */
> +	int width;		/* configuration register bit width */
> +};
> +
> +struct single_fdt_pin_cfg {
> +	fdt32_t reg;		/* configuration register offset */
> +	fdt32_t val;		/* configuration register value */
> +};
> +
> +/**
> + * single_configure_pins() - Configure pins based on FDT data
> + *
> + * @dev: Pointer to single pin configuration device which is the parent of
> + *       the pins node holding the pin configuration data.
> + * @pins: Pointer to the first element of an array of register/value pairs
> + *        of type 'struct single_fdt_pin_cfg'. Each such pair describes the
> + *        the pin to be configured and the value to be used for configuration.
> + *        This pointer points to a 'pinctrl-single,pins' property in the
> + *        device-tree.
> + * @size: Size of the 'pins' array in bytes.
> + *        The number of register/value pairs in the 'pins' array therefore
> + *        equals to 'size / sizeof(struct single_fdt_pin_cfg)'.
> + */
> +static int single_configure_pins(struct udevice *dev,
> +				 const struct single_fdt_pin_cfg *pins,
> +				 int size)
> +{
> +	struct single_pdata *pdata = dev->platdata;
> +	int count = size / sizeof(struct single_fdt_pin_cfg);
> +	int n, reg;
> +	u32 val;
> +
> +	for (n = 0; n < count; n++) {
> +		reg = fdt32_to_cpu(pins->reg);
> +		if ((reg < 0) || (reg > pdata->offset)) {
> +			dev_dbg(dev, "  invalid register offset 0x%08x\n", reg);
> +			pins++;
> +			continue;
> +		}
> +		reg += pdata->base;
> +		switch (pdata->width) {
> +		case 32:
> +			val = readl(reg) & ~pdata->mask;
> +			val |= fdt32_to_cpu(pins->val) & pdata->mask;
> +			writel(val, reg);
> +			dev_dbg(dev, "  reg/val 0x%08x/0x%08x\n",
> +				reg, val);
> +			break;
> +		default:
> +			dev_warn(dev, "unsupported register width %i\n",
> +				 pdata->width);
> +		}
> +		pins++;
> +	}
> +	return 0;
> +}
> +
> +static int single_set_state(struct udevice *dev, struct udevice *config)
> +{
> +	const void *fdt = gd->fdt_blob;
> +	const struct single_fdt_pin_cfg *prop;
> +	int len;
> +
> +	prop = fdt_getprop(fdt, config->of_offset, "pinctrl-single,pins", &len);
> +	if (prop) {
> +		dev_dbg(dev, "configuring pins for %s\n", config->name);
> +		if (len % sizeof(struct single_fdt_pin_cfg)) {
> +			dev_dbg(dev, "  invalid pin configuration in fdt\n");
> +			return -FDT_ERR_BADSTRUCTURE;
> +		}
> +		single_configure_pins(dev, prop, len);
> +		len = 0;
> +	}
> +
> +	return len;
> +}
> +
> +static int single_ofdata_to_platdata(struct udevice *dev)
> +{
> +	fdt_addr_t addr;
> +	u32 of_reg[2];
> +	int res;
> +	struct single_pdata *pdata = dev->platdata;
> +
> +	pdata->width = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +				      "pinctrl-single,register-width", 0);
> +
> +	res = fdtdec_get_int_array(gd->fdt_blob, dev->of_offset,
> +				   "reg", of_reg, 2);
> +	if (res)
> +		return res;
> +	pdata->offset = of_reg[1] - pdata->width / 8;
> +
> +	addr = dev_get_addr(dev);
> +	if (addr == FDT_ADDR_T_NONE) {
> +		dev_dbg(dev, "no valid base register address\n");
> +		return -EINVAL;
> +	}
> +	pdata->base = addr;
> +
> +	pdata->mask = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +				     "pinctrl-single,function-mask",
> +				     0xffffffff);
> +	return 0;
> +}
> +
> +const struct pinctrl_ops single_pinctrl_ops = {
> +	.set_state = single_set_state,
> +};
> +
> +static const struct udevice_id single_pinctrl_match[] = {
> +	{ .compatible = "pinctrl-single" },
> +	{ /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(single_pinctrl) = {
> +	.name = "single-pinctrl",
> +	.id = UCLASS_PINCTRL,
> +	.of_match = single_pinctrl_match,
> +	.ops = &single_pinctrl_ops,
> +	.flags = DM_FLAG_PRE_RELOC,
> +	.platdata_auto_alloc_size = sizeof(struct single_pdata),
> +	.ofdata_to_platdata = single_ofdata_to_platdata,
> +};
> 

[U-Boot] [PATCH v3] Add single register pin controller driver

[James]

Hi Felix,

Not 'hijacking' a patch, just following Linux Developer's Certificate of
Origin 1.1 guidelines.

As you know, I contacted you directly with this patch suggestion prior to
posting to the mailing list (keeping strictly to rule c). I had hoped you
would make a new version:

> On 09.03.2017 08:53, James Balean wrote:
>> Did you want to test/submit this?

Perhaps I misinterpreted your response, which stated:

On Fri, 10 Mar 2017 at 04:13, Felix Brack <fb@ltec.ch> wrote:
> This and probably more has to be accessible by _all_ mailing list
subscribers. Please post there.

This was confusing, as I could see no way to convert this work to a patch
of your unapproved v2 patch (being new to this process).

On Sat, 11 Mar 2017 at 00:07, Felix Brack <fb@ltec.ch> wrote:
> Remove this "Signed-off-by" tag as I neither made nor tested these
modifications.

My understanding from the 'submitting patches' guide is that the square
bracket nomenclature I used indicates minor changes to an existing patch,
thereby providing you with credit whilst also denoting that you do not
endorse the changes.

> What if I fix a
bug in my v2 patch? Should I then increase from v2 to v4?

Perhaps someone can clarify, but it seems logical that the version number
is in order of contribution to the project, rather than being tied to any
specific user. Especially given the software license it is under.


Kind regards,
James Balean

[U-Boot] [PATCH v3] Add single register pin controller driver

[Simon Glass]

Hi,

On 11 March 2017 at 04:52, James <james@balean.com.au> wrote:
> Hi Felix,
>
> Not 'hijacking' a patch, just following Linux Developer's Certificate of
> Origin 1.1 guidelines.
>
> As you know, I contacted you directly with this patch suggestion prior to
> posting to the mailing list (keeping strictly to rule c). I had hoped you
> would make a new version:
>
>> On 09.03.2017 08:53, James Balean wrote:
>>> Did you want to test/submit this?
>
> Perhaps I misinterpreted your response, which stated:
>
> On Fri, 10 Mar 2017 at 04:13, Felix Brack <fb@ltec.ch> wrote:
>> This and probably more has to be accessible by _all_ mailing list
> subscribers. Please post there.
>
> This was confusing, as I could see no way to convert this work to a patch
> of your unapproved v2 patch (being new to this process).
>
> On Sat, 11 Mar 2017 at 00:07, Felix Brack <fb@ltec.ch> wrote:
>> Remove this "Signed-off-by" tag as I neither made nor tested these
> modifications.
>
> My understanding from the 'submitting patches' guide is that the square
> bracket nomenclature I used indicates minor changes to an existing patch,
> thereby providing you with credit whilst also denoting that you do not
> endorse the changes.
>
>> What if I fix a
> bug in my v2 patch? Should I then increase from v2 to v4?
>
> Perhaps someone can clarify, but it seems logical that the version number
> is in order of contribution to the project, rather than being tied to any
> specific user. Especially given the software license it is under.

It is better to make comments on the patch and let the original author
respin it. If you have not heard after a week then I suppose you can
resend it with the changes, i.w.c. I think you DO need to keep the
original author's sign-off and increment the version.

Felix, are you planning to resend this?

Regards,
Simon

[U-Boot] [PATCH v3] Add single register pin controller driver

[Felix Brack]

Hello Simon,

On 16.03.2017 23:52, Simon Glass wrote:
> Hi,
> 
> On 11 March 2017 at 04:52, James <james@balean.com.au> wrote:
>> Hi Felix,
>>
>> Not 'hijacking' a patch, just following Linux Developer's Certificate of
>> Origin 1.1 guidelines.
>>
>> As you know, I contacted you directly with this patch suggestion prior to
>> posting to the mailing list (keeping strictly to rule c). I had hoped you
>> would make a new version:
>>
>>> On 09.03.2017 08:53, James Balean wrote:
>>>> Did you want to test/submit this?
>>
>> Perhaps I misinterpreted your response, which stated:
>>
>> On Fri, 10 Mar 2017 at 04:13, Felix Brack <fb@ltec.ch> wrote:
>>> This and probably more has to be accessible by _all_ mailing list
>> subscribers. Please post there.
>>
>> This was confusing, as I could see no way to convert this work to a patch
>> of your unapproved v2 patch (being new to this process).
>>
>> On Sat, 11 Mar 2017 at 00:07, Felix Brack <fb@ltec.ch> wrote:
>>> Remove this "Signed-off-by" tag as I neither made nor tested these
>> modifications.
>>
>> My understanding from the 'submitting patches' guide is that the square
>> bracket nomenclature I used indicates minor changes to an existing patch,
>> thereby providing you with credit whilst also denoting that you do not
>> endorse the changes.
>>
>>> What if I fix a
>> bug in my v2 patch? Should I then increase from v2 to v4?
>>
>> Perhaps someone can clarify, but it seems logical that the version number
>> is in order of contribution to the project, rather than being tied to any
>> specific user. Especially given the software license it is under.
> 
> It is better to make comments on the patch and let the original author
> respin it. If you have not heard after a week then I suppose you can
> resend it with the changes, i.w.c. I think you DO need to keep the
> original author's sign-off and increment the version.
>



> Felix, are you planning to resend this?
>

Yes, I will try to do so next week.

Regards Felix

[U-Boot] [PATCH v3] Add single register pin controller driver

[Felix Brack]

Hi James,

On 11.03.2017 12:52, James wrote:
> Hi Felix,
> 
> Not 'hijacking' a patch, just following Linux Developer's Certificate of
> Origin 1.1 guidelines.
> 
> As you know, I contacted you directly with this patch suggestion prior
> to posting to the mailing list (keeping strictly to rule c). I had hoped
> you would make a new version:

See following remark please.

> 
>> On 09.03.2017 08:53, James Balean wrote:
>>> Did you want to test/submit this?
> 
> Perhaps I misinterpreted your response, which stated:
> 

My answer to the question above was: 'If required I will fix and test
things myself. I will then post the next version of the patch ASAP, for
a further review of course.' Please quote completely and correctly.

> On Fri, 10 Mar 2017 at 04:13, Felix Brack <fb@ltec.ch
> <mailto:fb@ltec.ch>> wrote:
>> This and probably more has to be accessible by _all_ mailing list
> subscribers. Please post there.
> 
> This was confusing, as I could see no way to convert this work to a
> patch of your unapproved v2 patch (being new to this process).
>

Okay, maybe that was a bad formulation, sorry, I will try to rephrase.
Consider this to be my point of view rather then a rule being carved in
stone.
The patch originator posts the patch (this starts a new thread). Now the
probably most important part of the process starts: everyone is invited
to comment on the patch _within the newly started thread_. This will
make sure that the thread started by posting the patch contains all
information that would probably lead to further versions of that patch.
Also this will make sure that everyone interested will be able to follow
the entire discussion. These comments (your's too, of course) are
extremely appreciated - the more eyes looking at a patch, the better.
Whenever possible comments should also contain concrete suggestions
about how to do better as well as a little explanation about why to do
so (imho). After a more or less extensive process of commenting and
resubmitting of the patch the responsible custodian may finally pick it
up to be added to the U-Boot repository.

> On Sat, 11 Mar 2017 at 00:07, Felix Brack <fb@ltec.ch
> <mailto:fb@ltec.ch>> wrote:
>> Remove this "Signed-off-by" tag as I neither made nor tested these
> modifications. 
> 
> My understanding from the 'submitting patches' guide is that the square
> bracket nomenclature I used indicates minor changes to an existing
> patch, thereby providing you with credit whilst also denoting that you
> do not endorse the changes.
> 
>> What if I fix a
> bug in my v2 patch? Should I then increase from v2 to v4?
> 
> Perhaps someone can clarify, but it seems logical that the version
> number is in order of contribution to the project, rather than being
> tied to any specific user. Especially given the software license it is
> under.
> 
> 
> Kind regards,
> James Balean
> 

Regards, Felix