[v2,1/2] dt-bindings: usb: Add Intel SoCFPGA USB controller

From: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>

Existing binding intel,keembay-dwc3.yaml does not have the required
properties for Intel SoCFPGA devices.
Introduce new binding description for Intel SoCFPGA USB controller
which will be used for current and future SoCFPGA devices.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
---
 .../bindings/usb/intel,socfpga-dwc3.yaml      | 84 +++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml

On Mon, Jul 24, 2023 at 02:36:58PM +0800, adrian.ho.yin.ng@intel.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
> 
> Existing binding intel,keembay-dwc3.yaml does not have the required
> properties for Intel SoCFPGA devices.
> Introduce new binding description for Intel SoCFPGA USB controller
> which will be used for current and future SoCFPGA devices.
> 
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
> ---
>  .../bindings/usb/intel,socfpga-dwc3.yaml      | 84 +++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml b/Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> new file mode 100644
> index 000000000000..e36b087c2651
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/intel,socfpga-dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel SoCFPGA DWC3 USB controller
> +
> +maintainers:
> +  - Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - intel,agilex5-dwc3
> +      - const: intel,socfpga-dwc3
> +
> +  reg:
> +    description: Offset and length of DWC3 controller register
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Controller Master/Core clock
> +      - description: Controller Suspend clock
> +
> +  ranges: true
> +
> +  resets:
> +    description: A list of phandles for resets listed in reset-names
> +    maxItems: 2
> +
> +  reset-names:
> +    items:
> +      - const: dwc3
> +      - const: dwc3-ecc
> +
> +  '#address-cells':
> +    enum: [ 1, 2 ]
> +
> +  '#size-cells':
> +    enum: [ 1, 2 ]
> +
> +# Required child node:
> +
> +patternProperties:
> +  "^usb@[0-9a-f]+$":
> +    $ref: snps,dwc3.yaml#

One node, no wrapper node and dwc3 child node please unless you have 
actual registers for the wrapper. Based on the example having the same 
register addresses in both nodes, you don't need the wrapper.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - resets
> +  - ranges
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/reset/altr,rst-mgr.h>
> +
> +    usb@11000000 {
> +          compatible = "intel,agilex5-dwc3", "intel,socfpga-dwc3";
> +          reg = <0x11000000 0x100000>;

You really have 1MB worth of registers? That chews up 1MB of 
kernel virtual space. Not a big deal for 64-bit, but it is a problem on 
32-bit systems. Define the length to just what you need.

> +          ranges;
> +          clocks = <&clkmgr 54>,
> +                   <&clkmgr 55>;
> +          resets = <&rst USB0_RESET>, <&rst USB1_RESET>;
> +          reset-names = "dwc3", "dwc3-ecc";
> +          #address-cells = <1>;
> +          #size-cells = <1>;

BTW, this implies that you can only DMA 32-bit bits. Not sure if the 
dwc3 supports more. 

> +
> +          usb@11000000 {
> +                compatible = "snps,dwc3";
> +                reg = <0x11000000 0x100000>;
> +                interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> +                dr_mode = "host";
> +          };
> +    };
> +
> -- 
> 2.26.2
>

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Thursday, 27 July, 2023 12:29 AM
> To: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>
> Cc: gregkh@linuxfoundation.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; linux-usb@vger.kernel.org;
> devicetree@vger.kernel.org; Thinh.Nguyen@synopsys.com;
> p.zabel@pengutronix.de
> Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Add Intel SoCFPGA USB
> controller
> 
> On Mon, Jul 24, 2023 at 02:36:58PM +0800, adrian.ho.yin.ng@intel.com wrote:
> > From: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
> >
> > Existing binding intel,keembay-dwc3.yaml does not have the required
> > properties for Intel SoCFPGA devices.
> > Introduce new binding description for Intel SoCFPGA USB controller
> > which will be used for current and future SoCFPGA devices.
> >
> > Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
> > ---
> >  .../bindings/usb/intel,socfpga-dwc3.yaml      | 84 +++++++++++++++++++
> >  1 file changed, 84 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> > b/Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> > new file mode 100644
> > index 000000000000..e36b087c2651
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> > @@ -0,0 +1,84 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/intel,socfpga-dwc3.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Intel SoCFPGA DWC3 USB controller
> > +
> > +maintainers:
> > +  - Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - intel,agilex5-dwc3
> > +      - const: intel,socfpga-dwc3
> > +
> > +  reg:
> > +    description: Offset and length of DWC3 controller register
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: Controller Master/Core clock
> > +      - description: Controller Suspend clock
> > +
> > +  ranges: true
> > +
> > +  resets:
> > +    description: A list of phandles for resets listed in reset-names
> > +    maxItems: 2
> > +
> > +  reset-names:
> > +    items:
> > +      - const: dwc3
> > +      - const: dwc3-ecc
> > +
> > +  '#address-cells':
> > +    enum: [ 1, 2 ]
> > +
> > +  '#size-cells':
> > +    enum: [ 1, 2 ]
> > +
> > +# Required child node:
> > +
> > +patternProperties:
> > +  "^usb@[0-9a-f]+$":
> > +    $ref: snps,dwc3.yaml#
> 
> One node, no wrapper node and dwc3 child node please unless you have
> actual registers for the wrapper. Based on the example having the same
> register addresses in both nodes, you don't need the wrapper.
> 

Noted.  To remove child node in next patch submission.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - resets
> > +  - ranges
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/reset/altr,rst-mgr.h>
> > +
> > +    usb@11000000 {
> > +          compatible = "intel,agilex5-dwc3", "intel,socfpga-dwc3";
> > +          reg = <0x11000000 0x100000>;
> 
> You really have 1MB worth of registers? That chews up 1MB of kernel virtual
> space. Not a big deal for 64-bit, but it is a problem on 32-bit systems. Define
> the length to just what you need.
> 

To update reg length in next patch submission.

> > +          ranges;
> > +          clocks = <&clkmgr 54>,
> > +                   <&clkmgr 55>;
> > +          resets = <&rst USB0_RESET>, <&rst USB1_RESET>;
> > +          reset-names = "dwc3", "dwc3-ecc";
> > +          #address-cells = <1>;
> > +          #size-cells = <1>;
> 
> BTW, this implies that you can only DMA 32-bit bits. Not sure if the
> dwc3 supports more.
> 

DWC3 supports DMA 64 bits.
In properties address-cells support value 1 and 2.
To update example to indicate 64-bit DMA support.

Thank You.
Adrian Ng