Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Felipe Balbi]

Hi,

On Wed, Mar 26, 2014 at 03:37:52PM +0530, sundeep subbaraya wrote:
  + INIT_LIST_HEAD(req-queue);
 
  remove this INIT_LIST_HEAD();
 
  also, before returning, I suppose you probably want to link the request
  to the endpoint somehow. Usually people save the endpoint pointer inside
  the private request structure (iow: req-ep = ep;)
 
 
 in ep_queue (say,for IN transaction) the driver copies the gadget
 driver buffer to HW endpoint buffer and if whole data
 has been transferred successfully then request is not added to
 endpoint request list. Hence  INIT_LIST_HEAD(req-queue)
 is required since list_add_tail is not done every time in ep_queue. Is
 it ok to let this be here?o

my bad, you _do_ need this INIT_LIST_HEAD() here. When I first read, I
thought you were initializing struct usb_request list_head, and that is
not necessary ;-)

  + if (req != NULL)
  + list_add_tail(req-queue, ep-queue);
 Here. INIT_LIST_HEAD(req-queue) is required in alloc_req since
 list_add_tail is not executed every time.
 
 Could you please clarify my questions regarding requests queuing.
 1. If Device need to send 5120 bytes to host and
 ep-desc-wMaxPacketSize is 512 then single request with length 5120
 is issued or 10 requests are issued?

single request.

 2. Does device need to know OUT transactions before hand so that OUT
 requests are queued for endpoint before packets are received
 from host?

well, no. Gadget driver shouldn't depend on that. That's UDC driver's
responsability to manage that. I mean, if host sends OUT token and
there's nothing in the out queue, then UDC need to start transfer as
soon as gadget driver queues the request. If, on the other hand, gadget
driver queues packet before host has sent OUT token then you have two
choices:

1) start the transfer - most HW will wait for OUT token
2) wait for out token

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Alan Stern]

On Tue, 15 Apr 2014, Felipe Balbi wrote:

  2. Does device need to know OUT transactions before hand so that OUT
  requests are queued for endpoint before packets are received
  from host?
 
 well, no. Gadget driver shouldn't depend on that. That's UDC driver's
 responsability to manage that. I mean, if host sends OUT token and
 there's nothing in the out queue, then UDC need to start transfer as
 soon as gadget driver queues the request. If, on the other hand, gadget
 driver queues packet before host has sent OUT token then you have two
 choices:
 
 1) start the transfer - most HW will wait for OUT token
 2) wait for out token

I'm not familiar with the variations in all the different UDC hardware.  
Nevertheless, I wouldn't describe the situation in those terms.

If an OUT transaction occurs and the gadget driver hasn't queued a
request, the UDC hardware could store the incoming data in an internal
buffer or it could NAK the transaction.  There aren't any other
choices.  If there isn't enough space available in an internal buffer,
the only possible action is NAK.

Regardless, gadget drivers do not need to queue requests for OUT
endpoints before the host starts sending data.  When the request does
get queued, the UDC driver will make sure that the transfer takes
place.

Alan Stern

--
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 RFC] usb: gadget: Add xilinx axi usb2 device support

[Felipe Balbi]

Hi,

On Tue, Apr 15, 2014 at 01:55:38PM -0400, Alan Stern wrote:
 On Tue, 15 Apr 2014, Felipe Balbi wrote:
 
   2. Does device need to know OUT transactions before hand so that OUT
   requests are queued for endpoint before packets are received
   from host?
  
  well, no. Gadget driver shouldn't depend on that. That's UDC driver's
  responsability to manage that. I mean, if host sends OUT token and
  there's nothing in the out queue, then UDC need to start transfer as
  soon as gadget driver queues the request. If, on the other hand, gadget
  driver queues packet before host has sent OUT token then you have two
  choices:
  
  1) start the transfer - most HW will wait for OUT token
  2) wait for out token
 
 I'm not familiar with the variations in all the different UDC hardware.  
 Nevertheless, I wouldn't describe the situation in those terms.

OK, I've oversimplified... what I meant was that even if you start a
transfer at the UDC level, nothing will happen on the bus until HW sees
an OUT token. The buffer pointed to by req-buf won't get any writes,
DMA won't do anything.

On (2) I meant that some HW (e.g. dwc3) will assert the IRQ line once
they see a token for which they have to transfer descriptors in the
internal controller's cache.

 If an OUT transaction occurs and the gadget driver hasn't queued a
 request, the UDC hardware could store the incoming data in an internal
 buffer or it could NAK the transaction.  There aren't any other
 choices.  If there isn't enough space available in an internal buffer,
 the only possible action is NAK.

in HS there's also NYET

 Regardless, gadget drivers do not need to queue requests for OUT
 endpoints before the host starts sending data.  When the request does

they're not required, but they can. It's UDC driver's responsability to
start consuming the queue at the proper time.

 get queued, the UDC driver will make sure that the transfer takes
 place.

correct.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[sundeep subbaraya]

On Fri, Feb 21, 2014 at 9:09 PM, Felipe Balbi ba...@ti.com wrote:
 Hi,

 On Fri, Feb 21, 2014 at 11:27:07AM +, Subbaraya Sundeep Bhatta wrote:
  From the looks of it, I doubt this was actually tested, you need a lot
  of work on this driver.
 Tested on both ARM and Microblaze architectures with Mass storage gadget.
 Will send a v2 after addressing all your comments.

 clearly you didn't try to remove and reinsert the module or you would
 see a whole bunch of errors.


Yes you are correct. My console hung up as soon as i rmmod my driver.
Could you please point me where am wrong.

 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.

 remove this footer message, btw, when sending emails to public mailing
 lists.

Sure
 --
 balbi
--
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 RFC] usb: gadget: Add xilinx axi usb2 device support

[Felipe Balbi]

Hi,

On Mon, Mar 10, 2014 at 07:06:19PM +0530, sundeep subbaraya wrote:
 On Fri, Feb 21, 2014 at 9:09 PM, Felipe Balbi ba...@ti.com wrote:
  Hi,
 
  On Fri, Feb 21, 2014 at 11:27:07AM +, Subbaraya Sundeep Bhatta wrote:
   From the looks of it, I doubt this was actually tested, you need a lot
   of work on this driver.
  Tested on both ARM and Microblaze architectures with Mass storage gadget.
  Will send a v2 after addressing all your comments.
 
  clearly you didn't try to remove and reinsert the module or you would
  see a whole bunch of errors.
 
 
 Yes you are correct. My console hung up as soon as i rmmod my driver.
 Could you please point me where am wrong.

Many of your mistakes have already been pointed out here:

http://marc.info/?l=linux-usbm=139292068028649w=2

Please fix those and see if following script works:

#!/bin/sh

MODULE=xilinx_udc

for i in $(seq 500); do
modprobe $MODULE  /dev/null 21;
modprobe -r $MODULE  /dev/null 21;
if [ $? -eq 0 ]; then
echo -n .
else
echo -n F
fi
done

echo Finished

you shouldn't see any F characters and your machine shouldn't crash or
hang or reboot...

Also make sure to run g_zero testcases and mass storage tescases. I
wrote, long ago, a small tool to help testing UDC drivers running with
the mass storage gadget, which you can find in [1]. There's a runner
script which makes it easy to full test suite which you can find in [2].

[1] 
https://gitorious.org/usb/usb-tools/source/7eb7ef21de6cd124e0e0d0e7df9ddfff0e2f548e:msc.c
[2] 
https://gitorious.org/usb/usb-tools/source/7eb7ef21de6cd124e0e0d0e7df9ddfff0e2f548e:msc.sh

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Mark Rutland]

On Fri, Feb 21, 2014 at 01:41:03PM +, Michal Simek wrote:
 Hi Mark,
 
 On 02/21/2014 01:04 PM, Mark Rutland wrote:
  
  On Thu, Feb 20, 2014 at 06:23:13PM +, Felipe Balbi wrote:
  Hi,
 
  On Wed, Feb 19, 2014 at 03:10:45PM +0530, Subbaraya Sundeep Bhatta wrote:
  This patch adds xilinx axi usb2 device driver support
 
  Signed-off-by: Subbaraya Sundeep Bhatta sbha...@xilinx.com
  ---
   .../devicetree/bindings/usb/xilinx_usb.txt |   21 +
   drivers/usb/gadget/Kconfig |   14 +
   drivers/usb/gadget/Makefile|1 +
   drivers/usb/gadget/xilinx_udc.c| 2045 
  
   4 files changed, 2081 insertions(+), 0 deletions(-)
   create mode 100644 Documentation/devicetree/bindings/usb/xilinx_usb.txt
   create mode 100644 drivers/usb/gadget/xilinx_udc.c
 
  diff --git a/Documentation/devicetree/bindings/usb/xilinx_usb.txt 
  b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
  new file mode 100644
  index 000..acf03ab
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
  @@ -0,0 +1,21 @@
  +Xilinx AXI USB2 device controller
  +
  +Required properties:
  +- compatible : Should be xlnx,axi-usb2-device-4.00.a
  
  Is axi-usb2-device the official device name?
 
 It is the name of IP which Xilinx have and we are using names in this format.
 
 
  +- reg: Physical base address and size of the Axi USB2
  +   device registers map.
  +- interrupts : Property with a value describing the interrupt
  +   number.
  
  Does the device only have a single interrupt?
 
 I believe so.
 
 
  +- interrupt-parent   : Must be core interrupt controller
  
  Is this strictly necessary?
 
 I am not sure what do you mean by that. If you mean that interrupt-parent
 can be written to the root of DTS file then we can setup system with more
 interrupt controllers that's why it is required.

At which point it's not a required property of the node...

 If we can point to standard interrupt description then please point me to
 exact description you would like to see here and we can change it.

Unfortunately I'm not aware of a generic interrupts document. I just
don't see the point in each document listing interrupt-parent as a
requiredp roeprty when it's not. That said this is a trivial detail and
not really a blocker.

Cheers,
Mark
--
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 RFC] usb: gadget: Add xilinx axi usb2 device support

[Michal Simek]

 If we can point to standard interrupt description then please point me to
 exact description you would like to see here and we can change it.
 
 Unfortunately I'm not aware of a generic interrupts document. I just
 don't see the point in each document listing interrupt-parent as a
 requiredp roeprty when it's not. That said this is a trivial detail and
 not really a blocker.

I agree with you that copying this part again and again is just problematic.
Time to time I see that IRQs doesn't need to be described too.

I am also not sure if kernel can work without interrupt-parent at all.
I expect that it won't work and because we have interrupt parent in every
node (which is generated) it is probably required in our setup.

As you said it is just trivial detail for me too.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP - KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform




signature.asc
Description: OpenPGP digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Felipe Balbi]

Hi,

On Fri, Feb 21, 2014 at 11:27:07AM +, Subbaraya Sundeep Bhatta wrote:
  From the looks of it, I doubt this was actually tested, you need a lot
  of work on this driver.
 Tested on both ARM and Microblaze architectures with Mass storage gadget.
 Will send a v2 after addressing all your comments.

clearly you didn't try to remove and reinsert the module or you would
see a whole bunch of errors.

 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.

remove this footer message, btw, when sending emails to public mailing
lists.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Felipe Balbi]

Hi,

On Fri, Feb 21, 2014 at 12:04:54PM +, Mark Rutland wrote:
   +Example:
   + axi-usb2-device@42e0 {
   +compatible = xlnx,axi-usb2-device-4.00.a;
   +interrupt-parent = 0x1;
   +interrupts = 0x0 0x39 0x1;
   +reg = 0x42e0 0x1;
   +xlnx,include-dma = 0x1;
   +};
   +
  
  you need to Cc devicet...@vger.kernel.org for this.
 
 Cheers Filipe; thanks for adding us to Cc :)

sure, but it's Felipe ;-)

   + /* Map the registers */
   + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   + udc-base_address = devm_ioremap_nocache(pdev-dev, res-start,
   +  resource_size(res));
  
  use devm_ioremap_resource() instead.
 
 Also, res might be NULL. You should check that before dereferencing it.

not needed when using devm_ioremap_resource(), see the implementation.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Felipe Balbi]

Hi,

On Fri, Feb 21, 2014 at 02:41:03PM +0100, Michal Simek wrote:
  + /* Map the registers */
  + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  + udc-base_address = devm_ioremap_nocache(pdev-dev, res-start,
  +  resource_size(res));
 
  use devm_ioremap_resource() instead.
  
  Also, res might be NULL. You should check that before dereferencing it.
 
 yes it is necessary for both cases with devm_ioremap_nocache
 or with devm_ioremap_resource.

read the source Luke:

| void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res)
| {
|   resource_size_t size;
|   const char *name;
|   void __iomem *dest_ptr;
| 
|   BUG_ON(!dev);
| 
|   if (!res || resource_type(res) != IORESOURCE_MEM) {

already done for you

|   dev_err(dev, invalid resource\n);
|   return ERR_PTR(-EINVAL);
|   }
| 
|   size = resource_size(res);
|   name = res-name ?: dev_name(dev);
| 
|   if (!devm_request_mem_region(dev, res-start, size, name)) {
|   dev_err(dev, can't request region for resource %pR\n, res);
|   return ERR_PTR(-EBUSY);
|   }
| 
|   if (res-flags  IORESOURCE_CACHEABLE)
|   dest_ptr = devm_ioremap(dev, res-start, size);
|   else
|   dest_ptr = devm_ioremap_nocache(dev, res-start, size);
| 
|   if (!dest_ptr) {
|   dev_err(dev, ioremap failed for resource %pR\n, res);
|   devm_release_mem_region(dev, res-start, size);
|   dest_ptr = ERR_PTR(-ENOMEM);
|   }
| 
|   return dest_ptr;
| }

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Michal Simek]

On 02/21/2014 04:42 PM, Felipe Balbi wrote:
 Hi,
 
 On Fri, Feb 21, 2014 at 02:41:03PM +0100, Michal Simek wrote:
 + /* Map the registers */
 + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 + udc-base_address = devm_ioremap_nocache(pdev-dev, res-start,
 +  resource_size(res));

 use devm_ioremap_resource() instead.

 Also, res might be NULL. You should check that before dereferencing it.

 yes it is necessary for both cases with devm_ioremap_nocache
 or with devm_ioremap_resource.
 
 read the source Luke:
 
 | void __iomem *devm_ioremap_resource(struct device *dev, struct resource 
 *res)
 | {
 | resource_size_t size;
 | const char *name;
 | void __iomem *dest_ptr;
 | 
 | BUG_ON(!dev);
 | 
 | if (!res || resource_type(res) != IORESOURCE_MEM) {
 
   already done for you
 
 | dev_err(dev, invalid resource\n);
 | return ERR_PTR(-EINVAL);
 | }
 | 
 | size = resource_size(res);
 | name = res-name ?: dev_name(dev);
 | 
 | if (!devm_request_mem_region(dev, res-start, size, name)) {
 | dev_err(dev, can't request region for resource %pR\n, res);
 | return ERR_PTR(-EBUSY);
 | }
 | 
 | if (res-flags  IORESOURCE_CACHEABLE)
 | dest_ptr = devm_ioremap(dev, res-start, size);
 | else
 | dest_ptr = devm_ioremap_nocache(dev, res-start, size);

I have read it just not sure if IORESOURCE_CACHEABLE is setup by default
or not.
If yes, then you have to setup res-flags in your driver and have to
check it.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP - KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform




signature.asc
Description: OpenPGP digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Felipe Balbi]

On Fri, Feb 21, 2014 at 04:51:07PM +0100, Michal Simek wrote:
 On 02/21/2014 04:42 PM, Felipe Balbi wrote:
  Hi,
  
  On Fri, Feb 21, 2014 at 02:41:03PM +0100, Michal Simek wrote:
  +   /* Map the registers */
  +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  +   udc-base_address = devm_ioremap_nocache(pdev-dev, res-start,
  +resource_size(res));
 
  use devm_ioremap_resource() instead.
 
  Also, res might be NULL. You should check that before dereferencing it.
 
  yes it is necessary for both cases with devm_ioremap_nocache
  or with devm_ioremap_resource.
  
  read the source Luke:
  
  | void __iomem *devm_ioremap_resource(struct device *dev, struct resource 
  *res)
  | {
  |   resource_size_t size;
  |   const char *name;
  |   void __iomem *dest_ptr;
  | 
  |   BUG_ON(!dev);
  | 
  |   if (!res || resource_type(res) != IORESOURCE_MEM) {
  
  already done for you
  
  |   dev_err(dev, invalid resource\n);
  |   return ERR_PTR(-EINVAL);
  |   }
  | 
  |   size = resource_size(res);
  |   name = res-name ?: dev_name(dev);
  | 
  |   if (!devm_request_mem_region(dev, res-start, size, name)) {
  |   dev_err(dev, can't request region for resource %pR\n, res);
  |   return ERR_PTR(-EBUSY);
  |   }
  | 
  |   if (res-flags  IORESOURCE_CACHEABLE)
  |   dest_ptr = devm_ioremap(dev, res-start, size);
  |   else
  |   dest_ptr = devm_ioremap_nocache(dev, res-start, size);
 
 I have read it just not sure if IORESOURCE_CACHEABLE is setup by default
 or not.
 If yes, then you have to setup res-flags in your driver and have to
 check it.

you don't need IORESOURCe_CACHEABLE. It's fine the way it is, just use
the helper function ;-).

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Mark Rutland]

On Fri, Feb 21, 2014 at 03:41:03PM +, Felipe Balbi wrote:
 Hi,
 
 On Fri, Feb 21, 2014 at 12:04:54PM +, Mark Rutland wrote:
+Example:
+   axi-usb2-device@42e0 {
+compatible = xlnx,axi-usb2-device-4.00.a;
+interrupt-parent = 0x1;
+interrupts = 0x0 0x39 0x1;
+reg = 0x42e0 0x1;
+xlnx,include-dma = 0x1;
+};
+
   
   you need to Cc devicet...@vger.kernel.org for this.
  
  Cheers Filipe; thanks for adding us to Cc :)
 
 sure, but it's Felipe ;-)

Whoops; sorry Felipe :)

 
+   /* Map the registers */
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   udc-base_address = devm_ioremap_nocache(pdev-dev, res-start,
+resource_size(res));
   
   use devm_ioremap_resource() instead.
  
  Also, res might be NULL. You should check that before dereferencing it.
 
 not needed when using devm_ioremap_resource(), see the implementation.

Ah, good to know.

Cheers,
Mark.
--
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

SPDX-License-Identifier (was: Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support)

[Felipe Balbi]

On Fri, Feb 21, 2014 at 05:04:26PM +0100, Michal Simek wrote:
 On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote:
  On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote:
  BTW: u-boot started to use SPDX-License-Identifier
  which will be nice to start to use.
  
  I agree, feel free to start sending patches to use this type of
  identifier for drivers.
 
 But we probably need to add that Licenses to one location.
 Documentation/Licenses?

Just add to the drivers themselves, just like u-boot is doing. A simple:

$ git grep -e SPDX-License-Identifier

will tell you filename and license. Or did I misunderstand your question ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Arnd Bergmann]

On Friday 21 February 2014 16:51:07 Michal Simek wrote:
  | 
  | if (res-flags  IORESOURCE_CACHEABLE)
  | dest_ptr = devm_ioremap(dev, res-start, size);
  | else
  | dest_ptr = devm_ioremap_nocache(dev, res-start, size);
 
 I have read it just not sure if IORESOURCE_CACHEABLE is setup by default
 or not.
 If yes, then you have to setup res-flags in your driver and have to
 check it.

ioremap() and ioremap_nocache() are the same on all architectures.
If you want a cacheable mapping, you need to call ioremap_cache(),
which unfortunately doesn't exist on all architectures and also
doesn't have a devm_* variant.

I don't know how the above code made it into devm_ioremap_resource(),
it's clearly bogus. The only time we ever set IORESOURCE_CACHEABLE
is for ROM BARs on PCI, which in turn are rarely used in the kernel.
It's never set for any platform devices, aside from one use
in arch/arm/mach-clps711x/board-cdb89712.c.

Arnd
--
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 RFC] usb: gadget: Add xilinx axi usb2 device support

[Greg Kroah-Hartman]

On Fri, Feb 21, 2014 at 05:04:26PM +0100, Michal Simek wrote:
 On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote:
  On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote:
  BTW: u-boot started to use SPDX-License-Identifier
  which will be nice to start to use.
  
  I agree, feel free to start sending patches to use this type of
  identifier for drivers.
 
 But we probably need to add that Licenses to one location.

No, why would we need to do that?

--
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 RFC] usb: gadget: Add xilinx axi usb2 device support

[Felipe Balbi]

Hi,

On Wed, Feb 19, 2014 at 03:10:45PM +0530, Subbaraya Sundeep Bhatta wrote:
 This patch adds xilinx axi usb2 device driver support
 
 Signed-off-by: Subbaraya Sundeep Bhatta sbha...@xilinx.com
 ---
  .../devicetree/bindings/usb/xilinx_usb.txt |   21 +
  drivers/usb/gadget/Kconfig |   14 +
  drivers/usb/gadget/Makefile|1 +
  drivers/usb/gadget/xilinx_udc.c| 2045 
 
  4 files changed, 2081 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/usb/xilinx_usb.txt
  create mode 100644 drivers/usb/gadget/xilinx_udc.c
 
 diff --git a/Documentation/devicetree/bindings/usb/xilinx_usb.txt 
 b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
 new file mode 100644
 index 000..acf03ab
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
 @@ -0,0 +1,21 @@
 +Xilinx AXI USB2 device controller
 +
 +Required properties:
 +- compatible : Should be xlnx,axi-usb2-device-4.00.a
 +- reg: Physical base address and size of the Axi USB2
 +   device registers map.
 +- interrupts : Property with a value describing the interrupt
 +   number.
 +- interrupt-parent   : Must be core interrupt controller
 +- xlnx,include-dma   : Must be 1 or 0 based on whether DMA is included
 +   in IP or not.
 +
 +Example:
 + axi-usb2-device@42e0 {
 +compatible = xlnx,axi-usb2-device-4.00.a;
 +interrupt-parent = 0x1;
 +interrupts = 0x0 0x39 0x1;
 +reg = 0x42e0 0x1;
 +xlnx,include-dma = 0x1;
 +};
 +

you need to Cc devicet...@vger.kernel.org for this.

 diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
 index 8154165..0b284bf 100644
 --- a/drivers/usb/gadget/Kconfig
 +++ b/drivers/usb/gadget/Kconfig
 @@ -466,6 +466,20 @@ config USB_EG20T
 ML7213/ML7831 is companion chip for Intel Atom E6xx series.
 ML7213/ML7831 is completely compatible for Intel EG20T PCH.
  
 +config USB_GADGET_XILINX
 + tristate Xilinx USB Driver
 + depends on MICROBLAZE || ARCH_ZYNQ

This has the tendency to grow and I really don't like seeing a bunch of
arch-specific dependencies. At a minimum add COMPILE_TEST so I can build
test on my setup and make sure it builds fine on other architectures.

 + help
 +   USB peripheral controller driver for Xilinx AXI USB2 device.
 +   Xilinx AXI USB2 device is a soft IP which supports both full
 +   and high speed USB 2.0 data transfers. It has seven configurable
 +   endpoints(bulk or interrupt or isochronous), as well as
 +   endpoint zero(for control transfers).
 +
 +   Say y to link the driver statically, or m to build a
 +   dynamically linked module called xilinx_udc and force all
 +   gadget drivers to also be dynamically linked.
 +
  #
  # LAST -- dummy/emulated controller
  #
 diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
 index 5f150bc..3a55564 100644
 --- a/drivers/usb/gadget/Makefile
 +++ b/drivers/usb/gadget/Makefile
 @@ -36,6 +36,7 @@ obj-$(CONFIG_USB_FUSB300)   += fusb300_udc.o
  obj-$(CONFIG_USB_FOTG210_UDC)+= fotg210-udc.o
  obj-$(CONFIG_USB_MV_U3D) += mv_u3d_core.o
  obj-$(CONFIG_USB_GR_UDC) += gr_udc.o
 +obj-$(CONFIG_USB_GADGET_XILINX)  += xilinx_udc.o

let's start normalizing the names here (I'll patch the others later) and
call this udc-xilinx.o

 diff --git a/drivers/usb/gadget/xilinx_udc.c b/drivers/usb/gadget/xilinx_udc.c
 new file mode 100644
 index 000..3ee8359
 --- /dev/null
 +++ b/drivers/usb/gadget/xilinx_udc.c
 @@ -0,0 +1,2045 @@
 +/*
 + * Xilinx USB peripheral controller driver
 + *
 + * Copyright (C) 2004 by Thomas Rathbone
 + * Copyright (C) 2005 by HP Labs
 + * Copyright (C) 2005 by David Brownell

heh! :-) Brownell was really everywhere, good to still see code from him
;-)

 + * Copyright (C) 2010 - 2014 Xilinx, Inc.
 + *
 + * Some parts of this driver code is based on the driver for at91-series
 + * USB peripheral controller (at91_udc.c).
 + *
 + * This program is free software; you can redistribute it
 + * and/or modify it under the terms of the GNU General Public
 + * License as published by the Free Software Foundation;
 + * either version 2 of the License, or (at your option) any
 + * later version.

are you sure you want to allow people ot use GPL v3 on this driver ?

 +#include linux/interrupt.h
 +#include linux/device.h
 +#include linux/module.h
 +#include linux/prefetch.h
 +#include linux/usb/ch9.h
 +#include linux/usb/gadget.h
 +#include linux/io.h
 +#include linux/seq_file.h
 +#include linux/of_address.h
 +#include linux/of_device.h
 +#include linux/of_platform.h
 +#include linux/of_irq.h
 +#include linux/dma-mapping.h
 +#include gadget_chips.h
 +
 +/* 

Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support

[Michal Simek]

Hi,

 + * Copyright (C) 2010 - 2014 Xilinx, Inc.
 + *
 + * Some parts of this driver code is based on the driver for at91-series
 + * USB peripheral controller (at91_udc.c).
 + *
 + * This program is free software; you can redistribute it
 + * and/or modify it under the terms of the GNU General Public
 + * License as published by the Free Software Foundation;
 + * either version 2 of the License, or (at your option) any
 + * later version.
 
 are you sure you want to allow people ot use GPL v3 on this driver ?

The license is the same as is in at91_udc.c driver.
From my understanding if this driver is based on that one we
have to follow license from it.
And this is problem in general with all licenses in the kernel
which use this fragment.
I don't think we can change it to be just GPLv2.
Please correct me if I am wrong.

BTW: u-boot started to use SPDX-License-Identifier
which will be nice to start to use.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP - KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform




signature.asc
Description: OpenPGP digital signature