Re: [PATCH] Staging: silabs si4455 serial driver

2020-12-10 Thread Dan Carpenter
On Wed, Dec 09, 2020 at 07:41:53PM +, József Horváth wrote:
> > > +static int si4455_get_part_info(struct uart_port *port,
> > > +   struct si4455_part_info *result)
> > > +{
> > > +   int ret;
> > > +   u8 dataOut[] = { SI4455_CMD_ID_PART_INFO };
> > > +   u8 dataIn[SI4455_CMD_REPLY_COUNT_PART_INFO];
> > > +
> > > +   ret = si4455_send_command_get_response(port,
> > > +   sizeof(dataOut),
> > > +   dataOut,
> > > +   sizeof(dataIn),
> > > +   dataIn);
> > 
> > Why not:
> > 
> 
> I changed all like this in my code already. I test it, and I'll send it again.
> 
> Ps.: For my eyes is better to read line or list, reading table is harder :)
> 
>   line(arg1, arg2, arg3, arg4);
> 
>   list(arg1,
>   arg2,
>   arg3,
>   arg4);
> 
>   table(arg1, arg2,
>   arg3, arg4);
> 

Use spaces to make arguments have to line up properly.
`checkpatch.pl --strict` will complain if it's not done.

table(arg1, arg2,
  arg_whatver, foo);
[tab][space x 7]arg_whaver, foo);

But I think Jérôme's main point was to get rid of the dataOut buffer and
use "result" directly.

> 
> >ret = si4455_send_command_get_response(port,
> >   sizeof(*result), result,
> >   sizeof(dataIn), dataIn);
> > 
> > > +   if (ret == 0) {
> > > +   result->CHIPREV = dataIn[0];
> > > +   memcpy(&result->PART, &dataIn[1],sizeof(result->PART));
> > > +   result->PBUILD = dataIn[3];
> > > +   memcpy(&result->ID, &dataIn[4], sizeof(result->ID));
> > > +   result->CUSTOMER = dataIn[6];
> > > +   result->ROMID = dataIn[7];
> > > +   result->BOND = dataIn[8];
> > 
> > ... it would avoid all these lines.
> > 

regards,
dan carpenter

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


[PATCH] Staging: ralink-gdma: ralink-gdma: Fix a blank line coding style issue

2020-12-10 Thread Chris Bloomfield
Fix a coding style issue as identified by checkpatch.pl

Signed-off-by: Chris Bloomfield 
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index 655df317d0ee..a6181a167814 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -122,6 +122,7 @@ struct gdma_dma_dev {
struct gdma_data *data;
void __iomem *base;
struct tasklet_struct task;
+
volatile unsigned long chan_issued;
atomic_t cnt;
 
-- 
2.28.0

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


Re: [PATCH] Staging: ralink-gdma: ralink-gdma: Fix a blank line coding style issue

2020-12-10 Thread Greg KH
On Thu, Dec 10, 2020 at 10:06:57AM +, Chris Bloomfield wrote:
> Fix a coding style issue as identified by checkpatch.pl
> 
> Signed-off-by: Chris Bloomfield 
> ---
>  drivers/staging/ralink-gdma/ralink-gdma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
> b/drivers/staging/ralink-gdma/ralink-gdma.c
> index 655df317d0ee..a6181a167814 100644
> --- a/drivers/staging/ralink-gdma/ralink-gdma.c
> +++ b/drivers/staging/ralink-gdma/ralink-gdma.c
> @@ -122,6 +122,7 @@ struct gdma_dma_dev {
>   struct gdma_data *data;
>   void __iomem *base;
>   struct tasklet_struct task;
> +
>   volatile unsigned long chan_issued;
>   atomic_t cnt;
>  

With your knowledge of C, do you think the above change looks correct?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: silabs si4455 serial driver

2020-12-10 Thread József Horváth
I send this again, because my original mail content was corrupted.

This is a serial port driver for
Silicon Labs Si4455 Sub-GHz transciver.

Signed-off-by: József Horváth 
---
 .../bindings/staging/serial/silabs,si4455.txt |   39 +
 drivers/staging/Kconfig   |2 +
 drivers/staging/Makefile  |1 +
 drivers/staging/si4455/Kconfig|8 +
 drivers/staging/si4455/Makefile   |2 +
 drivers/staging/si4455/TODO   |3 +
 drivers/staging/si4455/si4455.c   | 1465 +
 drivers/staging/si4455/si4455_api.h   |   56 +
 8 files changed, 1576 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
 create mode 100644 drivers/staging/si4455/Kconfig
 create mode 100644 drivers/staging/si4455/Makefile
 create mode 100644 drivers/staging/si4455/TODO
 create mode 100644 drivers/staging/si4455/si4455.c
 create mode 100644 drivers/staging/si4455/si4455_api.h

diff --git a/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt 
b/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
new file mode 100644
index ..abd659b7b952
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
@@ -0,0 +1,39 @@
+* Silicon Labs Si4455 EASY-TO-USE, LOW-CURRENT OOK/(G)FSK SUB-GHZ TRANSCEIVER
+
+Required properties:
+- compatible: Should be one of the following:
+  - "silabs,si4455" for Silicon Labs Si4455-B1A or Si4455-C2A (driver 
automatically detects the part info),
+  - "silabs,si4455b1a" for Silicon Labs Si4455-B1A,
+  - "silabs,si4455c2a" for Silicon Labs Si4455-C2A,
+- reg: SPI chip select number.
+- interrupts: Specifies the interrupt source of the parent interrupt
+  controller. The format of the interrupt specifier depends on the
+  parent interrupt controller.
+- clocks: phandle to the IC source clock (only external clock source 
supported).
+- spi-max-frequency: maximum clock frequency on SPI port
+- shdn-gpios: gpio pin for SDN
+
+Example:
+
+/ {
+   clocks {
+si4455_1_2_osc: si4455_1_2_osc {
+compatible = "fixed-clock";
+#clock-cells = <0>;
+clock-frequency  = <3000>;
+};
+   };
+};
+
+&spi0 {
+   si4455: si4455@0 {
+   compatible = "silabs,si4455";
+   reg = <0>;
+   clocks = <&si4455_1_2_osc>;
+   interrupt-parent = <&gpio>;
+   interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+shdn-gpios = <&gpio 26 1>;
+status = "okay";
+spi-max-frequency = <300>;
+   };
+};
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 9b7cb7c5766a..2cf0c9bfe878 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -118,4 +118,6 @@ source "drivers/staging/wfx/Kconfig"
 
 source "drivers/staging/hikey9xx/Kconfig"
 
+source "drivers/staging/si4455/Kconfig"
+
 endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 38226737c9f3..043c63287bc6 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -49,3 +49,4 @@ obj-$(CONFIG_QLGE)+= qlge/
 obj-$(CONFIG_WIMAX)+= wimax/
 obj-$(CONFIG_WFX)  += wfx/
 obj-y  += hikey9xx/
+obj-$(CONFIG_SERIAL_SI4455)+= si4455/
diff --git a/drivers/staging/si4455/Kconfig b/drivers/staging/si4455/Kconfig
new file mode 100644
index ..666f726f2583
--- /dev/null
+++ b/drivers/staging/si4455/Kconfig
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+config SERIAL_SI4455
+   tristate "Si4455 support"
+   depends on SPI
+   select SERIAL_CORE
+   help
+ This driver is for Silicon Labs's Si4455 Sub-GHz transciver.
+ Say 'Y' here if you wish to use it as serial port.
diff --git a/drivers/staging/si4455/Makefile b/drivers/staging/si4455/Makefile
new file mode 100644
index ..1a6474673509
--- /dev/null
+++ b/drivers/staging/si4455/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_SERIAL_SI4455) := si4455.o
diff --git a/drivers/staging/si4455/TODO b/drivers/staging/si4455/TODO
new file mode 100644
index ..aee5c7613b31
--- /dev/null
+++ b/drivers/staging/si4455/TODO
@@ -0,0 +1,3 @@
+TODO:
+- add variable packet length support
+- add firmware patching support in case of Si4455-C2A
diff --git a/drivers/staging/si4455/si4455.c b/drivers/staging/si4455/si4455.c
new file mode 100644
index ..15f45f19ffdb
--- /dev/null
+++ b/drivers/staging/si4455/si4455.c
@@ -0,0 +1,1465 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 József Horváth 
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include

[PATCH v1 1/2] Staging: silabs si4455 serial driver: fix directory structure and coding style

2020-12-10 Thread József Horváth
fix: coding style
fix: error checking
remove: doc silabs,si4455.txt

Signed-off-by: József Horváth 
---
 .../bindings/staging/serial/silabs,si4455.txt |  39 -
 MAINTAINERS   |   7 +
 drivers/staging/Kconfig   |   2 -
 drivers/staging/Makefile  |   1 -
 drivers/tty/serial/Kconfig|   8 +
 drivers/tty/serial/Makefile   |   1 +
 .../{staging/si4455 => tty/serial}/si4455.c   | 956 --
 .../si4455 => tty/serial}/si4455_api.h|   0
 8 files changed, 419 insertions(+), 595 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
 rename drivers/{staging/si4455 => tty/serial}/si4455.c (57%)
 rename drivers/{staging/si4455 => tty/serial}/si4455_api.h (100%)

diff --git a/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt 
b/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
deleted file mode 100644
index abd659b7b952..
--- a/Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
+++ /dev/null
@@ -1,39 +0,0 @@
-* Silicon Labs Si4455 EASY-TO-USE, LOW-CURRENT OOK/(G)FSK SUB-GHZ TRANSCEIVER
-
-Required properties:
-- compatible: Should be one of the following:
-  - "silabs,si4455" for Silicon Labs Si4455-B1A or Si4455-C2A (driver 
automatically detects the part info),
-  - "silabs,si4455b1a" for Silicon Labs Si4455-B1A,
-  - "silabs,si4455c2a" for Silicon Labs Si4455-C2A,
-- reg: SPI chip select number.
-- interrupts: Specifies the interrupt source of the parent interrupt
-  controller. The format of the interrupt specifier depends on the
-  parent interrupt controller.
-- clocks: phandle to the IC source clock (only external clock source 
supported).
-- spi-max-frequency: maximum clock frequency on SPI port
-- shdn-gpios: gpio pin for SDN
-
-Example:
-
-/ {
-   clocks {
-si4455_1_2_osc: si4455_1_2_osc {
-compatible = "fixed-clock";
-#clock-cells = <0>;
-clock-frequency  = <3000>;
-};
-   };
-};
-
-&spi0 {
-   si4455: si4455@0 {
-   compatible = "silabs,si4455";
-   reg = <0>;
-   clocks = <&si4455_1_2_osc>;
-   interrupt-parent = <&gpio>;
-   interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
-shdn-gpios = <&gpio 26 1>;
-status = "okay";
-spi-max-frequency = <300>;
-   };
-};
diff --git a/MAINTAINERS b/MAINTAINERS
index 2ac5688db43a..a29bc17d446d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15934,6 +15934,13 @@ S: Maintained
 F: drivers/input/touchscreen/silead.c
 F: drivers/platform/x86/touchscreen_dmi.c
 
+SILICON LABS SI4455 SERIAL DRIVER
+M: József Horváth 
+S: Maintained
+F: Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
+F: drivers/tty/serial/si4455.c
+F: drivers/tty/serial/si4455_api.h
+
 SILICON LABS WIRELESS DRIVERS (for WFxxx series)
 M: Jérôme Pouiller 
 S: Supported
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 2cf0c9bfe878..9b7cb7c5766a 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -118,6 +118,4 @@ source "drivers/staging/wfx/Kconfig"
 
 source "drivers/staging/hikey9xx/Kconfig"
 
-source "drivers/staging/si4455/Kconfig"
-
 endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 043c63287bc6..38226737c9f3 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -49,4 +49,3 @@ obj-$(CONFIG_QLGE)+= qlge/
 obj-$(CONFIG_WIMAX)+= wimax/
 obj-$(CONFIG_WFX)  += wfx/
 obj-y  += hikey9xx/
-obj-$(CONFIG_SERIAL_SI4455)+= si4455/
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 28f22e58639c..560aa311cd03 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1583,6 +1583,14 @@ config SERIAL_MILBEAUT_USIO_CONSOLE
  receives all kernel messages and warnings and which allows logins in
  single user mode).
 
+config SERIAL_SI4455
+   tristate "Si4455 support"
+   depends on SPI
+   select SERIAL_CORE
+   help
+ This driver is for Silicon Labs's Si4455 Sub-GHz transciver.
+ Say 'Y' here if you wish to use it as serial port.
+
 endmenu
 
 config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index caf167f0c10a..94f8a727b2a1 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -96,3 +96,4 @@ obj-$(CONFIG_SERIAL_MCTRL_GPIO)   += serial_mctrl_gpio.o
 
 obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o
 obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o
+obj-$(CONFIG_SERIAL_SI4455) += si4455.o
diff --git a/drivers/staging/si4455/si4455.c b/drivers/tty/serial/si4455.c
similarity index 57%
rename from drivers/staging/s

[PATCH v1 2/2] Staging: silabs si4455 serial driver: docs device tree binding

2020-12-10 Thread József Horváth
add: device tree binding schema

Signed-off-by: József Horváth 
---
 .../bindings/serial/silabs,si4455.yaml| 53 +++
 MAINTAINERS   |  2 +-
 2 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/serial/silabs,si4455.yaml

diff --git a/Documentation/devicetree/bindings/serial/silabs,si4455.yaml 
b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml
new file mode 100644
index ..80a73a61755b
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/serial/silabs,si4455.yaml#";
+$schema: "http://devicetree.org/meta-schemas/core.yaml#";
+
+title: Silicon Labs Si4455 device tree bindings
+
+maintainers:
+  - József Horváth 
+
+allOf:
+  - $ref: "/schemas/serial.yaml#"
+
+properties:
+  compatible:
+const: silabs,si4455
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+
+  spi-max-frequency:
+description: maximum clock frequency on SPI port
+maximum: 50
+
+  shutdown-gpios:
+description: gpio pin for SDN
+maxItems: 1
+
+required:
+  - reg
+  - interrupts
+  - spi-max-frequency
+  - shutdown-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+&spi0 {
+  serial0: si4455@0 {
+compatible = "silabs,si4455";
+reg = <0>;
+interrupt-parent = <&gpio>;
+interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+shutdown-gpios = <&gpio 26 1>;
+spi-max-frequency = <30>;
+  };
+};
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index a29bc17d446d..16cc96971ac2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15937,7 +15937,7 @@ F:  drivers/platform/x86/touchscreen_dmi.c
 SILICON LABS SI4455 SERIAL DRIVER
 M: József Horváth 
 S: Maintained
-F: Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt
+F: Documentation/devicetree/bindings/serial/silabs,si4455.yaml
 F: drivers/tty/serial/si4455.c
 F: drivers/tty/serial/si4455_api.h
 
-- 
2.17.1

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


Re: [PATCH v1 1/2] Staging: silabs si4455 serial driver: fix directory structure and coding style

2020-12-10 Thread 'Greg Kroah-Hartman'
On Thu, Dec 10, 2020 at 12:20:59PM +, József Horváth wrote:
> fix: coding style
> fix: error checking
> remove: doc silabs,si4455.txt

What does all of this mean?

Please read the documentation for how to write an effective changelog
text, and where to put the "changes from the first version" text at.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Staging: silabs si4455 serial driver

2020-12-10 Thread 'Greg Kroah-Hartman'
On Thu, Dec 10, 2020 at 12:20:10PM +, József Horváth wrote:
> I send this again, because my original mail content was corrupted.
> 
> This is a serial port driver for
> Silicon Labs Si4455 Sub-GHz transciver.
> 
> Signed-off-by: József Horváth 
> ---
>  .../bindings/staging/serial/silabs,si4455.txt |   39 +
>  drivers/staging/Kconfig   |2 +
>  drivers/staging/Makefile  |1 +
>  drivers/staging/si4455/Kconfig|8 +
>  drivers/staging/si4455/Makefile   |2 +
>  drivers/staging/si4455/TODO   |3 +
>  drivers/staging/si4455/si4455.c   | 1465 +

I said I wasn't going to take this into drivers/staging/ so why is this
still here?

confused,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2020-12-10 Thread Rob Herring
On Tue, Dec 08, 2020 at 08:55:22AM +0100, Sergio Paracuellos wrote:
> The commit adds rt2880 compatible node in binding document.
> 
> Signed-off-by: Sergio Paracuellos 
> ---
>  .../pinctrl/ralink,rt2880-pinmux.yaml | 70 +++
>  1 file changed, 70 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 ..7dea3e26d99e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
> @@ -0,0 +1,70 @@
> +# 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 
> +
> +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

What's the control interface as you have no 'reg' property.

> +
> +  pinctrl-0:
> +description:
> +  A phandle to the node containing the subnodes containing default
> +  configurations. This is for pinctrl hogs.
> +
> +  pinctrl-names:
> +description:
> +  A pinctrl state named "default" can be defined.
> +const: default

These 2 properties go in consumer nodes.

> +
> +required:
> +  - compatible
> +
> +patternProperties:
> +  '[a-z0-9_-]+':
> +if:
> +  type: object
> +  description: node for pinctrl.
> +  $ref: "pinmux-node.yaml"
> +then:

For new bindings, don't do this hack. Just name the nodes '-pins$'

> +  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
 
> +
> +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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value

2020-12-10 Thread shaojie . dong
Hi

> 
> This function should not be calling register_netdev().  What does that
> have to do with firmware?  It should also not free_netdev() because
> that will just lead to a use after free in the caller.
>

--> check code history author changed synchronous 
firmware loading to asynchronous firmware loading
before this change, register_netdev() was not calling in firmware related 
function.
For asynchronous loading, maybe register_netdev() be calling in 
rtl871x_load_fw_cb() is to ensure the netdev be registered after firmware 
loading completed

--> for potential use after free issue
Could I only call "free_irq(adapter->pnetdev->irq, 
adapter->pnetdev)" when register_netdev() failed ?
If no need to change drivers/staging/rtl8712/hal_init.c file, I could give 
up my patch, thank you !

> -原始邮件-
> 发件人: "Dan Carpenter" 
> 发送时间: 2020-12-10 01:46:15 (星期四)
> 收件人: shaojie.d...@isrc.iscas.ac.cn
> 抄送: larry.fin...@lwfinger.net, florian.c.schilha...@googlemail.com, 
gre...@linuxfoundation.org, de...@driverdev.osuosl.org, 
linux-ker...@vger.kernel.org
> 主题: Re: [PATCH] staging: rtl8712: check register_netdev() return value
> 
> On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.d...@isrc.iscas.ac.cn 
wrote:
> > From: "shaojie.dong" 
> > 
> > Function register_netdev() can fail, so we should check it's return 
value
> > 
> > Signed-off-by: shaojie.dong 
> > ---
> >  drivers/staging/rtl8712/hal_init.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/rtl8712/hal_init.c 
b/drivers/staging/rtl8712/hal_init.c
> > index 715f1fe8b..38a3e3d44 100644
> > --- a/drivers/staging/rtl8712/hal_init.c
> > +++ b/drivers/staging/rtl8712/hal_init.c
> > @@ -45,7 +45,10 @@ static void rtl871x_load_fw_cb(const struct 
firmware *firmware, void *context)
> >   }
> >   adapter->fw = firmware;
> >   /* firmware available - start netdev */
> > - register_netdev(adapter->pnetdev);
> > + if (register_netdev(adapter->pnetdev) != 0) {
> > + netdev_err(adapter->pnetdev, "register_netdev() 
failed\n");
> > + free_netdev(adapter->pnetdev);
> > + }
> 
> This function should not be calling register_netdev().  What does that
> have to do with firmware?  It should also not free_netdev() because
> that will just lead to a use after free in the caller.
> 
> regards,
> dan carpenter
> 
> >   complete(&adapter->rtl8712_fw_ready);
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> > ___
> > devel mailing list
> > de...@linuxdriverproject.org
> > 
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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


Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value

2020-12-10 Thread shaojie . dong
Hi

Thanks !  I will modify sign name correctly later

Sorry to say that I have no rtl8712 hardware, so that I could not test it.

From Dan Carpenter's email reply, "free_netdev(adapter->pnetdev)" function 
may cause use after free issue
So that I reply email to ensure if this return value should be check or how to 
handle this return value error


> -原始邮件-
> 发件人: "Greg KH" 
> 发送时间: 2020-12-09 23:13:40 (星期三)
> 收件人: shaojie.d...@isrc.iscas.ac.cn
> 抄送: larry.fin...@lwfinger.net, florian.c.schilha...@googlemail.com, 
de...@driverdev.osuosl.org, linux-ker...@vger.kernel.org
> 主题: Re: [PATCH] staging: rtl8712: check register_netdev() return value
> 
> On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.d...@isrc.iscas.ac.cn 
wrote:
> > From: "shaojie.dong" 
> > 
> > Function register_netdev() can fail, so we should check it's return 
value
> > 
> > Signed-off-by: shaojie.dong 
> 
> I doubt you sign your name with a '.' in it, right?
> 
> Please resend with the correct name, and using Capital letters where
> needed.
> 
> > ---
> >  drivers/staging/rtl8712/hal_init.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/rtl8712/hal_init.c 
b/drivers/staging/rtl8712/hal_init.c
> > index 715f1fe8b..38a3e3d44 100644
> > --- a/drivers/staging/rtl8712/hal_init.c
> > +++ b/drivers/staging/rtl8712/hal_init.c
> > @@ -45,7 +45,10 @@ static void rtl871x_load_fw_cb(const struct 
firmware *firmware, void *context)
> >   }
> >   adapter->fw = firmware;
> >   /* firmware available - start netdev */
> > - register_netdev(adapter->pnetdev);
> > + if (register_netdev(adapter->pnetdev) != 0) {
> > + netdev_err(adapter->pnetdev, "register_netdev() 
failed\n");
> > + free_netdev(adapter->pnetdev);
> > + }
> 
> Did you test this to see if this really properly cleans everything up?
> 
> And your if statement can be simplified, please do so.
> 
> thanks,
> 
> greg k-h

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


Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value

2020-12-10 Thread Dan Carpenter
On Thu, Dec 10, 2020 at 11:05:34PM +0800, shaojie.d...@isrc.iscas.ac.cn wrote:
> Hi
> 
> > 
> > This function should not be calling register_netdev().  What does that
> > have to do with firmware?  It should also not free_netdev() because
> > that will just lead to a use after free in the caller.
> >
> 
> --> check code history author changed 
> synchronous firmware loading to asynchronous firmware loading
> before this change, register_netdev() was not calling in firmware related 
> function.
> For asynchronous loading, maybe register_netdev() be calling in 
> rtl871x_load_fw_cb() is to ensure the netdev be registered after firmware 
> loading completed
> 
> --> for potential use after free issue
> Could I only call "free_irq(adapter->pnetdev->irq, 
> adapter->pnetdev)" when register_netdev() failed ?
> If no need to change drivers/staging/rtl8712/hal_init.c file, I could 
> give up my patch, thank you !
> 

Cleaning this up is a bit complicated and requires reworking the
firmware loading and it requires testing.  I don't think you have the
hardware to actually test this driver?  Probably, just leave this code
for another day.

regards,
dan carpenter

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


Re: Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value

2020-12-10 Thread shaojie . dong
Hi

I do not have rtl8712 hardware
So that I would remain this code and give up my patch
Thank you !

> -原始邮件-
> 发件人: "Dan Carpenter" 
> 发送时间: 2020-12-10 23:16:31 (星期四)
> 收件人: shaojie.d...@isrc.iscas.ac.cn
> 抄送: larry.fin...@lwfinger.net, florian.c.schilha...@googlemail.com, 
gre...@linuxfoundation.org, de...@driverdev.osuosl.org, 
linux-ker...@vger.kernel.org
> 主题: Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value
> 
> On Thu, Dec 10, 2020 at 11:05:34PM +0800, shaojie.d...@isrc.iscas.ac.cn 
wrote:
> > Hi
> > 
> > > 
> > > This function should not be calling register_netdev().  What 
does that
> > > have to do with firmware?  It should also not free_netdev() 
because
> > > that will just lead to a use after free in the caller.
> > >
> > 
> > --> check code history author changed 
synchronous firmware loading to asynchronous firmware loading
> > before this change, register_netdev() was not calling in firmware 
related function.
> > For asynchronous loading, maybe register_netdev() be calling in 
rtl871x_load_fw_cb() is to ensure the netdev be registered after firmware 
loading completed
> > 
> > --> for potential use after free issue
> > Could I only call "free_irq(adapter->pnetdev->irq, 
adapter->pnetdev)" when register_netdev() failed ?
> > If no need to change drivers/staging/rtl8712/hal_init.c file, I 
could give up my patch, thank you !
> > 
> 
> Cleaning this up is a bit complicated and requires reworking the
> firmware loading and it requires testing.  I don't think you have the
> hardware to actually test this driver?  Probably, just leave this code
> for another day.
> 
> regards,
> dan carpenter

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


Re: [PATCH v1 1/2] Staging: silabs si4455 serial driver: fix directory structure and coding style

2020-12-10 Thread Dan Carpenter
On Thu, Dec 10, 2020 at 12:20:59PM +, József Horváth wrote:
> fix: coding style
> fix: error checking
> remove: doc silabs,si4455.txt
> 
> Signed-off-by: József Horváth 

Just fold (combine) this check in with the first.  This is much better
but there are still 54 checkpatch warnins.

total: 0 errors, 2 warnings, 52 checks, 1311 lines checked

This is much better.  Do we have to print so many warnings?  It's
useful for debugging but it makes the code messy.  The ioctl still has
some security issues with regards to user pointers (see bottom).


>  struct si4455_part_info {
> - u8  CHIPREV;
> - u16 PART;
> - u8  PBUILD;
> - u16 ID;
> - u8  CUSTOMER;
> - u8  ROMID;
> - u8  BOND;
> + u8 CHIPREV;
> + u16 PART;
> + u8 PBUILD;
> + u16 ID;
> + u8 CUSTOMER;
> + u8 ROMID;
> + u8 BOND;

Make these lower case letters.


>  static int si4455_get_response(struct uart_port *port,
>   int length,
>   u8 *data)
>  {
> - int ret = -EIO;
> - u8 dataOut[] = { SI4455_CMD_ID_READ_CMD_BUFF };
> - u8 *dataIn = devm_kzalloc(port->dev, 1 + length, GFP_KERNEL);
> - struct spi_transfer xfer[] = {
> - {
> - .tx_buf = dataOut,
> - .len = sizeof(dataOut),
> - }, {
> - .rx_buf = dataIn,
> - .len = 1 + length,
> - }
> - };
> - int errCnt = 1;
> + int ret;
> + u8 data_out[] = { SI4455_CMD_ID_READ_CMD_BUFF };
> + u8 *data_in = NULL;
> + struct spi_transfer xfer[2];
> + int err = 1;

Use "cnt" instead of "err" otherwise people will get confused and start
saying "err = some_function();"

>  
> - while (errCnt > 0) {
> - dataOut[0] = SI4455_CMD_ID_READ_CMD_BUFF;
> + data_in = kzalloc(1 + length, GFP_KERNEL);
> + if (!data_in)
> + return -ENOMEM;
> +
> + memset(&xfer, 0x00, sizeof(xfer));
> + xfer[0].tx_buf = data_out;
> + xfer[0].len = sizeof(data_out);
> + xfer[1].rx_buf = data_in;
> + xfer[1].len = 1 + length;
> +
> + while (--err > 0) {
> + data_out[0] = SI4455_CMD_ID_READ_CMD_BUFF;
>   ret = spi_sync_transfer(to_spi_device(port->dev),
>   xfer,
>   ARRAY_SIZE(xfer));
> - if (ret == 0) {
> - if (dataIn[0] == 0xFF) {
> - if ((length > 0) && (data != NULL)) {
> - memcpy(data, &dataIn[1], length);
> - } else {
> - if (length > 0)
> - ret = -EINVAL;
> - }
> - break;
> + if (ret) {
> + dev_err(port->dev, "%s: spi_sync_transfer error(%i)", 
> __func__, ret);
> + break;
> + }
> +
> + if (data_in[0] == 0xFF) {
> + if (length > 0 && data) {
> + memcpy(data, &data_in[1], length);
> + } else {
> + if (length > 0)
> + ret = -EINVAL;

Should this break if we encounter an error?

>   }
> - usleep_range(100, 200);
> - } else {
>   break;
>   }
> - errCnt--;
> + usleep_range(100, 200);
>   }
> - if (errCnt == 0) {
> - dev_err(port->dev, "si4455_poll_cts:errCnt==%i", errCnt);
> + if (err == 0) {
> + dev_err(port->dev, "%s:err==%i", __func__, err);
>   ret = -EIO;
>   }
> - if (dataIn != NULL)
> - devm_kfree(port->dev, dataIn);
> + kfree(data_in);
>   return ret;
>  }
>  
> +static int si4455_poll_cts(struct uart_port *port)
> +{
> + return si4455_get_response(port, 0, NULL);
> +}
> +
>  static int si4455_send_command(struct uart_port *port,
>   int length,
>   u8 *data)
> @@ -214,134 +193,118 @@ static int si4455_send_command(struct uart_port *port,
>   int ret;
>  
>   ret = si4455_poll_cts(port);
> - if (ret == 0) {
> - ret = spi_write(to_spi_device(port->dev), data, length);
> - if (ret) {
> - dev_err(port->dev,
> - "%s: spi_write error(%i)",
> - __func__,
> - ret);
> - }
> - } else {
> - dev_err(port->dev,
> - "%s: si4455_poll_cts error(%i)",
> + 

Re: [PATCH v1 2/2] Staging: silabs si4455 serial driver: docs device tree binding

2020-12-10 Thread Rob Herring
On Thu, Dec 10, 2020 at 12:21:56PM +, József Horváth wrote:
> add: device tree binding schema

For the subject, follow conventions of the directory. Something like:

dt-bindings: serial: Add SiLabs SI4455 schema

> Signed-off-by: József Horváth 
> ---
>  .../bindings/serial/silabs,si4455.yaml| 53 +++
>  MAINTAINERS   |  2 +-
>  2 files changed, 54 insertions(+), 1 deletion(-)
>  create mode 100644 
> Documentation/devicetree/bindings/serial/silabs,si4455.yaml
> 
> diff --git a/Documentation/devicetree/bindings/serial/silabs,si4455.yaml 
> b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml
> new file mode 100644
> index ..80a73a61755b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/silabs,si4455.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/serial/silabs,si4455.yaml#";
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
> +
> +title: Silicon Labs Si4455 device tree bindings

Add 'description' with some info on this h/w and possibly a link to 
datasheet if available.

> +
> +maintainers:
> +  - József Horváth 
> +
> +allOf:
> +  - $ref: "/schemas/serial.yaml#"
> +
> +properties:
> +  compatible:
> +const: silabs,si4455
> +
> +  reg:
> +maxItems: 1
> +
> +  interrupts:
> +maxItems: 1
> +
> +  spi-max-frequency:
> +description: maximum clock frequency on SPI port
> +maximum: 50
> +
> +  shutdown-gpios:
> +description: gpio pin for SDN
> +maxItems: 1
> +
> +required:
> +  - reg
> +  - interrupts
> +  - spi-max-frequency
> +  - shutdown-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +&spi0 {
> +  serial0: si4455@0 {
> +compatible = "silabs,si4455";
> +reg = <0>;
> +interrupt-parent = <&gpio>;
> +interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +shutdown-gpios = <&gpio 26 1>;
> +spi-max-frequency = <30>;
> +  };
> +};
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a29bc17d446d..16cc96971ac2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15937,7 +15937,7 @@ F:drivers/platform/x86/touchscreen_dmi.c
>  SILICON LABS SI4455 SERIAL DRIVER
>  M:   József Horváth 
>  S:   Maintained
> -F:   Documentation/devicetree/bindings/staging/serial/silabs,si4455.txt

This is a new file, right?

> +F:   Documentation/devicetree/bindings/serial/silabs,si4455.yaml
>  F:   drivers/tty/serial/si4455.c
>  F:   drivers/tty/serial/si4455_api.h
>  
> -- 
> 2.17.1
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v1 2/2] Staging: silabs si4455 serial driver: docs device tree binding

2020-12-10 Thread Rob Herring
On Thu, 10 Dec 2020 12:21:56 +, József Horváth wrote:
> add: device tree binding schema
> 
> Signed-off-by: József Horváth 
> ---
>  .../bindings/serial/silabs,si4455.yaml| 53 +++
>  MAINTAINERS   |  2 +-
>  2 files changed, 54 insertions(+), 1 deletion(-)
>  create mode 100644 
> Documentation/devicetree/bindings/serial/silabs,si4455.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: 
Documentation/devicetree/bindings/serial/silabs,si4455.example.dts:19.9-14 
syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:342: 
Documentation/devicetree/bindings/serial/silabs,si4455.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs
make: *** [Makefile:1364: dt_binding_check] Error 2


See https://patchwork.ozlabs.org/patch/1414082

The base for the patch is generally the last rc1. Any dependencies
should be noted.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

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


Re: [PATCH v2 16/19] dt-bindings: media: Add A83T MIPI CSI-2 bindings documentation

2020-12-10 Thread Rob Herring
On Sat, 28 Nov 2020 15:28:36 +0100, Paul Kocialkowski wrote:
> This introduces YAML bindings documentation for the A83T MIPI CSI-2
> controller.
> 
> Signed-off-by: Paul Kocialkowski 
> ---
>  .../media/allwinner,sun8i-a83t-mipi-csi2.yaml | 147 ++
>  1 file changed, 147 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-mipi-csi2.yaml
> 

Reviewed-by: Rob Herring 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[driver-core:driver-core-testing] BUILD SUCCESS WITH WARNING 2a0387e8128ae0c3de9ff976bc25afaae3d4a916

2020-12-10 Thread kernel test robot
 allyesconfig
mips  rb532_defconfig
sh  urquell_defconfig
sh  sdk7786_defconfig
powerpc  ppc64e_defconfig
arm palmz72_defconfig
sh  sh7785lcr_32bit_defconfig
arm shannon_defconfig
sh   j2_defconfig
m68k   m5249evb_defconfig
arm   versatile_defconfig
sh   rts7751r2dplus_defconfig
powerpc mpc834x_itx_defconfig
sh   alldefconfig
powerpc  cm5200_defconfig
powerpc  ppc40x_defconfig
mips  rm200_defconfig
arm mxs_defconfig
riscvalldefconfig
powerpc  arches_defconfig
m68km5272c3_defconfig
powerpc mpc837x_rdb_defconfig
ia64 allmodconfig
ia64defconfig
ia64 allyesconfig
m68k allmodconfig
m68kdefconfig
m68k allyesconfig
nios2   defconfig
arc  allyesconfig
nds32 allnoconfig
c6x  allyesconfig
nds32   defconfig
nios2allyesconfig
cskydefconfig
alpha   defconfig
alphaallyesconfig
xtensa   allyesconfig
h8300allyesconfig
arc defconfig
sh   allmodconfig
parisc  defconfig
s390 allyesconfig
parisc   allyesconfig
s390defconfig
i386 allyesconfig
sparc   defconfig
i386   tinyconfig
i386defconfig
sparcallyesconfig
mips allmodconfig
powerpc  allyesconfig
powerpc  allmodconfig
powerpc   allnoconfig
i386 randconfig-a004-20201209
i386 randconfig-a005-20201209
i386 randconfig-a001-20201209
i386 randconfig-a002-20201209
i386 randconfig-a006-20201209
i386 randconfig-a003-20201209
i386 randconfig-a001-20201210
i386 randconfig-a004-20201210
i386 randconfig-a003-20201210
i386 randconfig-a002-20201210
i386 randconfig-a005-20201210
i386 randconfig-a006-20201210
x86_64   randconfig-a016-20201209
x86_64   randconfig-a012-20201209
x86_64   randconfig-a013-20201209
x86_64   randconfig-a014-20201209
x86_64   randconfig-a015-20201209
x86_64   randconfig-a011-20201209
i386 randconfig-a013-20201209
i386 randconfig-a014-20201209
i386 randconfig-a011-20201209
i386 randconfig-a015-20201209
i386 randconfig-a012-20201209
i386 randconfig-a016-20201209
riscvnommu_k210_defconfig
riscvallyesconfig
riscvnommu_virt_defconfig
riscv allnoconfig
riscv   defconfig
riscv  rv32_defconfig
riscvallmodconfig
x86_64   rhel
x86_64   allyesconfig
x86_64rhel-7.6-kselftests
x86_64  defconfig
x86_64   rhel-8.3
x86_64  kexec

clang tested configs:
x86_64   randconfig-a004-20201209
x86_64   randconfig-a006-20201209
x86_64   randconfig-a005-20201209
x86_64   randconfig-a001-20201209
x86_64   randconfig-a002-20201209
x86_64   randconfig-a003-20201209
x86_64   randconfig-a003-20201210
x86_64   randconfig-a006-20201210
x86_64   randconfig-a002-20201210
x86_64   randconfig-a005-20201210
x86_64   randconfig-a004-20201210
x86_64   randconfig-a001-20201210

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
devel mailing list
de...@linuxdriverproject.org
http

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

2020-12-10 Thread Sergio Paracuellos
Hi Rob,

On Thu, Dec 10, 2020 at 2:47 PM Rob Herring  wrote:
>
> On Tue, Dec 08, 2020 at 08:55:22AM +0100, Sergio Paracuellos wrote:
> > The commit adds rt2880 compatible node in binding document.
> >
> > Signed-off-by: Sergio Paracuellos 
> > ---
> >  .../pinctrl/ralink,rt2880-pinmux.yaml | 70 +++
> >  1 file changed, 70 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 ..7dea3e26d99e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
> > @@ -0,0 +1,70 @@
> > +# 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 
> > +
> > +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
>
> What's the control interface as you have no 'reg' property.

There is not used in pinctrl. Every pin has a gpio function and
pinctrl and gpio are separate drivers. Here only pin functions and
groups are defined. The glue
code for this driver is done in arch/mips/ralink/mt7621.c using
specific pinmux.h header defined for ralink and then all that settings
are used in drivers through
the pinctrl driver.

>
> > +
> > +  pinctrl-0:
> > +description:
> > +  A phandle to the node containing the subnodes containing default
> > +  configurations. This is for pinctrl hogs.
> > +
> > +  pinctrl-names:
> > +description:
> > +  A pinctrl state named "default" can be defined.
> > +const: default
>
> These 2 properties go in consumer nodes.

Ok, So I have to remove them from here. I see.

>
> > +
> > +required:
> > +  - compatible
> > +
> > +patternProperties:
> > +  '[a-z0-9_-]+':
> > +if:
> > +  type: object
> > +  description: node for pinctrl.
> > +  $ref: "pinmux-node.yaml"
> > +then:
>
> For new bindings, don't do this hack. Just name the nodes '-pins$'

I see. I will update bindings for pinctrl in staging and avoid this
if-then clause.

>
> > +  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

Ok, I will add this.

>
> > +
> > +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
> >

Thanks for the review.

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