Add device tree bindings for the gpio/pin/mux controller that is part of
the RP1 multi function device, and relative entries in MAINTAINERS file.
Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
---
.../pinctrl/raspberrypi,rp1-gpio.yaml | 163 ++++++++++++++++++
MAINTAINERS | 2 +
2 files changed, 165 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
diff --git a/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
new file mode 100644
index 000000000000..465a53a6d84f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
@@ -0,0 +1,163 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/raspberrypi,rp1-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RaspberryPi RP1 GPIO/Pinconf/Pinmux Controller submodule
+
+maintainers:
+ - Andrea della Porta <andrea.porta@suse.com>
+
+description:
+ The RP1 chipset is a Multi Function Device containing, among other sub-peripherals,
+ a gpio/pinconf/mux controller whose 54 pins are grouped into 3 banks. It works also
+ as an interrupt controller for those gpios.
+
+properties:
+ compatible:
+ const: raspberrypi,rp1-gpio
+
+ reg:
+ maxItems: 3
+ description: One reg specifier for each one of the 3 pin banks.
+
+ '#gpio-cells':
+ description: The first cell is the pin number and the second cell is used
+ to specify the flags (see include/dt-bindings/gpio/gpio.h).
+ const: 2
+
+ gpio-controller: true
+
+ gpio-ranges:
+ maxItems: 1
+
+ gpio-line-names:
+ maxItems: 54
+
+ interrupts:
+ maxItems: 3
+ description: One interrupt specifier for each one of the 3 pin banks.
+
+ '#interrupt-cells':
+ description:
+ Specifies the Bank number [0, 1, 2] and Flags as defined in
+ include/dt-bindings/interrupt-controller/irq.h.
+ const: 2
+
+ interrupt-controller: true
+
+additionalProperties:
+ anyOf:
+ - type: object
+ additionalProperties: false
+ allOf:
+ - $ref: pincfg-node.yaml#
+ - $ref: pinmux-node.yaml#
+
+ description:
+ Pin controller client devices use pin configuration subnodes (children
+ and grandchildren) for desired pin configuration.
+ Client device subnodes use below standard properties.
+
+ properties:
+ pins:
+ description:
+ A string (or list of strings) adhering to the pattern 'gpio[0-5][0-9]'
+ function: true
+ bias-disable: true
+ bias-pull-down: true
+ bias-pull-up: true
+ slew-rate:
+ description: 0 is slow slew rate, 1 is fast slew rate
+ enum: [ 0, 1 ]
+ drive-strength:
+ enum: [ 2, 4, 8, 12 ]
+
+ - type: object
+ additionalProperties:
+ $ref: "#/additionalProperties/anyOf/0"
+
+allOf:
+ - $ref: pinctrl.yaml#
+
+required:
+ - reg
+ - compatible
+ - '#gpio-cells'
+ - gpio-controller
+ - interrupts
+ - '#interrupt-cells'
+ - interrupt-controller
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ rp1 {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ rp1_gpio: pinctrl@c0400d0000 {
+ reg = <0xc0 0x400d0000 0x0 0xc000>,
+ <0xc0 0x400e0000 0x0 0xc000>,
+ <0xc0 0x400f0000 0x0 0xc000>;
+ compatible = "raspberrypi,rp1-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>,
+ <1 IRQ_TYPE_LEVEL_HIGH>,
+ <2 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-line-names =
+ "ID_SDA", // GPIO0
+ "ID_SCL", // GPIO1
+ "GPIO2", "GPIO3", "GPIO4", "GPIO5", "GPIO6",
+ "GPIO7", "GPIO8", "GPIO9", "GPIO10", "GPIO11",
+ "GPIO12", "GPIO13", "GPIO14", "GPIO15", "GPIO16",
+ "GPIO17", "GPIO18", "GPIO19", "GPIO20", "GPIO21",
+ "GPIO22", "GPIO23", "GPIO24", "GPIO25", "GPIO26",
+ "GPIO27",
+ "PCIE_RP1_WAKE", // GPIO28
+ "FAN_TACH", // GPIO29
+ "HOST_SDA", // GPIO30
+ "HOST_SCL", // GPIO31
+ "ETH_RST_N", // GPIO32
+ "", // GPIO33
+ "CD0_IO0_MICCLK", // GPIO34
+ "CD0_IO0_MICDAT0", // GPIO35
+ "RP1_PCIE_CLKREQ_N", // GPIO36
+ "", // GPIO37
+ "CD0_SDA", // GPIO38
+ "CD0_SCL", // GPIO39
+ "CD1_SDA", // GPIO40
+ "CD1_SCL", // GPIO41
+ "USB_VBUS_EN", // GPIO42
+ "USB_OC_N", // GPIO43
+ "RP1_STAT_LED", // GPIO44
+ "FAN_PWM", // GPIO45
+ "CD1_IO0_MICCLK", // GPIO46
+ "2712_WAKE", // GPIO47
+ "CD1_IO1_MICDAT1", // GPIO48
+ "EN_MAX_USB_CUR", // GPIO49
+ "", // GPIO50
+ "", // GPIO51
+ "", // GPIO52
+ ""; // GPIO53
+
+ rp1-uart0-14-15 {
+ pin_txd {
+ function = "uart0";
+ pins = "gpio14";
+ bias-disable;
+ };
+
+ pin_rxd {
+ function = "uart0";
+ pins = "gpio15";
+ bias-pull-up;
+ };
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 75a66e3e34c9..c55d12550246 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19384,7 +19384,9 @@ RASPBERRY PI RP1 PCI DRIVER
M: Andrea della Porta <andrea.porta@suse.com>
S: Maintained
F: Documentation/devicetree/bindings/clock/raspberrypi,rp1-clocks.yaml
+F: Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
F: include/dt-bindings/clock/rp1.h
+F: include/dt-bindings/misc/rp1.h
RC-CORE / LIRC FRAMEWORK
M: Sean Young <sean@mess.org>
--
2.35.3On Mon, Oct 28, 2024 at 03:07:19PM +0100, Andrea della Porta wrote: > Add device tree bindings for the gpio/pin/mux controller that is part of > the RP1 multi function device, and relative entries in MAINTAINERS file. > > Signed-off-by: Andrea della Porta <andrea.porta@suse.com> > --- > .../pinctrl/raspberrypi,rp1-gpio.yaml | 163 ++++++++++++++++++ > MAINTAINERS | 2 + > 2 files changed, 165 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml > > diff --git a/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml > new file mode 100644 > index 000000000000..465a53a6d84f > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml > @@ -0,0 +1,163 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/raspberrypi,rp1-gpio.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: RaspberryPi RP1 GPIO/Pinconf/Pinmux Controller submodule > + > +maintainers: > + - Andrea della Porta <andrea.porta@suse.com> > + > +description: > + The RP1 chipset is a Multi Function Device containing, among other sub-peripherals, > + a gpio/pinconf/mux controller whose 54 pins are grouped into 3 banks. It works also Please wrap code according to coding style (checkpatch is not a coding style description but only a tool). > + as an interrupt controller for those gpios. > + > +properties: > + compatible: > + const: raspberrypi,rp1-gpio > + > + reg: > + maxItems: 3 > + description: One reg specifier for each one of the 3 pin banks. > + > + '#gpio-cells': > + description: The first cell is the pin number and the second cell is used > + to specify the flags (see include/dt-bindings/gpio/gpio.h). > + const: 2 > + > + gpio-controller: true > + > + gpio-ranges: > + maxItems: 1 > + > + gpio-line-names: > + maxItems: 54 > + > + interrupts: > + maxItems: 3 > + description: One interrupt specifier for each one of the 3 pin banks. > + > + '#interrupt-cells': > + description: > + Specifies the Bank number [0, 1, 2] and Flags as defined in > + include/dt-bindings/interrupt-controller/irq.h. > + const: 2 > + > + interrupt-controller: true > + > +additionalProperties: Not much improved. You are supposed to have here pattern, just like other bindings. I asked for this last time. And there are examples using it - almost all or most of pinctrl bindings, including bindings having subnodes (but you do not use such case here). > + anyOf: > + - type: object > + additionalProperties: false > + allOf: > + - $ref: pincfg-node.yaml# > + - $ref: pinmux-node.yaml# > + > + description: > + Pin controller client devices use pin configuration subnodes (children > + and grandchildren) for desired pin configuration. > + Client device subnodes use below standard properties. > + > + properties: > + pins: > + description: > + A string (or list of strings) adhering to the pattern 'gpio[0-5][0-9]' > + function: true > + bias-disable: true > + bias-pull-down: true > + bias-pull-up: true > + slew-rate: > + description: 0 is slow slew rate, 1 is fast slew rate > + enum: [ 0, 1 ] > + drive-strength: > + enum: [ 2, 4, 8, 12 ] > + > + - type: object > + additionalProperties: > + $ref: "#/additionalProperties/anyOf/0" Your example does not use any subnodes, so this looks not needed. Best regards, Krzysztof
Hi Krzysztof,
On 08:26 Tue 29 Oct , Krzysztof Kozlowski wrote:
> On Mon, Oct 28, 2024 at 03:07:19PM +0100, Andrea della Porta wrote:
> > Add device tree bindings for the gpio/pin/mux controller that is part of
> > the RP1 multi function device, and relative entries in MAINTAINERS file.
> >
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > ---
> > .../pinctrl/raspberrypi,rp1-gpio.yaml | 163 ++++++++++++++++++
> > MAINTAINERS | 2 +
> > 2 files changed, 165 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
> > new file mode 100644
> > index 000000000000..465a53a6d84f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
> > @@ -0,0 +1,163 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/raspberrypi,rp1-gpio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: RaspberryPi RP1 GPIO/Pinconf/Pinmux Controller submodule
> > +
> > +maintainers:
> > + - Andrea della Porta <andrea.porta@suse.com>
> > +
> > +description:
> > + The RP1 chipset is a Multi Function Device containing, among other sub-peripherals,
> > + a gpio/pinconf/mux controller whose 54 pins are grouped into 3 banks. It works also
>
> Please wrap code according to coding style (checkpatch is not a coding
> style description but only a tool).
Ack.
>
> > + as an interrupt controller for those gpios.
> > +
> > +properties:
> > + compatible:
> > + const: raspberrypi,rp1-gpio
> > +
> > + reg:
> > + maxItems: 3
> > + description: One reg specifier for each one of the 3 pin banks.
> > +
> > + '#gpio-cells':
> > + description: The first cell is the pin number and the second cell is used
> > + to specify the flags (see include/dt-bindings/gpio/gpio.h).
> > + const: 2
> > +
> > + gpio-controller: true
> > +
> > + gpio-ranges:
> > + maxItems: 1
> > +
> > + gpio-line-names:
> > + maxItems: 54
> > +
> > + interrupts:
> > + maxItems: 3
> > + description: One interrupt specifier for each one of the 3 pin banks.
> > +
> > + '#interrupt-cells':
> > + description:
> > + Specifies the Bank number [0, 1, 2] and Flags as defined in
> > + include/dt-bindings/interrupt-controller/irq.h.
> > + const: 2
> > +
> > + interrupt-controller: true
> > +
> > +additionalProperties:
>
> Not much improved. You are supposed to have here pattern, just like
> other bindings. I asked for this last time.
>
> And there are examples using it - almost all or most of pinctrl
> bindings, including bindings having subnodes (but you do not use such
> case here).
This is the same approach used in [1], which seems quite recent. I did't
use pattern because I wouldn't really want to enforce a particular naming
scheme. Subnodes are used, please see below. Since pinctrl.yaml explicitly
says that there is no common binding but each device has its own, I
thought that was reasonable choice. Should I enforce some common pattern,
then?
>
> > + anyOf:
> > + - type: object
> > + additionalProperties: false
> > + allOf:
> > + - $ref: pincfg-node.yaml#
> > + - $ref: pinmux-node.yaml#
> > +
> > + description:
> > + Pin controller client devices use pin configuration subnodes (children
> > + and grandchildren) for desired pin configuration.
> > + Client device subnodes use below standard properties.
> > +
> > + properties:
> > + pins:
> > + description:
> > + A string (or list of strings) adhering to the pattern 'gpio[0-5][0-9]'
> > + function: true
> > + bias-disable: true
> > + bias-pull-down: true
> > + bias-pull-up: true
> > + slew-rate:
> > + description: 0 is slow slew rate, 1 is fast slew rate
> > + enum: [ 0, 1 ]
> > + drive-strength:
> > + enum: [ 2, 4, 8, 12 ]
> > +
> > + - type: object
> > + additionalProperties:
> > + $ref: "#/additionalProperties/anyOf/0"
>
> Your example does not use any subnodes, so this looks not needed.
The example has subnodes, as in the following excerpt from the example:
rp1-uart0-14-15 {
pin_txd {
function = "uart0";
pins = "gpio14";
bias-disable;
};
pin_rxd {
function = "uart0";
pins = "gpio15";
bias-pull-up;
};
};
Many thanks,
Andrea
[1] - Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
>
> Best regards,
> Krzysztof
>
On 31/10/2024 15:07, Andrea della Porta wrote: > Hi Krzysztof, > > On 08:26 Tue 29 Oct , Krzysztof Kozlowski wrote: >> On Mon, Oct 28, 2024 at 03:07:19PM +0100, Andrea della Porta wrote: >>> Add device tree bindings for the gpio/pin/mux controller that is part of >>> the RP1 multi function device, and relative entries in MAINTAINERS file. >>> >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com> >>> --- >>> .../pinctrl/raspberrypi,rp1-gpio.yaml | 163 ++++++++++++++++++ >>> MAINTAINERS | 2 + >>> 2 files changed, 165 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml >>> new file mode 100644 >>> index 000000000000..465a53a6d84f >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml >>> @@ -0,0 +1,163 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/pinctrl/raspberrypi,rp1-gpio.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: RaspberryPi RP1 GPIO/Pinconf/Pinmux Controller submodule >>> + >>> +maintainers: >>> + - Andrea della Porta <andrea.porta@suse.com> >>> + >>> +description: >>> + The RP1 chipset is a Multi Function Device containing, among other sub-peripherals, >>> + a gpio/pinconf/mux controller whose 54 pins are grouped into 3 banks. It works also >> >> Please wrap code according to coding style (checkpatch is not a coding >> style description but only a tool). > > Ack. > >> >>> + as an interrupt controller for those gpios. >>> + >>> +properties: >>> + compatible: >>> + const: raspberrypi,rp1-gpio >>> + >>> + reg: >>> + maxItems: 3 >>> + description: One reg specifier for each one of the 3 pin banks. >>> + >>> + '#gpio-cells': >>> + description: The first cell is the pin number and the second cell is used >>> + to specify the flags (see include/dt-bindings/gpio/gpio.h). >>> + const: 2 >>> + >>> + gpio-controller: true >>> + >>> + gpio-ranges: >>> + maxItems: 1 >>> + >>> + gpio-line-names: >>> + maxItems: 54 >>> + >>> + interrupts: >>> + maxItems: 3 >>> + description: One interrupt specifier for each one of the 3 pin banks. >>> + >>> + '#interrupt-cells': >>> + description: >>> + Specifies the Bank number [0, 1, 2] and Flags as defined in >>> + include/dt-bindings/interrupt-controller/irq.h. >>> + const: 2 >>> + >>> + interrupt-controller: true >>> + >>> +additionalProperties: >> >> Not much improved. You are supposed to have here pattern, just like >> other bindings. I asked for this last time. >> >> And there are examples using it - almost all or most of pinctrl >> bindings, including bindings having subnodes (but you do not use such >> case here). > > This is the same approach used in [1], which seems quite recent. I did't 2021, so not that recent, but you are right that it's not the example I would recommend. See rather: git grep pins -- Documentation/devicetree/bindings/pinctrl/ | grep '\$' pins, groups, states, etc. > use pattern because I wouldn't really want to enforce a particular naming > scheme. Subnodes are used, please see below. Since pinctrl.yaml explicitly But we want to enforce, because it brings uniformity and matches partially generic naming patterns. > says that there is no common binding but each device has its own, I > thought that was reasonable choice. Should I enforce some common pattern, > then? Yes, you should. Again, look at other bindings, e.g. qcom tlmm or lpass lpi. > >> >>> + anyOf: >>> + - type: object >>> + additionalProperties: false >>> + allOf: >>> + - $ref: pincfg-node.yaml# >>> + - $ref: pinmux-node.yaml# >>> + >>> + description: >>> + Pin controller client devices use pin configuration subnodes (children >>> + and grandchildren) for desired pin configuration. >>> + Client device subnodes use below standard properties. >>> + >>> + properties: >>> + pins: >>> + description: >>> + A string (or list of strings) adhering to the pattern 'gpio[0-5][0-9]' >>> + function: true >>> + bias-disable: true >>> + bias-pull-down: true >>> + bias-pull-up: true >>> + slew-rate: >>> + description: 0 is slow slew rate, 1 is fast slew rate >>> + enum: [ 0, 1 ] >>> + drive-strength: >>> + enum: [ 2, 4, 8, 12 ] >>> + >>> + - type: object >>> + additionalProperties: >>> + $ref: "#/additionalProperties/anyOf/0" >> >> Your example does not use any subnodes, so this looks not needed. > > The example has subnodes, as in the following excerpt from the example: I meant, you do not need properties in subnodes (1st level). You only want properties in subnodes of subnodes, so 2nd level. What is the point of allowing them in 1st level? Best regards, Krzysztof
Hi Krzysztof,
On 19:10 Thu 31 Oct , Krzysztof Kozlowski wrote:
> On 31/10/2024 15:07, Andrea della Porta wrote:
> > Hi Krzysztof,
> >
> > On 08:26 Tue 29 Oct , Krzysztof Kozlowski wrote:
> >> On Mon, Oct 28, 2024 at 03:07:19PM +0100, Andrea della Porta wrote:
> >>> Add device tree bindings for the gpio/pin/mux controller that is part of
> >>> the RP1 multi function device, and relative entries in MAINTAINERS file.
> >>>
> >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> >>> ---
> >>> .../pinctrl/raspberrypi,rp1-gpio.yaml | 163 ++++++++++++++++++
> >>> MAINTAINERS | 2 +
> >>> 2 files changed, 165 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
> >>> new file mode 100644
> >>> index 000000000000..465a53a6d84f
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
> >>> @@ -0,0 +1,163 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/pinctrl/raspberrypi,rp1-gpio.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: RaspberryPi RP1 GPIO/Pinconf/Pinmux Controller submodule
> >>> +
> >>> +maintainers:
> >>> + - Andrea della Porta <andrea.porta@suse.com>
> >>> +
> >>> +description:
> >>> + The RP1 chipset is a Multi Function Device containing, among other sub-peripherals,
> >>> + a gpio/pinconf/mux controller whose 54 pins are grouped into 3 banks. It works also
> >>
> >> Please wrap code according to coding style (checkpatch is not a coding
> >> style description but only a tool).
> >
> > Ack.
> >
> >>
> >>> + as an interrupt controller for those gpios.
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + const: raspberrypi,rp1-gpio
> >>> +
> >>> + reg:
> >>> + maxItems: 3
> >>> + description: One reg specifier for each one of the 3 pin banks.
> >>> +
> >>> + '#gpio-cells':
> >>> + description: The first cell is the pin number and the second cell is used
> >>> + to specify the flags (see include/dt-bindings/gpio/gpio.h).
> >>> + const: 2
> >>> +
> >>> + gpio-controller: true
> >>> +
> >>> + gpio-ranges:
> >>> + maxItems: 1
> >>> +
> >>> + gpio-line-names:
> >>> + maxItems: 54
> >>> +
> >>> + interrupts:
> >>> + maxItems: 3
> >>> + description: One interrupt specifier for each one of the 3 pin banks.
> >>> +
> >>> + '#interrupt-cells':
> >>> + description:
> >>> + Specifies the Bank number [0, 1, 2] and Flags as defined in
> >>> + include/dt-bindings/interrupt-controller/irq.h.
> >>> + const: 2
> >>> +
> >>> + interrupt-controller: true
> >>> +
> >>> +additionalProperties:
> >>
> >> Not much improved. You are supposed to have here pattern, just like
> >> other bindings. I asked for this last time.
> >>
> >> And there are examples using it - almost all or most of pinctrl
> >> bindings, including bindings having subnodes (but you do not use such
> >> case here).
> >
> > This is the same approach used in [1], which seems quite recent. I did't
>
> 2021, so not that recent, but you are right that it's not the example I
> would recommend. See rather:
> git grep pins -- Documentation/devicetree/bindings/pinctrl/ | grep '\$'
>
>
> pins, groups, states, etc.
Perfect. Thanks for the example suggestion.
>
> > use pattern because I wouldn't really want to enforce a particular naming
> > scheme. Subnodes are used, please see below. Since pinctrl.yaml explicitly
>
> But we want to enforce, because it brings uniformity and matches
> partially generic naming patterns.
Ack.
>
> > says that there is no common binding but each device has its own, I
> > thought that was reasonable choice. Should I enforce some common pattern,
> > then?
>
> Yes, you should. Again, look at other bindings, e.g. qcom tlmm or lpass lpi.
Ok.
>
> >
> >>
> >>> + anyOf:
> >>> + - type: object
> >>> + additionalProperties: false
> >>> + allOf:
> >>> + - $ref: pincfg-node.yaml#
> >>> + - $ref: pinmux-node.yaml#
> >>> +
> >>> + description:
> >>> + Pin controller client devices use pin configuration subnodes (children
> >>> + and grandchildren) for desired pin configuration.
> >>> + Client device subnodes use below standard properties.
> >>> +
> >>> + properties:
> >>> + pins:
> >>> + description:
> >>> + A string (or list of strings) adhering to the pattern 'gpio[0-5][0-9]'
> >>> + function: true
> >>> + bias-disable: true
> >>> + bias-pull-down: true
> >>> + bias-pull-up: true
> >>> + slew-rate:
> >>> + description: 0 is slow slew rate, 1 is fast slew rate
> >>> + enum: [ 0, 1 ]
> >>> + drive-strength:
> >>> + enum: [ 2, 4, 8, 12 ]
> >>> +
> >>> + - type: object
> >>> + additionalProperties:
> >>> + $ref: "#/additionalProperties/anyOf/0"
> >>
> >> Your example does not use any subnodes, so this looks not needed.
> >
> > The example has subnodes, as in the following excerpt from the example:
>
> I meant, you do not need properties in subnodes (1st level). You only
> want properties in subnodes of subnodes, so 2nd level. What is the point
> of allowing them in 1st level?
I will add those two sub-nodes to the example:
rp1-i2s0-default-state {
function = "i2s0";
pins = "gpio18", "gpio19", "gpio20", "gpio21";
bias-disable;
};
rp1-uart0-default-state {
txd-pins {
function = "uart0";
pins = "gpio14";
bias-disable;
};
rxd-pins {
function = "uart0";
pins = "gpio15";
bias-pull-up;
};
};
The first is just a group of pins with the same settings, the second is
a pin group with different settings per pin. This is basically the same
usage as in qcom,sm4250-lpass-lpi-pinctrl.yaml.
Many thanks,
Andrea
>
>
>
> Best regards,
> Krzysztof
>
On 04/11/2024 12:11, Andrea della Porta wrote:
>>>>
>>>> Your example does not use any subnodes, so this looks not needed.
>>>
>>> The example has subnodes, as in the following excerpt from the example:
>>
>> I meant, you do not need properties in subnodes (1st level). You only
>> want properties in subnodes of subnodes, so 2nd level. What is the point
>> of allowing them in 1st level?
>
> I will add those two sub-nodes to the example:
>
> rp1-i2s0-default-state {
> function = "i2s0";
> pins = "gpio18", "gpio19", "gpio20", "gpio21";
> bias-disable;
> };
>
> rp1-uart0-default-state {
> txd-pins {
> function = "uart0";
> pins = "gpio14";
> bias-disable;
> };
>
> rxd-pins {
> function = "uart0";
> pins = "gpio15";
> bias-pull-up;
> };
> };
>
> The first is just a group of pins with the same settings, the second is
> a pin group with different settings per pin. This is basically the same
> usage as in qcom,sm4250-lpass-lpi-pinctrl.yaml.
>
Ack, that's ok then.
Best regards,
Krzysztof
© 2016 - 2024 Red Hat, Inc.