[PATCH v3] Axi-usb: Add support for 64-bit addressing.

[PATCH v3] Axi-usb: Add support for 64-bit addressing.

[Nava kishore Manne]

This patch updates the driver to support 64-bit DMA addressing.

Signed-off-by: Nava kishore Manne <navam@xilinx.com>
---
Changes for v3:
                -Added new compatable string for 5.00 IP version as suggested by
                 Arnd Bergmann.
                -Used write_fn() insted of lo_hi_writeq() as suggested by
                 Arnd Bergmann.

Changes for v2:
                -Added dma-ranges property in device tree as suggested by Arnd Bergmann.
                -Modified the driver code based on the xlnx,addrwidth.

 .../devicetree/bindings/usb/udc-xilinx.txt         | 21 +++++----
 drivers/usb/gadget/udc/udc-xilinx.c                | 53 ++++++++++++++++++++--
 2 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
index 47b4e39..09df757 100644
--- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
+++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
@@ -1,18 +1,23 @@
 Xilinx USB2 device controller
 
 Required properties:
-- compatible		: Should be "xlnx,usb2-device-4.00.a"
+- compatible		: Should be "xlnx,usb2-device-4.00.a" or
+			  "xlnx,usb2-device-5.00"
 - reg			: Physical base address and size of the USB2
 			  device registers map.
 - interrupts		: Should contain single irq line of USB2 device
 			  controller
 - xlnx,has-builtin-dma	: if DMA is included
+- dma-ranges		: Should be as the following
+			  <child-bus-address, parent-bus-address, length>
+- xlnx,addrwidth	: Should be the dma addressing size in bits(ex: 64 bits)
 
 Example:
- 		axi-usb2-device@42e00000 {
-                        compatible = "xlnx,usb2-device-4.00.a";
-                        interrupts = <0x0 0x39 0x1>;
-                        reg = <0x42e00000 0x10000>;
-                        xlnx,has-builtin-dma;
-                };
-
+		axi-usb2-device@42e00000 {
+			compatible = "xlnx,usb2-device-4.00.a";
+			interrupts = <0x0 0x39 0x1>;
+			reg = <0x42e00000 0x10000>;
+			dma-ranges = <0x00000000 0x00000000 0x40000000>;
+			xlnx,has-builtin-dma;
+			xlnx,addrwidth = <0x40>;
+		};
diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/udc/udc-xilinx.c
index 1cbb0ac..112a6f3 100644
--- a/drivers/usb/gadget/udc/udc-xilinx.c
+++ b/drivers/usb/gadget/udc/udc-xilinx.c
@@ -47,6 +47,15 @@
 #define XUSB_DMA_LENGTH_OFFSET		0x0210	/* DMA Length Register */
 #define XUSB_DMA_STATUS_OFFSET		0x0214	/* DMA Status Register */
 
+/* DMA source Address Reg for LSB */
+#define XUSB_DMA_DSAR_ADDR_OFFSET_LSB   0x0308
+/* DMA source Address Reg for MSB */
+#define XUSB_DMA_DSAR_ADDR_OFFSET_MSB   0x030C
+/* DMA destination Addr Reg LSB */
+#define XUSB_DMA_DDAR_ADDR_OFFSET_LSB   0x0310
+/* DMA destination Addr Reg MSB */
+#define XUSB_DMA_DDAR_ADDR_OFFSET_MSB   0x0314
+
 /* Endpoint Configuration Space offsets */
 #define XUSB_EP_CFGSTATUS_OFFSET	0x00	/* Endpoint Config Status  */
 #define XUSB_EP_BUF0COUNT_OFFSET	0x08	/* Buffer 0 Count */
@@ -193,7 +202,7 @@ struct xusb_udc {
 	void __iomem *addr;
 	spinlock_t lock;
 	bool dma_enabled;
-
+	u32 dma_addrwidth;
 	unsigned int (*read_fn)(void __iomem *);
 	void (*write_fn)(void __iomem *, u32, u32);
 };
@@ -214,6 +223,20 @@ static const struct usb_endpoint_descriptor config_bulk_out_desc = {
 	.wMaxPacketSize		= cpu_to_le16(EP0_MAX_PACKET),
 };
 
+/**
+ * xudc_write64 - write 64bit value to device registers
+ * @addr: base addr of device registers
+ * @offset: register offset
+ * @val: data to be written
+ **/
+static void xudc_write64(struct xusb_ep *ep, u32 offset, u64 val)
+{
+	struct xusb_udc *udc = ep->udc;
+
+	udc->write_fn(udc->addr, offset, lower_32_bits(val));
+	udc->write_fn(udc->addr, offset+0x04, upper_32_bits(val));
+}
+
 /**
  * xudc_write32 - little endian write to device registers
  * @addr: base addr of device registers
@@ -330,8 +353,13 @@ static int xudc_start_dma(struct xusb_ep *ep, dma_addr_t src,
 	 * destination registers and then set the length
 	 * into the DMA length register.
 	 */
-	udc->write_fn(udc->addr, XUSB_DMA_DSAR_ADDR_OFFSET, src);
-	udc->write_fn(udc->addr, XUSB_DMA_DDAR_ADDR_OFFSET, dst);
+	if (udc->dma_addrwidth > 32) {
+		xudc_write64(ep, XUSB_DMA_DSAR_ADDR_OFFSET_LSB, src);
+		xudc_write64(ep, XUSB_DMA_DDAR_ADDR_OFFSET_LSB, dst);
+	} else {
+		udc->write_fn(udc->addr, XUSB_DMA_DSAR_ADDR_OFFSET, src);
+		udc->write_fn(udc->addr, XUSB_DMA_DDAR_ADDR_OFFSET, dst);
+	}
 	udc->write_fn(udc->addr, XUSB_DMA_LENGTH_OFFSET, length);
 
 	/*
@@ -2097,6 +2125,24 @@ static int xudc_probe(struct platform_device *pdev)
 
 	udc->dma_enabled = of_property_read_bool(np, "xlnx,has-builtin-dma");
 
+	if (of_device_is_compatible(np, "xlnx,usb2-device-5.00")) {
+		ret = of_property_read_u32(np, "xlnx,addrwidth",
+						&udc->dma_addrwidth);
+		if (ret < 0)
+			dev_warn(&pdev->dev,
+				"missing xlnx,addrwidth property\n");
+	} else {
+		udc->dma_addrwidth = 32;
+	}
+
+	/* Set the dma mask bits */
+	ret = dma_set_mask_and_coherent(&pdev->dev,
+					DMA_BIT_MASK(udc->dma_addrwidth));
+	if (ret < 0) {
+		dev_dbg(&pdev->dev, "no usable DMA configuration");
+		goto fail;
+	}
+
 	/* Setup gadget structure */
 	udc->gadget.ops = &xusb_udc_ops;
 	udc->gadget.max_speed = USB_SPEED_HIGH;
@@ -2168,6 +2214,7 @@ static int xudc_remove(struct platform_device *pdev)
 /* Match table for of_platform binding */
 static const struct of_device_id usb_of_match[] = {
 	{ .compatible = "xlnx,usb2-device-4.00.a", },
+	{ .compatible = "xlnx,usb2-device-5.00", },
 	{ /* end of list */ },
 };
 MODULE_DEVICE_TABLE(of, usb_of_match);
-- 
2.8.3.394.g3916adf

Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.

[Arnd Bergmann]

On Tuesday, May 24, 2016 10:51:08 AM CEST Nava kishore Manne wrote:
> diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> index 47b4e39..09df757 100644
> --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> @@ -1,18 +1,23 @@
>  Xilinx USB2 device controller
>  
>  Required properties:
> -- compatible		: Should be "xlnx,usb2-device-4.00.a"
> +- compatible		: Should be "xlnx,usb2-device-4.00.a" or
> +			  "xlnx,usb2-device-5.00"
>  - reg			: Physical base address and size of the USB2
>  			  device registers map.
>  - interrupts		: Should contain single irq line of USB2 device
>  			  controller
>  - xlnx,has-builtin-dma	: if DMA is included
> +- dma-ranges		: Should be as the following
> +			  <child-bus-address, parent-bus-address, length>

A USB host should not have any children that are DMA capable, I think, so this
property doesn't make sense here. It should be part of the parent bus.

> +- xlnx,addrwidth	: Should be the dma addressing size in bits(ex: 64 bits)

I'm still unconvinced about the property definition here. What are the possible
options for the IP block? I don't think I ever saw a reply from you to my earlier
questions.

> @@ -214,6 +223,20 @@ static const struct usb_endpoint_descriptor config_bulk_out_desc = {
>  	.wMaxPacketSize		= cpu_to_le16(EP0_MAX_PACKET),
>  };
>  
> +/**
> + * xudc_write64 - write 64bit value to device registers
> + * @addr: base addr of device registers
> + * @offset: register offset
> + * @val: data to be written
> + **/
> +static void xudc_write64(struct xusb_ep *ep, u32 offset, u64 val)
> +{
> +	struct xusb_udc *udc = ep->udc;
> +
> +	udc->write_fn(udc->addr, offset, lower_32_bits(val));
> +	udc->write_fn(udc->addr, offset+0x04, upper_32_bits(val));
> +}
> +
>  /**
>   * xudc_write32 - little endian write to device registers
>   * @addr: base addr of device registers
> @@ -330,8 +353,13 @@ static int xudc_start_dma(struct xusb_ep *ep, dma_addr_t src,
>  	 * destination registers and then set the length
>  	 * into the DMA length register.
>  	 */
> -	udc->write_fn(udc->addr, XUSB_DMA_DSAR_ADDR_OFFSET, src);
> -	udc->write_fn(udc->addr, XUSB_DMA_DDAR_ADDR_OFFSET, dst);
> +	if (udc->dma_addrwidth > 32) {
> +		xudc_write64(ep, XUSB_DMA_DSAR_ADDR_OFFSET_LSB, src);
> +		xudc_write64(ep, XUSB_DMA_DDAR_ADDR_OFFSET_LSB, dst);
> +	} else {
> +		udc->write_fn(udc->addr, XUSB_DMA_DSAR_ADDR_OFFSET, src);
> +		udc->write_fn(udc->addr, XUSB_DMA_DDAR_ADDR_OFFSET, dst);
> +	}
>  	udc->write_fn(udc->addr, XUSB_DMA_LENGTH_OFFSET, length);
>  

This looks good.

	Arnd

RE: [PATCH v3] Axi-usb: Add support for 64-bit addressing.

[Nava kishore Manne]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Tuesday, May 24, 2016 2:21 PM
> To: Nava kishore Manne <navam@xilinx.com>
> Cc: robh+dt@kernel.org; pawel.moll@arm.com; mark.rutland@arm.com;
> ijc+devicetree@hellion.org.uk; galak@codeaurora.org; Michal Simek
> <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>;
> balbi@ti.com; gregkh@linuxfoundation.org; Hyun Kwon
> <hyunk@xilinx.com>; Nava kishore Manne <navam@xilinx.com>; Radhey
> Shyam Pandey <radheys@xilinx.com>; devicetree@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.
> 
> On Tuesday, May 24, 2016 10:51:08 AM CEST Nava kishore Manne wrote:
> > diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > index 47b4e39..09df757 100644
> > --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > @@ -1,18 +1,23 @@
> >  Xilinx USB2 device controller
> >
> >  Required properties:
> > -- compatible		: Should be "xlnx,usb2-device-4.00.a"
> > +- compatible		: Should be "xlnx,usb2-device-4.00.a" or
> > +			  "xlnx,usb2-device-5.00"
> >  - reg			: Physical base address and size of the USB2
> >  			  device registers map.
> >  - interrupts		: Should contain single irq line of USB2 device
> >  			  controller
> >  - xlnx,has-builtin-dma	: if DMA is included
> > +- dma-ranges		: Should be as the following
> > +			  <child-bus-address, parent-bus-address, length>
> 
> A USB host should not have any children that are DMA capable, I think, so
> this property doesn't make sense here. It should be part of the parent bus.
> 
Will send next version (v4) by removing this property from the DT.

> > +- xlnx,addrwidth	: Should be the dma addressing size in bits(ex: 64
> bits)
> 
> I'm still unconvinced about the property definition here. What are the
> possible options for the IP block? I don't think I ever saw a reply from you to
> my earlier questions.
> 

Sorry Let me clearly explain

>From the IP version 5.0 onwards The IP support both 32-bit and 64-bit addressing.
But the older version of the IP's supports only 32-bit addressing.

This addrwidth property differentiates the address width for the new IP (I mean 5.0 version on wards)
For older IP it will be always 32-bit.

Please let me know if you are still not clear.

Regards,
Navakishore.


> > @@ -214,6 +223,20 @@ static const struct usb_endpoint_descriptor
> config_bulk_out_desc = {
> >  	.wMaxPacketSize		= cpu_to_le16(EP0_MAX_PACKET),
> >  };
> >
> > +/**
> > + * xudc_write64 - write 64bit value to device registers
> > + * @addr: base addr of device registers
> > + * @offset: register offset
> > + * @val: data to be written
> > + **/
> > +static void xudc_write64(struct xusb_ep *ep, u32 offset, u64 val) {
> > +	struct xusb_udc *udc = ep->udc;
> > +
> > +	udc->write_fn(udc->addr, offset, lower_32_bits(val));
> > +	udc->write_fn(udc->addr, offset+0x04, upper_32_bits(val)); }
> > +
> >  /**
> >   * xudc_write32 - little endian write to device registers
> >   * @addr: base addr of device registers @@ -330,8 +353,13 @@ static
> > int xudc_start_dma(struct xusb_ep *ep, dma_addr_t src,
> >  	 * destination registers and then set the length
> >  	 * into the DMA length register.
> >  	 */
> > -	udc->write_fn(udc->addr, XUSB_DMA_DSAR_ADDR_OFFSET, src);
> > -	udc->write_fn(udc->addr, XUSB_DMA_DDAR_ADDR_OFFSET, dst);
> > +	if (udc->dma_addrwidth > 32) {
> > +		xudc_write64(ep, XUSB_DMA_DSAR_ADDR_OFFSET_LSB,
> src);
> > +		xudc_write64(ep, XUSB_DMA_DDAR_ADDR_OFFSET_LSB,
> dst);
> > +	} else {
> > +		udc->write_fn(udc->addr,
> XUSB_DMA_DSAR_ADDR_OFFSET, src);
> > +		udc->write_fn(udc->addr,
> XUSB_DMA_DDAR_ADDR_OFFSET, dst);
> > +	}
> >  	udc->write_fn(udc->addr, XUSB_DMA_LENGTH_OFFSET, length);
> >
> 
> This looks good.
> 
> 	Arnd

Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.

[Rob Herring]

On Tue, May 24, 2016 at 11:47:31AM +0000, Nava kishore Manne wrote:
> 
> 
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > Sent: Tuesday, May 24, 2016 2:21 PM
> > To: Nava kishore Manne <navam@xilinx.com>
> > Cc: robh+dt@kernel.org; pawel.moll@arm.com; mark.rutland@arm.com;
> > ijc+devicetree@hellion.org.uk; galak@codeaurora.org; Michal Simek
> > <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>;
> > balbi@ti.com; gregkh@linuxfoundation.org; Hyun Kwon
> > <hyunk@xilinx.com>; Nava kishore Manne <navam@xilinx.com>; Radhey
> > Shyam Pandey <radheys@xilinx.com>; devicetree@vger.kernel.org; linux-
> > arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.
> > 
> > On Tuesday, May 24, 2016 10:51:08 AM CEST Nava kishore Manne wrote:
> > > diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > index 47b4e39..09df757 100644
> > > --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > @@ -1,18 +1,23 @@
> > >  Xilinx USB2 device controller
> > >
> > >  Required properties:
> > > -- compatible		: Should be "xlnx,usb2-device-4.00.a"
> > > +- compatible		: Should be "xlnx,usb2-device-4.00.a" or
> > > +			  "xlnx,usb2-device-5.00"
> > >  - reg			: Physical base address and size of the USB2
> > >  			  device registers map.
> > >  - interrupts		: Should contain single irq line of USB2 device
> > >  			  controller
> > >  - xlnx,has-builtin-dma	: if DMA is included
> > > +- dma-ranges		: Should be as the following
> > > +			  <child-bus-address, parent-bus-address, length>
> > 
> > A USB host should not have any children that are DMA capable, I think, so
> > this property doesn't make sense here. It should be part of the parent bus.
> > 
> Will send next version (v4) by removing this property from the DT.
> 
> > > +- xlnx,addrwidth	: Should be the dma addressing size in bits(ex: 64
> > bits)
> > 
> > I'm still unconvinced about the property definition here. What are the
> > possible options for the IP block? I don't think I ever saw a reply from you to
> > my earlier questions.
> > 
> 
> Sorry Let me clearly explain
> 
> From the IP version 5.0 onwards The IP support both 32-bit and 64-bit addressing.
> But the older version of the IP's supports only 32-bit addressing.
> 
> This addrwidth property differentiates the address width for the new IP (I mean 5.0 version on wards)
> For older IP it will be always 32-bit.

Then I think you should have a simple boolean property for 64-bit 
configuration.

Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.

[Arnd Bergmann]

On Wednesday, May 25, 2016 12:34:19 PM CEST Rob Herring wrote:
> On Tue, May 24, 2016 at 11:47:31AM +0000, Nava kishore Manne wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > Sent: Tuesday, May 24, 2016 2:21 PM
> > > To: Nava kishore Manne <navam@xilinx.com>
> > > Cc: robh+dt@kernel.org; pawel.moll@arm.com; mark.rutland@arm.com;
> > > ijc+devicetree@hellion.org.uk; galak@codeaurora.org; Michal Simek
> > > <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>;
> > > balbi@ti.com; gregkh@linuxfoundation.org; Hyun Kwon
> > > <hyunk@xilinx.com>; Nava kishore Manne <navam@xilinx.com>; Radhey
> > > Shyam Pandey <radheys@xilinx.com>; devicetree@vger.kernel.org; linux-
> > > arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.
> > > 
> > > On Tuesday, May 24, 2016 10:51:08 AM CEST Nava kishore Manne wrote:
> > > > diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > > b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > > index 47b4e39..09df757 100644
> > > > --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > > +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > > @@ -1,18 +1,23 @@
> > > >  Xilinx USB2 device controller
> > > >
> > > >  Required properties:
> > > > -- compatible             : Should be "xlnx,usb2-device-4.00.a"
> > > > +- compatible             : Should be "xlnx,usb2-device-4.00.a" or
> > > > +                   "xlnx,usb2-device-5.00"
> > > >  - reg                    : Physical base address and size of the USB2
> > > >                     device registers map.
> > > >  - interrupts             : Should contain single irq line of USB2 device
> > > >                     controller
> > > >  - xlnx,has-builtin-dma   : if DMA is included
> > > > +- dma-ranges             : Should be as the following
> > > > +                   <child-bus-address, parent-bus-address, length>
> > > 
> > > A USB host should not have any children that are DMA capable, I think, so
> > > this property doesn't make sense here. It should be part of the parent bus.
> > > 
> > Will send next version (v4) by removing this property from the DT.
> > 
> > > > +- xlnx,addrwidth : Should be the dma addressing size in bits(ex: 64
> > > bits)
> > > 
> > > I'm still unconvinced about the property definition here. What are the
> > > possible options for the IP block? I don't think I ever saw a reply from you to
> > > my earlier questions.
> > > 
> > 
> > Sorry Let me clearly explain
> > 
> > From the IP version 5.0 onwards The IP support both 32-bit and 64-bit addressing.
> > But the older version of the IP's supports only 32-bit addressing.
> > 
> > This addrwidth property differentiates the address width for the new IP (I mean 5.0 version on wards)
> > For older IP it will be always 32-bit.
> 
> Then I think you should have a simple boolean property for 64-bit 
> configuration.

I think matching on the version number is slightly better, as we have the version
already and it identifies whether the register exists.

Having a boolean property of course works as well, it just duplicates that
information.

	Arnd

Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.

[Rob Herring]

On Wed, May 25, 2016 at 1:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday, May 25, 2016 12:34:19 PM CEST Rob Herring wrote:
>> On Tue, May 24, 2016 at 11:47:31AM +0000, Nava kishore Manne wrote:
>> >
>> >
>> > > -----Original Message-----
>> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
>> > > Sent: Tuesday, May 24, 2016 2:21 PM
>> > > To: Nava kishore Manne <navam@xilinx.com>
>> > > Cc: robh+dt@kernel.org; pawel.moll@arm.com; mark.rutland@arm.com;
>> > > ijc+devicetree@hellion.org.uk; galak@codeaurora.org; Michal Simek
>> > > <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>;
>> > > balbi@ti.com; gregkh@linuxfoundation.org; Hyun Kwon
>> > > <hyunk@xilinx.com>; Nava kishore Manne <navam@xilinx.com>; Radhey
>> > > Shyam Pandey <radheys@xilinx.com>; devicetree@vger.kernel.org; linux-
>> > > arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>> > > Subject: Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.
>> > >
>> > > On Tuesday, May 24, 2016 10:51:08 AM CEST Nava kishore Manne wrote:
>> > > > diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
>> > > > b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
>> > > > index 47b4e39..09df757 100644
>> > > > --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
>> > > > +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
>> > > > @@ -1,18 +1,23 @@
>> > > >  Xilinx USB2 device controller
>> > > >
>> > > >  Required properties:
>> > > > -- compatible             : Should be "xlnx,usb2-device-4.00.a"
>> > > > +- compatible             : Should be "xlnx,usb2-device-4.00.a" or
>> > > > +                   "xlnx,usb2-device-5.00"
>> > > >  - reg                    : Physical base address and size of the USB2
>> > > >                     device registers map.
>> > > >  - interrupts             : Should contain single irq line of USB2 device
>> > > >                     controller
>> > > >  - xlnx,has-builtin-dma   : if DMA is included
>> > > > +- dma-ranges             : Should be as the following
>> > > > +                   <child-bus-address, parent-bus-address, length>
>> > >
>> > > A USB host should not have any children that are DMA capable, I think, so
>> > > this property doesn't make sense here. It should be part of the parent bus.
>> > >
>> > Will send next version (v4) by removing this property from the DT.
>> >
>> > > > +- xlnx,addrwidth : Should be the dma addressing size in bits(ex: 64
>> > > bits)
>> > >
>> > > I'm still unconvinced about the property definition here. What are the
>> > > possible options for the IP block? I don't think I ever saw a reply from you to
>> > > my earlier questions.
>> > >
>> >
>> > Sorry Let me clearly explain
>> >
>> > From the IP version 5.0 onwards The IP support both 32-bit and 64-bit addressing.
>> > But the older version of the IP's supports only 32-bit addressing.
>> >
>> > This addrwidth property differentiates the address width for the new IP (I mean 5.0 version on wards)
>> > For older IP it will be always 32-bit.
>>
>> Then I think you should have a simple boolean property for 64-bit
>> configuration.
>
> I think matching on the version number is slightly better, as we have the version
> already and it identifies whether the register exists.
>
> Having a boolean property of course works as well, it just duplicates that
> information.

It doesn't because v5.0 h/w can be configured for either 32 or 64-bit
mode. Normally, we would encode configuration into the compatible
strings, but I view FPGA based blocks to be an exception. Of course,
since they can just "fix the h/w" we could just require h/w version
and feature registers. ;)

Rob

RE: [PATCH v3] Axi-usb: Add support for 64-bit addressing.

[Nava kishore Manne]

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Thursday, May 26, 2016 12:24 AM
> To: Arnd Bergmann <arnd@arndb.de>
> Cc: Nava kishore Manne <navam@xilinx.com>; pawel.moll@arm.com;
> mark.rutland@arm.com; ijc+devicetree@hellion.org.uk;
> galak@codeaurora.org; Michal Simek <michals@xilinx.com>; Soren
> Brinkmann <sorenb@xilinx.com>; balbi@ti.com;
> gregkh@linuxfoundation.org; Hyun Kwon <hyunk@xilinx.com>; Radhey
> Shyam Pandey <radheys@xilinx.com>; devicetree@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.
>
> On Wed, May 25, 2016 at 1:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday, May 25, 2016 12:34:19 PM CEST Rob Herring wrote:
> >> On Tue, May 24, 2016 at 11:47:31AM +0000, Nava kishore Manne wrote:
> >> >
> >> >
> >> > > -----Original Message-----
> >> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> >> > > Sent: Tuesday, May 24, 2016 2:21 PM
> >> > > To: Nava kishore Manne <navam@xilinx.com>
> >> > > Cc: robh+dt@kernel.org; pawel.moll@arm.com;
> mark.rutland@arm.com;
> >> > > ijc+devicetree@hellion.org.uk; galak@codeaurora.org; Michal Simek
> >> > > <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>;
> >> > > balbi@ti.com; gregkh@linuxfoundation.org; Hyun Kwon
> >> > > <hyunk@xilinx.com>; Nava kishore Manne <navam@xilinx.com>;
> Radhey
> >> > > Shyam Pandey <radheys@xilinx.com>; devicetree@vger.kernel.org;
> >> > > linux- arm-kernel@lists.infradead.org;
> >> > > linux-kernel@vger.kernel.org
> >> > > Subject: Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.
> >> > >
> >> > > On Tuesday, May 24, 2016 10:51:08 AM CEST Nava kishore Manne
> wrote:
> >> > > > diff --git
> >> > > > a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> >> > > > b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> >> > > > index 47b4e39..09df757 100644
> >> > > > --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> >> > > > +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> >> > > > @@ -1,18 +1,23 @@
> >> > > >  Xilinx USB2 device controller
> >> > > >
> >> > > >  Required properties:
> >> > > > -- compatible             : Should be "xlnx,usb2-device-4.00.a"
> >> > > > +- compatible             : Should be "xlnx,usb2-device-4.00.a" or
> >> > > > +                   "xlnx,usb2-device-5.00"
> >> > > >  - reg                    : Physical base address and size of the USB2
> >> > > >                     device registers map.
> >> > > >  - interrupts             : Should contain single irq line of USB2 device
> >> > > >                     controller
> >> > > >  - xlnx,has-builtin-dma   : if DMA is included
> >> > > > +- dma-ranges             : Should be as the following
> >> > > > +                   <child-bus-address, parent-bus-address,
> >> > > > +length>
> >> > >
> >> > > A USB host should not have any children that are DMA capable, I
> >> > > think, so this property doesn't make sense here. It should be part of
> the parent bus.
> >> > >
> >> > Will send next version (v4) by removing this property from the DT.
> >> >
> >> > > > +- xlnx,addrwidth : Should be the dma addressing size in
> >> > > > +bits(ex: 64
> >> > > bits)
> >> > >
> >> > > I'm still unconvinced about the property definition here. What
> >> > > are the possible options for the IP block? I don't think I ever
> >> > > saw a reply from you to my earlier questions.
> >> > >
> >> >
> >> > Sorry Let me clearly explain
> >> >
> >> > From the IP version 5.0 onwards The IP support both 32-bit and 64-bit
> addressing.
> >> > But the older version of the IP's supports only 32-bit addressing.
> >> >
> >> > This addrwidth property differentiates the address width for the
> >> > new IP (I mean 5.0 version on wards) For older IP it will be always 32-bit.
> >>
> >> Then I think you should have a simple boolean property for 64-bit
> >> configuration.
> >
> > I think matching on the version number is slightly better, as we have
> > the version already and it identifies whether the register exists.
> >
> > Having a boolean property of course works as well, it just duplicates
> > that information.
>
> It doesn't because v5.0 h/w can be configured for either 32 or 64-bit mode.
> Normally, we would encode configuration into the compatible strings, but I
> view FPGA based blocks to be an exception. Of course, since they can just
> "fix the h/w" we could just require h/w version and feature registers. ;)
>
The Axi-usb 5.00 IP is a FPGA based one. This IP needs to support two H/w designs one with 32 bit DMA addressing another one is 64 bit DMA addressing.
And also in the software point for view we don’t have any register to figure out whether it is supporting 32 bit DMA addressing or 64 bit DMA addressing.
To support both the designs I kept addrwidth property in the dt. I think here addrwidth property is make sense to differentiate the h/w configurations.
If you want me to changes it to boolean please let me know I will fix it in the next version.

Regards,
Navakishore.

> Rob


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: [PATCH v3] Axi-usb: Add support for 64-bit addressing.

[Arnd Bergmann]

On Monday, May 30, 2016 5:46:21 AM CEST Nava kishore Manne wrote:
> >
> The Axi-usb 5.00 IP is a FPGA based one. This IP needs to support two H/w designs one with 32 bit DMA addressing another one is 64 bit DMA addressing.
> And also in the software point for view we don’t have any register to figure out whether it is supporting 32 bit DMA addressing or 64 bit DMA addressing.
> To support both the designs I kept addrwidth property in the dt. I think here addrwidth property is make sense to differentiate the h/w configurations.
> If you want me to changes it to boolean please let me know I will fix it in the next version.

I think boolean would be more logical here, since what is configurable
here is not how many bits are supported, but whether the XUSB_DMA_DSAR_ADDR
register is present or not.

	Arnd