* [PATCH 0/3] usb: Add driver for USB signal re-mapper @ 2020-08-12 20:20 Al Cooper 2020-08-12 20:20 ` [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver Al Cooper ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Al Cooper @ 2020-08-12 20:20 UTC (permalink / raw) To: linux-kernel Cc: Al Cooper, bcm-kernel-feedback-list, devicetree, Florian Fainelli, Greg Kroah-Hartman, linux-arm-kernel, linux-usb, Rob Herring The Broadcom 7211 has new functionality that allows some USB low speed side band signals, that go from the XHCI host controller to pins on the chip, to be remapped to use any GPIO pin instead of the limited set selectable by hardware. This can be done without changing the standard driver for the host controller. There is currently support for three USB signals, PWRON, VBUS_PRESENT and PWRFLT. This driver will allow the remapping of any of these three signals based on settings in the Device Tree node for the driver. The driver was written so that it could handle additional signals added in the future by just adding the correct properties to the DT node. Al Cooper (3): dt-bindings: Add support for Broadcom USB pin map driver usb: Add driver to allow any GPIO to be used for 7211 USB signals usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap .../bindings/usb/brcm,usb-pinmap.yaml | 63 ++++ MAINTAINERS | 8 + drivers/usb/host/Kconfig | 4 + drivers/usb/host/Makefile | 1 + drivers/usb/host/brcmstb-usb-pinmap.c | 348 ++++++++++++++++++ 5 files changed, 424 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml create mode 100644 drivers/usb/host/brcmstb-usb-pinmap.c -- 2.17.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver 2020-08-12 20:20 [PATCH 0/3] usb: Add driver for USB signal re-mapper Al Cooper @ 2020-08-12 20:20 ` Al Cooper 2020-08-24 23:30 ` Rob Herring 2020-08-28 14:00 ` Linus Walleij 2020-08-12 20:20 ` [PATCH 2/3] usb: Add driver to allow any GPIO to be used for 7211 USB signals Al Cooper 2020-08-12 20:20 ` [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap Al Cooper 2 siblings, 2 replies; 15+ messages in thread From: Al Cooper @ 2020-08-12 20:20 UTC (permalink / raw) To: linux-kernel Cc: Al Cooper, bcm-kernel-feedback-list, devicetree, Florian Fainelli, Greg Kroah-Hartman, linux-arm-kernel, linux-usb, Rob Herring 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 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>; + }; + +... -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver 2020-08-12 20:20 ` [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver Al Cooper @ 2020-08-24 23:30 ` Rob Herring 2020-08-25 12:26 ` Alan Cooper 2020-08-28 14:00 ` Linus Walleij 1 sibling, 1 reply; 15+ messages in thread From: Rob Herring @ 2020-08-24 23:30 UTC (permalink / raw) To: Al Cooper Cc: linux-kernel, bcm-kernel-feedback-list, devicetree, Florian Fainelli, Greg Kroah-Hartman, linux-arm-kernel, linux-usb 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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver 2020-08-24 23:30 ` Rob Herring @ 2020-08-25 12:26 ` Alan Cooper 2020-08-25 15:46 ` Rob Herring 0 siblings, 1 reply; 15+ messages in thread From: Alan Cooper @ 2020-08-25 12:26 UTC (permalink / raw) To: Rob Herring Cc: : Linux Kernel Mailing List, BCM Kernel Feedback, DTML, Florian Fainelli, Greg Kroah-Hartman, moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE, USB list 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 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver 2020-08-25 12:26 ` Alan Cooper @ 2020-08-25 15:46 ` Rob Herring 2020-08-26 16:00 ` Alan Cooper 0 siblings, 1 reply; 15+ messages in thread From: Rob Herring @ 2020-08-25 15:46 UTC (permalink / raw) To: Alan Cooper, Linus Walleij Cc: : Linux Kernel Mailing List, BCM Kernel Feedback, DTML, Florian Fainelli, Greg Kroah-Hartman, moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE, USB list +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 > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver 2020-08-25 15:46 ` Rob Herring @ 2020-08-26 16:00 ` Alan Cooper 0 siblings, 0 replies; 15+ messages in thread From: Alan Cooper @ 2020-08-26 16:00 UTC (permalink / raw) To: Rob Herring Cc: Linus Walleij, : Linux Kernel Mailing List, BCM Kernel Feedback, DTML, Florian Fainelli, Greg Kroah-Hartman, moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE, USB list 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 > > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver 2020-08-12 20:20 ` [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver Al Cooper 2020-08-24 23:30 ` Rob Herring @ 2020-08-28 14:00 ` Linus Walleij 2020-08-28 14:18 ` Geert Uytterhoeven 1 sibling, 1 reply; 15+ messages in thread From: Linus Walleij @ 2020-08-28 14:00 UTC (permalink / raw) To: Al Cooper, Geert Uytterhoeven Cc: linux-kernel, bcm-kernel-feedback-list, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Florian Fainelli, Greg Kroah-Hartman, Linux ARM, linux-usb, Rob Herring 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver 2020-08-28 14:00 ` Linus Walleij @ 2020-08-28 14:18 ` Geert Uytterhoeven 0 siblings, 0 replies; 15+ messages in thread From: Geert Uytterhoeven @ 2020-08-28 14:18 UTC (permalink / raw) To: Linus Walleij Cc: Al Cooper, linux-kernel, bcm-kernel-feedback-list, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Florian Fainelli, Greg Kroah-Hartman, Linux ARM, linux-usb, Rob Herring 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 -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/3] usb: Add driver to allow any GPIO to be used for 7211 USB signals 2020-08-12 20:20 [PATCH 0/3] usb: Add driver for USB signal re-mapper Al Cooper 2020-08-12 20:20 ` [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver Al Cooper @ 2020-08-12 20:20 ` Al Cooper 2020-08-12 20:20 ` [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap Al Cooper 2 siblings, 0 replies; 15+ messages in thread From: Al Cooper @ 2020-08-12 20:20 UTC (permalink / raw) To: linux-kernel Cc: Al Cooper, Al Cooper, bcm-kernel-feedback-list, devicetree, Florian Fainelli, Greg Kroah-Hartman, linux-arm-kernel, linux-usb, Rob Herring From: Al Cooper <al.cooper@broadcom.com> The Broadcom 7211 has new functionality that allows some USB low speed side band signals, that go from the XHCI host controller to pins on the chip, to be remapped to use any GPIO pin instead of the limited set selectable by hardware. This can be done without changing the standard driver for the host controller. There is currently support for three USB signals, PWRON, VBUS_PRESENT and PWRFLT. This driver will allow the remapping of any of these three signals based on settings in the Device Tree node for the driver. The driver was written so that it could handle additional signals added in the future by just adding the correct properties to the DT node. Below is an example of a DT node that would remap all three signals: usb_pinmap: usb-pinmap@22000d0 { compatible = "brcm,usb-pinmap"; reg = <0x22000d0 0x4>; in-names = "VBUS", "PWRFLT"; in-gpios = <&gpio 18 0>, <&gpio 19 0>; in-masks = <0x8000 0x40000 0x10000 0x80000>; out-names = "PWRON"; out-gpios = <&gpio 20 0>; out-masks = <0x20000 0x800000 0x400000 0x200000>; interrupts = <0x0 0xb2 0x4>; }; refs #SWLINUX-5537 Signed-off-by: Al Cooper <al.cooper@broadcom.com> --- drivers/usb/host/brcmstb-usb-pinmap.c | 348 ++++++++++++++++++++++++++ 1 file changed, 348 insertions(+) create mode 100644 drivers/usb/host/brcmstb-usb-pinmap.c diff --git a/drivers/usb/host/brcmstb-usb-pinmap.c b/drivers/usb/host/brcmstb-usb-pinmap.c new file mode 100644 index 000000000000..647373b7e53b --- /dev/null +++ b/drivers/usb/host/brcmstb-usb-pinmap.c @@ -0,0 +1,348 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2020, Broadcom */ + +#include <linux/init.h> +#include <linux/types.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/device.h> +#include <linux/of.h> +#include <linux/kernel.h> +#include <linux/kdebug.h> +#include <linux/gpio/consumer.h> + +struct out_pin { + u32 enable_mask; + u32 value_mask; + u32 changed_mask; + u32 clr_changed_mask; + struct gpio_desc *gpiod; + const char *name; +}; + +struct in_pin { + u32 enable_mask; + u32 value_mask; + struct gpio_desc *gpiod; + const char *name; + struct brcmstb_usb_pinmap_data *pdata; +}; + +struct brcmstb_usb_pinmap_data { + void __iomem *regs; + int in_count; + struct in_pin *in_pins; + int out_count; + struct out_pin *out_pins; +}; + + +static void pinmap_set(void __iomem *reg, u32 mask) +{ + u32 val; + + val = readl(reg); + val |= mask; + writel(val, reg); +} + +static void pinmap_unset(void __iomem *reg, u32 mask) +{ + u32 val; + + val = readl(reg); + val &= ~mask; + writel(val, reg); +} + +static void sync_in_pin(struct in_pin *pin) +{ + u32 val; + + val = gpiod_get_value(pin->gpiod); + if (val) + pinmap_set(pin->pdata->regs, pin->value_mask); + else + pinmap_unset(pin->pdata->regs, pin->value_mask); +} + +/* + * Interrupt from override register, propagate from override bit + * to GPIO. + */ +static irqreturn_t brcmstb_usb_pinmap_ovr_isr(int irq, void *dev_id) +{ + struct brcmstb_usb_pinmap_data *pdata = dev_id; + struct out_pin *pout; + u32 val; + u32 bit; + int x; + + pr_debug("%s: reg: 0x%x\n", __func__, readl(pdata->regs)); + pout = pdata->out_pins; + for (x = 0; x < pdata->out_count; x++) { + val = readl(pdata->regs); + if (val & pout->changed_mask) { + pinmap_set(pdata->regs, pout->clr_changed_mask); + pinmap_unset(pdata->regs, pout->clr_changed_mask); + bit = val & pout->value_mask; + gpiod_set_value(pout->gpiod, bit ? 1 : 0); + pr_debug("%s: %s bit changed state to %d\n", + __func__, pout->name, bit ? 1 : 0); + } + } + return IRQ_HANDLED; +} + +/* + * Interrupt from GPIO, propagate from GPIO to override bit. + */ +static irqreturn_t brcmstb_usb_pinmap_gpio_isr(int irq, void *dev_id) +{ + struct in_pin *pin = dev_id; + + pr_debug("%s: %s pin changed state\n", __func__, pin->name); + sync_in_pin(pin); + return IRQ_HANDLED; +} + + +static void get_pin_counts(struct device_node *dn, int *in_count, + int *out_count) +{ + int in; + int out; + + *in_count = 0; + *out_count = 0; + in = of_property_count_strings(dn, "in-names"); + if (in < 0) + return; + out = of_property_count_strings(dn, "out-names"); + if (out < 0) + return; + *in_count = in; + *out_count = out; +} + +static int parse_pins(struct device *dev, struct device_node *dn, + struct brcmstb_usb_pinmap_data *pdata) +{ + struct out_pin *pout; + struct in_pin *pin; + int index; + int res; + int x; + + pin = pdata->in_pins; + for (x = 0, index = 0; x < pdata->in_count; x++) { + pin->gpiod = devm_gpiod_get_index(dev, "in", x, GPIOD_IN); + if (IS_ERR(pin->gpiod)) { + dev_err(dev, "Error getting gpio %s\n", pin->name); + return PTR_ERR(pin->gpiod); + + } + res = of_property_read_string_index(dn, "in-names", x, + &pin->name); + if (res < 0) { + dev_err(dev, "Error getting in-names for %s\n", + pin->name); + return res; + } + res = of_property_read_u32_index(dn, "in-masks", index++, + &pin->enable_mask); + if (res < 0) { + dev_err(dev, "Error getting first in-masks for %s\n", + pin->name); + return res; + } + res = of_property_read_u32_index(dn, "in-masks", index++, + &pin->value_mask); + if (res < 0) { + dev_err(dev, "Error getting second in-masks for %s\n", + pin->name); + return res; + } + pin->pdata = pdata; + pin++; + } + pout = pdata->out_pins; + for (x = 0, index = 0; x < pdata->out_count; x++) { + pout->gpiod = devm_gpiod_get_index(dev, "out", x, + GPIOD_OUT_HIGH); + if (IS_ERR(pout->gpiod)) { + dev_err(dev, "Error getting gpio %s\n", pin->name); + return PTR_ERR(pout->gpiod); + } + res = of_property_read_string_index(dn, "out-names", x, + &pout->name); + if (res < 0) { + dev_err(dev, "Error getting out-names for %s\n", + pout->name); + return res; + } + res = of_property_read_u32_index(dn, "out-masks", index++, + &pout->enable_mask); + if (res < 0) { + dev_err(dev, "Error getting first out-masks for %s\n", + pout->name); + return res; + } + res = of_property_read_u32_index(dn, "out-masks", index++, + &pout->value_mask); + if (res < 0) { + dev_err(dev, "Error getting second out-masks for %s\n", + pout->name); + return res; + } + res = of_property_read_u32_index(dn, "out-masks", index++, + &pout->changed_mask); + if (res < 0) { + dev_err(dev, "Error getting third out-masks for %s\n", + pout->name); + return res; + } + res = of_property_read_u32_index(dn, "out-masks", index++, + &pout->clr_changed_mask); + if (res < 0) { + dev_err(dev, "Error getting fourth out-masks for %s\n", + pout->name); + return res; + } + pout++; + } + return 0; +} + +void sync_all_pins(struct brcmstb_usb_pinmap_data *pdata) +{ + struct out_pin *pout; + struct in_pin *pin; + int val; + int x; + + /* + * Enable the override, clear any changed condition and + * propagate the state to the GPIO for all out pins. + */ + pout = pdata->out_pins; + for (x = 0; x < pdata->out_count; x++) { + pinmap_set(pdata->regs, pout->enable_mask); + pinmap_set(pdata->regs, pout->clr_changed_mask); + pinmap_unset(pdata->regs, pout->clr_changed_mask); + val = readl(pdata->regs) & pout->value_mask; + gpiod_set_value(pout->gpiod, val ? 1 : 0); + pout++; + } + + /* sync and enable all in pins. */ + pin = pdata->in_pins; + for (x = 0; x < pdata->in_count; x++) { + sync_in_pin(pin); + pinmap_set(pdata->regs, pin->enable_mask); + pin++; + } +} + +static int __init brcmstb_usb_pinmap_probe(struct platform_device *pdev) +{ + struct device_node *dn = pdev->dev.of_node; + struct brcmstb_usb_pinmap_data *pdata; + struct in_pin *pin; + struct resource *r; + int out_count; + int in_count; + int err; + int irq; + int x; + + get_pin_counts(dn, &in_count, &out_count); + if ((in_count + out_count) == 0) + return -EINVAL; + + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + + pdata = devm_kzalloc(&pdev->dev, + sizeof(*pdata) + + (sizeof(struct in_pin) * in_count) + + (sizeof(struct out_pin) * out_count), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + + pdata->in_count = in_count; + pdata->out_count = out_count; + pdata->in_pins = (struct in_pin *)(pdata + 1); + pdata->out_pins = (struct out_pin *)(pdata->in_pins + in_count); + + pdata->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r)); + if (IS_ERR(pdata->regs)) + return PTR_ERR(pdata->regs); + platform_set_drvdata(pdev, pdata); + + err = parse_pins(&pdev->dev, dn, pdata); + if (err) + return err; + + sync_all_pins(pdata); + + if (out_count) { + + /* Enable interrupt for out pins */ + irq = platform_get_irq(pdev, 0); + err = devm_request_irq(&pdev->dev, irq, + brcmstb_usb_pinmap_ovr_isr, + IRQF_TRIGGER_RISING, + pdev->name, pdata); + if (err < 0) { + dev_err(&pdev->dev, "Error requesting IRQ\n"); + return err; + } + } + + for (x = 0, pin = pdata->in_pins; x < pdata->in_count; x++, pin++) { + irq = gpiod_to_irq(pin->gpiod); + if (irq < 0) { + dev_err(&pdev->dev, "Error getting IRQ for %s pin\n", + pin->name); + return irq; + } + err = devm_request_irq(&pdev->dev, irq, + brcmstb_usb_pinmap_gpio_isr, + IRQF_SHARED | IRQF_TRIGGER_RISING | + IRQF_TRIGGER_FALLING, + pdev->name, pin); + if (err < 0) { + dev_err(&pdev->dev, "Error requesting IRQ for %s pin\n", + pin->name); + return err; + } + } + + dev_info(&pdev->dev, "Driver probe succeeded\n"); + dev_dbg(&pdev->dev, "In pin count: %d, out pin count: %d\n", + pdata->in_count, pdata->out_count); + return 0; +} + + +static const struct of_device_id brcmstb_usb_pinmap_of_match[] = { + { .compatible = "brcm,usb-pinmap" }, + { }, +}; + +static struct platform_driver brcmstb_usb_pinmap_driver = { + .driver = { + .name = "brcm-usb-pinmap", + .of_match_table = brcmstb_usb_pinmap_of_match, + }, +}; + +static int __init brcmstb_usb_pinmap_init(void) +{ + return platform_driver_probe(&brcmstb_usb_pinmap_driver, + brcmstb_usb_pinmap_probe); +} + +module_init(brcmstb_usb_pinmap_init); -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap 2020-08-12 20:20 [PATCH 0/3] usb: Add driver for USB signal re-mapper Al Cooper 2020-08-12 20:20 ` [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver Al Cooper 2020-08-12 20:20 ` [PATCH 2/3] usb: Add driver to allow any GPIO to be used for 7211 USB signals Al Cooper @ 2020-08-12 20:20 ` Al Cooper 2020-08-12 21:59 ` kernel test robot ` (3 more replies) 2 siblings, 4 replies; 15+ messages in thread From: Al Cooper @ 2020-08-12 20:20 UTC (permalink / raw) To: linux-kernel Cc: Al Cooper, Al Cooper, bcm-kernel-feedback-list, devicetree, Florian Fainelli, Greg Kroah-Hartman, linux-arm-kernel, linux-usb, Rob Herring From: Al Cooper <al.cooper@broadcom.com> Add Kconfig and Makefile changes to build brcmstb-usb-pinmap and update MAINTAINERS for the new driver. refs #SWLINUX-5537 Signed-off-by: Al Cooper <al.cooper@broadcom.com> --- MAINTAINERS | 8 ++++++++ drivers/usb/host/Kconfig | 4 ++++ drivers/usb/host/Makefile | 1 + 3 files changed, 13 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index f0569cf304ca..3a44ac61899b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3527,6 +3527,14 @@ S: Maintained F: Documentation/devicetree/bindings/usb/brcm,bcm7445-ehci.yaml F: drivers/usb/host/ehci-brcm.* +BROADCOM BRCMSTB USB PIN MAP DRIVER +M: Al Cooper <alcooperx@gmail.com> +L: linux-usb@vger.kernel.org +L: bcm-kernel-feedback-list@broadcom.com +S: Maintained +F: Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml +F: drivers/usb/host/brcmstb-usb-pinmap.c + BROADCOM BRCMSTB USB2 and USB3 PHY DRIVER M: Al Cooper <alcooperx@gmail.com> L: linux-kernel@vger.kernel.org diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 1cb3004ea7b2..9c285053bb0c 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -109,12 +109,16 @@ endif # USB_XHCI_HCD config USB_EHCI_BRCMSTB tristate +config BRCM_USB_PINMAP + tristate + config USB_BRCMSTB tristate "Broadcom STB USB support" depends on (ARCH_BRCMSTB && PHY_BRCM_USB) || COMPILE_TEST select USB_OHCI_HCD_PLATFORM if USB_OHCI_HCD select USB_EHCI_BRCMSTB if USB_EHCI_HCD select USB_XHCI_PLATFORM if USB_XHCI_HCD + select BRCM_USB_PINMAP help Enables support for XHCI, EHCI and OHCI host controllers found in Broadcom STB SoC's. diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index bc731332fed9..0e63ef94790d 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -90,3 +90,4 @@ obj-$(CONFIG_USB_HCD_BCMA) += bcma-hcd.o obj-$(CONFIG_USB_HCD_SSB) += ssb-hcd.o obj-$(CONFIG_USB_FOTG210_HCD) += fotg210-hcd.o obj-$(CONFIG_USB_MAX3421_HCD) += max3421-hcd.o +obj-$(CONFIG_BRCM_USB_PINMAP) += brcmstb-usb-pinmap.o -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap 2020-08-12 20:20 ` [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap Al Cooper @ 2020-08-12 21:59 ` kernel test robot 2020-08-13 5:40 ` Greg Kroah-Hartman ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2020-08-12 21:59 UTC (permalink / raw) To: Al Cooper, linux-kernel Cc: kbuild-all, Al Cooper, bcm-kernel-feedback-list, devicetree, Florian Fainelli, Greg Kroah-Hartman, linux-arm-kernel, linux-usb, Rob Herring [-- Attachment #1: Type: text/plain, Size: 3585 bytes --] Hi Al, I love your patch! Perhaps something to improve: [auto build test WARNING on usb/usb-testing] [also build test WARNING on linux/master linus/master v5.8 next-20200812] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Al-Cooper/usb-Add-driver-for-USB-signal-re-mapper/20200813-042328 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/usb/host/brcmstb-usb-pinmap.c:219:6: warning: no previous prototype for 'sync_all_pins' [-Wmissing-prototypes] 219 | void sync_all_pins(struct brcmstb_usb_pinmap_data *pdata) | ^~~~~~~~~~~~~ vim +/sync_all_pins +219 drivers/usb/host/brcmstb-usb-pinmap.c 06f59ffd15dd0d5 Al Cooper 2020-08-12 218 06f59ffd15dd0d5 Al Cooper 2020-08-12 @219 void sync_all_pins(struct brcmstb_usb_pinmap_data *pdata) 06f59ffd15dd0d5 Al Cooper 2020-08-12 220 { 06f59ffd15dd0d5 Al Cooper 2020-08-12 221 struct out_pin *pout; 06f59ffd15dd0d5 Al Cooper 2020-08-12 222 struct in_pin *pin; 06f59ffd15dd0d5 Al Cooper 2020-08-12 223 int val; 06f59ffd15dd0d5 Al Cooper 2020-08-12 224 int x; 06f59ffd15dd0d5 Al Cooper 2020-08-12 225 06f59ffd15dd0d5 Al Cooper 2020-08-12 226 /* 06f59ffd15dd0d5 Al Cooper 2020-08-12 227 * Enable the override, clear any changed condition and 06f59ffd15dd0d5 Al Cooper 2020-08-12 228 * propagate the state to the GPIO for all out pins. 06f59ffd15dd0d5 Al Cooper 2020-08-12 229 */ 06f59ffd15dd0d5 Al Cooper 2020-08-12 230 pout = pdata->out_pins; 06f59ffd15dd0d5 Al Cooper 2020-08-12 231 for (x = 0; x < pdata->out_count; x++) { 06f59ffd15dd0d5 Al Cooper 2020-08-12 232 pinmap_set(pdata->regs, pout->enable_mask); 06f59ffd15dd0d5 Al Cooper 2020-08-12 233 pinmap_set(pdata->regs, pout->clr_changed_mask); 06f59ffd15dd0d5 Al Cooper 2020-08-12 234 pinmap_unset(pdata->regs, pout->clr_changed_mask); 06f59ffd15dd0d5 Al Cooper 2020-08-12 235 val = readl(pdata->regs) & pout->value_mask; 06f59ffd15dd0d5 Al Cooper 2020-08-12 236 gpiod_set_value(pout->gpiod, val ? 1 : 0); 06f59ffd15dd0d5 Al Cooper 2020-08-12 237 pout++; 06f59ffd15dd0d5 Al Cooper 2020-08-12 238 } 06f59ffd15dd0d5 Al Cooper 2020-08-12 239 06f59ffd15dd0d5 Al Cooper 2020-08-12 240 /* sync and enable all in pins. */ 06f59ffd15dd0d5 Al Cooper 2020-08-12 241 pin = pdata->in_pins; 06f59ffd15dd0d5 Al Cooper 2020-08-12 242 for (x = 0; x < pdata->in_count; x++) { 06f59ffd15dd0d5 Al Cooper 2020-08-12 243 sync_in_pin(pin); 06f59ffd15dd0d5 Al Cooper 2020-08-12 244 pinmap_set(pdata->regs, pin->enable_mask); 06f59ffd15dd0d5 Al Cooper 2020-08-12 245 pin++; 06f59ffd15dd0d5 Al Cooper 2020-08-12 246 } 06f59ffd15dd0d5 Al Cooper 2020-08-12 247 } 06f59ffd15dd0d5 Al Cooper 2020-08-12 248 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 55790 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap 2020-08-12 20:20 ` [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap Al Cooper 2020-08-12 21:59 ` kernel test robot @ 2020-08-13 5:40 ` Greg Kroah-Hartman 2020-08-13 15:01 ` Alan Cooper 2020-08-24 15:48 ` kernel test robot 2020-08-24 15:48 ` [RFC PATCH] usb: sync_all_pins() can be static kernel test robot 3 siblings, 1 reply; 15+ messages in thread From: Greg Kroah-Hartman @ 2020-08-13 5:40 UTC (permalink / raw) To: Al Cooper Cc: linux-kernel, Al Cooper, bcm-kernel-feedback-list, devicetree, Florian Fainelli, linux-arm-kernel, linux-usb, Rob Herring On Wed, Aug 12, 2020 at 04:20:18PM -0400, Al Cooper wrote: > From: Al Cooper <al.cooper@broadcom.com> > > Add Kconfig and Makefile changes to build brcmstb-usb-pinmap and > update MAINTAINERS for the new driver. This can be part of the previous patch, or at least the Kconfig and Makefile changes should be there so that we build the code when we add it. > refs #SWLINUX-5537 What is this? > > Signed-off-by: Al Cooper <al.cooper@broadcom.com> > --- > MAINTAINERS | 8 ++++++++ > drivers/usb/host/Kconfig | 4 ++++ > drivers/usb/host/Makefile | 1 + > 3 files changed, 13 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index f0569cf304ca..3a44ac61899b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3527,6 +3527,14 @@ S: Maintained > F: Documentation/devicetree/bindings/usb/brcm,bcm7445-ehci.yaml > F: drivers/usb/host/ehci-brcm.* > > +BROADCOM BRCMSTB USB PIN MAP DRIVER > +M: Al Cooper <alcooperx@gmail.com> > +L: linux-usb@vger.kernel.org > +L: bcm-kernel-feedback-list@broadcom.com > +S: Maintained > +F: Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml > +F: drivers/usb/host/brcmstb-usb-pinmap.c > + > BROADCOM BRCMSTB USB2 and USB3 PHY DRIVER > M: Al Cooper <alcooperx@gmail.com> > L: linux-kernel@vger.kernel.org > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > index 1cb3004ea7b2..9c285053bb0c 100644 > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -109,12 +109,16 @@ endif # USB_XHCI_HCD > config USB_EHCI_BRCMSTB > tristate > > +config BRCM_USB_PINMAP > + tristate > + > config USB_BRCMSTB > tristate "Broadcom STB USB support" > depends on (ARCH_BRCMSTB && PHY_BRCM_USB) || COMPILE_TEST > select USB_OHCI_HCD_PLATFORM if USB_OHCI_HCD > select USB_EHCI_BRCMSTB if USB_EHCI_HCD > select USB_XHCI_PLATFORM if USB_XHCI_HCD > + select BRCM_USB_PINMAP > help > Enables support for XHCI, EHCI and OHCI host controllers > found in Broadcom STB SoC's. > diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile > index bc731332fed9..0e63ef94790d 100644 > --- a/drivers/usb/host/Makefile > +++ b/drivers/usb/host/Makefile > @@ -90,3 +90,4 @@ obj-$(CONFIG_USB_HCD_BCMA) += bcma-hcd.o > obj-$(CONFIG_USB_HCD_SSB) += ssb-hcd.o > obj-$(CONFIG_USB_FOTG210_HCD) += fotg210-hcd.o > obj-$(CONFIG_USB_MAX3421_HCD) += max3421-hcd.o > +obj-$(CONFIG_BRCM_USB_PINMAP) += brcmstb-usb-pinmap.o Shouldn't this driver be in usb/misc/ with other drivers like this? Why host? Wait, why is this a separate driver at all? Why not just build it into the USB_BRCMSTB driver? thanks, greg k-h ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap 2020-08-13 5:40 ` Greg Kroah-Hartman @ 2020-08-13 15:01 ` Alan Cooper 0 siblings, 0 replies; 15+ messages in thread From: Alan Cooper @ 2020-08-13 15:01 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: : Linux Kernel Mailing List, Al Cooper, BCM Kernel Feedback, DTML, Florian Fainelli, moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE, USB list, Rob Herring On Thu, Aug 13, 2020 at 1:40 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Wed, Aug 12, 2020 at 04:20:18PM -0400, Al Cooper wrote: > > From: Al Cooper <al.cooper@broadcom.com> > > > > Add Kconfig and Makefile changes to build brcmstb-usb-pinmap and > > update MAINTAINERS for the new driver. > > This can be part of the previous patch, or at least the Kconfig and > Makefile changes should be there so that we build the code when we add > it. I'll combine the 2 patches. > > > refs #SWLINUX-5537 > > What is this? This will be removed. > > > > > Signed-off-by: Al Cooper <al.cooper@broadcom.com> > > --- > > MAINTAINERS | 8 ++++++++ > > drivers/usb/host/Kconfig | 4 ++++ > > drivers/usb/host/Makefile | 1 + > > 3 files changed, 13 insertions(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f0569cf304ca..3a44ac61899b 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -3527,6 +3527,14 @@ S: Maintained > > F: Documentation/devicetree/bindings/usb/brcm,bcm7445-ehci.yaml > > F: drivers/usb/host/ehci-brcm.* > > > > +BROADCOM BRCMSTB USB PIN MAP DRIVER > > +M: Al Cooper <alcooperx@gmail.com> > > +L: linux-usb@vger.kernel.org > > +L: bcm-kernel-feedback-list@broadcom.com > > +S: Maintained > > +F: Documentation/devicetree/bindings/usb/brcm,usb-pinmap.yaml > > +F: drivers/usb/host/brcmstb-usb-pinmap.c > > + > > BROADCOM BRCMSTB USB2 and USB3 PHY DRIVER > > M: Al Cooper <alcooperx@gmail.com> > > L: linux-kernel@vger.kernel.org > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > > index 1cb3004ea7b2..9c285053bb0c 100644 > > --- a/drivers/usb/host/Kconfig > > +++ b/drivers/usb/host/Kconfig > > @@ -109,12 +109,16 @@ endif # USB_XHCI_HCD > > config USB_EHCI_BRCMSTB > > tristate > > > > +config BRCM_USB_PINMAP > > + tristate > > + > > config USB_BRCMSTB > > tristate "Broadcom STB USB support" > > depends on (ARCH_BRCMSTB && PHY_BRCM_USB) || COMPILE_TEST > > select USB_OHCI_HCD_PLATFORM if USB_OHCI_HCD > > select USB_EHCI_BRCMSTB if USB_EHCI_HCD > > select USB_XHCI_PLATFORM if USB_XHCI_HCD > > + select BRCM_USB_PINMAP > > help > > Enables support for XHCI, EHCI and OHCI host controllers > > found in Broadcom STB SoC's. > > diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile > > index bc731332fed9..0e63ef94790d 100644 > > --- a/drivers/usb/host/Makefile > > +++ b/drivers/usb/host/Makefile > > @@ -90,3 +90,4 @@ obj-$(CONFIG_USB_HCD_BCMA) += bcma-hcd.o > > obj-$(CONFIG_USB_HCD_SSB) += ssb-hcd.o > > obj-$(CONFIG_USB_FOTG210_HCD) += fotg210-hcd.o > > obj-$(CONFIG_USB_MAX3421_HCD) += max3421-hcd.o > > +obj-$(CONFIG_BRCM_USB_PINMAP) += brcmstb-usb-pinmap.o > > Shouldn't this driver be in usb/misc/ with other drivers like this? Why > host? That does seem like a better choice, I'll move it. > > Wait, why is this a separate driver at all? Why not just build it into > the USB_BRCMSTB driver? Of the 20 or so chips supported by the BRCMSTB USB drivers, only one currently has this functionality so it seems better to have this code in it's own driver so it can be enabled in device-tree for just this chip. I also wanted to leave this in it's own driver because in the future the same hardware functionality may be added for some eMMC signals. Thanks Al > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap 2020-08-12 20:20 ` [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap Al Cooper 2020-08-12 21:59 ` kernel test robot 2020-08-13 5:40 ` Greg Kroah-Hartman @ 2020-08-24 15:48 ` kernel test robot 2020-08-24 15:48 ` [RFC PATCH] usb: sync_all_pins() can be static kernel test robot 3 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2020-08-24 15:48 UTC (permalink / raw) To: Al Cooper, linux-kernel Cc: kbuild-all, Al Cooper, bcm-kernel-feedback-list, devicetree, Florian Fainelli, Greg Kroah-Hartman, linux-arm-kernel, linux-usb, Rob Herring [-- Attachment #1: Type: text/plain, Size: 1344 bytes --] Hi Al, I love your patch! Perhaps something to improve: [auto build test WARNING on usb/usb-testing] [also build test WARNING on linux/master linus/master v5.9-rc2 next-20200824] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Al-Cooper/usb-Add-driver-for-USB-signal-re-mapper/20200813-042328 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: i386-randconfig-s002-20200824 (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-191-g10164920-dirty # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> drivers/usb/host/brcmstb-usb-pinmap.c:219:6: sparse: sparse: symbol 'sync_all_pins' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 40549 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH] usb: sync_all_pins() can be static 2020-08-12 20:20 ` [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap Al Cooper ` (2 preceding siblings ...) 2020-08-24 15:48 ` kernel test robot @ 2020-08-24 15:48 ` kernel test robot 3 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2020-08-24 15:48 UTC (permalink / raw) To: Al Cooper, linux-kernel Cc: kbuild-all, Al Cooper, bcm-kernel-feedback-list, devicetree, Florian Fainelli, Greg Kroah-Hartman, linux-arm-kernel, linux-usb, Rob Herring Signed-off-by: kernel test robot <lkp@intel.com> --- brcmstb-usb-pinmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/brcmstb-usb-pinmap.c b/drivers/usb/host/brcmstb-usb-pinmap.c index 647373b7e53b7a..2dd117d4fcfefc 100644 --- a/drivers/usb/host/brcmstb-usb-pinmap.c +++ b/drivers/usb/host/brcmstb-usb-pinmap.c @@ -216,7 +216,7 @@ static int parse_pins(struct device *dev, struct device_node *dn, return 0; } -void sync_all_pins(struct brcmstb_usb_pinmap_data *pdata) +static void sync_all_pins(struct brcmstb_usb_pinmap_data *pdata) { struct out_pin *pout; struct in_pin *pin; ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-08-28 14:18 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-12 20:20 [PATCH 0/3] usb: Add driver for USB signal re-mapper Al Cooper 2020-08-12 20:20 ` [PATCH 1/3] dt-bindings: Add support for Broadcom USB pin map driver Al Cooper 2020-08-24 23:30 ` Rob Herring 2020-08-25 12:26 ` Alan Cooper 2020-08-25 15:46 ` Rob Herring 2020-08-26 16:00 ` Alan Cooper 2020-08-28 14:00 ` Linus Walleij 2020-08-28 14:18 ` Geert Uytterhoeven 2020-08-12 20:20 ` [PATCH 2/3] usb: Add driver to allow any GPIO to be used for 7211 USB signals Al Cooper 2020-08-12 20:20 ` [PATCH 3/3] usb: Add Kconfig and Makefile changes to build brcmstb-usb-pinmap Al Cooper 2020-08-12 21:59 ` kernel test robot 2020-08-13 5:40 ` Greg Kroah-Hartman 2020-08-13 15:01 ` Alan Cooper 2020-08-24 15:48 ` kernel test robot 2020-08-24 15:48 ` [RFC PATCH] usb: sync_all_pins() can be static kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).