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

[Nava kishore Manne]

> -Original Message-
> From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org]
> Sent: Friday, September 02, 2016 12:09 PM
> To: Nava kishore Manne <na...@xilinx.com>
> Cc: Soren Brinkmann <sor...@xilinx.com>; robh...@kernel.org;
> pawel.m...@arm.com; mark.rutl...@arm.com;
> ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; ba...@kernel.org;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: Re: [PATCH v5] Axi-usb: Add support for 64-bit addressing.
> 
> On Fri, Sep 02, 2016 at 06:02:14AM +, Nava kishore Manne wrote:
> > 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.
> >
> 
> Now deleted.
> 
> I have told you this before, this type of footer guarantees that your patches,
> or emails, will not get reviewed.  It is the exact opposite of working in 
> public
> together.
> 
> Please have your company fix this.

Sorry something fishy happened with my email-client
I forgot to keep no-footer option in my email-client. Next time onwards will 
take care before giving replies...

Regards,
Navakishore.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[gre...@linuxfoundation.org]

On Fri, Sep 02, 2016 at 06:02:14AM +, Nava kishore Manne wrote:
> 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.
> 

Now deleted.

I have told you this before, this type of footer guarantees that your
patches, or emails, will not get reviewed.  It is the exact opposite of
working in public together.

Please have your company fix this.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Nava kishore Manne]

HI Soren,

Thanks for the review

> -Original Message-
> From: Sören Brinkmann [mailto:soren.brinkm...@xilinx.com]
> Sent: Thursday, September 01, 2016 11:27 PM
> To: Nava kishore Manne <na...@xilinx.com>
> Cc: robh...@kernel.org; pawel.m...@arm.com; mark.rutl...@arm.com;
> ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; ba...@kernel.org;
> gre...@linuxfoundation.org; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-ker...@vger.kernel.org; linux-
> u...@vger.kernel.org; Nava kishore Manne <na...@xilinx.com>
> Subject: Re: [PATCH v5] Axi-usb: Add support for 64-bit addressing.
>
> On Thu, 2016-09-01 at 14:22:56 +0530, Nava kishore Manne wrote:
> > This patch updates the driver to support 64-bit DMA addressing.
> >
> > Signed-off-by: Nava kishore Manne <na...@xilinx.com>
> > Acked-by: Rob Herring <r...@kernel.org>
> > ---
> > Changes for v5:
> > -None.
> >
> > Changes for v4:
> > -Used boolen property insted of addrwith property in the DT
> >  as suggested by Arnd Bergmann.
> > -Adopt the DT relevant changes into the driver.
> >
> > 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 |  5 ++-
> >  drivers/usb/gadget/udc/udc-xilinx.c| 52
> +-
> >  2 files changed, 54 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > index 47b4e39..d08d972 100644
> > --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > @@ -1,12 +1,14 @@
> >  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
> > +- xlnx,has-64bit-dma   : if DMA is included 64-bit addressing support.
>
> We add these properties to describe the IP configuration + we have the
> version indicator in the compatible string. Do we really need both? It seems
> that all versions use the same driver and differentiation is made through the
> DT properties that describe the IP configuration parameters.
> Wouldn't it be better to just have one generic 'xlnx,axi-usb2' (or
> whatever) compatible string instead of patching the driver for each new
> version without really needing to differentiate the IP versions through it?
>
I think matching on the version number is slightly better, as we have the 
version
Already and it identifies the 64-bit  registers existence.

Regards,
Navakishore


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 v5] Axi-usb: Add support for 64-bit addressing.

[Sören Brinkmann]

On Thu, 2016-09-01 at 14:22:56 +0530, Nava kishore Manne wrote:
> This patch updates the driver to support 64-bit DMA addressing.
> 
> Signed-off-by: Nava kishore Manne 
> Acked-by: Rob Herring 
> ---
> Changes for v5:
> -None.
> 
> Changes for v4:
> -Used boolen property insted of addrwith property in the DT
>  as suggested by Arnd Bergmann.
> -Adopt the DT relevant changes into the driver.
> 
> 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 |  5 ++-
>  drivers/usb/gadget/udc/udc-xilinx.c| 52 
> +-
>  2 files changed, 54 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt 
> b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> index 47b4e39..d08d972 100644
> --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> @@ -1,12 +1,14 @@
>  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
> +- xlnx,has-64bit-dma : if DMA is included 64-bit addressing support.

We add these properties to describe the IP configuration + we have the
version indicator in the compatible string. Do we really need both? It
seems that all versions use the same driver and differentiation is made
through the DT properties that describe the IP configuration parameters.
Wouldn't it be better to just have one generic 'xlnx,axi-usb2' (or
whatever) compatible string instead of patching the driver for each new
version without really needing to differentiate the IP versions through
it?

Sören
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5] 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 
Acked-by: Rob Herring 
---
Changes for v5:
-None.

Changes for v4:
-Used boolen property insted of addrwith property in the DT
 as suggested by Arnd Bergmann.
-Adopt the DT relevant changes into the driver.

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 |  5 ++-
 drivers/usb/gadget/udc/udc-xilinx.c| 52 +-
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt 
b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
index 47b4e39..d08d972 100644
--- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
+++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
@@ -1,12 +1,14 @@
 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
+- xlnx,has-64bit-dma   : if DMA is included 64-bit addressing support.
 
 Example:
axi-usb2-device@42e0 {
@@ -14,5 +16,6 @@ Example:
 interrupts = <0x0 0x39 0x1>;
 reg = <0x42e0 0x1>;
 xlnx,has-builtin-dma;
+   xlnx,has-64bit-dma;
 };
 
diff --git a/drivers/usb/gadget/udc/udc-xilinx.c 
b/drivers/usb/gadget/udc/udc-xilinx.c
index 1cbb0ac..6fb80c6 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 */
@@ -169,6 +178,7 @@ struct xusb_ep {
  * @setup: usb_ctrlrequest structure for control requests
  * @req: pointer to dummy request for get status command
  * @dev: pointer to device structure in gadget
+ * @is_extend_dma: flag indiacting whether the dma is 64-bit support or not.
  * @usb_state: device in suspended state or not
  * @remote_wkp: remote wakeup enabled by host
  * @setupseqtx: tx status
@@ -186,6 +196,7 @@ struct xusb_udc {
struct usb_ctrlrequest setup;
struct xusb_req *req;
struct device *dev;
+   bool is_extend_dma;
u32 usb_state;
u32 remote_wkp;
u32 setupseqtx;
@@ -215,6 +226,20 @@ static const struct usb_endpoint_descriptor 
config_bulk_out_desc = {
 };
 
 /**
+ * xudc_write64 - write 64bit value to device registers
+ * @ep: pointer to the usb device endpoint structure.
+ * @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
  * @offset: register offset
@@ -330,8 +355,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->is_extend_dma) {
+   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);
+