diff mbox series

[1/3] dt-bindings: Add support for Broadcom USB pin map driver

Message ID 20200812202018.49046-2-alcooperx@gmail.com
State Changes Requested, archived
Headers show
Series usb: Add driver for USB signal re-mapper | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema success

Commit Message

Alan Cooper Aug. 12, 2020, 8:20 p.m. UTC
Add DT bindings for the Broadcom USB pin map driver. This driver allows
some USB input and output signals to be mapped to any GPIO instead
of the normal dedicated pins to/from the XHCI controller.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 .../bindings/usb/brcm,usb-pinmap.yaml         | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml

Comments

Rob Herring Aug. 24, 2020, 11:30 p.m. UTC | #1
On Wed, Aug 12, 2020 at 04:20:16PM -0400, Al Cooper wrote:
> Add DT bindings for the Broadcom USB pin map driver. This driver allows
> some USB input and output signals to be mapped to any GPIO instead
> of the normal dedicated pins to/from the XHCI controller.

Is this a driver or h/w block because bindings are for h/w blocks?

> 
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
> ---
>  .../bindings/usb/brcm,usb-pinmap.yaml         | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> new file mode 100644
> index 000000000000..19cf6ad36373
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/brcm,usb-pinmap.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom USB pin map Controller Device Tree Bindings
> +
> +maintainers:
> +  - Al Cooper <alcooperx@gmail.com>
> +
> +properties:
> +  compatible:
> +      items:
> +          - const: brcm,usb-pinmap

2 space indentation please.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    description: Must be defined if any out-gpios are specified.

'dependencies' can express this in schema.

> +
> +  in-gpios:
> +    description: Array of one or more GPIO pins used for input signals.

You need to define how many GPIOs are valid.

> +
> +  in-names:
> +    description: Array of input signal names, one per gpio in in-gpios.

No, this isn't how we name GPIOs. The part before '-gpios' is how.

> +
> +  in-masks:
> +    description: Array of enable and mask pairs, one per gpio in-gpios.

Needs a vendor prefix.

> +
> +  out-gpios:
> +    description: Array of one or more GPIO pins used for output signals.
> +
> +  out-names:
> +    description: Array of output signal names, one per gpio in out-gpios.
> +
> +  out-masks:
> +    description: Array of enable, value, changed and clear masks, one
> +      per gpio in out-gpios.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    usb_pinmap: usb-pinmap@22000d0 {
> +        compatible = "brcm,usb-pinmap";
> +        reg = <0x22000d0 0x4>;
> +        in-gpios = <&gpio 18 0>, <&gpio 19 0>;
> +        in-names = "VBUS", "PWRFLT";
> +        in-masks = <0x8000 0x40000 0x10000 0x80000>;
> +        out-gpios = <&gpio 20 0>;
> +        out-names = "PWRON";
> +        out-masks = <0x20000 0x800000 0x400000 0x200000>;
> +        interrupts = <0x0 0xb2 0x4>;
> +    };
> +
> +...
> -- 
> 2.17.1
>
Alan Cooper Aug. 25, 2020, 12:26 p.m. UTC | #2
On Mon, Aug 24, 2020 at 7:30 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Aug 12, 2020 at 04:20:16PM -0400, Al Cooper wrote:
> > Add DT bindings for the Broadcom USB pin map driver. This driver allows
> > some USB input and output signals to be mapped to any GPIO instead
> > of the normal dedicated pins to/from the XHCI controller.
>
> Is this a driver or h/w block because bindings are for h/w blocks?

This is a hardware block. I'll remove "driver" from the description.

>
> >
> > Signed-off-by: Al Cooper <alcooperx@gmail.com>
> > ---
> >  .../bindings/usb/brcm,usb-pinmap.yaml         | 63 +++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> > new file mode 100644
> > index 000000000000..19cf6ad36373
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> > @@ -0,0 +1,63 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/brcm,usb-pinmap.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom USB pin map Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Al Cooper <alcooperx@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +      items:
> > +          - const: brcm,usb-pinmap
>
> 2 space indentation please.

Fixed.

>
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +    description: Must be defined if any out-gpios are specified.
>
> 'dependencies' can express this in schema.

Okay.

>
> > +
> > +  in-gpios:
> > +    description: Array of one or more GPIO pins used for input signals.
>
> You need to define how many GPIOs are valid.

I tried to avoid doing this because there is a possibility that future
chips will have a few more signals added and the driver was written so
new signals can be added entirely in device tree without any changes
to the driver. If this is unacceptable, I can add the current max in
and out valid gpios.

>
> > +
> > +  in-names:
> > +    description: Array of input signal names, one per gpio in in-gpios.
>
> No, this isn't how we name GPIOs. The part before '-gpios' is how.

This is the meant to be a description of how each gpio is being used
to help with error messages in the driver.
What if I use "brcmstb,in-functions" instead?

>
> > +
> > +  in-masks:
> > +    description: Array of enable and mask pairs, one per gpio in-gpios.
>
> Needs a vendor prefix.

I'll change it to "brcmstb,in-masks"

>
> > +
> > +  out-gpios:
> > +    description: Array of one or more GPIO pins used for output signals.
> > +
> > +  out-names:
> > +    description: Array of output signal names, one per gpio in out-gpios.
> > +
> > +  out-masks:
> > +    description: Array of enable, value, changed and clear masks, one
> > +      per gpio in out-gpios.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    usb_pinmap: usb-pinmap@22000d0 {
> > +        compatible = "brcm,usb-pinmap";
> > +        reg = <0x22000d0 0x4>;
> > +        in-gpios = <&gpio 18 0>, <&gpio 19 0>;
> > +        in-names = "VBUS", "PWRFLT";
> > +        in-masks = <0x8000 0x40000 0x10000 0x80000>;
> > +        out-gpios = <&gpio 20 0>;
> > +        out-names = "PWRON";
> > +        out-masks = <0x20000 0x800000 0x400000 0x200000>;
> > +        interrupts = <0x0 0xb2 0x4>;
> > +    };
> > +
> > +...
> > --
> > 2.17.1
> >
Rob Herring Aug. 25, 2020, 3:46 p.m. UTC | #3
+Linus W

On Tue, Aug 25, 2020 at 6:26 AM Alan Cooper <alcooperx@gmail.com> wrote:
>
> On Mon, Aug 24, 2020 at 7:30 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Aug 12, 2020 at 04:20:16PM -0400, Al Cooper wrote:
> > > Add DT bindings for the Broadcom USB pin map driver. This driver allows
> > > some USB input and output signals to be mapped to any GPIO instead
> > > of the normal dedicated pins to/from the XHCI controller.
> >
> > Is this a driver or h/w block because bindings are for h/w blocks?
>
> This is a hardware block. I'll remove "driver" from the description.

Another question, this kind of looks like a pin mux controller. Is
that not a fit for this? If not, why?

> > > Signed-off-by: Al Cooper <alcooperx@gmail.com>
> > > ---
> > >  .../bindings/usb/brcm,usb-pinmap.yaml         | 63 +++++++++++++++++++
> > >  1 file changed, 63 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> > > new file mode 100644
> > > index 000000000000..19cf6ad36373
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> > > @@ -0,0 +1,63 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/usb/brcm,usb-pinmap.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Broadcom USB pin map Controller Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Al Cooper <alcooperx@gmail.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +      items:
> > > +          - const: brcm,usb-pinmap
> >
> > 2 space indentation please.
>
> Fixed.
>
> >
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +    description: Must be defined if any out-gpios are specified.
> >
> > 'dependencies' can express this in schema.
>
> Okay.
>
> >
> > > +
> > > +  in-gpios:
> > > +    description: Array of one or more GPIO pins used for input signals.
> >
> > You need to define how many GPIOs are valid.
>
> I tried to avoid doing this because there is a possibility that future
> chips will have a few more signals added and the driver was written so
> new signals can be added entirely in device tree without any changes
> to the driver. If this is unacceptable, I can add the current max in
> and out valid gpios.

A 'should be enough for a while' value is fine. The driver doesn't
have to have a max. I'd expect the binding to be updated for new SoCs
anyways.

> >
> > > +
> > > +  in-names:
> > > +    description: Array of input signal names, one per gpio in in-gpios.
> >
> > No, this isn't how we name GPIOs. The part before '-gpios' is how.
>
> This is the meant to be a description of how each gpio is being used
> to help with error messages in the driver.
> What if I use "brcmstb,in-functions" instead?

'brcmstb' is not a vendor. But brcm,in-functions is fine.

> > > +
> > > +  in-masks:
> > > +    description: Array of enable and mask pairs, one per gpio in-gpios.
> >
> > Needs a vendor prefix.
>
> I'll change it to "brcmstb,in-masks"
>
> >
> > > +
> > > +  out-gpios:
> > > +    description: Array of one or more GPIO pins used for output signals.
> > > +
> > > +  out-names:
> > > +    description: Array of output signal names, one per gpio in out-gpios.
> > > +
> > > +  out-masks:
> > > +    description: Array of enable, value, changed and clear masks, one
> > > +      per gpio in out-gpios.
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    usb_pinmap: usb-pinmap@22000d0 {
> > > +        compatible = "brcm,usb-pinmap";
> > > +        reg = <0x22000d0 0x4>;
> > > +        in-gpios = <&gpio 18 0>, <&gpio 19 0>;
> > > +        in-names = "VBUS", "PWRFLT";
> > > +        in-masks = <0x8000 0x40000 0x10000 0x80000>;
> > > +        out-gpios = <&gpio 20 0>;
> > > +        out-names = "PWRON";
> > > +        out-masks = <0x20000 0x800000 0x400000 0x200000>;
> > > +        interrupts = <0x0 0xb2 0x4>;
> > > +    };
> > > +
> > > +...
> > > --
> > > 2.17.1
> > >
Alan Cooper Aug. 26, 2020, 4 p.m. UTC | #4
On Tue, Aug 25, 2020 at 11:46 AM Rob Herring <robh@kernel.org> wrote:
>
> +Linus W
>
> On Tue, Aug 25, 2020 at 6:26 AM Alan Cooper <alcooperx@gmail.com> wrote:
> >
> > On Mon, Aug 24, 2020 at 7:30 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Wed, Aug 12, 2020 at 04:20:16PM -0400, Al Cooper wrote:
> > > > Add DT bindings for the Broadcom USB pin map driver. This driver allows
> > > > some USB input and output signals to be mapped to any GPIO instead
> > > > of the normal dedicated pins to/from the XHCI controller.
> > >
> > > Is this a driver or h/w block because bindings are for h/w blocks?
> >
> > This is a hardware block. I'll remove "driver" from the description.
>
> Another question, this kind of looks like a pin mux controller. Is
> that not a fit for this? If not, why?

This driver is not doing any pin-muxing of a physical pin on the chip.
Instead it's using standard gpio's, through gpiolib, and propagating
the gpio state for in-coming signals to a special register that feeds
into a XHCI host controller register and it's propagating the state of
out-going signals from the special register fed by a XHCI controller
register to a gpio. Both directions are interrupt driven and
continually mirroring the state between the XHCI host controller
registers and the gpios. I don't see any pinmux/pinctrl driver doing
this kind of thing.

Thanks
Al

>
> > > > Signed-off-by: Al Cooper <alcooperx@gmail.com>
> > > > ---
> > > >  .../bindings/usb/brcm,usb-pinmap.yaml         | 63 +++++++++++++++++++
> > > >  1 file changed, 63 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> > > > new file mode 100644
> > > > index 000000000000..19cf6ad36373
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
> > > > @@ -0,0 +1,63 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/usb/brcm,usb-pinmap.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Broadcom USB pin map Controller Device Tree Bindings
> > > > +
> > > > +maintainers:
> > > > +  - Al Cooper <alcooperx@gmail.com>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +      items:
> > > > +          - const: brcm,usb-pinmap
> > >
> > > 2 space indentation please.
> >
> > Fixed.
> >
> > >
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 1
> > > > +    description: Must be defined if any out-gpios are specified.
> > >
> > > 'dependencies' can express this in schema.
> >
> > Okay.
> >
> > >
> > > > +
> > > > +  in-gpios:
> > > > +    description: Array of one or more GPIO pins used for input signals.
> > >
> > > You need to define how many GPIOs are valid.
> >
> > I tried to avoid doing this because there is a possibility that future
> > chips will have a few more signals added and the driver was written so
> > new signals can be added entirely in device tree without any changes
> > to the driver. If this is unacceptable, I can add the current max in
> > and out valid gpios.
>
> A 'should be enough for a while' value is fine. The driver doesn't
> have to have a max. I'd expect the binding to be updated for new SoCs
> anyways.
>
> > >
> > > > +
> > > > +  in-names:
> > > > +    description: Array of input signal names, one per gpio in in-gpios.
> > >
> > > No, this isn't how we name GPIOs. The part before '-gpios' is how.
> >
> > This is the meant to be a description of how each gpio is being used
> > to help with error messages in the driver.
> > What if I use "brcmstb,in-functions" instead?
>
> 'brcmstb' is not a vendor. But brcm,in-functions is fine.
>
> > > > +
> > > > +  in-masks:
> > > > +    description: Array of enable and mask pairs, one per gpio in-gpios.
> > >
> > > Needs a vendor prefix.
> >
> > I'll change it to "brcmstb,in-masks"
> >
> > >
> > > > +
> > > > +  out-gpios:
> > > > +    description: Array of one or more GPIO pins used for output signals.
> > > > +
> > > > +  out-names:
> > > > +    description: Array of output signal names, one per gpio in out-gpios.
> > > > +
> > > > +  out-masks:
> > > > +    description: Array of enable, value, changed and clear masks, one
> > > > +      per gpio in out-gpios.
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    usb_pinmap: usb-pinmap@22000d0 {
> > > > +        compatible = "brcm,usb-pinmap";
> > > > +        reg = <0x22000d0 0x4>;
> > > > +        in-gpios = <&gpio 18 0>, <&gpio 19 0>;
> > > > +        in-names = "VBUS", "PWRFLT";
> > > > +        in-masks = <0x8000 0x40000 0x10000 0x80000>;
> > > > +        out-gpios = <&gpio 20 0>;
> > > > +        out-names = "PWRON";
> > > > +        out-masks = <0x20000 0x800000 0x400000 0x200000>;
> > > > +        interrupts = <0x0 0xb2 0x4>;
> > > > +    };
> > > > +
> > > > +...
> > > > --
> > > > 2.17.1
> > > >
Linus Walleij Aug. 28, 2020, 2 p.m. UTC | #5
On Wed, Aug 12, 2020 at 10:20 PM Al Cooper <alcooperx@gmail.com> wrote:

> Add DT bindings for the Broadcom USB pin map driver. This driver allows
> some USB input and output signals to be mapped to any GPIO instead
> of the normal dedicated pins to/from the XHCI controller.
>
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
(...)
> +title: Broadcom USB pin map Controller Device Tree Bindings
> +
> +maintainers:
> +  - Al Cooper <alcooperx@gmail.com>
> +
> +properties:
> +  compatible:
> +      items:
> +          - const: brcm,usb-pinmap
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    description: Must be defined if any out-gpios are specified.
> +
> +  in-gpios:
> +    description: Array of one or more GPIO pins used for input signals.
> +
> +  in-names:
> +    description: Array of input signal names, one per gpio in in-gpios.
> +
> +  in-masks:
> +    description: Array of enable and mask pairs, one per gpio in-gpios.
> +
> +  out-gpios:
> +    description: Array of one or more GPIO pins used for output signals.
> +
> +  out-names:
> +    description: Array of output signal names, one per gpio in out-gpios.
> +
> +  out-masks:
> +    description: Array of enable, value, changed and clear masks, one
> +      per gpio in out-gpios.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    usb_pinmap: usb-pinmap@22000d0 {
> +        compatible = "brcm,usb-pinmap";
> +        reg = <0x22000d0 0x4>;
> +        in-gpios = <&gpio 18 0>, <&gpio 19 0>;
> +        in-names = "VBUS", "PWRFLT";
> +        in-masks = <0x8000 0x40000 0x10000 0x80000>;
> +        out-gpios = <&gpio 20 0>;
> +        out-names = "PWRON";
> +        out-masks = <0x20000 0x800000 0x400000 0x200000>;
> +        interrupts = <0x0 0xb2 0x4>;
> +    };

Wow look at that.

This looks very much like Geert's just invented GPIO aggregator.
But in hardware!

See:
drivers/gpio/gpio-aggregator.c

I think Geert is intending to add bindings to the aggregator, and
while I do think this should be its own driver (in drivers/usb) these
bindings and whatever Geert want to use for the aggregator
should certainly be the same.

Geert what do you think?

Here is the actual driver:
https://lore.kernel.org/linux-usb/20200812202018.49046-3-alcooperx@gmail.com/

Yours,
Linus Walleij
Geert Uytterhoeven Aug. 28, 2020, 2:18 p.m. UTC | #6
Hi Linus et al,

On Fri, Aug 28, 2020 at 4:00 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Aug 12, 2020 at 10:20 PM Al Cooper <alcooperx@gmail.com> wrote:
> > Add DT bindings for the Broadcom USB pin map driver. This driver allows
> > some USB input and output signals to be mapped to any GPIO instead
> > of the normal dedicated pins to/from the XHCI controller.
> >
> > Signed-off-by: Al Cooper <alcooperx@gmail.com>
> (...)
> > +title: Broadcom USB pin map Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Al Cooper <alcooperx@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +      items:
> > +          - const: brcm,usb-pinmap
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +    description: Must be defined if any out-gpios are specified.
> > +
> > +  in-gpios:
> > +    description: Array of one or more GPIO pins used for input signals.
> > +
> > +  in-names:
> > +    description: Array of input signal names, one per gpio in in-gpios.
> > +
> > +  in-masks:
> > +    description: Array of enable and mask pairs, one per gpio in-gpios.
> > +
> > +  out-gpios:
> > +    description: Array of one or more GPIO pins used for output signals.
> > +
> > +  out-names:
> > +    description: Array of output signal names, one per gpio in out-gpios.
> > +
> > +  out-masks:
> > +    description: Array of enable, value, changed and clear masks, one
> > +      per gpio in out-gpios.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    usb_pinmap: usb-pinmap@22000d0 {
> > +        compatible = "brcm,usb-pinmap";
> > +        reg = <0x22000d0 0x4>;
> > +        in-gpios = <&gpio 18 0>, <&gpio 19 0>;
> > +        in-names = "VBUS", "PWRFLT";
> > +        in-masks = <0x8000 0x40000 0x10000 0x80000>;
> > +        out-gpios = <&gpio 20 0>;
> > +        out-names = "PWRON";
> > +        out-masks = <0x20000 0x800000 0x400000 0x200000>;
> > +        interrupts = <0x0 0xb2 0x4>;
> > +    };
>
> Wow look at that.
>
> This looks very much like Geert's just invented GPIO aggregator.
> But in hardware!
>
> See:
> drivers/gpio/gpio-aggregator.c
>
> I think Geert is intending to add bindings to the aggregator, and
> while I do think this should be its own driver (in drivers/usb) these
> bindings and whatever Geert want to use for the aggregator
> should certainly be the same.

I don't intend to add any DT bindings to the GPIO Aggregator, as it's
meant to be an "abstract base" driver.  Actual hardware blocks for which
the GPIO Aggregator could be a suitable driver should have their own DT
bindings, and their compatible values added to the GPIO Aggregator
driver's match table.

Anyway, DT bindings would just be a compatible value, and a gpios
property.

> Geert what do you think?

This USB pin map driver seems to map GPIO pins to USB pins, not other
GPIO pins, so to me it looks like something different than the GPIO
Aggregator: a hardware mux instead of a software mux.

BTW, at least on most Renesas SoCs, you can usually mux output functions
to multiple pins at the same time, which could be considered mirroring,
too.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
new file mode 100644
index 000000000000..19cf6ad36373
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml
@@ -0,0 +1,63 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/brcm,usb-pinmap.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom USB pin map Controller Device Tree Bindings
+
+maintainers:
+  - Al Cooper <alcooperx@gmail.com>
+
+properties:
+  compatible:
+      items:
+          - const: brcm,usb-pinmap
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description: Must be defined if any out-gpios are specified.
+
+  in-gpios:
+    description: Array of one or more GPIO pins used for input signals.
+
+  in-names:
+    description: Array of input signal names, one per gpio in in-gpios.
+
+  in-masks:
+    description: Array of enable and mask pairs, one per gpio in-gpios.
+
+  out-gpios:
+    description: Array of one or more GPIO pins used for output signals.
+
+  out-names:
+    description: Array of output signal names, one per gpio in out-gpios.
+
+  out-masks:
+    description: Array of enable, value, changed and clear masks, one
+      per gpio in out-gpios.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    usb_pinmap: usb-pinmap@22000d0 {
+        compatible = "brcm,usb-pinmap";
+        reg = <0x22000d0 0x4>;
+        in-gpios = <&gpio 18 0>, <&gpio 19 0>;
+        in-names = "VBUS", "PWRFLT";
+        in-masks = <0x8000 0x40000 0x10000 0x80000>;
+        out-gpios = <&gpio 20 0>;
+        out-names = "PWRON";
+        out-masks = <0x20000 0x800000 0x400000 0x200000>;
+        interrupts = <0x0 0xb2 0x4>;
+    };
+
+...