2016-04-11 07:42:10

by Nava kishore Manne

[permalink] [raw]
Subject: [PATCH] Axi-usb: Add support for 64-bit addressing.

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

Signed-off-by: Nava kishore Manne <[email protected]>
---
.../devicetree/bindings/usb/udc-xilinx.txt | 3 +-
drivers/usb/gadget/udc/udc-xilinx.c | 38 ++++++++++++++++++++--
2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
index 47b4e39..d417872 100644
--- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
+++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
@@ -7,12 +7,13 @@ Required properties:
- interrupts : Should contain single irq line of USB2 device
controller
- xlnx,has-builtin-dma : if DMA is included
-
+- xlnx,addrwidth : Should be the dma addressing size in bits(ex: 40 bits).
Example:
axi-usb2-device@42e00000 {
compatible = "xlnx,usb2-device-4.00.a";
interrupts = <0x0 0x39 0x1>;
reg = <0x42e00000 0x10000>;
xlnx,has-builtin-dma;
+ xlnx,addrwidth = <0x28>;
};

diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/udc/udc-xilinx.c
index 1cbb0ac..eeb1401 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 */
@@ -176,6 +185,7 @@ struct xusb_ep {
* @addr: the usb device base address
* @lock: instance of spinlock
* @dma_enabled: flag indicating whether the dma is included in the system
+ * @dma_addrwidth:Indicate the DMA address width.
* @read_fn: function pointer to read device registers
* @write_fn: function pointer to write to device registers
*/
@@ -193,7 +203,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);
};
@@ -215,6 +225,19 @@ static const struct usb_endpoint_descriptor config_bulk_out_desc = {
};

/**
+ * 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(void __iomem *addr, u32 offset, u64 val)
+{
+#if defined(CONFIG_PHYS_ADDR_T_64BIT)
+ writeq(val, addr + offset);
+#endif
+}
+
+/**
* xudc_write32 - little endian write to device registers
* @addr: base addr of device registers
* @offset: register offset
@@ -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(udc->addr, XUSB_DMA_DSAR_ADDR_OFFSET_LSB, src);
+ xudc_write64(udc->addr, 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,10 @@ static int xudc_probe(struct platform_device *pdev)

udc->dma_enabled = of_property_read_bool(np, "xlnx,has-builtin-dma");

+ ret = of_property_read_u32(np, "xlnx,addrwidth", &udc->dma_addrwidth);
+ if (ret < 0)
+ dev_warn(&pdev->dev, "missing xlnx,addrwidth property\n");
+
/* Setup gadget structure */
udc->gadget.ops = &xusb_udc_ops;
udc->gadget.max_speed = USB_SPEED_HIGH;
--
2.1.2


2016-04-11 08:09:21

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] Axi-usb: Add support for 64-bit addressing.

Hi Nava,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.6-rc3 next-20160411]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url: https://github.com/0day-ci/linux/commits/Nava-kishore-Manne/Axi-usb-Add-support-for-64-bit-addressing/20160411-154657
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux for-next
config: i386-randconfig-x015-201615 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

All errors (new ones prefixed by >>):

drivers/usb/gadget/udc/udc-xilinx.c: In function 'xudc_write64':
>> drivers/usb/gadget/udc/udc-xilinx.c:236:2: error: implicit declaration of function 'writeq' [-Werror=implicit-function-declaration]
writeq(val, addr + offset);
^
cc1: some warnings being treated as errors

vim +/writeq +236 drivers/usb/gadget/udc/udc-xilinx.c

230 * @offset: register offset
231 * @val: data to be written
232 **/
233 static void xudc_write64(void __iomem *addr, u32 offset, u64 val)
234 {
235 #if defined(CONFIG_PHYS_ADDR_T_64BIT)
> 236 writeq(val, addr + offset);
237 #endif
238 }
239

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.34 kB)
.config.gz (23.81 kB)
Download all attachments

2016-04-12 14:04:00

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] Axi-usb: Add support for 64-bit addressing.

On Mon, Apr 11, 2016 at 01:11:46PM +0530, Nava kishore Manne wrote:
> This patch updates the driver to support 64-bit DMA
> addressing.
>
> Signed-off-by: Nava kishore Manne <[email protected]>
> ---
> .../devicetree/bindings/usb/udc-xilinx.txt | 3 +-
> drivers/usb/gadget/udc/udc-xilinx.c | 38 ++++++++++++++++++++--
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> index 47b4e39..d417872 100644
> --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> @@ -7,12 +7,13 @@ Required properties:
> - interrupts : Should contain single irq line of USB2 device
> controller
> - xlnx,has-builtin-dma : if DMA is included
> -
> +- xlnx,addrwidth : Should be the dma addressing size in bits(ex: 40 bits).

Now this property shows up in a 2nd device. Now I'm more convinced this
is the wrong approach and should use dma-ranges.

Rob

2016-04-17 13:15:25

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] Axi-usb: Add support for 64-bit addressing.

On Tuesday 12 April 2016 09:03:38 Rob Herring wrote:
> On Mon, Apr 11, 2016 at 01:11:46PM +0530, Nava kishore Manne wrote:
> > This patch updates the driver to support 64-bit DMA
> > addressing.
> >
> > Signed-off-by: Nava kishore Manne <[email protected]>
> > ---
> > .../devicetree/bindings/usb/udc-xilinx.txt | 3 +-
> > drivers/usb/gadget/udc/udc-xilinx.c | 38 ++++++++++++++++++++--
> > 2 files changed, 37 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > index 47b4e39..d417872 100644
> > --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > @@ -7,12 +7,13 @@ Required properties:
> > - interrupts : Should contain single irq line of USB2 device
> > controller
> > - xlnx,has-builtin-dma : if DMA is included
> > -
> > +- xlnx,addrwidth : Should be the dma addressing size in bits(ex: 40 bits).
>
> Now this property shows up in a 2nd device. Now I'm more convinced this
> is the wrong approach and should use dma-ranges.

Not necessarily: We need to be careful not to mix up two different things here:

* dma-ranges describes the address width of a bus, along with possible offsets.
In order to do wider than 32-bit addressing, all upstream busses must be
capable of supporting this, and AXI can have either 64-bit or 32-bit addressing.

* A device may have a register set that allows wider DMA. This is normally
identified through the 'compatible' property, and you can have all
combinations with bus addressing: a device with 32-bit DMA can be connected
to a 64-bit AXI bus, and a device with 64-bit DMA can have an upstream parent
or grandparent that is limited to 32 bits (or any other width really).

The patch here is almost certainly wrong. For one thing, it never sets the
DMA mask to the correct value, and it also infers the presence of the
0x308..0x317 register range from an arbitrary DT property that describes
something else (the supported width of the DMA).

Someone with access to the data sheet of the hardware should look up
what the device capabilities actually are, and add the necessary properties
accordingly.

Arnd

2016-04-17 14:10:45

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] Axi-usb: Add support for 64-bit addressing.

On Monday 11 April 2016 16:08:14 kbuild test robot wrote:
>
> 230 * @offset: register offset
> 231 * @val: data to be written
> 232 **/
> 233 static void xudc_write64(void __iomem *addr, u32 offset, u64 val)
> 234 {
> 235 #if defined(CONFIG_PHYS_ADDR_T_64BIT)
> > 236 writeq(val, addr + offset);
> 237 #endif
> 238 }
>

I think you want lo_hi_writeq() rather than writeq().

Arnd

2016-04-18 14:29:18

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] Axi-usb: Add support for 64-bit addressing.

On Sun, Apr 17, 2016 at 03:14:34PM +0200, Arnd Bergmann wrote:
> On Tuesday 12 April 2016 09:03:38 Rob Herring wrote:
> > On Mon, Apr 11, 2016 at 01:11:46PM +0530, Nava kishore Manne wrote:
> > > This patch updates the driver to support 64-bit DMA
> > > addressing.
> > >
> > > Signed-off-by: Nava kishore Manne <[email protected]>
> > > ---
> > > .../devicetree/bindings/usb/udc-xilinx.txt | 3 +-
> > > drivers/usb/gadget/udc/udc-xilinx.c | 38 ++++++++++++++++++++--
> > > 2 files changed, 37 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/udc-xilinx.txt b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > index 47b4e39..d417872 100644
> > > --- a/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > +++ b/Documentation/devicetree/bindings/usb/udc-xilinx.txt
> > > @@ -7,12 +7,13 @@ Required properties:
> > > - interrupts : Should contain single irq line of USB2 device
> > > controller
> > > - xlnx,has-builtin-dma : if DMA is included
> > > -
> > > +- xlnx,addrwidth : Should be the dma addressing size in bits(ex: 40 bits).
> >
> > Now this property shows up in a 2nd device. Now I'm more convinced this
> > is the wrong approach and should use dma-ranges.
>
> Not necessarily: We need to be careful not to mix up two different things here:
>
> * dma-ranges describes the address width of a bus, along with possible offsets.
> In order to do wider than 32-bit addressing, all upstream busses must be
> capable of supporting this, and AXI can have either 64-bit or 32-bit addressing.
>
> * A device may have a register set that allows wider DMA. This is normally
> identified through the 'compatible' property, and you can have all
> combinations with bus addressing: a device with 32-bit DMA can be connected
> to a 64-bit AXI bus, and a device with 64-bit DMA can have an upstream parent
> or grandparent that is limited to 32 bits (or any other width really).
>
> The patch here is almost certainly wrong. For one thing, it never sets the
> DMA mask to the correct value, and it also infers the presence of the
> 0x308..0x317 register range from an arbitrary DT property that describes
> something else (the supported width of the DMA).

Right, you don't need to know the exact bus width for determining the
register/descriptor set is 32 or 64 bit addesses. I'm fine with a
property for that, but if limiting the actual connected address bits is
needed, then dma-ranges should be used.

Rob

2016-04-18 14:36:00

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] Axi-usb: Add support for 64-bit addressing.

On Monday 18 April 2016 09:29:09 Rob Herring wrote:
>
> Right, you don't need to know the exact bus width for determining the
> register/descriptor set is 32 or 64 bit addesses. I'm fine with a
> property for that, but if limiting the actual connected address bits is
> needed, then dma-ranges should be used.

The other way round: dma-ranges is needed to allow 64-bit addressing,
the default is to only allow 32-bit addressing (and arm64 has a known
bug here, it just allows it anyway when it shouldn't).

Arnd

2016-04-19 09:29:53

by Nava kishore Manne

[permalink] [raw]
Subject: RE: [PATCH] Axi-usb: Add support for 64-bit addressing.



> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Monday, April 18, 2016 8:05 PM
> To: Rob Herring <[email protected]>
> Cc: [email protected]; Nava kishore Manne
> <[email protected]>; [email protected]; [email protected];
> Nava kishore Manne <[email protected]>; Hyun Kwon
> <[email protected]>; [email protected]; [email protected];
> [email protected]; Radhey Shyam Pandey <[email protected]>;
> Michal Simek <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Soren Brinkmann
> <[email protected]>
> Subject: Re: [PATCH] Axi-usb: Add support for 64-bit addressing.
>
> On Monday 18 April 2016 09:29:09 Rob Herring wrote:
> >
> > Right, you don't need to know the exact bus width for determining the
> > register/descriptor set is 32 or 64 bit addesses. I'm fine with a
> > property for that, but if limiting the actual connected address bits
> > is needed, then dma-ranges should be used.
>
> The other way round: dma-ranges is needed to allow 64-bit addressing, the
> default is to only allow 32-bit addressing (and arm64 has a known bug here, it
> just allows it anyway when it shouldn't).

AXI-USB IP configurable for 32-bit or 64-bit addressing.

In any of the configuration I mean if it is a 32-bit or 64-bit addressing there is a flexibility for the dma to choose the
Memory range supported by the AXI-USB...

For example if AXI-USB (dma ) is configured for 40-bit addressing.

Theoretically it can access memory up to 1TB if it is true we can get the
Address width using log2 of the dma-ranges size as you mentioned above.

But in real use case user won't map the entire memory
He will map only the memory of his own choice...

For example user mapped 2GB then dma-ranges property will be like below.

dma-ranges = <0x00000000 0x00000000 0x80000000>;

In this case log2 of the dma-ranges size won't give exact value for address width.
That's why used separate h/w property for getting address width of the IP...

Will address the remaining comments in v2.

Regards,
Navakishore.

> Arnd


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.

2016-04-19 14:07:57

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] Axi-usb: Add support for 64-bit addressing.

On Tuesday 19 April 2016 09:15:01 Nava kishore Manne wrote:
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:[email protected]]
> > Sent: Monday, April 18, 2016 8:05 PM
> > To: Rob Herring <[email protected]>
> > Cc: [email protected]; Nava kishore Manne
> > <[email protected]>; [email protected]; [email protected];
> > Nava kishore Manne <[email protected]>; Hyun Kwon
> > <[email protected]>; [email protected]; [email protected];
> > [email protected]; Radhey Shyam Pandey <[email protected]>;
> > Michal Simek <[email protected]>; [email protected]; linux-
> > [email protected]; [email protected]; Soren Brinkmann
> > <[email protected]>
> > Subject: Re: [PATCH] Axi-usb: Add support for 64-bit addressing.
> >
> > On Monday 18 April 2016 09:29:09 Rob Herring wrote:
> > >
> > > Right, you don't need to know the exact bus width for determining the
> > > register/descriptor set is 32 or 64 bit addesses. I'm fine with a
> > > property for that, but if limiting the actual connected address bits
> > > is needed, then dma-ranges should be used.
> >
> > The other way round: dma-ranges is needed to allow 64-bit addressing, the
> > default is to only allow 32-bit addressing (and arm64 has a known bug here, it
> > just allows it anyway when it shouldn't).
>
> AXI-USB IP configurable for 32-bit or 64-bit addressing.
>
> In any of the configuration I mean if it is a 32-bit or 64-bit addressing there is a flexibility for the dma to choose the
> Memory range supported by the AXI-USB...
>
> For example if AXI-USB (dma ) is configured for 40-bit addressing.

You seem to contradict yourself here, saying that it can be either
32-bit or 64-bit, but then you say it is configured for 40-bit mode.

Can you be more specific of which configurations are possible in the
AXI-USB hardware block? Does it have three modes (32, 40, 64) or just
two modes where it always truncates the 64-bit register values
to 40 bits before sending a transaction out on the AXI master port?


> Theoretically it can access memory up to 1TB if it is true we can get the
> Address width using log2 of the dma-ranges size as you mentioned above.
>
> But in real use case user won't map the entire memory
> He will map only the memory of his own choice...
>
> For example user mapped 2GB then dma-ranges property will be like below.
>
> dma-ranges = <0x00000000 0x00000000 0x80000000>;

Again, that is not how dma-ranges works. The dma-ranges should list how
the bus address space translates into the CPU address space, and that
is completely independent of how much memory is installed. Unfortunately
this was designed back when we had hierarchical buses, whereas nowadays
we use point-to-point connections. We can normally work around that
by translating each AXI link in the SoC design into a single line
in the dma-ranges property, except when two devices on the same logical
bus (which doesn't exist on AXI) have AXI master ports connecting to
different places using the same address.

Arnd