[v7,00/22] pinctrl: add BCM63XX pincontrol support

Message ID 20210315114214.3096-1-noltari@gmail.com
Headers show
Series
  • pinctrl: add BCM63XX pincontrol support
Related show

Message

Álvaro Fernández Rojas March 15, 2021, 11:41 a.m.
First of all, I've based this on the patches sent by Jonas Gorski back in
2016:
https://www.spinics.net/lists/linux-gpio/msg15983.html
http://patchwork.ozlabs.org/project/linux-gpio/patch/1471604025-21575-2-git-send-email-jonas.gorski@gmail.com/

I've tried to address all coments from Linus Walleij, but I know that
this may still need some other modifications

This patchset adds appropriate binding documentation and drivers for
pin controller cores found in the BCM63XX MIPS SoCs currently supported.

While the GPIO part is always the same, the pinmux part varies quite a
lot between different SoCs. Sometimes they have defined groups which
can be muxed into different functions, sometimes each function has a
different group. Sometimes you can mux individual pins. Often it is a
combination of single pins and groups.

Some core versions require the GPIO direction to be set according to the
function, most do not. Sometimes the mux register(s) contain bits for
unrelated other functions.

v7: introduce changes suggested by Rob Herring.
v6: introduce changes suggested by Rob Herring and Andy Shevchenko.
v5: introduce changes suggested by Andy Shevchenko.
v4: fix gpiochip_irqchip_add_domain(), remove IRQ Kconfig selections and add
 missing of_node_put().
v3: introduce new files for shared code and add more changes suggested by
 Linus Walleij. Also add a new patch needed for properly parsing gpio-ranges.
v2: introduce changes suggested by Linus Walleij and remove interrupts
 - In order to use GPIO_REGMAP, the need to get gpio_chip from gpio_regmap
 and use it for pinctrl_add_gpio_range() and gpio_chip.direction_input()
 and gpio_chip.direction_output().

Álvaro Fernández Rojas (22):
  gpio: guard gpiochip_irqchip_add_domain() with GPIOLIB_IRQCHIP
  gpio: regmap: set gpio_chip of_node
  dt-bindings: add BCM63XX GPIO binding documentation
  pinctrl: bcm: add bcm63xx base code
  dt-bindings: add BCM6328 pincontroller binding documentation
  dt-bindings: add BCM6328 GPIO sysctl binding documentation
  pinctrl: add a pincontrol driver for BCM6328
  dt-bindings: add BCM6358 pincontroller binding documentation
  dt-bindings: add BCM6358 GPIO sysctl binding documentation
  pinctrl: add a pincontrol driver for BCM6358
  dt-bindings: add BCM6362 pincontroller binding documentation
  dt-bindings: add BCM6362 GPIO sysctl binding documentation
  pinctrl: add a pincontrol driver for BCM6362
  dt-bindings: add BCM6368 pincontroller binding documentation
  dt-bindings: add BCM6368 GPIO sysctl binding documentation
  pinctrl: add a pincontrol driver for BCM6368
  dt-bindings: add BCM63268 pincontroller binding documentation
  dt-bindings: add BCM63268 GPIO sysctl binding documentation
  pinctrl: add a pincontrol driver for BCM63268
  dt-bindings: add BCM6318 pincontroller binding documentation
  dt-bindings: add BCM6318 GPIO sysctl binding documentation
  pinctrl: add a pincontrol driver for BCM6318

 .../bindings/gpio/brcm,bcm63xx-gpio.yaml      |  83 +++
 .../mfd/brcm,bcm6318-gpio-sysctl.yaml         | 179 +++++
 .../mfd/brcm,bcm63268-gpio-sysctl.yaml        | 196 ++++++
 .../mfd/brcm,bcm6328-gpio-sysctl.yaml         | 164 +++++
 .../mfd/brcm,bcm6358-gpio-sysctl.yaml         | 132 ++++
 .../mfd/brcm,bcm6362-gpio-sysctl.yaml         | 238 +++++++
 .../mfd/brcm,bcm6368-gpio-sysctl.yaml         | 248 +++++++
 .../pinctrl/brcm,bcm6318-pinctrl.yaml         | 148 ++++
 .../pinctrl/brcm,bcm63268-pinctrl.yaml        | 169 +++++
 .../pinctrl/brcm,bcm6328-pinctrl.yaml         | 132 ++++
 .../pinctrl/brcm,bcm6358-pinctrl.yaml         |  98 +++
 .../pinctrl/brcm,bcm6362-pinctrl.yaml         | 211 ++++++
 .../pinctrl/brcm,bcm6368-pinctrl.yaml         | 222 ++++++
 drivers/gpio/gpio-regmap.c                    |   2 +
 drivers/pinctrl/bcm/Kconfig                   |  55 ++
 drivers/pinctrl/bcm/Makefile                  |   7 +
 drivers/pinctrl/bcm/pinctrl-bcm6318.c         | 498 ++++++++++++++
 drivers/pinctrl/bcm/pinctrl-bcm63268.c        | 643 ++++++++++++++++++
 drivers/pinctrl/bcm/pinctrl-bcm6328.c         | 404 +++++++++++
 drivers/pinctrl/bcm/pinctrl-bcm6358.c         | 369 ++++++++++
 drivers/pinctrl/bcm/pinctrl-bcm6362.c         | 617 +++++++++++++++++
 drivers/pinctrl/bcm/pinctrl-bcm6368.c         | 523 ++++++++++++++
 drivers/pinctrl/bcm/pinctrl-bcm63xx.c         | 113 +++
 drivers/pinctrl/bcm/pinctrl-bcm63xx.h         |  43 ++
 include/linux/gpio/driver.h                   |   9 +
 include/linux/gpio/regmap.h                   |   4 +
 26 files changed, 5507 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/brcm,bcm6318-gpio-sysctl.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/brcm,bcm6328-gpio-sysctl.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/brcm,bcm6358-gpio-sysctl.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/brcm,bcm6362-gpio-sysctl.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/brcm,bcm6368-gpio-sysctl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6318-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm63268-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6362-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6368-pinctrl.yaml
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6318.c
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63268.c
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6328.c
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6358.c
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6362.c
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6368.c
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.c
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.h

Comments

Linus Walleij March 16, 2021, 10:13 a.m. | #1
On Mon, Mar 15, 2021 at 12:42 PM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:

> v7: introduce changes suggested by Rob Herring.


If Rob is happy with the bindings like this (GPIO as parallel node rathern
than subnode) I am ready to merge this.

As long as the bindings are OK I am pretty sure any remaining nits can
be fixed in-tree.

Yours,
Linus Walleij
Rob Herring March 16, 2021, 8:59 p.m. | #2
On Mon, Mar 15, 2021 at 12:41:57PM +0100, Álvaro Fernández Rojas wrote:
> Add binding documentation for the pincontrol core found in BCM6328 SoCs.

> 

> Co-developed-by: Jonas Gorski <jonas.gorski@gmail.com>

> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

> ---

>  v7: add changes suggested by Rob Herring

>  v6: add changes suggested by Rob Herring

>  v5: change Documentation to dt-bindings in commit title

>  v4: no changes

>  v3: add new gpio node

>  v2: remove interrupts

> 

>  .../pinctrl/brcm,bcm6328-pinctrl.yaml         | 132 ++++++++++++++++++

>  1 file changed, 132 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml

> 

> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml

> new file mode 100644

> index 000000000000..e1ecdc578f32

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml

> @@ -0,0 +1,132 @@

> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/pinctrl/brcm,bcm6328-pinctrl.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +

> +title: Broadcom BCM6328 pin controller

> +

> +maintainers:

> +  - Álvaro Fernández Rojas <noltari@gmail.com>

> +  - Jonas Gorski <jonas.gorski@gmail.com>

> +

> +description: |+

> +  The pin controller node should be the child of a syscon node.

> +

> +  Refer to the the bindings described in

> +  Documentation/devicetree/bindings/mfd/syscon.yaml

> +

> +properties:

> +  compatible:

> +    const: brcm,bcm6328-pinctrl

> +

> +  reg:

> +    maxItems: 1

> +

> +patternProperties:

> +  '^.*-pins$':


'-pins$' is equivalent.

> +    if:

> +      type: object

> +    then:


You don't need this if/then. This should be:

'-pins$':
  type: object
  $ref: pinmux-node.yaml#

  additionalProperties: false
  properties:
    ...

> +      properties:

> +        function:

> +          $ref: "pinmux-node.yaml#/properties/function"


Drop this as you need the $ref up a level.

> +          enum: [ serial_led_data, serial_led_clk, inet_act_led, pcie_clkreq,

> +                  led, ephy0_act_led, ephy1_act_led, ephy2_act_led,

> +                  ephy3_act_led, hsspi_cs1, usb_device_port, usb_host_port ]

> +

> +        pins:

> +          $ref: "pinmux-node.yaml#/properties/pins"

> +          enum: [ gpio6, gpio7, gpio11, gpio16, gpio17, gpio18, gpio19,

> +                  gpio20, gpio25, gpio26, gpio27, gpio28, hsspi_cs1,

> +                  usb_port1 ]

> +

> +required:

> +  - compatible

> +  - reg

> +

> +additionalProperties: false

> +

> +examples:

> +  - |

> +    pinctrl@18 {

> +      compatible = "brcm,bcm6328-pinctrl";

> +      reg = <0x18 0x10>;

> +

> +      pinctrl_serial_led: serial_led-pins {

> +        pinctrl_serial_led_data: serial_led_data-pins {

> +          function = "serial_led_data";

> +          pins = "gpio6";

> +        };

> +

> +        pinctrl_serial_led_clk: serial_led_clk-pins {

> +          function = "serial_led_clk";

> +          pins = "gpio7";

> +        };

> +      };

> +

> +      pinctrl_inet_act_led: inet_act_led-pins {

> +        function = "inet_act_led";

> +        pins = "gpio11";

> +      };

> +

> +      pinctrl_pcie_clkreq: pcie_clkreq-pins {

> +        function = "pcie_clkreq";

> +        pins = "gpio16";

> +      };

> +

> +      pinctrl_ephy0_spd_led: ephy0_spd_led-pins {

> +        function = "led";

> +        pins = "gpio17";

> +      };

> +

> +      pinctrl_ephy1_spd_led: ephy1_spd_led-pins {

> +        function = "led";

> +        pins = "gpio18";

> +      };

> +

> +      pinctrl_ephy2_spd_led: ephy2_spd_led-pins {

> +        function = "led";

> +        pins = "gpio19";

> +      };

> +

> +      pinctrl_ephy3_spd_led: ephy3_spd_led-pins {

> +        function = "led";

> +        pins = "gpio20";

> +      };

> +

> +      pinctrl_ephy0_act_led: ephy0_act_led-pins {

> +        function = "ephy0_act_led";

> +        pins = "gpio25";

> +      };

> +

> +      pinctrl_ephy1_act_led: ephy1_act_led-pins {

> +        function = "ephy1_act_led";

> +        pins = "gpio26";

> +      };

> +

> +      pinctrl_ephy2_act_led: ephy2_act_led-pins {

> +        function = "ephy2_act_led";

> +        pins = "gpio27";

> +      };

> +

> +      pinctrl_ephy3_act_led: ephy3_act_led-pins {

> +        function = "ephy3_act_led";

> +        pins = "gpio28";

> +      };

> +

> +      pinctrl_hsspi_cs1: hsspi_cs1-pins {

> +        function = "hsspi_cs1";

> +        pins = "hsspi_cs1";

> +      };

> +

> +      pinctrl_usb_port1_device: usb_port1_device-pins {

> +        function = "usb_device_port";

> +        pins = "usb_port1";

> +      };

> +

> +      pinctrl_usb_port1_host: usb_port1_host-pins {

> +        function = "usb_host_port";

> +        pins = "usb_port1";

> +      };

> +    };

> -- 

> 2.20.1

>
Rob Herring March 16, 2021, 9:20 p.m. | #3
On Mon, Mar 15, 2021 at 5:42 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>

> Add binding documentation for the GPIO sysctl found in BCM6318 SoCs.

>

> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

> ---

>  v7: add changes suggested by Rob Herring

>

>  .../mfd/brcm,bcm6318-gpio-sysctl.yaml         | 179 ++++++++++++++++++

>  1 file changed, 179 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/mfd/brcm,bcm6318-gpio-sysctl.yaml

>

> diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm6318-gpio-sysctl.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm6318-gpio-sysctl.yaml

> new file mode 100644

> index 000000000000..7056a490a27d

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/mfd/brcm,bcm6318-gpio-sysctl.yaml

> @@ -0,0 +1,179 @@

> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/mfd/brcm,bcm6318-gpio-sysctl.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +

> +title: Broadcom BCM6318 GPIO System Controller Device Tree Bindings

> +

> +maintainers:

> +  - Álvaro Fernández Rojas <noltari@gmail.com>

> +  - Jonas Gorski <jonas.gorski@gmail.com>

> +

> +description:

> +  Broadcom BCM6318 SoC GPIO system controller which provides a register map

> +  for controlling the GPIO and pins of the SoC.


Perhaps a blurb about other registers in this block. From the
registers, it looked like LED and PHY control at least.

> +

> +properties:

> +  "#address-cells": true

> +

> +  "#size-cells": true

> +

> +  compatible:

> +    items:

> +      - const: brcm,bcm6318-gpio-sysctl

> +      - const: syscon

> +      - const: simple-mfd

> +

> +  ranges:

> +    maxItems: 1

> +

> +  reg:

> +    maxItems: 1

> +

> +patternProperties:

> +  "^gpio@[0-9a-f]+$":

> +    # Child node

> +    type: object

> +    $ref: "../gpio/brcm,bcm63xx-gpio.yaml"

> +    description:

> +      GPIO controller for the SoC GPIOs. This child node definition

> +      should follow the bindings specified in

> +      Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml.

> +

> +  "^pinctrl@[0-9a-f]+$":

> +    # Child node

> +    type: object

> +    $ref: "../pinctrl/brcm,bcm6318-pinctrl.yaml"

> +    description:

> +      Pin controller for the SoC pins. This child node definition

> +      should follow the bindings specified in

> +      Documentation/devicetree/bindings/pinctrl/brcm,bcm6318-pinctrl.yaml.

> +

> +required:

> +  - "#address-cells"

> +  - compatible

> +  - ranges

> +  - reg

> +  - "#size-cells"

> +

> +additionalProperties: false

> +

> +examples:

> +  - |

> +    syscon@10000080 {

> +      #address-cells = <1>;

> +      #size-cells = <1>;

> +      compatible = "brcm,bcm6318-gpio-sysctl", "syscon", "simple-mfd";

> +      reg = <0x10000080 0x80>;

> +      ranges = <0 0x10000080 0x80>;

> +

> +      gpio@0 {

> +        compatible = "brcm,bcm6318-gpio";

> +        reg = <0x0 0x10>;

> +

> +        data = <0xc>;

> +        dirout = <0x4>;

> +

> +        gpio-controller;

> +        gpio-ranges = <&pinctrl 0 0 50>;

> +        #gpio-cells = <2>;

> +      };

> +

> +      pinctrl: pinctrl@10 {

> +        compatible = "brcm,bcm6318-pinctrl";

> +        reg = <0x18 0x10>, <0x54 0x18>;

> +

> +        pinctrl_ephy0_spd_led: ephy0_spd_led-pins {

> +          function = "ephy0_spd_led";

> +          pins = "gpio0";

> +        };

> +

> +        pinctrl_ephy1_spd_led: ephy1_spd_led-pins {

> +          function = "ephy1_spd_led";

> +          pins = "gpio1";

> +        };

> +

> +        pinctrl_ephy2_spd_led: ephy2_spd_led-pins {

> +          function = "ephy2_spd_led";

> +          pins = "gpio2";

> +        };

> +

> +        pinctrl_ephy3_spd_led: ephy3_spd_led-pins {

> +          function = "ephy3_spd_led";

> +          pins = "gpio3";

> +        };

> +

> +        pinctrl_ephy0_act_led: ephy0_act_led-pins {

> +          function = "ephy0_act_led";

> +          pins = "gpio4";

> +        };

> +

> +        pinctrl_ephy1_act_led: ephy1_act_led-pins {

> +          function = "ephy1_act_led";

> +          pins = "gpio5";

> +        };

> +

> +        pinctrl_ephy2_act_led: ephy2_act_led-pins {

> +          function = "ephy2_act_led";

> +          pins = "gpio6";

> +        };

> +

> +        pinctrl_ephy3_act_led: ephy3_act_led-pins {

> +          function = "ephy3_act_led";

> +          pins = "gpio7";

> +        };

> +

> +        pinctrl_serial_led: serial_led-pins {

> +          pinctrl_serial_led_data: serial_led_data-pins {

> +            function = "serial_led_data";

> +            pins = "gpio6";

> +          };

> +

> +          pinctrl_serial_led_clk: serial_led_clk-pins {

> +            function = "serial_led_clk";

> +            pins = "gpio7";

> +          };

> +        };

> +

> +        pinctrl_inet_act_led: inet_act_led-pins {

> +          function = "inet_act_led";

> +          pins = "gpio8";

> +        };

> +

> +        pinctrl_inet_fail_led: inet_fail_led-pins {

> +          function = "inet_fail_led";

> +          pins = "gpio9";

> +        };

> +

> +        pinctrl_dsl_led: dsl_led-pins {

> +          function = "dsl_led";

> +          pins = "gpio10";

> +        };

> +

> +        pinctrl_post_fail_led: post_fail_led-pins {

> +          function = "post_fail_led";

> +          pins = "gpio11";

> +        };

> +

> +        pinctrl_wlan_wps_led: wlan_wps_led-pins {

> +          function = "wlan_wps_led";

> +          pins = "gpio12";

> +        };

> +

> +        pinctrl_usb_pwron: usb_pwron-pins {

> +          function = "usb_pwron";

> +          pins = "gpio13";

> +        };

> +

> +        pinctrl_usb_device_led: usb_device_led-pins {

> +          function = "usb_device_led";

> +          pins = "gpio13";

> +        };

> +

> +        pinctrl_usb_active: usb_active-pins {

> +          function = "usb_active";

> +          pins = "gpio40";

> +        };

> +      };

> +    };

> --

> 2.20.1

>
Álvaro Fernández Rojas March 17, 2021, 11:20 a.m. | #4
Hi Linus,

> El 16 mar 2021, a las 11:13, Linus Walleij <linus.walleij@linaro.org> escribió:

> 

> On Mon, Mar 15, 2021 at 12:42 PM Álvaro Fernández Rojas

> <noltari@gmail.com> wrote:

> 

>> v7: introduce changes suggested by Rob Herring.

> 

> If Rob is happy with the bindings like this (GPIO as parallel node rathern

> than subnode) I am ready to merge this.


I appreciate that, but I’m having a hard time in understanding what Rob wants and since there are no examples available on most of the stuff he’s requesting this is really frustrating...

> 

> As long as the bindings are OK I am pretty sure any remaining nits can

> be fixed in-tree.

> 

> Yours,

> Linus Walleij


Best regards,
Álvaro.
Álvaro Fernández Rojas March 17, 2021, 12:54 p.m. | #5
Hi Rob,

> El 16 mar 2021, a las 21:59, Rob Herring <robh@kernel.org> escribió:
> 
> On Mon, Mar 15, 2021 at 12:41:57PM +0100, Álvaro Fernández Rojas wrote:
>> Add binding documentation for the pincontrol core found in BCM6328 SoCs.
>> 
>> Co-developed-by: Jonas Gorski <jonas.gorski@gmail.com>
>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>> ---
>> v7: add changes suggested by Rob Herring
>> v6: add changes suggested by Rob Herring
>> v5: change Documentation to dt-bindings in commit title
>> v4: no changes
>> v3: add new gpio node
>> v2: remove interrupts
>> 
>> .../pinctrl/brcm,bcm6328-pinctrl.yaml         | 132 ++++++++++++++++++
>> 1 file changed, 132 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml
>> new file mode 100644
>> index 000000000000..e1ecdc578f32
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml
>> @@ -0,0 +1,132 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pinctrl/brcm,bcm6328-pinctrl.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Broadcom BCM6328 pin controller
>> +
>> +maintainers:
>> +  - Álvaro Fernández Rojas <noltari@gmail.com>
>> +  - Jonas Gorski <jonas.gorski@gmail.com>
>> +
>> +description: |+
>> +  The pin controller node should be the child of a syscon node.
>> +
>> +  Refer to the the bindings described in
>> +  Documentation/devicetree/bindings/mfd/syscon.yaml
>> +
>> +properties:
>> +  compatible:
>> +    const: brcm,bcm6328-pinctrl
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +patternProperties:
>> +  '^.*-pins$':
> 
> '-pins$' is equivalent.
> 
>> +    if:
>> +      type: object
>> +    then:
> 
> You don't need this if/then. This should be:
> 
> '-pins$':
>  type: object
>  $ref: pinmux-node.yaml#
> 
>  additionalProperties: false
>  properties:
>    ...

If I add "additionalProperties: false" I get the following error:
/home/noltari/workspace/linux/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.example.dt.yaml: pinctrl@18: serial_led-pins: 'serial_led_clk-pins', 'serial_led_data-pins' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /home/noltari/workspace/linux/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml

> 
>> +      properties:
>> +        function:
>> +          $ref: "pinmux-node.yaml#/properties/function"
> 
> Drop this as you need the $ref up a level.
> 
>> +          enum: [ serial_led_data, serial_led_clk, inet_act_led, pcie_clkreq,
>> +                  led, ephy0_act_led, ephy1_act_led, ephy2_act_led,
>> +                  ephy3_act_led, hsspi_cs1, usb_device_port, usb_host_port ]
>> +
>> +        pins:
>> +          $ref: "pinmux-node.yaml#/properties/pins"
>> +          enum: [ gpio6, gpio7, gpio11, gpio16, gpio17, gpio18, gpio19,
>> +                  gpio20, gpio25, gpio26, gpio27, gpio28, hsspi_cs1,
>> +                  usb_port1 ]
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    pinctrl@18 {
>> +      compatible = "brcm,bcm6328-pinctrl";
>> +      reg = <0x18 0x10>;
>> +
>> +      pinctrl_serial_led: serial_led-pins {
>> +        pinctrl_serial_led_data: serial_led_data-pins {
>> +          function = "serial_led_data";
>> +          pins = "gpio6";
>> +        };
>> +
>> +        pinctrl_serial_led_clk: serial_led_clk-pins {
>> +          function = "serial_led_clk";
>> +          pins = "gpio7";
>> +        };
>> +      };
>> +
>> +      pinctrl_inet_act_led: inet_act_led-pins {
>> +        function = "inet_act_led";
>> +        pins = "gpio11";
>> +      };
>> +
>> +      pinctrl_pcie_clkreq: pcie_clkreq-pins {
>> +        function = "pcie_clkreq";
>> +        pins = "gpio16";
>> +      };
>> +
>> +      pinctrl_ephy0_spd_led: ephy0_spd_led-pins {
>> +        function = "led";
>> +        pins = "gpio17";
>> +      };
>> +
>> +      pinctrl_ephy1_spd_led: ephy1_spd_led-pins {
>> +        function = "led";
>> +        pins = "gpio18";
>> +      };
>> +
>> +      pinctrl_ephy2_spd_led: ephy2_spd_led-pins {
>> +        function = "led";
>> +        pins = "gpio19";
>> +      };
>> +
>> +      pinctrl_ephy3_spd_led: ephy3_spd_led-pins {
>> +        function = "led";
>> +        pins = "gpio20";
>> +      };
>> +
>> +      pinctrl_ephy0_act_led: ephy0_act_led-pins {
>> +        function = "ephy0_act_led";
>> +        pins = "gpio25";
>> +      };
>> +
>> +      pinctrl_ephy1_act_led: ephy1_act_led-pins {
>> +        function = "ephy1_act_led";
>> +        pins = "gpio26";
>> +      };
>> +
>> +      pinctrl_ephy2_act_led: ephy2_act_led-pins {
>> +        function = "ephy2_act_led";
>> +        pins = "gpio27";
>> +      };
>> +
>> +      pinctrl_ephy3_act_led: ephy3_act_led-pins {
>> +        function = "ephy3_act_led";
>> +        pins = "gpio28";
>> +      };
>> +
>> +      pinctrl_hsspi_cs1: hsspi_cs1-pins {
>> +        function = "hsspi_cs1";
>> +        pins = "hsspi_cs1";
>> +      };
>> +
>> +      pinctrl_usb_port1_device: usb_port1_device-pins {
>> +        function = "usb_device_port";
>> +        pins = "usb_port1";
>> +      };
>> +
>> +      pinctrl_usb_port1_host: usb_port1_host-pins {
>> +        function = "usb_host_port";
>> +        pins = "usb_port1";
>> +      };
>> +    };
>> -- 
>> 2.20.1
Linus Walleij March 20, 2021, 11:38 a.m. | #6
On Wed, Mar 17, 2021 at 12:20 PM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:

> I appreciate that, but I’m having a hard time in understanding what
> Rob wants and since there are no examples available on most of the
> stuff he’s requesting this is really frustrating...

I am sorry that the situation can be stressful.

This is not Rob's fault, at least it's also mine.

The real problem we have is lack of hardware people
reviewing hardware descriptions, to put it simple.
As reviewers we get a bit confused, then we try to make
a mind map of the hardware as most driver developers do,
but as we are not chip designers we will make
mistakes and get confused and there will be a bit
of back-and-forth and inconsistencies.

The bindings have very high ambitions (to describe all
hardware) but it's a bit like food: the less you know
about how it's produced, the better the taste.
In fact it is a best effort and involves a bit of guesswork
and group effort and you are part of the group effort
now :)

Yours,
Linus Walleij