[PATCH 0/3] pinctrl: ralink: pinctrl driver for the rt2880 family

[PATCH 0/3] pinctrl: ralink: pinctrl driver for the rt2880 family

[Sergio Paracuellos]

This series adds a pinctrl driver for ralink rt2880 SoC.

After last cleanup in staging I was told [0] this driver is ready to be
promoted from staging.

This series are rebased on the top of staging-testing.

Thanks in advance for your time.

Best regards,
    Sergio Paracuellos

[0]: http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2020-December/149178.html
Sergio Paracuellos (3):
  dt-bindings: pinctrl: rt2880: add binding document
  pinctrl: ralink: add a pinctrl driver for the rt2880 family
  staging: mt7621-pinctrl: remove driver from staging

 .../pinctrl/ralink,rt2880-pinmux.yaml         | 82 +++++++++++++++++++
 drivers/pinctrl/Kconfig                       |  6 ++
 drivers/pinctrl/Makefile                      |  1 +
 .../pinctrl-rt2880.c                          |  0
 drivers/staging/Kconfig                       |  2 -
 drivers/staging/Makefile                      |  1 -
 drivers/staging/mt7621-pinctrl/Kconfig        |  6 --
 drivers/staging/mt7621-pinctrl/Makefile       |  4 -
 drivers/staging/mt7621-pinctrl/TODO           |  6 --
 9 files changed, 89 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
 rename drivers/{staging/mt7621-pinctrl => pinctrl}/pinctrl-rt2880.c (100%)
 delete mode 100644 drivers/staging/mt7621-pinctrl/Kconfig
 delete mode 100644 drivers/staging/mt7621-pinctrl/Makefile
 delete mode 100644 drivers/staging/mt7621-pinctrl/TODO

-- 
2.25.1

[PATCH 0/3] pinctrl: ralink: pinctrl driver for the rt2880 family

[Sergio Paracuellos]

This series adds a pinctrl driver for ralink rt2880 SoC.

After last cleanup in staging I was told [0] this driver is ready to be
promoted from staging.

This series are rebased on the top of staging-testing.

Thanks in advance for your time.

Best regards,
    Sergio Paracuellos

[0]: http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2020-December/149178.html
Sergio Paracuellos (3):
  dt-bindings: pinctrl: rt2880: add binding document
  pinctrl: ralink: add a pinctrl driver for the rt2880 family
  staging: mt7621-pinctrl: remove driver from staging

 .../pinctrl/ralink,rt2880-pinmux.yaml         | 82 +++++++++++++++++++
 drivers/pinctrl/Kconfig                       |  6 ++
 drivers/pinctrl/Makefile                      |  1 +
 .../pinctrl-rt2880.c                          |  0
 drivers/staging/Kconfig                       |  2 -
 drivers/staging/Makefile                      |  1 -
 drivers/staging/mt7621-pinctrl/Kconfig        |  6 --
 drivers/staging/mt7621-pinctrl/Makefile       |  4 -
 drivers/staging/mt7621-pinctrl/TODO           |  6 --
 9 files changed, 89 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
 rename drivers/{staging/mt7621-pinctrl => pinctrl}/pinctrl-rt2880.c (100%)
 delete mode 100644 drivers/staging/mt7621-pinctrl/Kconfig
 delete mode 100644 drivers/staging/mt7621-pinctrl/Makefile
 delete mode 100644 drivers/staging/mt7621-pinctrl/TODO

-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document

[Sergio Paracuellos]

The commit adds rt2880 compatible node in binding document.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../pinctrl/ralink,rt2880-pinmux.yaml         | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml

diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
new file mode 100644
index 000000000000..d946219a115c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/ralink,rt2880-pinmux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ralink rt2880 pinmux controller
+
+maintainers:
+  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
+
+description:
+  The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual pins
+  is not supported. There is no pinconf support.
+
+properties:
+  compatible:
+    enum:
+      - ralink,rt2880-pinmux
+
+  pinctrl-0:
+    description:
+      A phandle to the node containing the subnodes containing default
+      configurations.
+
+  pinctrl-names:
+    description:
+      A pinctrl state named "default" must be defined.
+    const: default
+
+required:
+  - compatible
+  - pinctrl-names
+  - pinctrl-0
+
+patternProperties:
+  '^.*$':
+    if:
+      type: object
+      description: |
+        A pinctrl node should contain at least one subnodes representing the
+        pinctrl groups available on the machine.
+      $ref: "pinmux-node.yaml"
+      required:
+        - groups
+        - function
+      additionalProperties: false
+    then:
+      properties:
+        groups:
+          description:
+            Name of the pin group to use for the functions.
+          $ref: "/schemas/types.yaml#/definitions/string"
+          enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
+                 pcie, sdhci]
+        function:
+          description:
+            The mux function to select
+          $ref: "/schemas/types.yaml#/definitions/string"
+          enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
+                 mdio, nand1, nand2, sdhci]
+
+additionalProperties: false
+
+examples:
+  # Pinmux controller node
+  - |
+    pinctrl {
+      compatible = "ralink,rt2880-pinmux";
+      pinctrl-names = "default";
+      pinctrl-0 = <&state_default>;
+
+      state_default: pinctrl0 {
+      };
+
+      i2c_pins: i2c0 {
+        i2c0 {
+          groups = "i2c";
+          function = "i2c";
+        };
+      };
+    };
-- 
2.25.1

[PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document

[Sergio Paracuellos]

The commit adds rt2880 compatible node in binding document.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../pinctrl/ralink,rt2880-pinmux.yaml         | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml

diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
new file mode 100644
index 000000000000..d946219a115c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/ralink,rt2880-pinmux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ralink rt2880 pinmux controller
+
+maintainers:
+  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
+
+description:
+  The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual pins
+  is not supported. There is no pinconf support.
+
+properties:
+  compatible:
+    enum:
+      - ralink,rt2880-pinmux
+
+  pinctrl-0:
+    description:
+      A phandle to the node containing the subnodes containing default
+      configurations.
+
+  pinctrl-names:
+    description:
+      A pinctrl state named "default" must be defined.
+    const: default
+
+required:
+  - compatible
+  - pinctrl-names
+  - pinctrl-0
+
+patternProperties:
+  '^.*$':
+    if:
+      type: object
+      description: |
+        A pinctrl node should contain at least one subnodes representing the
+        pinctrl groups available on the machine.
+      $ref: "pinmux-node.yaml"
+      required:
+        - groups
+        - function
+      additionalProperties: false
+    then:
+      properties:
+        groups:
+          description:
+            Name of the pin group to use for the functions.
+          $ref: "/schemas/types.yaml#/definitions/string"
+          enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
+                 pcie, sdhci]
+        function:
+          description:
+            The mux function to select
+          $ref: "/schemas/types.yaml#/definitions/string"
+          enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
+                 mdio, nand1, nand2, sdhci]
+
+additionalProperties: false
+
+examples:
+  # Pinmux controller node
+  - |
+    pinctrl {
+      compatible = "ralink,rt2880-pinmux";
+      pinctrl-names = "default";
+      pinctrl-0 = <&state_default>;
+
+      state_default: pinctrl0 {
+      };
+
+      i2c_pins: i2c0 {
+        i2c0 {
+          groups = "i2c";
+          function = "i2c";
+        };
+      };
+    };
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Sergio Paracuellos]

These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
pin individually, these socs have mux groups that when set will effect 1-N pins.
Pin groups have a 2, 4 or 8 different muxes.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/pinctrl/Kconfig          |   6 +
 drivers/pinctrl/Makefile         |   1 +
 drivers/pinctrl/pinctrl-rt2880.c | 370 +++++++++++++++++++++++++++++++
 3 files changed, 377 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-rt2880.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 815095326e2d..e97970ec1210 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -215,6 +215,12 @@ config PINCTRL_ROCKCHIP
 	select MFD_SYSCON
 	select OF_GPIO
 
+config PINCTRL_RT2880
+        bool "RT2880 pinctrl driver for RALINK/Mediatek SOCs"
+        depends on RALINK
+        select PINMUX
+        select GENERIC_PINCONF
+
 config PINCTRL_SINGLE
 	tristate "One-register-per-pin type device tree based pinctrl driver"
 	depends on OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f53933b2ff02..0ebc24dad259 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_STMFX) 	+= pinctrl-stmfx.o
 obj-$(CONFIG_PINCTRL_ZYNQ)	+= pinctrl-zynq.o
 obj-$(CONFIG_PINCTRL_INGENIC)	+= pinctrl-ingenic.o
 obj-$(CONFIG_PINCTRL_RK805)	+= pinctrl-rk805.o
+obj-$(CONFIG_PINCTRL_RT2880)   += pinctrl-rt2880.o
 obj-$(CONFIG_PINCTRL_OCELOT)	+= pinctrl-ocelot.o
 obj-$(CONFIG_PINCTRL_EQUILIBRIUM)   += pinctrl-equilibrium.o
 
diff --git a/drivers/pinctrl/pinctrl-rt2880.c b/drivers/pinctrl/pinctrl-rt2880.c
new file mode 100644
index 000000000000..e61dbe186bc9
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-rt2880.c
@@ -0,0 +1,370 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2013 John Crispin <blogic@openwrt.org>
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/machine.h>
+
+#include <asm/mach-ralink/ralink_regs.h>
+#include <asm/mach-ralink/pinmux.h>
+#include <asm/mach-ralink/mt7620.h>
+
+#include "core.h"
+#include "pinctrl-utils.h"
+
+#define SYSC_REG_GPIO_MODE	0x60
+#define SYSC_REG_GPIO_MODE2	0x64
+
+struct rt2880_priv {
+	struct device *dev;
+
+	struct pinctrl_pin_desc *pads;
+	struct pinctrl_desc *desc;
+
+	struct rt2880_pmx_func **func;
+	int func_count;
+
+	struct rt2880_pmx_group *groups;
+	const char **group_names;
+	int group_count;
+
+	u8 *gpio;
+	int max_pins;
+};
+
+static int rt2880_get_group_count(struct pinctrl_dev *pctrldev)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	return p->group_count;
+}
+
+static const char *rt2880_get_group_name(struct pinctrl_dev *pctrldev,
+					 unsigned int group)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	return (group >= p->group_count) ? NULL : p->group_names[group];
+}
+
+static int rt2880_get_group_pins(struct pinctrl_dev *pctrldev,
+				 unsigned int group,
+				 const unsigned int **pins,
+				 unsigned int *num_pins)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	if (group >= p->group_count)
+		return -EINVAL;
+
+	*pins = p->groups[group].func[0].pins;
+	*num_pins = p->groups[group].func[0].pin_count;
+
+	return 0;
+}
+
+static const struct pinctrl_ops rt2880_pctrl_ops = {
+	.get_groups_count	= rt2880_get_group_count,
+	.get_group_name		= rt2880_get_group_name,
+	.get_group_pins		= rt2880_get_group_pins,
+	.dt_node_to_map		= pinconf_generic_dt_node_to_map_all,
+	.dt_free_map		= pinconf_generic_dt_free_map,
+};
+
+static int rt2880_pmx_func_count(struct pinctrl_dev *pctrldev)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	return p->func_count;
+}
+
+static const char *rt2880_pmx_func_name(struct pinctrl_dev *pctrldev,
+					unsigned int func)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	return p->func[func]->name;
+}
+
+static int rt2880_pmx_group_get_groups(struct pinctrl_dev *pctrldev,
+				       unsigned int func,
+				       const char * const **groups,
+				       unsigned int * const num_groups)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	if (p->func[func]->group_count == 1)
+		*groups = &p->group_names[p->func[func]->groups[0]];
+	else
+		*groups = p->group_names;
+
+	*num_groups = p->func[func]->group_count;
+
+	return 0;
+}
+
+static int rt2880_pmx_group_enable(struct pinctrl_dev *pctrldev,
+				   unsigned int func, unsigned int group)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+	u32 mode = 0;
+	u32 reg = SYSC_REG_GPIO_MODE;
+	int i;
+	int shift;
+
+	/* dont allow double use */
+	if (p->groups[group].enabled) {
+		dev_err(p->dev, "%s is already enabled\n",
+			p->groups[group].name);
+		return -EBUSY;
+	}
+
+	p->groups[group].enabled = 1;
+	p->func[func]->enabled = 1;
+
+	shift = p->groups[group].shift;
+	if (shift >= 32) {
+		shift -= 32;
+		reg = SYSC_REG_GPIO_MODE2;
+	}
+	mode = rt_sysc_r32(reg);
+	mode &= ~(p->groups[group].mask << shift);
+
+	/* mark the pins as gpio */
+	for (i = 0; i < p->groups[group].func[0].pin_count; i++)
+		p->gpio[p->groups[group].func[0].pins[i]] = 1;
+
+	/* function 0 is gpio and needs special handling */
+	if (func == 0) {
+		mode |= p->groups[group].gpio << shift;
+	} else {
+		for (i = 0; i < p->func[func]->pin_count; i++)
+			p->gpio[p->func[func]->pins[i]] = 0;
+		mode |= p->func[func]->value << shift;
+	}
+	rt_sysc_w32(mode, reg);
+
+	return 0;
+}
+
+static int rt2880_pmx_group_gpio_request_enable(struct pinctrl_dev *pctrldev,
+						struct pinctrl_gpio_range *range,
+						unsigned int pin)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	if (!p->gpio[pin]) {
+		dev_err(p->dev, "pin %d is not set to gpio mux\n", pin);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct pinmux_ops rt2880_pmx_group_ops = {
+	.get_functions_count	= rt2880_pmx_func_count,
+	.get_function_name	= rt2880_pmx_func_name,
+	.get_function_groups	= rt2880_pmx_group_get_groups,
+	.set_mux		= rt2880_pmx_group_enable,
+	.gpio_request_enable	= rt2880_pmx_group_gpio_request_enable,
+};
+
+static struct pinctrl_desc rt2880_pctrl_desc = {
+	.owner		= THIS_MODULE,
+	.name		= "rt2880-pinmux",
+	.pctlops	= &rt2880_pctrl_ops,
+	.pmxops		= &rt2880_pmx_group_ops,
+};
+
+static struct rt2880_pmx_func gpio_func = {
+	.name = "gpio",
+};
+
+static int rt2880_pinmux_index(struct rt2880_priv *p)
+{
+	struct rt2880_pmx_func **f;
+	struct rt2880_pmx_group *mux = p->groups;
+	int i, j, c = 0;
+
+	/* count the mux functions */
+	while (mux->name) {
+		p->group_count++;
+		mux++;
+	}
+
+	/* allocate the group names array needed by the gpio function */
+	p->group_names = devm_kcalloc(p->dev, p->group_count,
+				      sizeof(char *), GFP_KERNEL);
+	if (!p->group_names)
+		return -1;
+
+	for (i = 0; i < p->group_count; i++) {
+		p->group_names[i] = p->groups[i].name;
+		p->func_count += p->groups[i].func_count;
+	}
+
+	/* we have a dummy function[0] for gpio */
+	p->func_count++;
+
+	/* allocate our function and group mapping index buffers */
+	f = p->func = devm_kcalloc(p->dev,
+				   p->func_count,
+				   sizeof(*p->func),
+				   GFP_KERNEL);
+	gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
+					GFP_KERNEL);
+	if (!f || !gpio_func.groups)
+		return -1;
+
+	/* add a backpointer to the function so it knows its group */
+	gpio_func.group_count = p->group_count;
+	for (i = 0; i < gpio_func.group_count; i++)
+		gpio_func.groups[i] = i;
+
+	f[c] = &gpio_func;
+	c++;
+
+	/* add remaining functions */
+	for (i = 0; i < p->group_count; i++) {
+		for (j = 0; j < p->groups[i].func_count; j++) {
+			f[c] = &p->groups[i].func[j];
+			f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
+						    GFP_KERNEL);
+			f[c]->groups[0] = i;
+			f[c]->group_count = 1;
+			c++;
+		}
+	}
+	return 0;
+}
+
+static int rt2880_pinmux_pins(struct rt2880_priv *p)
+{
+	int i, j;
+
+	/*
+	 * loop over the functions and initialize the pins array.
+	 * also work out the highest pin used.
+	 */
+	for (i = 0; i < p->func_count; i++) {
+		int pin;
+
+		if (!p->func[i]->pin_count)
+			continue;
+
+		p->func[i]->pins = devm_kcalloc(p->dev,
+						p->func[i]->pin_count,
+						sizeof(int),
+						GFP_KERNEL);
+		for (j = 0; j < p->func[i]->pin_count; j++)
+			p->func[i]->pins[j] = p->func[i]->pin_first + j;
+
+		pin = p->func[i]->pin_first + p->func[i]->pin_count;
+		if (pin > p->max_pins)
+			p->max_pins = pin;
+	}
+
+	/* the buffer that tells us which pins are gpio */
+	p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
+	/* the pads needed to tell pinctrl about our pins */
+	p->pads = devm_kcalloc(p->dev, p->max_pins,
+			       sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
+	if (!p->pads || !p->gpio) {
+		dev_err(p->dev, "Failed to allocate gpio data\n");
+		return -ENOMEM;
+	}
+
+	memset(p->gpio, 1, sizeof(u8) * p->max_pins);
+	for (i = 0; i < p->func_count; i++) {
+		if (!p->func[i]->pin_count)
+			continue;
+
+		for (j = 0; j < p->func[i]->pin_count; j++)
+			p->gpio[p->func[i]->pins[j]] = 0;
+	}
+
+	/* pin 0 is always a gpio */
+	p->gpio[0] = 1;
+
+	/* set the pads */
+	for (i = 0; i < p->max_pins; i++) {
+		/* strlen("ioXY") + 1 = 5 */
+		char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
+
+		if (!name)
+			return -ENOMEM;
+		snprintf(name, 5, "io%d", i);
+		p->pads[i].number = i;
+		p->pads[i].name = name;
+	}
+	p->desc->pins = p->pads;
+	p->desc->npins = p->max_pins;
+
+	return 0;
+}
+
+static int rt2880_pinmux_probe(struct platform_device *pdev)
+{
+	struct rt2880_priv *p;
+	struct pinctrl_dev *dev;
+
+	if (!rt2880_pinmux_data)
+		return -ENOTSUPP;
+
+	/* setup the private data */
+	p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	p->dev = &pdev->dev;
+	p->desc = &rt2880_pctrl_desc;
+	p->groups = rt2880_pinmux_data;
+	platform_set_drvdata(pdev, p);
+
+	/* init the device */
+	if (rt2880_pinmux_index(p)) {
+		dev_err(&pdev->dev, "failed to load index\n");
+		return -EINVAL;
+	}
+	if (rt2880_pinmux_pins(p)) {
+		dev_err(&pdev->dev, "failed to load pins\n");
+		return -EINVAL;
+	}
+	dev = pinctrl_register(p->desc, &pdev->dev, p);
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+
+	return 0;
+}
+
+static const struct of_device_id rt2880_pinmux_match[] = {
+	{ .compatible = "ralink,rt2880-pinmux" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rt2880_pinmux_match);
+
+static struct platform_driver rt2880_pinmux_driver = {
+	.probe = rt2880_pinmux_probe,
+	.driver = {
+		.name = "rt2880-pinmux",
+		.of_match_table = rt2880_pinmux_match,
+	},
+};
+
+int __init rt2880_pinmux_init(void)
+{
+	return platform_driver_register(&rt2880_pinmux_driver);
+}
+
+core_initcall_sync(rt2880_pinmux_init);
-- 
2.25.1

[PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Sergio Paracuellos]

These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
pin individually, these socs have mux groups that when set will effect 1-N pins.
Pin groups have a 2, 4 or 8 different muxes.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/pinctrl/Kconfig          |   6 +
 drivers/pinctrl/Makefile         |   1 +
 drivers/pinctrl/pinctrl-rt2880.c | 370 +++++++++++++++++++++++++++++++
 3 files changed, 377 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-rt2880.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 815095326e2d..e97970ec1210 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -215,6 +215,12 @@ config PINCTRL_ROCKCHIP
 	select MFD_SYSCON
 	select OF_GPIO
 
+config PINCTRL_RT2880
+        bool "RT2880 pinctrl driver for RALINK/Mediatek SOCs"
+        depends on RALINK
+        select PINMUX
+        select GENERIC_PINCONF
+
 config PINCTRL_SINGLE
 	tristate "One-register-per-pin type device tree based pinctrl driver"
 	depends on OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f53933b2ff02..0ebc24dad259 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_STMFX) 	+= pinctrl-stmfx.o
 obj-$(CONFIG_PINCTRL_ZYNQ)	+= pinctrl-zynq.o
 obj-$(CONFIG_PINCTRL_INGENIC)	+= pinctrl-ingenic.o
 obj-$(CONFIG_PINCTRL_RK805)	+= pinctrl-rk805.o
+obj-$(CONFIG_PINCTRL_RT2880)   += pinctrl-rt2880.o
 obj-$(CONFIG_PINCTRL_OCELOT)	+= pinctrl-ocelot.o
 obj-$(CONFIG_PINCTRL_EQUILIBRIUM)   += pinctrl-equilibrium.o
 
diff --git a/drivers/pinctrl/pinctrl-rt2880.c b/drivers/pinctrl/pinctrl-rt2880.c
new file mode 100644
index 000000000000..e61dbe186bc9
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-rt2880.c
@@ -0,0 +1,370 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2013 John Crispin <blogic@openwrt.org>
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/machine.h>
+
+#include <asm/mach-ralink/ralink_regs.h>
+#include <asm/mach-ralink/pinmux.h>
+#include <asm/mach-ralink/mt7620.h>
+
+#include "core.h"
+#include "pinctrl-utils.h"
+
+#define SYSC_REG_GPIO_MODE	0x60
+#define SYSC_REG_GPIO_MODE2	0x64
+
+struct rt2880_priv {
+	struct device *dev;
+
+	struct pinctrl_pin_desc *pads;
+	struct pinctrl_desc *desc;
+
+	struct rt2880_pmx_func **func;
+	int func_count;
+
+	struct rt2880_pmx_group *groups;
+	const char **group_names;
+	int group_count;
+
+	u8 *gpio;
+	int max_pins;
+};
+
+static int rt2880_get_group_count(struct pinctrl_dev *pctrldev)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	return p->group_count;
+}
+
+static const char *rt2880_get_group_name(struct pinctrl_dev *pctrldev,
+					 unsigned int group)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	return (group >= p->group_count) ? NULL : p->group_names[group];
+}
+
+static int rt2880_get_group_pins(struct pinctrl_dev *pctrldev,
+				 unsigned int group,
+				 const unsigned int **pins,
+				 unsigned int *num_pins)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	if (group >= p->group_count)
+		return -EINVAL;
+
+	*pins = p->groups[group].func[0].pins;
+	*num_pins = p->groups[group].func[0].pin_count;
+
+	return 0;
+}
+
+static const struct pinctrl_ops rt2880_pctrl_ops = {
+	.get_groups_count	= rt2880_get_group_count,
+	.get_group_name		= rt2880_get_group_name,
+	.get_group_pins		= rt2880_get_group_pins,
+	.dt_node_to_map		= pinconf_generic_dt_node_to_map_all,
+	.dt_free_map		= pinconf_generic_dt_free_map,
+};
+
+static int rt2880_pmx_func_count(struct pinctrl_dev *pctrldev)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	return p->func_count;
+}
+
+static const char *rt2880_pmx_func_name(struct pinctrl_dev *pctrldev,
+					unsigned int func)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	return p->func[func]->name;
+}
+
+static int rt2880_pmx_group_get_groups(struct pinctrl_dev *pctrldev,
+				       unsigned int func,
+				       const char * const **groups,
+				       unsigned int * const num_groups)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	if (p->func[func]->group_count == 1)
+		*groups = &p->group_names[p->func[func]->groups[0]];
+	else
+		*groups = p->group_names;
+
+	*num_groups = p->func[func]->group_count;
+
+	return 0;
+}
+
+static int rt2880_pmx_group_enable(struct pinctrl_dev *pctrldev,
+				   unsigned int func, unsigned int group)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+	u32 mode = 0;
+	u32 reg = SYSC_REG_GPIO_MODE;
+	int i;
+	int shift;
+
+	/* dont allow double use */
+	if (p->groups[group].enabled) {
+		dev_err(p->dev, "%s is already enabled\n",
+			p->groups[group].name);
+		return -EBUSY;
+	}
+
+	p->groups[group].enabled = 1;
+	p->func[func]->enabled = 1;
+
+	shift = p->groups[group].shift;
+	if (shift >= 32) {
+		shift -= 32;
+		reg = SYSC_REG_GPIO_MODE2;
+	}
+	mode = rt_sysc_r32(reg);
+	mode &= ~(p->groups[group].mask << shift);
+
+	/* mark the pins as gpio */
+	for (i = 0; i < p->groups[group].func[0].pin_count; i++)
+		p->gpio[p->groups[group].func[0].pins[i]] = 1;
+
+	/* function 0 is gpio and needs special handling */
+	if (func == 0) {
+		mode |= p->groups[group].gpio << shift;
+	} else {
+		for (i = 0; i < p->func[func]->pin_count; i++)
+			p->gpio[p->func[func]->pins[i]] = 0;
+		mode |= p->func[func]->value << shift;
+	}
+	rt_sysc_w32(mode, reg);
+
+	return 0;
+}
+
+static int rt2880_pmx_group_gpio_request_enable(struct pinctrl_dev *pctrldev,
+						struct pinctrl_gpio_range *range,
+						unsigned int pin)
+{
+	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
+
+	if (!p->gpio[pin]) {
+		dev_err(p->dev, "pin %d is not set to gpio mux\n", pin);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct pinmux_ops rt2880_pmx_group_ops = {
+	.get_functions_count	= rt2880_pmx_func_count,
+	.get_function_name	= rt2880_pmx_func_name,
+	.get_function_groups	= rt2880_pmx_group_get_groups,
+	.set_mux		= rt2880_pmx_group_enable,
+	.gpio_request_enable	= rt2880_pmx_group_gpio_request_enable,
+};
+
+static struct pinctrl_desc rt2880_pctrl_desc = {
+	.owner		= THIS_MODULE,
+	.name		= "rt2880-pinmux",
+	.pctlops	= &rt2880_pctrl_ops,
+	.pmxops		= &rt2880_pmx_group_ops,
+};
+
+static struct rt2880_pmx_func gpio_func = {
+	.name = "gpio",
+};
+
+static int rt2880_pinmux_index(struct rt2880_priv *p)
+{
+	struct rt2880_pmx_func **f;
+	struct rt2880_pmx_group *mux = p->groups;
+	int i, j, c = 0;
+
+	/* count the mux functions */
+	while (mux->name) {
+		p->group_count++;
+		mux++;
+	}
+
+	/* allocate the group names array needed by the gpio function */
+	p->group_names = devm_kcalloc(p->dev, p->group_count,
+				      sizeof(char *), GFP_KERNEL);
+	if (!p->group_names)
+		return -1;
+
+	for (i = 0; i < p->group_count; i++) {
+		p->group_names[i] = p->groups[i].name;
+		p->func_count += p->groups[i].func_count;
+	}
+
+	/* we have a dummy function[0] for gpio */
+	p->func_count++;
+
+	/* allocate our function and group mapping index buffers */
+	f = p->func = devm_kcalloc(p->dev,
+				   p->func_count,
+				   sizeof(*p->func),
+				   GFP_KERNEL);
+	gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
+					GFP_KERNEL);
+	if (!f || !gpio_func.groups)
+		return -1;
+
+	/* add a backpointer to the function so it knows its group */
+	gpio_func.group_count = p->group_count;
+	for (i = 0; i < gpio_func.group_count; i++)
+		gpio_func.groups[i] = i;
+
+	f[c] = &gpio_func;
+	c++;
+
+	/* add remaining functions */
+	for (i = 0; i < p->group_count; i++) {
+		for (j = 0; j < p->groups[i].func_count; j++) {
+			f[c] = &p->groups[i].func[j];
+			f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
+						    GFP_KERNEL);
+			f[c]->groups[0] = i;
+			f[c]->group_count = 1;
+			c++;
+		}
+	}
+	return 0;
+}
+
+static int rt2880_pinmux_pins(struct rt2880_priv *p)
+{
+	int i, j;
+
+	/*
+	 * loop over the functions and initialize the pins array.
+	 * also work out the highest pin used.
+	 */
+	for (i = 0; i < p->func_count; i++) {
+		int pin;
+
+		if (!p->func[i]->pin_count)
+			continue;
+
+		p->func[i]->pins = devm_kcalloc(p->dev,
+						p->func[i]->pin_count,
+						sizeof(int),
+						GFP_KERNEL);
+		for (j = 0; j < p->func[i]->pin_count; j++)
+			p->func[i]->pins[j] = p->func[i]->pin_first + j;
+
+		pin = p->func[i]->pin_first + p->func[i]->pin_count;
+		if (pin > p->max_pins)
+			p->max_pins = pin;
+	}
+
+	/* the buffer that tells us which pins are gpio */
+	p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
+	/* the pads needed to tell pinctrl about our pins */
+	p->pads = devm_kcalloc(p->dev, p->max_pins,
+			       sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
+	if (!p->pads || !p->gpio) {
+		dev_err(p->dev, "Failed to allocate gpio data\n");
+		return -ENOMEM;
+	}
+
+	memset(p->gpio, 1, sizeof(u8) * p->max_pins);
+	for (i = 0; i < p->func_count; i++) {
+		if (!p->func[i]->pin_count)
+			continue;
+
+		for (j = 0; j < p->func[i]->pin_count; j++)
+			p->gpio[p->func[i]->pins[j]] = 0;
+	}
+
+	/* pin 0 is always a gpio */
+	p->gpio[0] = 1;
+
+	/* set the pads */
+	for (i = 0; i < p->max_pins; i++) {
+		/* strlen("ioXY") + 1 = 5 */
+		char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
+
+		if (!name)
+			return -ENOMEM;
+		snprintf(name, 5, "io%d", i);
+		p->pads[i].number = i;
+		p->pads[i].name = name;
+	}
+	p->desc->pins = p->pads;
+	p->desc->npins = p->max_pins;
+
+	return 0;
+}
+
+static int rt2880_pinmux_probe(struct platform_device *pdev)
+{
+	struct rt2880_priv *p;
+	struct pinctrl_dev *dev;
+
+	if (!rt2880_pinmux_data)
+		return -ENOTSUPP;
+
+	/* setup the private data */
+	p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	p->dev = &pdev->dev;
+	p->desc = &rt2880_pctrl_desc;
+	p->groups = rt2880_pinmux_data;
+	platform_set_drvdata(pdev, p);
+
+	/* init the device */
+	if (rt2880_pinmux_index(p)) {
+		dev_err(&pdev->dev, "failed to load index\n");
+		return -EINVAL;
+	}
+	if (rt2880_pinmux_pins(p)) {
+		dev_err(&pdev->dev, "failed to load pins\n");
+		return -EINVAL;
+	}
+	dev = pinctrl_register(p->desc, &pdev->dev, p);
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+
+	return 0;
+}
+
+static const struct of_device_id rt2880_pinmux_match[] = {
+	{ .compatible = "ralink,rt2880-pinmux" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rt2880_pinmux_match);
+
+static struct platform_driver rt2880_pinmux_driver = {
+	.probe = rt2880_pinmux_probe,
+	.driver = {
+		.name = "rt2880-pinmux",
+		.of_match_table = rt2880_pinmux_match,
+	},
+};
+
+int __init rt2880_pinmux_init(void)
+{
+	return platform_driver_register(&rt2880_pinmux_driver);
+}
+
+core_initcall_sync(rt2880_pinmux_init);
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] staging: mt7621-pinctrl: remove driver from staging

[Sergio Paracuellos]

Remove this driver from staging because it has been moved
into its properly place in the kernel.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/Kconfig                       |   2 -
 drivers/staging/Makefile                      |   1 -
 drivers/staging/mt7621-pinctrl/Kconfig        |   6 -
 drivers/staging/mt7621-pinctrl/Makefile       |   4 -
 drivers/staging/mt7621-pinctrl/TODO           |   6 -
 .../staging/mt7621-pinctrl/pinctrl-rt2880.c   | 370 ------------------
 6 files changed, 389 deletions(-)
 delete mode 100644 drivers/staging/mt7621-pinctrl/Kconfig
 delete mode 100644 drivers/staging/mt7621-pinctrl/Makefile
 delete mode 100644 drivers/staging/mt7621-pinctrl/TODO
 delete mode 100644 drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 9b7cb7c5766a..c42708e60afc 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -94,8 +94,6 @@ source "drivers/staging/mt7621-pci/Kconfig"
 
 source "drivers/staging/mt7621-pci-phy/Kconfig"
 
-source "drivers/staging/mt7621-pinctrl/Kconfig"
-
 source "drivers/staging/mt7621-dma/Kconfig"
 
 source "drivers/staging/ralink-gdma/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 38226737c9f3..ebcc646d7b51 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -37,7 +37,6 @@ obj-$(CONFIG_BCM2835_VCHIQ)	+= vc04_services/
 obj-$(CONFIG_PI433)		+= pi433/
 obj-$(CONFIG_PCI_MT7621)	+= mt7621-pci/
 obj-$(CONFIG_PCI_MT7621_PHY)	+= mt7621-pci-phy/
-obj-$(CONFIG_PINCTRL_RT2880)	+= mt7621-pinctrl/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-dma/
 obj-$(CONFIG_DMA_RALINK)	+= ralink-gdma/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-dts/
diff --git a/drivers/staging/mt7621-pinctrl/Kconfig b/drivers/staging/mt7621-pinctrl/Kconfig
deleted file mode 100644
index f42974026480..000000000000
--- a/drivers/staging/mt7621-pinctrl/Kconfig
+++ /dev/null
@@ -1,6 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-config PINCTRL_RT2880
-	bool "RT2800 pinctrl driver for RALINK/Mediatek SOCs"
-	depends on RALINK
-	select PINMUX
-	select GENERIC_PINCONF
diff --git a/drivers/staging/mt7621-pinctrl/Makefile b/drivers/staging/mt7621-pinctrl/Makefile
deleted file mode 100644
index 49445f40c3cd..000000000000
--- a/drivers/staging/mt7621-pinctrl/Makefile
+++ /dev/null
@@ -1,4 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_PINCTRL_RT2880)   += pinctrl-rt2880.o
-
-ccflags-y += -I$(srctree)/drivers/pinctrl
diff --git a/drivers/staging/mt7621-pinctrl/TODO b/drivers/staging/mt7621-pinctrl/TODO
deleted file mode 100644
index b2c235a16d5c..000000000000
--- a/drivers/staging/mt7621-pinctrl/TODO
+++ /dev/null
@@ -1,6 +0,0 @@
-
-- general code review and cleanup
-- should probably be always selected by 'config RALINK'
-- ensure device-tree requirements are documented
-
-Cc: NeilBrown <neil@brown.name>
diff --git a/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c b/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
deleted file mode 100644
index e61dbe186bc9..000000000000
--- a/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
+++ /dev/null
@@ -1,370 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- *  Copyright (C) 2013 John Crispin <blogic@openwrt.org>
- */
-
-#include <linux/module.h>
-#include <linux/device.h>
-#include <linux/io.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
-#include <linux/of.h>
-#include <linux/pinctrl/pinctrl.h>
-#include <linux/pinctrl/pinconf.h>
-#include <linux/pinctrl/pinconf-generic.h>
-#include <linux/pinctrl/pinmux.h>
-#include <linux/pinctrl/consumer.h>
-#include <linux/pinctrl/machine.h>
-
-#include <asm/mach-ralink/ralink_regs.h>
-#include <asm/mach-ralink/pinmux.h>
-#include <asm/mach-ralink/mt7620.h>
-
-#include "core.h"
-#include "pinctrl-utils.h"
-
-#define SYSC_REG_GPIO_MODE	0x60
-#define SYSC_REG_GPIO_MODE2	0x64
-
-struct rt2880_priv {
-	struct device *dev;
-
-	struct pinctrl_pin_desc *pads;
-	struct pinctrl_desc *desc;
-
-	struct rt2880_pmx_func **func;
-	int func_count;
-
-	struct rt2880_pmx_group *groups;
-	const char **group_names;
-	int group_count;
-
-	u8 *gpio;
-	int max_pins;
-};
-
-static int rt2880_get_group_count(struct pinctrl_dev *pctrldev)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	return p->group_count;
-}
-
-static const char *rt2880_get_group_name(struct pinctrl_dev *pctrldev,
-					 unsigned int group)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	return (group >= p->group_count) ? NULL : p->group_names[group];
-}
-
-static int rt2880_get_group_pins(struct pinctrl_dev *pctrldev,
-				 unsigned int group,
-				 const unsigned int **pins,
-				 unsigned int *num_pins)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	if (group >= p->group_count)
-		return -EINVAL;
-
-	*pins = p->groups[group].func[0].pins;
-	*num_pins = p->groups[group].func[0].pin_count;
-
-	return 0;
-}
-
-static const struct pinctrl_ops rt2880_pctrl_ops = {
-	.get_groups_count	= rt2880_get_group_count,
-	.get_group_name		= rt2880_get_group_name,
-	.get_group_pins		= rt2880_get_group_pins,
-	.dt_node_to_map		= pinconf_generic_dt_node_to_map_all,
-	.dt_free_map		= pinconf_generic_dt_free_map,
-};
-
-static int rt2880_pmx_func_count(struct pinctrl_dev *pctrldev)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	return p->func_count;
-}
-
-static const char *rt2880_pmx_func_name(struct pinctrl_dev *pctrldev,
-					unsigned int func)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	return p->func[func]->name;
-}
-
-static int rt2880_pmx_group_get_groups(struct pinctrl_dev *pctrldev,
-				       unsigned int func,
-				       const char * const **groups,
-				       unsigned int * const num_groups)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	if (p->func[func]->group_count == 1)
-		*groups = &p->group_names[p->func[func]->groups[0]];
-	else
-		*groups = p->group_names;
-
-	*num_groups = p->func[func]->group_count;
-
-	return 0;
-}
-
-static int rt2880_pmx_group_enable(struct pinctrl_dev *pctrldev,
-				   unsigned int func, unsigned int group)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-	u32 mode = 0;
-	u32 reg = SYSC_REG_GPIO_MODE;
-	int i;
-	int shift;
-
-	/* dont allow double use */
-	if (p->groups[group].enabled) {
-		dev_err(p->dev, "%s is already enabled\n",
-			p->groups[group].name);
-		return -EBUSY;
-	}
-
-	p->groups[group].enabled = 1;
-	p->func[func]->enabled = 1;
-
-	shift = p->groups[group].shift;
-	if (shift >= 32) {
-		shift -= 32;
-		reg = SYSC_REG_GPIO_MODE2;
-	}
-	mode = rt_sysc_r32(reg);
-	mode &= ~(p->groups[group].mask << shift);
-
-	/* mark the pins as gpio */
-	for (i = 0; i < p->groups[group].func[0].pin_count; i++)
-		p->gpio[p->groups[group].func[0].pins[i]] = 1;
-
-	/* function 0 is gpio and needs special handling */
-	if (func == 0) {
-		mode |= p->groups[group].gpio << shift;
-	} else {
-		for (i = 0; i < p->func[func]->pin_count; i++)
-			p->gpio[p->func[func]->pins[i]] = 0;
-		mode |= p->func[func]->value << shift;
-	}
-	rt_sysc_w32(mode, reg);
-
-	return 0;
-}
-
-static int rt2880_pmx_group_gpio_request_enable(struct pinctrl_dev *pctrldev,
-						struct pinctrl_gpio_range *range,
-						unsigned int pin)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	if (!p->gpio[pin]) {
-		dev_err(p->dev, "pin %d is not set to gpio mux\n", pin);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static const struct pinmux_ops rt2880_pmx_group_ops = {
-	.get_functions_count	= rt2880_pmx_func_count,
-	.get_function_name	= rt2880_pmx_func_name,
-	.get_function_groups	= rt2880_pmx_group_get_groups,
-	.set_mux		= rt2880_pmx_group_enable,
-	.gpio_request_enable	= rt2880_pmx_group_gpio_request_enable,
-};
-
-static struct pinctrl_desc rt2880_pctrl_desc = {
-	.owner		= THIS_MODULE,
-	.name		= "rt2880-pinmux",
-	.pctlops	= &rt2880_pctrl_ops,
-	.pmxops		= &rt2880_pmx_group_ops,
-};
-
-static struct rt2880_pmx_func gpio_func = {
-	.name = "gpio",
-};
-
-static int rt2880_pinmux_index(struct rt2880_priv *p)
-{
-	struct rt2880_pmx_func **f;
-	struct rt2880_pmx_group *mux = p->groups;
-	int i, j, c = 0;
-
-	/* count the mux functions */
-	while (mux->name) {
-		p->group_count++;
-		mux++;
-	}
-
-	/* allocate the group names array needed by the gpio function */
-	p->group_names = devm_kcalloc(p->dev, p->group_count,
-				      sizeof(char *), GFP_KERNEL);
-	if (!p->group_names)
-		return -1;
-
-	for (i = 0; i < p->group_count; i++) {
-		p->group_names[i] = p->groups[i].name;
-		p->func_count += p->groups[i].func_count;
-	}
-
-	/* we have a dummy function[0] for gpio */
-	p->func_count++;
-
-	/* allocate our function and group mapping index buffers */
-	f = p->func = devm_kcalloc(p->dev,
-				   p->func_count,
-				   sizeof(*p->func),
-				   GFP_KERNEL);
-	gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
-					GFP_KERNEL);
-	if (!f || !gpio_func.groups)
-		return -1;
-
-	/* add a backpointer to the function so it knows its group */
-	gpio_func.group_count = p->group_count;
-	for (i = 0; i < gpio_func.group_count; i++)
-		gpio_func.groups[i] = i;
-
-	f[c] = &gpio_func;
-	c++;
-
-	/* add remaining functions */
-	for (i = 0; i < p->group_count; i++) {
-		for (j = 0; j < p->groups[i].func_count; j++) {
-			f[c] = &p->groups[i].func[j];
-			f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
-						    GFP_KERNEL);
-			f[c]->groups[0] = i;
-			f[c]->group_count = 1;
-			c++;
-		}
-	}
-	return 0;
-}
-
-static int rt2880_pinmux_pins(struct rt2880_priv *p)
-{
-	int i, j;
-
-	/*
-	 * loop over the functions and initialize the pins array.
-	 * also work out the highest pin used.
-	 */
-	for (i = 0; i < p->func_count; i++) {
-		int pin;
-
-		if (!p->func[i]->pin_count)
-			continue;
-
-		p->func[i]->pins = devm_kcalloc(p->dev,
-						p->func[i]->pin_count,
-						sizeof(int),
-						GFP_KERNEL);
-		for (j = 0; j < p->func[i]->pin_count; j++)
-			p->func[i]->pins[j] = p->func[i]->pin_first + j;
-
-		pin = p->func[i]->pin_first + p->func[i]->pin_count;
-		if (pin > p->max_pins)
-			p->max_pins = pin;
-	}
-
-	/* the buffer that tells us which pins are gpio */
-	p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
-	/* the pads needed to tell pinctrl about our pins */
-	p->pads = devm_kcalloc(p->dev, p->max_pins,
-			       sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
-	if (!p->pads || !p->gpio) {
-		dev_err(p->dev, "Failed to allocate gpio data\n");
-		return -ENOMEM;
-	}
-
-	memset(p->gpio, 1, sizeof(u8) * p->max_pins);
-	for (i = 0; i < p->func_count; i++) {
-		if (!p->func[i]->pin_count)
-			continue;
-
-		for (j = 0; j < p->func[i]->pin_count; j++)
-			p->gpio[p->func[i]->pins[j]] = 0;
-	}
-
-	/* pin 0 is always a gpio */
-	p->gpio[0] = 1;
-
-	/* set the pads */
-	for (i = 0; i < p->max_pins; i++) {
-		/* strlen("ioXY") + 1 = 5 */
-		char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
-
-		if (!name)
-			return -ENOMEM;
-		snprintf(name, 5, "io%d", i);
-		p->pads[i].number = i;
-		p->pads[i].name = name;
-	}
-	p->desc->pins = p->pads;
-	p->desc->npins = p->max_pins;
-
-	return 0;
-}
-
-static int rt2880_pinmux_probe(struct platform_device *pdev)
-{
-	struct rt2880_priv *p;
-	struct pinctrl_dev *dev;
-
-	if (!rt2880_pinmux_data)
-		return -ENOTSUPP;
-
-	/* setup the private data */
-	p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
-	if (!p)
-		return -ENOMEM;
-
-	p->dev = &pdev->dev;
-	p->desc = &rt2880_pctrl_desc;
-	p->groups = rt2880_pinmux_data;
-	platform_set_drvdata(pdev, p);
-
-	/* init the device */
-	if (rt2880_pinmux_index(p)) {
-		dev_err(&pdev->dev, "failed to load index\n");
-		return -EINVAL;
-	}
-	if (rt2880_pinmux_pins(p)) {
-		dev_err(&pdev->dev, "failed to load pins\n");
-		return -EINVAL;
-	}
-	dev = pinctrl_register(p->desc, &pdev->dev, p);
-	if (IS_ERR(dev))
-		return PTR_ERR(dev);
-
-	return 0;
-}
-
-static const struct of_device_id rt2880_pinmux_match[] = {
-	{ .compatible = "ralink,rt2880-pinmux" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, rt2880_pinmux_match);
-
-static struct platform_driver rt2880_pinmux_driver = {
-	.probe = rt2880_pinmux_probe,
-	.driver = {
-		.name = "rt2880-pinmux",
-		.of_match_table = rt2880_pinmux_match,
-	},
-};
-
-int __init rt2880_pinmux_init(void)
-{
-	return platform_driver_register(&rt2880_pinmux_driver);
-}
-
-core_initcall_sync(rt2880_pinmux_init);
-- 
2.25.1

[PATCH 3/3] staging: mt7621-pinctrl: remove driver from staging

[Sergio Paracuellos]

Remove this driver from staging because it has been moved
into its properly place in the kernel.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/Kconfig                       |   2 -
 drivers/staging/Makefile                      |   1 -
 drivers/staging/mt7621-pinctrl/Kconfig        |   6 -
 drivers/staging/mt7621-pinctrl/Makefile       |   4 -
 drivers/staging/mt7621-pinctrl/TODO           |   6 -
 .../staging/mt7621-pinctrl/pinctrl-rt2880.c   | 370 ------------------
 6 files changed, 389 deletions(-)
 delete mode 100644 drivers/staging/mt7621-pinctrl/Kconfig
 delete mode 100644 drivers/staging/mt7621-pinctrl/Makefile
 delete mode 100644 drivers/staging/mt7621-pinctrl/TODO
 delete mode 100644 drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 9b7cb7c5766a..c42708e60afc 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -94,8 +94,6 @@ source "drivers/staging/mt7621-pci/Kconfig"
 
 source "drivers/staging/mt7621-pci-phy/Kconfig"
 
-source "drivers/staging/mt7621-pinctrl/Kconfig"
-
 source "drivers/staging/mt7621-dma/Kconfig"
 
 source "drivers/staging/ralink-gdma/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 38226737c9f3..ebcc646d7b51 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -37,7 +37,6 @@ obj-$(CONFIG_BCM2835_VCHIQ)	+= vc04_services/
 obj-$(CONFIG_PI433)		+= pi433/
 obj-$(CONFIG_PCI_MT7621)	+= mt7621-pci/
 obj-$(CONFIG_PCI_MT7621_PHY)	+= mt7621-pci-phy/
-obj-$(CONFIG_PINCTRL_RT2880)	+= mt7621-pinctrl/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-dma/
 obj-$(CONFIG_DMA_RALINK)	+= ralink-gdma/
 obj-$(CONFIG_SOC_MT7621)	+= mt7621-dts/
diff --git a/drivers/staging/mt7621-pinctrl/Kconfig b/drivers/staging/mt7621-pinctrl/Kconfig
deleted file mode 100644
index f42974026480..000000000000
--- a/drivers/staging/mt7621-pinctrl/Kconfig
+++ /dev/null
@@ -1,6 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-config PINCTRL_RT2880
-	bool "RT2800 pinctrl driver for RALINK/Mediatek SOCs"
-	depends on RALINK
-	select PINMUX
-	select GENERIC_PINCONF
diff --git a/drivers/staging/mt7621-pinctrl/Makefile b/drivers/staging/mt7621-pinctrl/Makefile
deleted file mode 100644
index 49445f40c3cd..000000000000
--- a/drivers/staging/mt7621-pinctrl/Makefile
+++ /dev/null
@@ -1,4 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_PINCTRL_RT2880)   += pinctrl-rt2880.o
-
-ccflags-y += -I$(srctree)/drivers/pinctrl
diff --git a/drivers/staging/mt7621-pinctrl/TODO b/drivers/staging/mt7621-pinctrl/TODO
deleted file mode 100644
index b2c235a16d5c..000000000000
--- a/drivers/staging/mt7621-pinctrl/TODO
+++ /dev/null
@@ -1,6 +0,0 @@
-
-- general code review and cleanup
-- should probably be always selected by 'config RALINK'
-- ensure device-tree requirements are documented
-
-Cc: NeilBrown <neil@brown.name>
diff --git a/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c b/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
deleted file mode 100644
index e61dbe186bc9..000000000000
--- a/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
+++ /dev/null
@@ -1,370 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- *  Copyright (C) 2013 John Crispin <blogic@openwrt.org>
- */
-
-#include <linux/module.h>
-#include <linux/device.h>
-#include <linux/io.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
-#include <linux/of.h>
-#include <linux/pinctrl/pinctrl.h>
-#include <linux/pinctrl/pinconf.h>
-#include <linux/pinctrl/pinconf-generic.h>
-#include <linux/pinctrl/pinmux.h>
-#include <linux/pinctrl/consumer.h>
-#include <linux/pinctrl/machine.h>
-
-#include <asm/mach-ralink/ralink_regs.h>
-#include <asm/mach-ralink/pinmux.h>
-#include <asm/mach-ralink/mt7620.h>
-
-#include "core.h"
-#include "pinctrl-utils.h"
-
-#define SYSC_REG_GPIO_MODE	0x60
-#define SYSC_REG_GPIO_MODE2	0x64
-
-struct rt2880_priv {
-	struct device *dev;
-
-	struct pinctrl_pin_desc *pads;
-	struct pinctrl_desc *desc;
-
-	struct rt2880_pmx_func **func;
-	int func_count;
-
-	struct rt2880_pmx_group *groups;
-	const char **group_names;
-	int group_count;
-
-	u8 *gpio;
-	int max_pins;
-};
-
-static int rt2880_get_group_count(struct pinctrl_dev *pctrldev)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	return p->group_count;
-}
-
-static const char *rt2880_get_group_name(struct pinctrl_dev *pctrldev,
-					 unsigned int group)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	return (group >= p->group_count) ? NULL : p->group_names[group];
-}
-
-static int rt2880_get_group_pins(struct pinctrl_dev *pctrldev,
-				 unsigned int group,
-				 const unsigned int **pins,
-				 unsigned int *num_pins)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	if (group >= p->group_count)
-		return -EINVAL;
-
-	*pins = p->groups[group].func[0].pins;
-	*num_pins = p->groups[group].func[0].pin_count;
-
-	return 0;
-}
-
-static const struct pinctrl_ops rt2880_pctrl_ops = {
-	.get_groups_count	= rt2880_get_group_count,
-	.get_group_name		= rt2880_get_group_name,
-	.get_group_pins		= rt2880_get_group_pins,
-	.dt_node_to_map		= pinconf_generic_dt_node_to_map_all,
-	.dt_free_map		= pinconf_generic_dt_free_map,
-};
-
-static int rt2880_pmx_func_count(struct pinctrl_dev *pctrldev)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	return p->func_count;
-}
-
-static const char *rt2880_pmx_func_name(struct pinctrl_dev *pctrldev,
-					unsigned int func)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	return p->func[func]->name;
-}
-
-static int rt2880_pmx_group_get_groups(struct pinctrl_dev *pctrldev,
-				       unsigned int func,
-				       const char * const **groups,
-				       unsigned int * const num_groups)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	if (p->func[func]->group_count == 1)
-		*groups = &p->group_names[p->func[func]->groups[0]];
-	else
-		*groups = p->group_names;
-
-	*num_groups = p->func[func]->group_count;
-
-	return 0;
-}
-
-static int rt2880_pmx_group_enable(struct pinctrl_dev *pctrldev,
-				   unsigned int func, unsigned int group)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-	u32 mode = 0;
-	u32 reg = SYSC_REG_GPIO_MODE;
-	int i;
-	int shift;
-
-	/* dont allow double use */
-	if (p->groups[group].enabled) {
-		dev_err(p->dev, "%s is already enabled\n",
-			p->groups[group].name);
-		return -EBUSY;
-	}
-
-	p->groups[group].enabled = 1;
-	p->func[func]->enabled = 1;
-
-	shift = p->groups[group].shift;
-	if (shift >= 32) {
-		shift -= 32;
-		reg = SYSC_REG_GPIO_MODE2;
-	}
-	mode = rt_sysc_r32(reg);
-	mode &= ~(p->groups[group].mask << shift);
-
-	/* mark the pins as gpio */
-	for (i = 0; i < p->groups[group].func[0].pin_count; i++)
-		p->gpio[p->groups[group].func[0].pins[i]] = 1;
-
-	/* function 0 is gpio and needs special handling */
-	if (func == 0) {
-		mode |= p->groups[group].gpio << shift;
-	} else {
-		for (i = 0; i < p->func[func]->pin_count; i++)
-			p->gpio[p->func[func]->pins[i]] = 0;
-		mode |= p->func[func]->value << shift;
-	}
-	rt_sysc_w32(mode, reg);
-
-	return 0;
-}
-
-static int rt2880_pmx_group_gpio_request_enable(struct pinctrl_dev *pctrldev,
-						struct pinctrl_gpio_range *range,
-						unsigned int pin)
-{
-	struct rt2880_priv *p = pinctrl_dev_get_drvdata(pctrldev);
-
-	if (!p->gpio[pin]) {
-		dev_err(p->dev, "pin %d is not set to gpio mux\n", pin);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static const struct pinmux_ops rt2880_pmx_group_ops = {
-	.get_functions_count	= rt2880_pmx_func_count,
-	.get_function_name	= rt2880_pmx_func_name,
-	.get_function_groups	= rt2880_pmx_group_get_groups,
-	.set_mux		= rt2880_pmx_group_enable,
-	.gpio_request_enable	= rt2880_pmx_group_gpio_request_enable,
-};
-
-static struct pinctrl_desc rt2880_pctrl_desc = {
-	.owner		= THIS_MODULE,
-	.name		= "rt2880-pinmux",
-	.pctlops	= &rt2880_pctrl_ops,
-	.pmxops		= &rt2880_pmx_group_ops,
-};
-
-static struct rt2880_pmx_func gpio_func = {
-	.name = "gpio",
-};
-
-static int rt2880_pinmux_index(struct rt2880_priv *p)
-{
-	struct rt2880_pmx_func **f;
-	struct rt2880_pmx_group *mux = p->groups;
-	int i, j, c = 0;
-
-	/* count the mux functions */
-	while (mux->name) {
-		p->group_count++;
-		mux++;
-	}
-
-	/* allocate the group names array needed by the gpio function */
-	p->group_names = devm_kcalloc(p->dev, p->group_count,
-				      sizeof(char *), GFP_KERNEL);
-	if (!p->group_names)
-		return -1;
-
-	for (i = 0; i < p->group_count; i++) {
-		p->group_names[i] = p->groups[i].name;
-		p->func_count += p->groups[i].func_count;
-	}
-
-	/* we have a dummy function[0] for gpio */
-	p->func_count++;
-
-	/* allocate our function and group mapping index buffers */
-	f = p->func = devm_kcalloc(p->dev,
-				   p->func_count,
-				   sizeof(*p->func),
-				   GFP_KERNEL);
-	gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
-					GFP_KERNEL);
-	if (!f || !gpio_func.groups)
-		return -1;
-
-	/* add a backpointer to the function so it knows its group */
-	gpio_func.group_count = p->group_count;
-	for (i = 0; i < gpio_func.group_count; i++)
-		gpio_func.groups[i] = i;
-
-	f[c] = &gpio_func;
-	c++;
-
-	/* add remaining functions */
-	for (i = 0; i < p->group_count; i++) {
-		for (j = 0; j < p->groups[i].func_count; j++) {
-			f[c] = &p->groups[i].func[j];
-			f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
-						    GFP_KERNEL);
-			f[c]->groups[0] = i;
-			f[c]->group_count = 1;
-			c++;
-		}
-	}
-	return 0;
-}
-
-static int rt2880_pinmux_pins(struct rt2880_priv *p)
-{
-	int i, j;
-
-	/*
-	 * loop over the functions and initialize the pins array.
-	 * also work out the highest pin used.
-	 */
-	for (i = 0; i < p->func_count; i++) {
-		int pin;
-
-		if (!p->func[i]->pin_count)
-			continue;
-
-		p->func[i]->pins = devm_kcalloc(p->dev,
-						p->func[i]->pin_count,
-						sizeof(int),
-						GFP_KERNEL);
-		for (j = 0; j < p->func[i]->pin_count; j++)
-			p->func[i]->pins[j] = p->func[i]->pin_first + j;
-
-		pin = p->func[i]->pin_first + p->func[i]->pin_count;
-		if (pin > p->max_pins)
-			p->max_pins = pin;
-	}
-
-	/* the buffer that tells us which pins are gpio */
-	p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
-	/* the pads needed to tell pinctrl about our pins */
-	p->pads = devm_kcalloc(p->dev, p->max_pins,
-			       sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
-	if (!p->pads || !p->gpio) {
-		dev_err(p->dev, "Failed to allocate gpio data\n");
-		return -ENOMEM;
-	}
-
-	memset(p->gpio, 1, sizeof(u8) * p->max_pins);
-	for (i = 0; i < p->func_count; i++) {
-		if (!p->func[i]->pin_count)
-			continue;
-
-		for (j = 0; j < p->func[i]->pin_count; j++)
-			p->gpio[p->func[i]->pins[j]] = 0;
-	}
-
-	/* pin 0 is always a gpio */
-	p->gpio[0] = 1;
-
-	/* set the pads */
-	for (i = 0; i < p->max_pins; i++) {
-		/* strlen("ioXY") + 1 = 5 */
-		char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
-
-		if (!name)
-			return -ENOMEM;
-		snprintf(name, 5, "io%d", i);
-		p->pads[i].number = i;
-		p->pads[i].name = name;
-	}
-	p->desc->pins = p->pads;
-	p->desc->npins = p->max_pins;
-
-	return 0;
-}
-
-static int rt2880_pinmux_probe(struct platform_device *pdev)
-{
-	struct rt2880_priv *p;
-	struct pinctrl_dev *dev;
-
-	if (!rt2880_pinmux_data)
-		return -ENOTSUPP;
-
-	/* setup the private data */
-	p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
-	if (!p)
-		return -ENOMEM;
-
-	p->dev = &pdev->dev;
-	p->desc = &rt2880_pctrl_desc;
-	p->groups = rt2880_pinmux_data;
-	platform_set_drvdata(pdev, p);
-
-	/* init the device */
-	if (rt2880_pinmux_index(p)) {
-		dev_err(&pdev->dev, "failed to load index\n");
-		return -EINVAL;
-	}
-	if (rt2880_pinmux_pins(p)) {
-		dev_err(&pdev->dev, "failed to load pins\n");
-		return -EINVAL;
-	}
-	dev = pinctrl_register(p->desc, &pdev->dev, p);
-	if (IS_ERR(dev))
-		return PTR_ERR(dev);
-
-	return 0;
-}
-
-static const struct of_device_id rt2880_pinmux_match[] = {
-	{ .compatible = "ralink,rt2880-pinmux" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, rt2880_pinmux_match);
-
-static struct platform_driver rt2880_pinmux_driver = {
-	.probe = rt2880_pinmux_probe,
-	.driver = {
-		.name = "rt2880-pinmux",
-		.of_match_table = rt2880_pinmux_match,
-	},
-};
-
-int __init rt2880_pinmux_init(void)
-{
-	return platform_driver_register(&rt2880_pinmux_driver);
-}
-
-core_initcall_sync(rt2880_pinmux_init);
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document

[Linus Walleij]

Hi Sergio,

thanks for driving this!

On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:

> The commit adds rt2880 compatible node in binding document.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
(...)
> +description:
> +  The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual pins
> +  is not supported. There is no pinconf support.

OK!

> +properties:
> +  compatible:
> +    enum:
> +      - ralink,rt2880-pinmux
> +
> +  pinctrl-0:
> +    description:
> +      A phandle to the node containing the subnodes containing default
> +      configurations.

As it is a node on the pin controller itself, this is a hog so write something
about that this is for pinctrl hogs.

> +  pinctrl-names:
> +    description:
> +      A pinctrl state named "default" must be defined.
> +    const: default

Is it really compulsory?

> +required:
> +  - compatible
> +  - pinctrl-names
> +  - pinctrl-0

I wonder if the hogs are really compulsory.

> +patternProperties:
> +  '^.*$':

That's liberal node naming!
What about [a-z0-9_-]+ or something?

> +    if:
> +      type: object
> +      description: |
> +        A pinctrl node should contain at least one subnodes representing the
> +        pinctrl groups available on the machine.
> +      $ref: "pinmux-node.yaml"
> +      required:
> +        - groups
> +        - function
> +      additionalProperties: false
> +    then:
> +      properties:
> +        groups:
> +          description:
> +            Name of the pin group to use for the functions.
> +          $ref: "/schemas/types.yaml#/definitions/string"
> +          enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
> +                 pcie, sdhci]
> +        function:
> +          description:
> +            The mux function to select
> +          $ref: "/schemas/types.yaml#/definitions/string"
> +          enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
> +                 mdio, nand1, nand2, sdhci]

Why do we have this complex if: clause?
$ref: "pinmux-node.yaml" should bring in the groups and
function properties. Then you can add some further restrictions
on top of that, right?

I would just do:

patternProperties:
  '^[a-z0-9_]+$':
    type: object
      description: node for pinctrl
      $ref "pinmux-node.yaml"

      properties:
        groups:
          description: groups...
          enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
pcie, sdhci]
        function:
          description: function...
          enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
mdio, nand1, nand2, sdhci]

Note: the function names are fine but the group names are a bit
confusion since often a group can be used for more than one
function, and e.g.

function = "i2c";
group = "uart1";

to use the uart1 pins for an i2c is gonna look mildly confusing.

But if this is what the hardware calls it I suppose it is
fine.

Yours,
Linus Walleij

Re: [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document

[Linus Walleij]

Hi Sergio,

thanks for driving this!

On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:

> The commit adds rt2880 compatible node in binding document.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
(...)
> +description:
> +  The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual pins
> +  is not supported. There is no pinconf support.

OK!

> +properties:
> +  compatible:
> +    enum:
> +      - ralink,rt2880-pinmux
> +
> +  pinctrl-0:
> +    description:
> +      A phandle to the node containing the subnodes containing default
> +      configurations.

As it is a node on the pin controller itself, this is a hog so write something
about that this is for pinctrl hogs.

> +  pinctrl-names:
> +    description:
> +      A pinctrl state named "default" must be defined.
> +    const: default

Is it really compulsory?

> +required:
> +  - compatible
> +  - pinctrl-names
> +  - pinctrl-0

I wonder if the hogs are really compulsory.

> +patternProperties:
> +  '^.*$':

That's liberal node naming!
What about [a-z0-9_-]+ or something?

> +    if:
> +      type: object
> +      description: |
> +        A pinctrl node should contain at least one subnodes representing the
> +        pinctrl groups available on the machine.
> +      $ref: "pinmux-node.yaml"
> +      required:
> +        - groups
> +        - function
> +      additionalProperties: false
> +    then:
> +      properties:
> +        groups:
> +          description:
> +            Name of the pin group to use for the functions.
> +          $ref: "/schemas/types.yaml#/definitions/string"
> +          enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
> +                 pcie, sdhci]
> +        function:
> +          description:
> +            The mux function to select
> +          $ref: "/schemas/types.yaml#/definitions/string"
> +          enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
> +                 mdio, nand1, nand2, sdhci]

Why do we have this complex if: clause?
$ref: "pinmux-node.yaml" should bring in the groups and
function properties. Then you can add some further restrictions
on top of that, right?

I would just do:

patternProperties:
  '^[a-z0-9_]+$':
    type: object
      description: node for pinctrl
      $ref "pinmux-node.yaml"

      properties:
        groups:
          description: groups...
          enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
pcie, sdhci]
        function:
          description: function...
          enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
mdio, nand1, nand2, sdhci]

Note: the function names are fine but the group names are a bit
confusion since often a group can be used for more than one
function, and e.g.

function = "i2c";
group = "uart1";

to use the uart1 pins for an i2c is gonna look mildly confusing.

But if this is what the hardware calls it I suppose it is
fine.

Yours,
Linus Walleij
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Linus Walleij]

Hi Serigio,

I dug around some to try to understand the patch I think I get
it now :)

Squash this with the third patch so it becomes a
"move" of this file, preserving history. With that:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

I have ideas, but it is better to move the driver out
of staging and improve it in pinctrl.

Since there might be many sub-SoCs for this pin
controller, what about creating
drivers/pinctrl/ralink/* for this at the same time?

On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
> pin individually, these socs have mux groups that when set will effect 1-N pins.
> Pin groups have a 2, 4 or 8 different muxes.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
(...)
> +#include <asm/mach-ralink/ralink_regs.h>
> +#include <asm/mach-ralink/pinmux.h>
> +#include <asm/mach-ralink/mt7620.h>

I think in the next step we should move the contents of
rt2880_pinmux_data into this driver, then we can drop these
mach-headers and show the way for the rest of the ralink
chips to push their data down into this driver (or subdrivers)
and depopulate mach-ralink a bit.

> +       p->groups = rt2880_pinmux_data;

So this is where the driver actually gets a pointer to all
groups and functions, and these groups and functions all
come from arch/mips/ralink/mt7621.c right?

I think after this first step we should move mt7621.c
to pinctrl and become a subdriver for this pin controller
and then we can hopefully move the rest as well once
you set the pattern for how we do this.

Yours,
Linus Walleij

Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Linus Walleij]

Hi Serigio,

I dug around some to try to understand the patch I think I get
it now :)

Squash this with the third patch so it becomes a
"move" of this file, preserving history. With that:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

I have ideas, but it is better to move the driver out
of staging and improve it in pinctrl.

Since there might be many sub-SoCs for this pin
controller, what about creating
drivers/pinctrl/ralink/* for this at the same time?

On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
> pin individually, these socs have mux groups that when set will effect 1-N pins.
> Pin groups have a 2, 4 or 8 different muxes.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
(...)
> +#include <asm/mach-ralink/ralink_regs.h>
> +#include <asm/mach-ralink/pinmux.h>
> +#include <asm/mach-ralink/mt7620.h>

I think in the next step we should move the contents of
rt2880_pinmux_data into this driver, then we can drop these
mach-headers and show the way for the rest of the ralink
chips to push their data down into this driver (or subdrivers)
and depopulate mach-ralink a bit.

> +       p->groups = rt2880_pinmux_data;

So this is where the driver actually gets a pointer to all
groups and functions, and these groups and functions all
come from arch/mips/ralink/mt7621.c right?

I think after this first step we should move mt7621.c
to pinctrl and become a subdriver for this pin controller
and then we can hopefully move the rest as well once
you set the pattern for how we do this.

Yours,
Linus Walleij
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document

[Sergio Paracuellos]

Hi Linus,

Thanks for the review. There weren't too many yaml samples for this so
as you had seen this was a bit messy and I really needed this review,
especially in the 'if' clause part :).

On Mon, Dec 7, 2020 at 11:42 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Sergio,
>
> thanks for driving this!
>
> On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
>
> > The commit adds rt2880 compatible node in binding document.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> (...)
> > +description:
> > +  The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual pins
> > +  is not supported. There is no pinconf support.
>
> OK!
>
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ralink,rt2880-pinmux
> > +
> > +  pinctrl-0:
> > +    description:
> > +      A phandle to the node containing the subnodes containing default
> > +      configurations.
>
> As it is a node on the pin controller itself, this is a hog so write something
> about that this is for pinctrl hogs.

Ok, will do.

>
> > +  pinctrl-names:
> > +    description:
> > +      A pinctrl state named "default" must be defined.
> > +    const: default
>
> Is it really compulsory?

Not really, I guess. The current device tree contains one so I added
here because of this.
>
> > +required:
> > +  - compatible
> > +  - pinctrl-names
> > +  - pinctrl-0
>
> I wonder if the hogs are really compulsory.

Ok, so I guess I should remove both 'pinctrl-names' and ' pinctrl-0'
from the required but maintain its desciption.

>
> > +patternProperties:
> > +  '^.*$':
>
> That's liberal node naming!
> What about [a-z0-9_-]+ or something?

hahaha. Yeah, I like freedom :), but yes, you are right, I will change
the pattern using the one proposed here.

>
> > +    if:
> > +      type: object
> > +      description: |
> > +        A pinctrl node should contain at least one subnodes representing the
> > +        pinctrl groups available on the machine.
> > +      $ref: "pinmux-node.yaml"
> > +      required:
> > +        - groups
> > +        - function
> > +      additionalProperties: false
> > +    then:
> > +      properties:
> > +        groups:
> > +          description:
> > +            Name of the pin group to use for the functions.
> > +          $ref: "/schemas/types.yaml#/definitions/string"
> > +          enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
> > +                 pcie, sdhci]
> > +        function:
> > +          description:
> > +            The mux function to select
> > +          $ref: "/schemas/types.yaml#/definitions/string"
> > +          enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
> > +                 mdio, nand1, nand2, sdhci]
>
> Why do we have this complex if: clause?

To be honest to avoid problems with other pinctrl root nodes because
they are not type 'object' and not having real idea in what way this
should be achieved :).

> $ref: "pinmux-node.yaml" should bring in the groups and
> function properties. Then you can add some further restrictions
> on top of that, right?
>
> I would just do:
>
> patternProperties:
>   '^[a-z0-9_]+$':
>     type: object
>       description: node for pinctrl
>       $ref "pinmux-node.yaml"
>
>       properties:
>         groups:
>           description: groups...
>           enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
> pcie, sdhci]
>         function:
>           description: function...
>           enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
> mdio, nand1, nand2, sdhci]
>
> Note: the function names are fine but the group names are a bit
> confusion since often a group can be used for more than one
> function, and e.g.
>
> function = "i2c";
> group = "uart1";
>
> to use the uart1 pins for an i2c is gonna look mildly confusing.
>
> But if this is what the hardware calls it I suppose it is
> fine.

This is the way is currently being used in the device tree.

>
> Yours,
> Linus Walleij

Thanks again. I will change this and send v2.

Best regards,
     Sergio Paracuellos

Re: [PATCH 1/3] dt-bindings: pinctrl: rt2880: add binding document

[Sergio Paracuellos]

Hi Linus,

Thanks for the review. There weren't too many yaml samples for this so
as you had seen this was a bit messy and I really needed this review,
especially in the 'if' clause part :).

On Mon, Dec 7, 2020 at 11:42 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Sergio,
>
> thanks for driving this!
>
> On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
>
> > The commit adds rt2880 compatible node in binding document.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> (...)
> > +description:
> > +  The rt2880 pinmux can only set the muxing of pin groups. muxing indiviual pins
> > +  is not supported. There is no pinconf support.
>
> OK!
>
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ralink,rt2880-pinmux
> > +
> > +  pinctrl-0:
> > +    description:
> > +      A phandle to the node containing the subnodes containing default
> > +      configurations.
>
> As it is a node on the pin controller itself, this is a hog so write something
> about that this is for pinctrl hogs.

Ok, will do.

>
> > +  pinctrl-names:
> > +    description:
> > +      A pinctrl state named "default" must be defined.
> > +    const: default
>
> Is it really compulsory?

Not really, I guess. The current device tree contains one so I added
here because of this.
>
> > +required:
> > +  - compatible
> > +  - pinctrl-names
> > +  - pinctrl-0
>
> I wonder if the hogs are really compulsory.

Ok, so I guess I should remove both 'pinctrl-names' and ' pinctrl-0'
from the required but maintain its desciption.

>
> > +patternProperties:
> > +  '^.*$':
>
> That's liberal node naming!
> What about [a-z0-9_-]+ or something?

hahaha. Yeah, I like freedom :), but yes, you are right, I will change
the pattern using the one proposed here.

>
> > +    if:
> > +      type: object
> > +      description: |
> > +        A pinctrl node should contain at least one subnodes representing the
> > +        pinctrl groups available on the machine.
> > +      $ref: "pinmux-node.yaml"
> > +      required:
> > +        - groups
> > +        - function
> > +      additionalProperties: false
> > +    then:
> > +      properties:
> > +        groups:
> > +          description:
> > +            Name of the pin group to use for the functions.
> > +          $ref: "/schemas/types.yaml#/definitions/string"
> > +          enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
> > +                 pcie, sdhci]
> > +        function:
> > +          description:
> > +            The mux function to select
> > +          $ref: "/schemas/types.yaml#/definitions/string"
> > +          enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
> > +                 mdio, nand1, nand2, sdhci]
>
> Why do we have this complex if: clause?

To be honest to avoid problems with other pinctrl root nodes because
they are not type 'object' and not having real idea in what way this
should be achieved :).

> $ref: "pinmux-node.yaml" should bring in the groups and
> function properties. Then you can add some further restrictions
> on top of that, right?
>
> I would just do:
>
> patternProperties:
>   '^[a-z0-9_]+$':
>     type: object
>       description: node for pinctrl
>       $ref "pinmux-node.yaml"
>
>       properties:
>         groups:
>           description: groups...
>           enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
> pcie, sdhci]
>         function:
>           description: function...
>           enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
> mdio, nand1, nand2, sdhci]
>
> Note: the function names are fine but the group names are a bit
> confusion since often a group can be used for more than one
> function, and e.g.
>
> function = "i2c";
> group = "uart1";
>
> to use the uart1 pins for an i2c is gonna look mildly confusing.
>
> But if this is what the hardware calls it I suppose it is
> fine.

This is the way is currently being used in the device tree.

>
> Yours,
> Linus Walleij

Thanks again. I will change this and send v2.

Best regards,
     Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Sergio Paracuellos]

Hi Linus,

On Tue, Dec 8, 2020 at 12:00 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Serigio,
>
> I dug around some to try to understand the patch I think I get
> it now :)
>
> Squash this with the third patch so it becomes a
> "move" of this file, preserving history. With that:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Ok, will squash those two if you prefer that way with your 'Acked-by'.

>
> I have ideas, but it is better to move the driver out
> of staging and improve it in pinctrl.
>
> Since there might be many sub-SoCs for this pin
> controller, what about creating
> drivers/pinctrl/ralink/* for this at the same time?

Ok, I will put this inside a ralink subdirectory in pinctrl.

>
> On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
> > pin individually, these socs have mux groups that when set will effect 1-N pins.
> > Pin groups have a 2, 4 or 8 different muxes.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> (...)
> > +#include <asm/mach-ralink/ralink_regs.h>
> > +#include <asm/mach-ralink/pinmux.h>
> > +#include <asm/mach-ralink/mt7620.h>
>
> I think in the next step we should move the contents of
> rt2880_pinmux_data into this driver, then we can drop these
> mach-headers and show the way for the rest of the ralink
> chips to push their data down into this driver (or subdrivers)
> and depopulate mach-ralink a bit.

Agree. Doing that no arch dependencies are included and we can cleanly
enable the driver also for COMPILE_TEST without adding special flags
in pinctrl Makefile.

>
> > +       p->groups = rt2880_pinmux_data;
>
> So this is where the driver actually gets a pointer to all
> groups and functions, and these groups and functions all
> come from arch/mips/ralink/mt7621.c right?

Yes, all of that is defined there.

>
> I think after this first step we should move mt7621.c
> to pinctrl and become a subdriver for this pin controller
> and then we can hopefully move the rest as well once
> you set the pattern for how we do this.

I see. Thanks for advices.

>
> Yours,
> Linus Walleij

Best regards,
    Sergio Paracuellos

Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Sergio Paracuellos]

Hi Linus,

On Tue, Dec 8, 2020 at 12:00 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Serigio,
>
> I dug around some to try to understand the patch I think I get
> it now :)
>
> Squash this with the third patch so it becomes a
> "move" of this file, preserving history. With that:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Ok, will squash those two if you prefer that way with your 'Acked-by'.

>
> I have ideas, but it is better to move the driver out
> of staging and improve it in pinctrl.
>
> Since there might be many sub-SoCs for this pin
> controller, what about creating
> drivers/pinctrl/ralink/* for this at the same time?

Ok, I will put this inside a ralink subdirectory in pinctrl.

>
> On Mon, Dec 7, 2020 at 8:21 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > These Socs have 1-3 banks of 8-32 gpios. Rather then setting the muxing of each
> > pin individually, these socs have mux groups that when set will effect 1-N pins.
> > Pin groups have a 2, 4 or 8 different muxes.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> (...)
> > +#include <asm/mach-ralink/ralink_regs.h>
> > +#include <asm/mach-ralink/pinmux.h>
> > +#include <asm/mach-ralink/mt7620.h>
>
> I think in the next step we should move the contents of
> rt2880_pinmux_data into this driver, then we can drop these
> mach-headers and show the way for the rest of the ralink
> chips to push their data down into this driver (or subdrivers)
> and depopulate mach-ralink a bit.

Agree. Doing that no arch dependencies are included and we can cleanly
enable the driver also for COMPILE_TEST without adding special flags
in pinctrl Makefile.

>
> > +       p->groups = rt2880_pinmux_data;
>
> So this is where the driver actually gets a pointer to all
> groups and functions, and these groups and functions all
> come from arch/mips/ralink/mt7621.c right?

Yes, all of that is defined there.

>
> I think after this first step we should move mt7621.c
> to pinctrl and become a subdriver for this pin controller
> and then we can hopefully move the rest as well once
> you set the pattern for how we do this.

I see. Thanks for advices.

>
> Yours,
> Linus Walleij

Best regards,
    Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Dan Carpenter]

On Mon, Dec 07, 2020 at 08:21:03PM +0100, Sergio Paracuellos wrote:
> +static struct pinctrl_desc rt2880_pctrl_desc = {
> +	.owner		= THIS_MODULE,
> +	.name		= "rt2880-pinmux",
> +	.pctlops	= &rt2880_pctrl_ops,
> +	.pmxops		= &rt2880_pmx_group_ops,
> +};
> +
> +static struct rt2880_pmx_func gpio_func = {
> +	.name = "gpio",
> +};
> +
> +static int rt2880_pinmux_index(struct rt2880_priv *p)


This function name is not great.  I assumed that it would return the
index.

> +{
> +	struct rt2880_pmx_func **f;

Get rid of this "f" variable and use "p->func" instead.

> +	struct rt2880_pmx_group *mux = p->groups;
> +	int i, j, c = 0;
> +
> +	/* count the mux functions */
> +	while (mux->name) {
> +		p->group_count++;
> +		mux++;
> +	}
> +
> +	/* allocate the group names array needed by the gpio function */
> +	p->group_names = devm_kcalloc(p->dev, p->group_count,
> +				      sizeof(char *), GFP_KERNEL);
> +	if (!p->group_names)
> +		return -1;

Return proper error codes in this function.  s/-1/-ENOMEM/

> +
> +	for (i = 0; i < p->group_count; i++) {
> +		p->group_names[i] = p->groups[i].name;
> +		p->func_count += p->groups[i].func_count;
> +	}
> +
> +	/* we have a dummy function[0] for gpio */
> +	p->func_count++;
> +
> +	/* allocate our function and group mapping index buffers */
> +	f = p->func = devm_kcalloc(p->dev,
> +				   p->func_count,
> +				   sizeof(*p->func),
> +				   GFP_KERNEL);
> +	gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
> +					GFP_KERNEL);
> +	if (!f || !gpio_func.groups)
> +		return -1;
> +
> +	/* add a backpointer to the function so it knows its group */
> +	gpio_func.group_count = p->group_count;
> +	for (i = 0; i < gpio_func.group_count; i++)
> +		gpio_func.groups[i] = i;
> +
> +	f[c] = &gpio_func;
> +	c++;
> +
> +	/* add remaining functions */
> +	for (i = 0; i < p->group_count; i++) {
> +		for (j = 0; j < p->groups[i].func_count; j++) {
> +			f[c] = &p->groups[i].func[j];
> +			f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
> +						    GFP_KERNEL);

Add a NULL check.

> +			f[c]->groups[0] = i;
> +			f[c]->group_count = 1;
> +			c++;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int rt2880_pinmux_pins(struct rt2880_priv *p)
> +{
> +	int i, j;
> +
> +	/*
> +	 * loop over the functions and initialize the pins array.
> +	 * also work out the highest pin used.
> +	 */
> +	for (i = 0; i < p->func_count; i++) {
> +		int pin;
> +
> +		if (!p->func[i]->pin_count)
> +			continue;
> +
> +		p->func[i]->pins = devm_kcalloc(p->dev,
> +						p->func[i]->pin_count,
> +						sizeof(int),
> +						GFP_KERNEL);

This can fit on two lines.

		p->func[i]->pins = devm_kcalloc(p->dev, p->func[i]->pin_count,
						sizeof(int), GFP_KERNEL);

> +		for (j = 0; j < p->func[i]->pin_count; j++)
> +			p->func[i]->pins[j] = p->func[i]->pin_first + j;
> +
> +		pin = p->func[i]->pin_first + p->func[i]->pin_count;
> +		if (pin > p->max_pins)
> +			p->max_pins = pin;
> +	}
> +
> +	/* the buffer that tells us which pins are gpio */
> +	p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
> +	/* the pads needed to tell pinctrl about our pins */
> +	p->pads = devm_kcalloc(p->dev, p->max_pins,
> +			       sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
> +	if (!p->pads || !p->gpio) {
> +		dev_err(p->dev, "Failed to allocate gpio data\n");

Delete this error message.  #checkpatch.pl

> +		return -ENOMEM;
> +	}
> +
> +	memset(p->gpio, 1, sizeof(u8) * p->max_pins);
> +	for (i = 0; i < p->func_count; i++) {
> +		if (!p->func[i]->pin_count)
> +			continue;
> +
> +		for (j = 0; j < p->func[i]->pin_count; j++)
> +			p->gpio[p->func[i]->pins[j]] = 0;
> +	}
> +
> +	/* pin 0 is always a gpio */
> +	p->gpio[0] = 1;
> +
> +	/* set the pads */
> +	for (i = 0; i < p->max_pins; i++) {
> +		/* strlen("ioXY") + 1 = 5 */
> +		char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
> +

		char *name;

		name = kasprintf(GFP_KERNEL, "io%d", i);


> +		if (!name)
> +			return -ENOMEM;
> +		snprintf(name, 5, "io%d", i);
> +		p->pads[i].number = i;
> +		p->pads[i].name = name;
> +	}
> +	p->desc->pins = p->pads;
> +	p->desc->npins = p->max_pins;
> +
> +	return 0;
> +}
> +
> +static int rt2880_pinmux_probe(struct platform_device *pdev)
> +{
> +	struct rt2880_priv *p;
> +	struct pinctrl_dev *dev;
> +
> +	if (!rt2880_pinmux_data)
> +		return -ENOTSUPP;
> +
> +	/* setup the private data */
> +	p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	p->dev = &pdev->dev;
> +	p->desc = &rt2880_pctrl_desc;
> +	p->groups = rt2880_pinmux_data;
> +	platform_set_drvdata(pdev, p);
> +
> +	/* init the device */
> +	if (rt2880_pinmux_index(p)) {
> +		dev_err(&pdev->dev, "failed to load index\n");
> +		return -EINVAL;

Preserve the error code:

	err = rt2880_pinmux_index(p);
	if (err) {
		dev_err(&pdev->dev, "failed to load index\n");
		return err;
	}


> +	}
> +	if (rt2880_pinmux_pins(p)) {
> +		dev_err(&pdev->dev, "failed to load pins\n");
> +		return -EINVAL;

Same.

> +	}
> +	dev = pinctrl_register(p->desc, &pdev->dev, p);
> +	if (IS_ERR(dev))
> +		return PTR_ERR(dev);
> +
> +	return 0;
> +}

regards,
dan carpenter

Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Dan Carpenter]

On Mon, Dec 07, 2020 at 08:21:03PM +0100, Sergio Paracuellos wrote:
> +static struct pinctrl_desc rt2880_pctrl_desc = {
> +	.owner		= THIS_MODULE,
> +	.name		= "rt2880-pinmux",
> +	.pctlops	= &rt2880_pctrl_ops,
> +	.pmxops		= &rt2880_pmx_group_ops,
> +};
> +
> +static struct rt2880_pmx_func gpio_func = {
> +	.name = "gpio",
> +};
> +
> +static int rt2880_pinmux_index(struct rt2880_priv *p)


This function name is not great.  I assumed that it would return the
index.

> +{
> +	struct rt2880_pmx_func **f;

Get rid of this "f" variable and use "p->func" instead.

> +	struct rt2880_pmx_group *mux = p->groups;
> +	int i, j, c = 0;
> +
> +	/* count the mux functions */
> +	while (mux->name) {
> +		p->group_count++;
> +		mux++;
> +	}
> +
> +	/* allocate the group names array needed by the gpio function */
> +	p->group_names = devm_kcalloc(p->dev, p->group_count,
> +				      sizeof(char *), GFP_KERNEL);
> +	if (!p->group_names)
> +		return -1;

Return proper error codes in this function.  s/-1/-ENOMEM/

> +
> +	for (i = 0; i < p->group_count; i++) {
> +		p->group_names[i] = p->groups[i].name;
> +		p->func_count += p->groups[i].func_count;
> +	}
> +
> +	/* we have a dummy function[0] for gpio */
> +	p->func_count++;
> +
> +	/* allocate our function and group mapping index buffers */
> +	f = p->func = devm_kcalloc(p->dev,
> +				   p->func_count,
> +				   sizeof(*p->func),
> +				   GFP_KERNEL);
> +	gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
> +					GFP_KERNEL);
> +	if (!f || !gpio_func.groups)
> +		return -1;
> +
> +	/* add a backpointer to the function so it knows its group */
> +	gpio_func.group_count = p->group_count;
> +	for (i = 0; i < gpio_func.group_count; i++)
> +		gpio_func.groups[i] = i;
> +
> +	f[c] = &gpio_func;
> +	c++;
> +
> +	/* add remaining functions */
> +	for (i = 0; i < p->group_count; i++) {
> +		for (j = 0; j < p->groups[i].func_count; j++) {
> +			f[c] = &p->groups[i].func[j];
> +			f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
> +						    GFP_KERNEL);

Add a NULL check.

> +			f[c]->groups[0] = i;
> +			f[c]->group_count = 1;
> +			c++;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int rt2880_pinmux_pins(struct rt2880_priv *p)
> +{
> +	int i, j;
> +
> +	/*
> +	 * loop over the functions and initialize the pins array.
> +	 * also work out the highest pin used.
> +	 */
> +	for (i = 0; i < p->func_count; i++) {
> +		int pin;
> +
> +		if (!p->func[i]->pin_count)
> +			continue;
> +
> +		p->func[i]->pins = devm_kcalloc(p->dev,
> +						p->func[i]->pin_count,
> +						sizeof(int),
> +						GFP_KERNEL);

This can fit on two lines.

		p->func[i]->pins = devm_kcalloc(p->dev, p->func[i]->pin_count,
						sizeof(int), GFP_KERNEL);

> +		for (j = 0; j < p->func[i]->pin_count; j++)
> +			p->func[i]->pins[j] = p->func[i]->pin_first + j;
> +
> +		pin = p->func[i]->pin_first + p->func[i]->pin_count;
> +		if (pin > p->max_pins)
> +			p->max_pins = pin;
> +	}
> +
> +	/* the buffer that tells us which pins are gpio */
> +	p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
> +	/* the pads needed to tell pinctrl about our pins */
> +	p->pads = devm_kcalloc(p->dev, p->max_pins,
> +			       sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
> +	if (!p->pads || !p->gpio) {
> +		dev_err(p->dev, "Failed to allocate gpio data\n");

Delete this error message.  #checkpatch.pl

> +		return -ENOMEM;
> +	}
> +
> +	memset(p->gpio, 1, sizeof(u8) * p->max_pins);
> +	for (i = 0; i < p->func_count; i++) {
> +		if (!p->func[i]->pin_count)
> +			continue;
> +
> +		for (j = 0; j < p->func[i]->pin_count; j++)
> +			p->gpio[p->func[i]->pins[j]] = 0;
> +	}
> +
> +	/* pin 0 is always a gpio */
> +	p->gpio[0] = 1;
> +
> +	/* set the pads */
> +	for (i = 0; i < p->max_pins; i++) {
> +		/* strlen("ioXY") + 1 = 5 */
> +		char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
> +

		char *name;

		name = kasprintf(GFP_KERNEL, "io%d", i);


> +		if (!name)
> +			return -ENOMEM;
> +		snprintf(name, 5, "io%d", i);
> +		p->pads[i].number = i;
> +		p->pads[i].name = name;
> +	}
> +	p->desc->pins = p->pads;
> +	p->desc->npins = p->max_pins;
> +
> +	return 0;
> +}
> +
> +static int rt2880_pinmux_probe(struct platform_device *pdev)
> +{
> +	struct rt2880_priv *p;
> +	struct pinctrl_dev *dev;
> +
> +	if (!rt2880_pinmux_data)
> +		return -ENOTSUPP;
> +
> +	/* setup the private data */
> +	p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	p->dev = &pdev->dev;
> +	p->desc = &rt2880_pctrl_desc;
> +	p->groups = rt2880_pinmux_data;
> +	platform_set_drvdata(pdev, p);
> +
> +	/* init the device */
> +	if (rt2880_pinmux_index(p)) {
> +		dev_err(&pdev->dev, "failed to load index\n");
> +		return -EINVAL;

Preserve the error code:

	err = rt2880_pinmux_index(p);
	if (err) {
		dev_err(&pdev->dev, "failed to load index\n");
		return err;
	}


> +	}
> +	if (rt2880_pinmux_pins(p)) {
> +		dev_err(&pdev->dev, "failed to load pins\n");
> +		return -EINVAL;

Same.

> +	}
> +	dev = pinctrl_register(p->desc, &pdev->dev, p);
> +	if (IS_ERR(dev))
> +		return PTR_ERR(dev);
> +
> +	return 0;
> +}

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Sergio Paracuellos]

Hi Dan,

Thanks for the review.

On Tue, Dec 8, 2020 at 11:17 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Mon, Dec 07, 2020 at 08:21:03PM +0100, Sergio Paracuellos wrote:
> > +static struct pinctrl_desc rt2880_pctrl_desc = {
> > +     .owner          = THIS_MODULE,
> > +     .name           = "rt2880-pinmux",
> > +     .pctlops        = &rt2880_pctrl_ops,
> > +     .pmxops         = &rt2880_pmx_group_ops,
> > +};
> > +
> > +static struct rt2880_pmx_func gpio_func = {
> > +     .name = "gpio",
> > +};
> > +
> > +static int rt2880_pinmux_index(struct rt2880_priv *p)
>
>
> This function name is not great.  I assumed that it would return the
> index.
>
> > +{
> > +     struct rt2880_pmx_func **f;
>
> Get rid of this "f" variable and use "p->func" instead.
>
> > +     struct rt2880_pmx_group *mux = p->groups;
> > +     int i, j, c = 0;
> > +
> > +     /* count the mux functions */
> > +     while (mux->name) {
> > +             p->group_count++;
> > +             mux++;
> > +     }
> > +
> > +     /* allocate the group names array needed by the gpio function */
> > +     p->group_names = devm_kcalloc(p->dev, p->group_count,
> > +                                   sizeof(char *), GFP_KERNEL);
> > +     if (!p->group_names)
> > +             return -1;
>
> Return proper error codes in this function.  s/-1/-ENOMEM/
>
> > +
> > +     for (i = 0; i < p->group_count; i++) {
> > +             p->group_names[i] = p->groups[i].name;
> > +             p->func_count += p->groups[i].func_count;
> > +     }
> > +
> > +     /* we have a dummy function[0] for gpio */
> > +     p->func_count++;
> > +
> > +     /* allocate our function and group mapping index buffers */
> > +     f = p->func = devm_kcalloc(p->dev,
> > +                                p->func_count,
> > +                                sizeof(*p->func),
> > +                                GFP_KERNEL);
> > +     gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
> > +                                     GFP_KERNEL);
> > +     if (!f || !gpio_func.groups)
> > +             return -1;
> > +
> > +     /* add a backpointer to the function so it knows its group */
> > +     gpio_func.group_count = p->group_count;
> > +     for (i = 0; i < gpio_func.group_count; i++)
> > +             gpio_func.groups[i] = i;
> > +
> > +     f[c] = &gpio_func;
> > +     c++;
> > +
> > +     /* add remaining functions */
> > +     for (i = 0; i < p->group_count; i++) {
> > +             for (j = 0; j < p->groups[i].func_count; j++) {
> > +                     f[c] = &p->groups[i].func[j];
> > +                     f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
> > +                                                 GFP_KERNEL);
>
> Add a NULL check.
>
> > +                     f[c]->groups[0] = i;
> > +                     f[c]->group_count = 1;
> > +                     c++;
> > +             }
> > +     }
> > +     return 0;
> > +}
> > +
> > +static int rt2880_pinmux_pins(struct rt2880_priv *p)
> > +{
> > +     int i, j;
> > +
> > +     /*
> > +      * loop over the functions and initialize the pins array.
> > +      * also work out the highest pin used.
> > +      */
> > +     for (i = 0; i < p->func_count; i++) {
> > +             int pin;
> > +
> > +             if (!p->func[i]->pin_count)
> > +                     continue;
> > +
> > +             p->func[i]->pins = devm_kcalloc(p->dev,
> > +                                             p->func[i]->pin_count,
> > +                                             sizeof(int),
> > +                                             GFP_KERNEL);
>
> This can fit on two lines.
>
>                 p->func[i]->pins = devm_kcalloc(p->dev, p->func[i]->pin_count,
>                                                 sizeof(int), GFP_KERNEL);
>
> > +             for (j = 0; j < p->func[i]->pin_count; j++)
> > +                     p->func[i]->pins[j] = p->func[i]->pin_first + j;
> > +
> > +             pin = p->func[i]->pin_first + p->func[i]->pin_count;
> > +             if (pin > p->max_pins)
> > +                     p->max_pins = pin;
> > +     }
> > +
> > +     /* the buffer that tells us which pins are gpio */
> > +     p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
> > +     /* the pads needed to tell pinctrl about our pins */
> > +     p->pads = devm_kcalloc(p->dev, p->max_pins,
> > +                            sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
> > +     if (!p->pads || !p->gpio) {
> > +             dev_err(p->dev, "Failed to allocate gpio data\n");
>
> Delete this error message.  #checkpatch.pl
>
> > +             return -ENOMEM;
> > +     }
> > +
> > +     memset(p->gpio, 1, sizeof(u8) * p->max_pins);
> > +     for (i = 0; i < p->func_count; i++) {
> > +             if (!p->func[i]->pin_count)
> > +                     continue;
> > +
> > +             for (j = 0; j < p->func[i]->pin_count; j++)
> > +                     p->gpio[p->func[i]->pins[j]] = 0;
> > +     }
> > +
> > +     /* pin 0 is always a gpio */
> > +     p->gpio[0] = 1;
> > +
> > +     /* set the pads */
> > +     for (i = 0; i < p->max_pins; i++) {
> > +             /* strlen("ioXY") + 1 = 5 */
> > +             char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
> > +
>
>                 char *name;
>
>                 name = kasprintf(GFP_KERNEL, "io%d", i);
>
>
> > +             if (!name)
> > +                     return -ENOMEM;
> > +             snprintf(name, 5, "io%d", i);
> > +             p->pads[i].number = i;
> > +             p->pads[i].name = name;
> > +     }
> > +     p->desc->pins = p->pads;
> > +     p->desc->npins = p->max_pins;
> > +
> > +     return 0;
> > +}
> > +
> > +static int rt2880_pinmux_probe(struct platform_device *pdev)
> > +{
> > +     struct rt2880_priv *p;
> > +     struct pinctrl_dev *dev;
> > +
> > +     if (!rt2880_pinmux_data)
> > +             return -ENOTSUPP;
> > +
> > +     /* setup the private data */
> > +     p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
> > +     if (!p)
> > +             return -ENOMEM;
> > +
> > +     p->dev = &pdev->dev;
> > +     p->desc = &rt2880_pctrl_desc;
> > +     p->groups = rt2880_pinmux_data;
> > +     platform_set_drvdata(pdev, p);
> > +
> > +     /* init the device */
> > +     if (rt2880_pinmux_index(p)) {
> > +             dev_err(&pdev->dev, "failed to load index\n");
> > +             return -EINVAL;
>
> Preserve the error code:
>
>         err = rt2880_pinmux_index(p);
>         if (err) {
>                 dev_err(&pdev->dev, "failed to load index\n");
>                 return err;
>         }
>
>
> > +     }
> > +     if (rt2880_pinmux_pins(p)) {
> > +             dev_err(&pdev->dev, "failed to load pins\n");
> > +             return -EINVAL;
>
> Same.
>
> > +     }
> > +     dev = pinctrl_register(p->desc, &pdev->dev, p);
> > +     if (IS_ERR(dev))
> > +             return PTR_ERR(dev);
> > +
> > +     return 0;
> > +}

This is already applied but needs more work in its correct place
afterwards. So I will take into account all of these comments and will
send patches to properly address all of them.

>
> regards,
> dan carpenter

Thanks again.

Best regards,
   Sergio Paracuellos

Re: [PATCH 2/3] pinctrl: ralink: add a pinctrl driver for the rt2880 family

[Sergio Paracuellos]

Hi Dan,

Thanks for the review.

On Tue, Dec 8, 2020 at 11:17 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Mon, Dec 07, 2020 at 08:21:03PM +0100, Sergio Paracuellos wrote:
> > +static struct pinctrl_desc rt2880_pctrl_desc = {
> > +     .owner          = THIS_MODULE,
> > +     .name           = "rt2880-pinmux",
> > +     .pctlops        = &rt2880_pctrl_ops,
> > +     .pmxops         = &rt2880_pmx_group_ops,
> > +};
> > +
> > +static struct rt2880_pmx_func gpio_func = {
> > +     .name = "gpio",
> > +};
> > +
> > +static int rt2880_pinmux_index(struct rt2880_priv *p)
>
>
> This function name is not great.  I assumed that it would return the
> index.
>
> > +{
> > +     struct rt2880_pmx_func **f;
>
> Get rid of this "f" variable and use "p->func" instead.
>
> > +     struct rt2880_pmx_group *mux = p->groups;
> > +     int i, j, c = 0;
> > +
> > +     /* count the mux functions */
> > +     while (mux->name) {
> > +             p->group_count++;
> > +             mux++;
> > +     }
> > +
> > +     /* allocate the group names array needed by the gpio function */
> > +     p->group_names = devm_kcalloc(p->dev, p->group_count,
> > +                                   sizeof(char *), GFP_KERNEL);
> > +     if (!p->group_names)
> > +             return -1;
>
> Return proper error codes in this function.  s/-1/-ENOMEM/
>
> > +
> > +     for (i = 0; i < p->group_count; i++) {
> > +             p->group_names[i] = p->groups[i].name;
> > +             p->func_count += p->groups[i].func_count;
> > +     }
> > +
> > +     /* we have a dummy function[0] for gpio */
> > +     p->func_count++;
> > +
> > +     /* allocate our function and group mapping index buffers */
> > +     f = p->func = devm_kcalloc(p->dev,
> > +                                p->func_count,
> > +                                sizeof(*p->func),
> > +                                GFP_KERNEL);
> > +     gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
> > +                                     GFP_KERNEL);
> > +     if (!f || !gpio_func.groups)
> > +             return -1;
> > +
> > +     /* add a backpointer to the function so it knows its group */
> > +     gpio_func.group_count = p->group_count;
> > +     for (i = 0; i < gpio_func.group_count; i++)
> > +             gpio_func.groups[i] = i;
> > +
> > +     f[c] = &gpio_func;
> > +     c++;
> > +
> > +     /* add remaining functions */
> > +     for (i = 0; i < p->group_count; i++) {
> > +             for (j = 0; j < p->groups[i].func_count; j++) {
> > +                     f[c] = &p->groups[i].func[j];
> > +                     f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
> > +                                                 GFP_KERNEL);
>
> Add a NULL check.
>
> > +                     f[c]->groups[0] = i;
> > +                     f[c]->group_count = 1;
> > +                     c++;
> > +             }
> > +     }
> > +     return 0;
> > +}
> > +
> > +static int rt2880_pinmux_pins(struct rt2880_priv *p)
> > +{
> > +     int i, j;
> > +
> > +     /*
> > +      * loop over the functions and initialize the pins array.
> > +      * also work out the highest pin used.
> > +      */
> > +     for (i = 0; i < p->func_count; i++) {
> > +             int pin;
> > +
> > +             if (!p->func[i]->pin_count)
> > +                     continue;
> > +
> > +             p->func[i]->pins = devm_kcalloc(p->dev,
> > +                                             p->func[i]->pin_count,
> > +                                             sizeof(int),
> > +                                             GFP_KERNEL);
>
> This can fit on two lines.
>
>                 p->func[i]->pins = devm_kcalloc(p->dev, p->func[i]->pin_count,
>                                                 sizeof(int), GFP_KERNEL);
>
> > +             for (j = 0; j < p->func[i]->pin_count; j++)
> > +                     p->func[i]->pins[j] = p->func[i]->pin_first + j;
> > +
> > +             pin = p->func[i]->pin_first + p->func[i]->pin_count;
> > +             if (pin > p->max_pins)
> > +                     p->max_pins = pin;
> > +     }
> > +
> > +     /* the buffer that tells us which pins are gpio */
> > +     p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
> > +     /* the pads needed to tell pinctrl about our pins */
> > +     p->pads = devm_kcalloc(p->dev, p->max_pins,
> > +                            sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
> > +     if (!p->pads || !p->gpio) {
> > +             dev_err(p->dev, "Failed to allocate gpio data\n");
>
> Delete this error message.  #checkpatch.pl
>
> > +             return -ENOMEM;
> > +     }
> > +
> > +     memset(p->gpio, 1, sizeof(u8) * p->max_pins);
> > +     for (i = 0; i < p->func_count; i++) {
> > +             if (!p->func[i]->pin_count)
> > +                     continue;
> > +
> > +             for (j = 0; j < p->func[i]->pin_count; j++)
> > +                     p->gpio[p->func[i]->pins[j]] = 0;
> > +     }
> > +
> > +     /* pin 0 is always a gpio */
> > +     p->gpio[0] = 1;
> > +
> > +     /* set the pads */
> > +     for (i = 0; i < p->max_pins; i++) {
> > +             /* strlen("ioXY") + 1 = 5 */
> > +             char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
> > +
>
>                 char *name;
>
>                 name = kasprintf(GFP_KERNEL, "io%d", i);
>
>
> > +             if (!name)
> > +                     return -ENOMEM;
> > +             snprintf(name, 5, "io%d", i);
> > +             p->pads[i].number = i;
> > +             p->pads[i].name = name;
> > +     }
> > +     p->desc->pins = p->pads;
> > +     p->desc->npins = p->max_pins;
> > +
> > +     return 0;
> > +}
> > +
> > +static int rt2880_pinmux_probe(struct platform_device *pdev)
> > +{
> > +     struct rt2880_priv *p;
> > +     struct pinctrl_dev *dev;
> > +
> > +     if (!rt2880_pinmux_data)
> > +             return -ENOTSUPP;
> > +
> > +     /* setup the private data */
> > +     p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
> > +     if (!p)
> > +             return -ENOMEM;
> > +
> > +     p->dev = &pdev->dev;
> > +     p->desc = &rt2880_pctrl_desc;
> > +     p->groups = rt2880_pinmux_data;
> > +     platform_set_drvdata(pdev, p);
> > +
> > +     /* init the device */
> > +     if (rt2880_pinmux_index(p)) {
> > +             dev_err(&pdev->dev, "failed to load index\n");
> > +             return -EINVAL;
>
> Preserve the error code:
>
>         err = rt2880_pinmux_index(p);
>         if (err) {
>                 dev_err(&pdev->dev, "failed to load index\n");
>                 return err;
>         }
>
>
> > +     }
> > +     if (rt2880_pinmux_pins(p)) {
> > +             dev_err(&pdev->dev, "failed to load pins\n");
> > +             return -EINVAL;
>
> Same.
>
> > +     }
> > +     dev = pinctrl_register(p->desc, &pdev->dev, p);
> > +     if (IS_ERR(dev))
> > +             return PTR_ERR(dev);
> > +
> > +     return 0;
> > +}

This is already applied but needs more work in its correct place
afterwards. So I will take into account all of these comments and will
send patches to properly address all of them.

>
> regards,
> dan carpenter

Thanks again.

Best regards,
   Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel