[PATCH 0/8] pinctrl: ralink: rt2880: Some minimal clean ups

[PATCH 0/8] pinctrl: ralink: rt2880: Some minimal clean ups

[Sergio Paracuellos]

After this driver was moved from staging into pinctrl subsytems
some reviews for bindigns and driver itself comes from Ron Herring
and Dan Carpenter. Get rid of all the comments to properly be in
a good shape before merge window.

Best regards,
    Sergio Paracuellos

Sergio Paracuellos (8):
  dt-bindings: pinctrl: rt2880: properly redo bindings
  pinctrl: ralink: rt2880: avoid double pointer to simplify code
  pinctrl: ralink: rt2880: return proper error code
  pinctrl: ralink: rt2880: add missing NULL check
  pinctrl: ralink: rt2880: delete not needed error message
  pinctrl: ralink: rt2880: preserve error codes
  pinctrl: ralink: rt2880: use 'PTR_ERR_OR_ZERO'
  staging: mt7621-dts: properly name pinctrl related nodes

 .../pinctrl/ralink,rt2880-pinmux.yaml         | 62 +++++++++----------
 drivers/pinctrl/ralink/pinctrl-rt2880.c       | 45 +++++++-------
 drivers/staging/mt7621-dts/mt7621.dtsi        | 46 +++++++-------
 3 files changed, 73 insertions(+), 80 deletions(-)

-- 
2.25.1

[PATCH 1/8] dt-bindings: pinctrl: rt2880: properly redo bindings

[Sergio Paracuellos]

When this bindings where applied there weren't already
reviewed and some old hacks was being used to properly
pass the schemas checks. This commits fix them up:
 - Instead of using 'if-then' clause use '-pins$'.
 - 'groups' and 'function' are included inside a new
   '^(.*-)?pinmux$' node.
 - compatible string is not an 'enum' but a 'const'.
 - 'pinctrl-0' and 'pinctrl-names' removed since they are
    used in consumer nodes.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../pinctrl/ralink,rt2880-pinmux.yaml         | 62 +++++++++----------
 1 file changed, 28 insertions(+), 34 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
index 7dea3e26d99e..b32f2676cab5 100644
--- a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
@@ -15,39 +15,38 @@ description:
 
 properties:
   compatible:
-    enum:
-      - ralink,rt2880-pinmux
+    const: ralink,rt2880-pinmux
 
-  pinctrl-0:
-    description:
-      A phandle to the node containing the subnodes containing default
-      configurations. This is for pinctrl hogs.
+patternProperties:
+  '-pins$':
+    type: object
+    patternProperties:
+      '^(.*-)?pinmux$':
+        type: object
+        description: node for pinctrl.
+        $ref: pinmux-node.yaml#
+
+        properties:
+          groups:
+            description: Name of the pin group to use for the functions.
+            enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
+                   pcie, sdhci]
+          function:
+            description: The mux function to select
+            enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
+                   mdio, nand1, nand2, sdhci]
+
+        required:
+          - groups
+          - function
+
+        additionalProperties: false
 
-  pinctrl-names:
-    description:
-      A pinctrl state named "default" can be defined.
-    const: default
+    additionalProperties: false
 
 required:
   - compatible
 
-patternProperties:
-  '[a-z0-9_-]+':
-    if:
-      type: object
-      description: node for pinctrl.
-      $ref: "pinmux-node.yaml"
-    then:
-      properties:
-        groups:
-          description: Name of the pin group to use for the functions.
-          enum: [i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2, mdio,
-                 pcie, sdhci]
-        function:
-          description: The mux function to select
-          enum: [gpio, i2c, spi, uart1, uart2, uart3, rgmii1, rgmii2,
-                 mdio, nand1, nand2, sdhci]
-
 additionalProperties: false
 
 examples:
@@ -55,14 +54,9 @@ examples:
   - |
     pinctrl {
       compatible = "ralink,rt2880-pinmux";
-      pinctrl-names = "default";
-      pinctrl-0 = <&state_default>;
-
-      state_default: pinctrl0 {
-      };
 
-      i2c_pins: i2c0 {
-        i2c0 {
+      i2c_pins: i2c0-pins {
+        pinmux {
           groups = "i2c";
           function = "i2c";
         };
-- 
2.25.1

[PATCH 2/8] pinctrl: ralink: rt2880: avoid double pointer to simplify code

[Sergio Paracuellos]

Double pointer is being used and assigned in a bit dirty way to
assign functions in pinctrl. Instead of doing this just avoid it
and use directly 'p->func' instead.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/pinctrl/ralink/pinctrl-rt2880.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/pinctrl/ralink/pinctrl-rt2880.c b/drivers/pinctrl/ralink/pinctrl-rt2880.c
index 42b1c6cecb57..c933e1a1d4fa 100644
--- a/drivers/pinctrl/ralink/pinctrl-rt2880.c
+++ b/drivers/pinctrl/ralink/pinctrl-rt2880.c
@@ -193,7 +193,6 @@ static struct rt2880_pmx_func gpio_func = {
 
 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;
 
@@ -218,31 +217,29 @@ static int rt2880_pinmux_index(struct rt2880_priv *p)
 	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);
+	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;
+	if (!p->func || !gpio_func.groups)
+		return -ENOMEM;
 
 	/* 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;
+	p->func[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),
+			p->func[c] = &p->groups[i].func[j];
+			p->func[c]->groups = devm_kzalloc(p->dev, sizeof(int),
 						    GFP_KERNEL);
-			f[c]->groups[0] = i;
-			f[c]->group_count = 1;
+			p->func[c]->groups[0] = i;
+			p->func[c]->group_count = 1;
 			c++;
 		}
 	}
-- 
2.25.1

[PATCH 3/8] pinctrl: ralink: rt2880: return proper error code

[Sergio Paracuellos]

Check for NULL shall return '-ENOMEM' instead of '-1'.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/pinctrl/ralink/pinctrl-rt2880.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/ralink/pinctrl-rt2880.c b/drivers/pinctrl/ralink/pinctrl-rt2880.c
index c933e1a1d4fa..6e0bf7c4837b 100644
--- a/drivers/pinctrl/ralink/pinctrl-rt2880.c
+++ b/drivers/pinctrl/ralink/pinctrl-rt2880.c
@@ -206,7 +206,7 @@ static int rt2880_pinmux_index(struct rt2880_priv *p)
 	p->group_names = devm_kcalloc(p->dev, p->group_count,
 				      sizeof(char *), GFP_KERNEL);
 	if (!p->group_names)
-		return -1;
+		return -ENOMEM;
 
 	for (i = 0; i < p->group_count; i++) {
 		p->group_names[i] = p->groups[i].name;
-- 
2.25.1

[PATCH 4/8] pinctrl: ralink: rt2880: add missing NULL check

[Sergio Paracuellos]

Memory is being requested to the kernel but there is
a missing check for NULL. Hence, add it.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/pinctrl/ralink/pinctrl-rt2880.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pinctrl/ralink/pinctrl-rt2880.c b/drivers/pinctrl/ralink/pinctrl-rt2880.c
index 6e0bf7c4837b..3c3336b724ca 100644
--- a/drivers/pinctrl/ralink/pinctrl-rt2880.c
+++ b/drivers/pinctrl/ralink/pinctrl-rt2880.c
@@ -238,6 +238,8 @@ static int rt2880_pinmux_index(struct rt2880_priv *p)
 			p->func[c] = &p->groups[i].func[j];
 			p->func[c]->groups = devm_kzalloc(p->dev, sizeof(int),
 						    GFP_KERNEL);
+			if (!p->func[c]->groups)
+				return -ENOMEM;
 			p->func[c]->groups[0] = i;
 			p->func[c]->group_count = 1;
 			c++;
-- 
2.25.1

[PATCH 5/8] pinctrl: ralink: rt2880: delete not needed error message

[Sergio Paracuellos]

When '-ENOMEM' is returned there is not need at all to
add custom error messages. Hence delete it.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/pinctrl/ralink/pinctrl-rt2880.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pinctrl/ralink/pinctrl-rt2880.c b/drivers/pinctrl/ralink/pinctrl-rt2880.c
index 3c3336b724ca..4725aa34328a 100644
--- a/drivers/pinctrl/ralink/pinctrl-rt2880.c
+++ b/drivers/pinctrl/ralink/pinctrl-rt2880.c
@@ -279,10 +279,8 @@ static int rt2880_pinmux_pins(struct rt2880_priv *p)
 	/* 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");
+	if (!p->pads || !p->gpio)
 		return -ENOMEM;
-	}
 
 	memset(p->gpio, 1, sizeof(u8) * p->max_pins);
 	for (i = 0; i < p->func_count; i++) {
-- 
2.25.1

[PATCH 6/8] pinctrl: ralink: rt2880: preserve error codes

[Sergio Paracuellos]

Some paths in probe function are returning '-EINVAL'
instead of preserve original code from called functions.
Change them to preserve all of them.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/pinctrl/ralink/pinctrl-rt2880.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/ralink/pinctrl-rt2880.c b/drivers/pinctrl/ralink/pinctrl-rt2880.c
index 4725aa34328a..5af1c8a20322 100644
--- a/drivers/pinctrl/ralink/pinctrl-rt2880.c
+++ b/drivers/pinctrl/ralink/pinctrl-rt2880.c
@@ -315,6 +315,7 @@ static int rt2880_pinmux_probe(struct platform_device *pdev)
 {
 	struct rt2880_priv *p;
 	struct pinctrl_dev *dev;
+	int err;
 
 	if (!rt2880_pinmux_data)
 		return -ENOTSUPP;
@@ -330,13 +331,16 @@ static int rt2880_pinmux_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, p);
 
 	/* init the device */
-	if (rt2880_pinmux_index(p)) {
+	err = rt2880_pinmux_index(p);
+	if (err) {
 		dev_err(&pdev->dev, "failed to load index\n");
-		return -EINVAL;
+		return err;
 	}
-	if (rt2880_pinmux_pins(p)) {
+
+	err = rt2880_pinmux_pins(p);
+	if (err) {
 		dev_err(&pdev->dev, "failed to load pins\n");
-		return -EINVAL;
+		return err;
 	}
 	dev = pinctrl_register(p->desc, &pdev->dev, p);
 	if (IS_ERR(dev))
-- 
2.25.1

[PATCH 7/8] pinctrl: ralink: rt2880: use 'PTR_ERR_OR_ZERO'

[Sergio Paracuellos]

 Avoid some boilerplate code using 'PTR_ERR_OR_ZERO'
 in probe function.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/pinctrl/ralink/pinctrl-rt2880.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pinctrl/ralink/pinctrl-rt2880.c b/drivers/pinctrl/ralink/pinctrl-rt2880.c
index 5af1c8a20322..983da62ff3be 100644
--- a/drivers/pinctrl/ralink/pinctrl-rt2880.c
+++ b/drivers/pinctrl/ralink/pinctrl-rt2880.c
@@ -343,10 +343,8 @@ static int rt2880_pinmux_probe(struct platform_device *pdev)
 		return err;
 	}
 	dev = pinctrl_register(p->desc, &pdev->dev, p);
-	if (IS_ERR(dev))
-		return PTR_ERR(dev);
 
-	return 0;
+	return PTR_ERR_OR_ZERO(dev);
 }
 
 static const struct of_device_id rt2880_pinmux_match[] = {
-- 
2.25.1

[PATCH 8/8] staging: mt7621-dts: properly name pinctrl related nodes

[Sergio Paracuellos]

According to the binding documentation pinctrl related nodes
must use '-pins$' and ''^(.*-)?pinmux$'' as names. Change all
of them to properly match them.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 46 +++++++++++++-------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index 5b9d3bf82cb1..f05b7dec703b 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -228,83 +228,83 @@ pinctrl: pinctrl {
 		state_default: pinctrl0 {
 		};
 
-		i2c_pins: i2c0 {
-			i2c0 {
+		i2c_pins: i2c0-pins {
+			pinmux {
 				groups = "i2c";
 				function = "i2c";
 			};
 		};
 
-		spi_pins: spi0 {
-			spi0 {
+		spi_pins: spi0-pins {
+			pinmux {
 				groups = "spi";
 				function = "spi";
 			};
 		};
 
-		uart1_pins: uart1 {
-			uart1 {
+		uart1_pins: uart1-pins {
+			pinmux {
 				groups = "uart1";
 				function = "uart1";
 			};
 		};
 
-		uart2_pins: uart2 {
-			uart2 {
+		uart2_pins: uart2-pins {
+			pinmux {
 				groups = "uart2";
 				function = "uart2";
 			};
 		};
 
-		uart3_pins: uart3 {
-			uart3 {
+		uart3_pins: uart3-pins {
+			pinmux {
 				groups = "uart3";
 				function = "uart3";
 			};
 		};
 
-		rgmii1_pins: rgmii1 {
-			rgmii1 {
+		rgmii1_pins: rgmii1-pins {
+			pinmux {
 				groups = "rgmii1";
 				function = "rgmii1";
 			};
 		};
 
-		rgmii2_pins: rgmii2 {
-			rgmii2 {
+		rgmii2_pins: rgmii2-pins {
+			pinmux {
 				groups = "rgmii2";
 				function = "rgmii2";
 			};
 		};
 
-		mdio_pins: mdio0 {
-			mdio0 {
+		mdio_pins: mdio0-pins {
+			pinmux {
 				groups = "mdio";
 				function = "mdio";
 			};
 		};
 
-		pcie_pins: pcie0 {
-			pcie0 {
+		pcie_pins: pcie0-pins {
+			pinmux {
 				groups = "pcie";
 				function = "gpio";
 			};
 		};
 
-		nand_pins: nand0 {
-			spi-nand {
+		nand_pins: nand0-pins {
+			spi-pinmux {
 				groups = "spi";
 				function = "nand1";
 			};
 
-			sdhci-nand {
+			sdhci-pinmux {
 				groups = "sdhci";
 				function = "nand2";
 			};
 		};
 
-		sdhci_pins: sdhci0 {
-			sdhci0 {
+		sdhci_pins: sdhci0-pins {
+			pinmux {
 				groups = "sdhci";
 				function = "sdhci";
 			};
-- 
2.25.1

Re: [PATCH 0/8] pinctrl: ralink: rt2880: Some minimal clean ups

[Linus Walleij]

On Sun, Dec 13, 2020 at 5:17 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:

> After this driver was moved from staging into pinctrl subsytems
> some reviews for bindigns and driver itself comes from Ron Herring
> and Dan Carpenter. Get rid of all the comments to properly be in
> a good shape before merge window.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

If Greg wants he can queue them last minute. Else I'll apply these
after the merge window, no big deal.

Yours,
Linus Walleij

Re: [PATCH 0/8] pinctrl: ralink: rt2880: Some minimal clean ups

[Sergio Paracuellos]

On Mon, Dec 14, 2020 at 10:02 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Sun, Dec 13, 2020 at 5:17 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
>
> > After this driver was moved from staging into pinctrl subsytems
> > some reviews for bindigns and driver itself comes from Ron Herring
> > and Dan Carpenter. Get rid of all the comments to properly be in
> > a good shape before merge window.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> If Greg wants he can queue them last minute. Else I'll apply these
> after the merge window, no big deal.

Thanks, and sorry for the last minute.

>
> Yours,
> Linus Walleij

Best regards,
    Sergio Paracuellos

Re: [PATCH 1/8] dt-bindings: pinctrl: rt2880: properly redo bindings

[Rob Herring]

On Sun, 13 Dec 2020 17:17:14 +0100, Sergio Paracuellos wrote:
> When this bindings where applied there weren't already
> reviewed and some old hacks was being used to properly
> pass the schemas checks. This commits fix them up:
>  - Instead of using 'if-then' clause use '-pins$'.
>  - 'groups' and 'function' are included inside a new
>    '^(.*-)?pinmux$' node.
>  - compatible string is not an 'enum' but a 'const'.
>  - 'pinctrl-0' and 'pinctrl-names' removed since they are
>     used in consumer nodes.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../pinctrl/ralink,rt2880-pinmux.yaml         | 62 +++++++++----------
>  1 file changed, 28 insertions(+), 34 deletions(-)
> 

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

Re: [PATCH 0/8] pinctrl: ralink: rt2880: Some minimal clean ups

[Linus Walleij]

On Sun, Dec 13, 2020 at 5:17 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:

> After this driver was moved from staging into pinctrl subsytems
> some reviews for bindigns and driver itself comes from Ron Herring
> and Dan Carpenter. Get rid of all the comments to properly be in
> a good shape before merge window.

Applied patches 1-7 to the pinctrl tree, patch 8 needs to be sent
to Greg.

Yours,
Linus Walleij

Re: [PATCH 0/8] pinctrl: ralink: rt2880: Some minimal clean ups

[Sergio Paracuellos]

Hi,

On Mon, Jan 4, 2021 at 3:39 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Sun, Dec 13, 2020 at 5:17 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
>
> > After this driver was moved from staging into pinctrl subsytems
> > some reviews for bindigns and driver itself comes from Ron Herring
> > and Dan Carpenter. Get rid of all the comments to properly be in
> > a good shape before merge window.
>
> Applied patches 1-7 to the pinctrl tree, patch 8 needs to be sent
> to Greg.

Thanks Linus. PATCH 8 should apply directly as it is in this series
also in staging. Greg, can you take PATCH 8 from this series or do you
want me to resend it?

>
> Yours,
> Linus Walleij

Best regards,
    Sergio Paracuellos

Re: [PATCH 0/8] pinctrl: ralink: rt2880: Some minimal clean ups

[Greg KH]

On Mon, Jan 04, 2021 at 03:45:36PM +0100, Sergio Paracuellos wrote:
> Hi,
> 
> On Mon, Jan 4, 2021 at 3:39 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Sun, Dec 13, 2020 at 5:17 PM Sergio Paracuellos
> > <sergio.paracuellos@gmail.com> wrote:
> >
> > > After this driver was moved from staging into pinctrl subsytems
> > > some reviews for bindigns and driver itself comes from Ron Herring
> > > and Dan Carpenter. Get rid of all the comments to properly be in
> > > a good shape before merge window.
> >
> > Applied patches 1-7 to the pinctrl tree, patch 8 needs to be sent
> > to Greg.
> 
> Thanks Linus. PATCH 8 should apply directly as it is in this series
> also in staging. Greg, can you take PATCH 8 from this series or do you
> want me to resend it?

Resend please.

thanks,

greg k-h