Re: usb HC busted?

[Sudip Mukherjee]

Hi Mathias,

On Thu, May 24, 2018 at 04:35:34PM +0300, Mathias Nyman wrote:
> Hi
> 
> On 24.05.2018 00:29, Sudip Mukherjee wrote:
> > Hi Mathias,
> > 
> > On Fri, May 18, 2018 at 04:19:02PM +0300, Mathias Nyman wrote:
> > > On 18.05.2018 16:04, Sudip Mukherjee wrote:

> > > > 
> > We have finally reproduced the error while traces were on. The trace and
> > the relevant part of the dmesg (when the error starts) are in:
> > https://drive.google.com/open?id=1PbcYwL1a9ndtHw1MNjE6uVqb0fFX9jV8
> > 
> > Will request you to have a look and suggest what might be going wrong here.
> > 
> 
> Log show two rings having the same TRB segment dma address, this will 
> completely mess up the transfer:
> 
> While allocating rigs the enque pointers for the two rings are the same:
> 
> 461.859315: xhci_ring_alloc: ISOC efa4e580: enq 
> 0x33386000(0x33386000) deq 
> 0x33386000(0x33386000) segs 2 stream 0 ...bs
> 461.859320: xhci_ring_alloc: ISOC f0ce1f00: enq 
> 0x33386000(0x33386000) deq 
> 0x33386000(0x33386000) segs 2 stream 0 ...
> 
> URBs for ISOC IN transfers are queued on EP3 at enqueue address (33386000 to 
> 33386140)
> 
> 461.859998: xhci_urb_enqueue: ep3in-isoc: urb f0ec0e00 pipe 4294528 slot 8 
> length 0/170 sgs 0/0 stream 0 flags 00010302
> 461.860004: xhci_queue_trb: ISOC: Buffer 2b480240 length 17 TD size 0 
> intr 0 type 'Isoch' flags b:i:I:c:s:I:e:c
> 461.860006: xhci_inc_enq: ISOC f0ce1f00: enq 
> 0x33386010(0x33386000) deq 
> 0x33386000(0x33386000
> 
> Later URBs for ISOC OUT transfers are queued at the same address, this should 
> not happen:
> 
> 461.901175: xhci_urb_enqueue: ep3out-isoc: urb ecec2600 pipe 100096 slot 8 
> length 0/51 sgs 0/0 stream 0 flags 00010002
> 461.901180: xhci_queue_trb: ISOC: Buffer 2d9fa805 length 17 TD size 0 
> intr 0 type 'Isoch' flags b:i:I:c:s:i:e:c
> 461.901181: xhci_inc_enq: ISOC efa4e580: enq 
> 0x33386010(0x33386000) deq 
> 0x33386000(0x33386000)
> 
> So something goes really wrong when allocating or setting up the rings in one 
> of these functions:
> xhci_ring_alloc()
> xhci_alloc_segments_for_ring()
> xhci_initialize_ring_info()
> xhci_segment_alloc()
> xhci_link_segments()
> dma_pool_zalloc()
> 
> To verify and rule out dma_pool_zalloc(), could you apply the attached patch 
> and reproduce with new logs?

I spoke too soon in my yesterday's mail. We were able to reproduce it
on the automated tests. The log and the trace is at:
https://drive.google.com/open?id=1h-3r-1lfjg8oblBGkzdRIq8z3ZNgGZx-

Will request you to have a look at it.

--
Regards
Sudip
--
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: usb HC busted?

[Sudip Mukherjee]

Hi Mathias,

On Thu, May 24, 2018 at 04:35:34PM +0300, Mathias Nyman wrote:
> Hi
> 
> On 24.05.2018 00:29, Sudip Mukherjee wrote:
> >Hi Mathias,
> >
> >>>On Fri, May 18, 2018 at 03:55:04PM +0300, Mathias Nyman wrote:
> >>>>Hi,

> >>>>
> >>>>
> >>>>Can you enable tracing for xhci and send me the output.
> >>>
> >We have finally reproduced the error while traces were on. The trace and
> >the relevant part of the dmesg (when the error starts) are in:
> >https://drive.google.com/open?id=1PbcYwL1a9ndtHw1MNjE6uVqb0fFX9jV8
> >
> >Will request you to have a look and suggest what might be going wrong here.
> >
> 
> Log show two rings having the same TRB segment dma address, this will 
> completely mess up the transfer:
> 
> While allocating rigs the enque pointers for the two rings are the same:
> 
> 461.859315: xhci_ring_alloc: ISOC efa4e580: enq 
> 0x33386000(0x33386000) deq 
> 0x33386000(0x33386000) segs 2 stream 0 ...bs
> 461.859320: xhci_ring_alloc: ISOC f0ce1f00: enq 
> 0x33386000(0x33386000) deq 
> 0x33386000(0x33386000) segs 2 stream 0 ...
> 
> URBs for ISOC IN transfers are queued on EP3 at enqueue address (33386000 to 
> 33386140)
> 
> 461.859998: xhci_urb_enqueue: ep3in-isoc: urb f0ec0e00 pipe 4294528 slot 8 
> length 0/170 sgs 0/0 stream 0 flags 00010302
> 461.860004: xhci_queue_trb: ISOC: Buffer 2b480240 length 17 TD size 0 
> intr 0 type 'Isoch' flags b:i:I:c:s:I:e:c
> 461.860006: xhci_inc_enq: ISOC f0ce1f00: enq 
> 0x33386010(0x33386000) deq 
> 0x33386000(0x33386000
> 
> Later URBs for ISOC OUT transfers are queued at the same address, this should 
> not happen:
> 
> 461.901175: xhci_urb_enqueue: ep3out-isoc: urb ecec2600 pipe 100096 slot 8 
> length 0/51 sgs 0/0 stream 0 flags 00010002
> 461.901180: xhci_queue_trb: ISOC: Buffer 2d9fa805 length 17 TD size 0 
> intr 0 type 'Isoch' flags b:i:I:c:s:i:e:c
> 461.901181: xhci_inc_enq: ISOC efa4e580: enq 
> 0x33386010(0x33386000) deq 
> 0x33386000(0x33386000)
> 
> So something goes really wrong when allocating or setting up the rings in one 
> of these functions:
> xhci_ring_alloc()
> xhci_alloc_segments_for_ring()
> xhci_initialize_ring_info()
> xhci_segment_alloc()
> xhci_link_segments()
> dma_pool_zalloc()
> 
> To verify and rule out dma_pool_zalloc(), could you apply the attached patch 
> and reproduce with new logs?

We tested for the full week but still could not reproduce with the patch
applied. We are still trying and will be setting up automated tests for
this. And, since we are not able to reproduce it, I was wondering if it
is somekind of race and the applied patch with extra tracing has changed
the timing in such a way that it is not seen now. And also, wondering if
2b3ff282dff3 ("xhci: Don't add a virt_dev to the devs array before it's fully 
allocated")
will be of any help to us.

--
Regards
Sudip
--
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: usb HC busted?

[Sudip Mukherjee]

Hi Mathias,

On Fri, May 18, 2018 at 04:19:02PM +0300, Mathias Nyman wrote:
> On 18.05.2018 16:04, Sudip Mukherjee wrote:
> > Hi Mathias,
> > 
> > On Fri, May 18, 2018 at 03:55:04PM +0300, Mathias Nyman wrote:
> > > Hi,
> > > 
> > > Looks like event for Transfer block (TRB) at 0x32a21060 was never 
> > > completed,
> > > or at least not handled by xhci driver.
> > > (either the event was never issued by hw, or something got messed up in 
> > > the driver along the way)
> > > 
> > > HC doesn't look busted, it continues sending transfer completions events.
> > > it is already at event 0x32a211d0, which is 23 TRBS later. (one TRB is 
> > > 0x10)
> > > 
> > > This small log sinppet doesnt' say much about the reasons.
> > > 
> > > Can you enable tracing for xhci and send me the output.
> > 
> > I saw in another thread that you will ask for the trace log and so I
> > was making a kernel with tracer enabled. I was also thinking of getting
> > the logs from usbmon. Will that be of any help?
> > 
> 
> Not sure yet, in this case i think traces are most interesting, then dynamic 
> debug messages
> (which spams dmesg and affects timing) and last usbmon.

We have finally reproduced the error while traces were on. The trace and
the relevant part of the dmesg (when the error starts) are in:
https://drive.google.com/open?id=1PbcYwL1a9ndtHw1MNjE6uVqb0fFX9jV8

Will request you to have a look and suggest what might be going wrong here.


--
Regards
Sudip
--
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: usb HC busted?

[Sudip Mukherjee]

Hi Mathias,

On Fri, May 18, 2018 at 03:55:04PM +0300, Mathias Nyman wrote:
> Hi,
> 
> Looks like event for Transfer block (TRB) at 0x32a21060 was never completed,
> or at least not handled by xhci driver.
> (either the event was never issued by hw, or something got messed up in the 
> driver along the way)
> 
> HC doesn't look busted, it continues sending transfer completions events.
> it is already at event 0x32a211d0, which is 23 TRBS later. (one TRB is 0x10)
> 
> This small log sinppet doesnt' say much about the reasons.
> 
> Can you enable tracing for xhci and send me the output.

I saw in another thread that you will ask for the trace log and so I
was making a kernel with tracer enabled. I was also thinking of getting
the logs from usbmon. Will that be of any help?

--
Regards
Sudip
--
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

usb HC busted?

[Sudip Mukherjee]

Hi Mathias,

We are using an Intel Atom based board and the usb controller is:

00:14.0 USB Controller: Intel Corporation Device 0f35 (rev 11) (prog-if 30)
Flags: bus master, medium devsel, latency 0, IRQ 130
Memory at df7e (64-bit, non-prefetchable) [size=64K]
Capabilities: [70] Power Management version 2
Capabilities: [80] Message Signalled Interrupts: Mask- 64bit+ Queue=0/3 
Enable+
Kernel driver in use: xhci_hcd

Occassionally we are seeing that the dmesg is flooded with errors like:
[  607.763965] xhci_hcd :00:14.0: WARN Event TRB for slot 8 ep 5 with no 
TDs queued?
[  607.764968] xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part 
of current TD ep_index 5 comp_code 1
[  607.764976] xhci_hcd :00:14.0: Looking for event-dma 32a211d0 
trb-start 32a21060 trb-end 32a21060 seg-start 32a21000 
seg-end 32a21ff0

We are using v4.14.2 kernel and I have also applied:
e4ec40ec4b26 ("xhci: Don't show incorrect WARN message about events for empty 
rings")
on top of it. The code says "HC is busted, give up!".
Should I really give up or something can be done at this point?

Will really your appreciate your help in this.

--
Regards
Sudip
--
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 v2] usb: host: oxu210hp-hcd: remove unused variable

[Sudip Mukherjee]

From: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>

The variable live was assigned the host controller running status but
it was never used or checked after that.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---

v2: added From: tag

 drivers/usb/host/oxu210hp-hcd.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index 4e4d601..bcf531c 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -2288,9 +2288,7 @@ static void scan_periodic(struct oxu_hcd *oxu)
 
while (q.ptr != NULL) {
union ehci_shadow temp;
-   int live;
 
-   live = HC_IS_RUNNING(oxu_to_hcd(oxu)->state);
switch (type) {
case Q_TYPE_QH:
/* handle any completions */
-- 
1.9.1

--
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 v2] usb: storage: ene_ub6250: remove unused variable

[Sudip Mukherjee]

From: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>

The variable Newblk was only being assigned some value but was never
used after that.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---

v2: added From: line

 drivers/usb/storage/ene_ub6250.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index 02bdaa9..369f3c2 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -1364,7 +1364,6 @@ static int ms_lib_read_extra(struct us_data *us, u32 
PhyBlock,
 
 static int ms_libsearch_block_from_physical(struct us_data *us, u16 phyblk)
 {
-   u16 Newblk;
u16 blk;
struct ms_lib_type_extdat extdat; /* need check */
struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;
@@ -1377,7 +1376,6 @@ static int ms_libsearch_block_from_physical(struct 
us_data *us, u16 phyblk)
if ((blk & MS_PHYSICAL_BLOCKS_PER_SEGMENT_MASK) == 0)
blk -= MS_PHYSICAL_BLOCKS_PER_SEGMENT;
 
-   Newblk = info->MS_Lib.Phy2LogMap[blk];
if (info->MS_Lib.Phy2LogMap[blk] == MS_LB_NOT_USED_ERASED) {
return blk;
} else if (info->MS_Lib.Phy2LogMap[blk] == MS_LB_NOT_USED) {
-- 
1.9.1

--
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] usb: storage: ene_ub6250: remove unused variable

[Sudip Mukherjee]

On Thursday 05 January 2017 06:36 PM, Greg Kroah-Hartman wrote:

On Sun, Dec 18, 2016 at 10:34:31PM +, Sudip Mukherjee wrote:

The variable Newblk was only being assigned some value but was never
used after that.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
  drivers/usb/storage/ene_ub6250.c | 2 --
  1 file changed, 2 deletions(-)


from and signed-off-by does not match :(

Same with your other usb patch...



I can add the From: tag in the patch. But this is from the last time we 
had this same conversation.


http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-February/065121.html


Regards
Sudip
--
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 v2] USB: cypress_m8: remove unused variable

[Sudip Mukherjee]

From: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>

The variable havedata was only being set but never used afterwards.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---

v2: changed the from line

 drivers/usb/serial/cypress_m8.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index bbeeb2b..90110de 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -1069,7 +1069,6 @@ static void cypress_read_int_callback(struct urb *urb)
unsigned char *data = urb->transfer_buffer;
unsigned long flags;
char tty_flag = TTY_NORMAL;
-   int havedata = 0;
int bytes = 0;
int result;
int i = 0;
@@ -1118,16 +1117,12 @@ static void cypress_read_int_callback(struct urb *urb)
priv->current_status = data[0] & 0xF8;
bytes = data[1] + 2;
i = 2;
-   if (bytes > 2)
-   havedata = 1;
break;
case packet_format_2:
/* This is for the CY7C63743... */
priv->current_status = data[0] & 0xF8;
bytes = (data[0] & 0x07) + 1;
i = 1;
-   if (bytes > 1)
-   havedata = 1;
break;
}
spin_unlock_irqrestore(>lock, flags);
-- 
1.9.1

--
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] usbip: vudc: check for NULL before use

[Sudip Mukherjee]

On Wed, Dec 21, 2016 at 07:38:21AM -0700, Shuah Khan wrote:
> Hi Sudip,
> 
> On Wed, Dec 21, 2016 at 6:33 AM, Sudip Mukherjee
> <sudipm.mukher...@gmail.com> wrote:
> > On Tue, Dec 20, 2016 at 07:31:44AM -0700, Shuah Khan wrote:
> >> On 12/18/2016 03:44 PM, Sudip Mukherjee wrote:
> >> > to_vep() is doing a container_of() on _ep. It is better to do the NULL
> >> > check first and then use it.
> >> >
> >> > Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
> >> > ---
> >> >  drivers/usb/usbip/vudc_dev.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c
> >> > index 968471b..32ea604 100644
> >> > --- a/drivers/usb/usbip/vudc_dev.c
> >> > +++ b/drivers/usb/usbip/vudc_dev.c
> >> > @@ -388,10 +388,10 @@ static int vep_dequeue(struct usb_ep *_ep, struct 
> >> > usb_request *_req)
> >> > unsigned long flags;
> >> > int ret = 0;
> >> >
> >> > -   ep = to_vep(_ep);
> >> > if (!_ep)
> >> > return -EINVAL;
> >>
> >> Hmm. Linus's latest checks _ep and _req. Are you sure you are working
> >> with the latest tree?
> >
> > I checked with next-20161221 and its still there.
> 
> This is for  vep_dequeue() - Are you sure both linux-next and Linus's tree 
> show
> the following:

This is for vep_set_halt_and_wedge(). I do not have any idea why the
patch says its vep_dequeue(). I tried generating the patch again and
it still shows as vep_dequeue(). But the line number 388 is correct and
if you try to apply, it applies correctly.

regards
sudip
--
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] usbip: vudc: check for NULL before use

[Sudip Mukherjee]

On Tue, Dec 20, 2016 at 07:31:44AM -0700, Shuah Khan wrote:
> On 12/18/2016 03:44 PM, Sudip Mukherjee wrote:
> > to_vep() is doing a container_of() on _ep. It is better to do the NULL
> > check first and then use it.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
> > ---
> >  drivers/usb/usbip/vudc_dev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c
> > index 968471b..32ea604 100644
> > --- a/drivers/usb/usbip/vudc_dev.c
> > +++ b/drivers/usb/usbip/vudc_dev.c
> > @@ -388,10 +388,10 @@ static int vep_dequeue(struct usb_ep *_ep, struct 
> > usb_request *_req)
> > unsigned long flags;
> > int ret = 0;
> >  
> > -   ep = to_vep(_ep);
> > if (!_ep)
> > return -EINVAL;
> 
> Hmm. Linus's latest checks _ep and _req. Are you sure you are working
> with the latest tree?

I checked with next-20161221 and its still there.

regards
sudip
--
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] usb: host: oxu210hp-hcd: remove unused variable

[Sudip Mukherjee]

The variable live was assigned the host controller running status but
it was never used or checked after that.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/host/oxu210hp-hcd.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index 4e4d601..bcf531c 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -2288,9 +2288,7 @@ static void scan_periodic(struct oxu_hcd *oxu)
 
while (q.ptr != NULL) {
union ehci_shadow temp;
-   int live;
 
-   live = HC_IS_RUNNING(oxu_to_hcd(oxu)->state);
switch (type) {
case Q_TYPE_QH:
/* handle any completions */
-- 
1.9.1

--
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] usbip: vudc: check for NULL before use

[Sudip Mukherjee]

to_vep() is doing a container_of() on _ep. It is better to do the NULL
check first and then use it.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/usbip/vudc_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c
index 968471b..32ea604 100644
--- a/drivers/usb/usbip/vudc_dev.c
+++ b/drivers/usb/usbip/vudc_dev.c
@@ -388,10 +388,10 @@ static int vep_dequeue(struct usb_ep *_ep, struct 
usb_request *_req)
unsigned long flags;
int ret = 0;
 
-   ep = to_vep(_ep);
if (!_ep)
return -EINVAL;
 
+   ep = to_vep(_ep);
udc = ep_to_vudc(ep);
if (!udc->driver)
return -ESHUTDOWN;
-- 
1.9.1

--
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] usb: storage: sddr09: remove unused variable

[Sudip Mukherjee]

The variable isnew was only assigned 0 or 1 but was never used after
that.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/storage/sddr09.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c
index 3aeaa53..44f8ffc 100644
--- a/drivers/usb/storage/sddr09.c
+++ b/drivers/usb/storage/sddr09.c
@@ -870,13 +870,12 @@ struct sddr09_card_info {
unsigned int pagelen;
unsigned char *bptr, *cptr, *xptr;
unsigned char ecc[3];
-   int i, result, isnew;
+   int i, result;
 
lbap = ((lba % 1000) << 1) | 0x1000;
if (parity[MSB_of(lbap) ^ LSB_of(lbap)])
lbap ^= 1;
pba = info->lba_to_pba[lba];
-   isnew = 0;
 
if (pba == UNDEF) {
pba = sddr09_find_unused_pba(info, lba);
@@ -887,7 +886,6 @@ struct sddr09_card_info {
}
info->pba_to_lba[pba] = lba;
info->lba_to_pba[lba] = pba;
-   isnew = 1;
}
 
if (pba == 1) {
-- 
1.9.1

--
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] usb: storage: ene_ub6250: remove unused variable

[Sudip Mukherjee]

The variable Newblk was only being assigned some value but was never
used after that.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/storage/ene_ub6250.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index 02bdaa9..369f3c2 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -1364,7 +1364,6 @@ static int ms_lib_read_extra(struct us_data *us, u32 
PhyBlock,
 
 static int ms_libsearch_block_from_physical(struct us_data *us, u16 phyblk)
 {
-   u16 Newblk;
u16 blk;
struct ms_lib_type_extdat extdat; /* need check */
struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;
@@ -1377,7 +1376,6 @@ static int ms_libsearch_block_from_physical(struct 
us_data *us, u16 phyblk)
if ((blk & MS_PHYSICAL_BLOCKS_PER_SEGMENT_MASK) == 0)
blk -= MS_PHYSICAL_BLOCKS_PER_SEGMENT;
 
-   Newblk = info->MS_Lib.Phy2LogMap[blk];
if (info->MS_Lib.Phy2LogMap[blk] == MS_LB_NOT_USED_ERASED) {
return blk;
} else if (info->MS_Lib.Phy2LogMap[blk] == MS_LB_NOT_USED) {
-- 
1.9.1

--
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] USB: cypress_m8: remove unused variable

[Sudip Mukherjee]

The variable havedata was only being set but never used afterwards.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/serial/cypress_m8.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index bbeeb2b..90110de 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -1069,7 +1069,6 @@ static void cypress_read_int_callback(struct urb *urb)
unsigned char *data = urb->transfer_buffer;
unsigned long flags;
char tty_flag = TTY_NORMAL;
-   int havedata = 0;
int bytes = 0;
int result;
int i = 0;
@@ -1118,16 +1117,12 @@ static void cypress_read_int_callback(struct urb *urb)
priv->current_status = data[0] & 0xF8;
bytes = data[1] + 2;
i = 2;
-   if (bytes > 2)
-   havedata = 1;
break;
case packet_format_2:
/* This is for the CY7C63743... */
priv->current_status = data[0] & 0xF8;
bytes = (data[0] & 0x07) + 1;
i = 1;
-   if (bytes > 1)
-   havedata = 1;
break;
}
spin_unlock_irqrestore(>lock, flags);
-- 
1.9.1

--
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] usb: c67x00: remove unused variable

[Sudip Mukherjee]

The pointer urbp was only assigned some address but was never used.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/c67x00/c67x00-sched.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/c67x00/c67x00-sched.c 
b/drivers/usb/c67x00/c67x00-sched.c
index 7311ed6..0b68cd6 100644
--- a/drivers/usb/c67x00/c67x00-sched.c
+++ b/drivers/usb/c67x00/c67x00-sched.c
@@ -966,13 +966,11 @@ static void c67x00_handle_successful_td(struct c67x00_hcd 
*c67x00,
 static void c67x00_handle_isoc(struct c67x00_hcd *c67x00, struct c67x00_td *td)
 {
struct urb *urb = td->urb;
-   struct c67x00_urb_priv *urbp;
int cnt;
 
if (!urb)
return;
 
-   urbp = urb->hcpriv;
cnt = td->privdata;
 
if (td->status & TD_ERROR_MASK)
-- 
1.9.1

--
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] usb: atm: cxacru: remove impossible condition

[Sudip Mukherjee]

The variable index is unsigned and so it can never be less than 0.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/atm/cxacru.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index f9fe86b6..d65a64c 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -474,7 +474,7 @@ static ssize_t cxacru_sysfs_store_adsl_config(struct device 
*dev,
ret = sscanf(buf + pos, "%x=%x%n", , , );
if (ret < 2)
return -EINVAL;
-   if (index < 0 || index > 0x7f)
+   if (index > 0x7f)
return -EINVAL;
if (tmp < 0 || tmp > len - pos)
return -EINVAL;
-- 
1.9.1

--
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] usb: mtu3: declare functions static

[Sudip Mukherjee]

The only user of the  functions mtu3_irq() and gpd_ring_empty() are in
the same file. They can be declared as static.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/mtu3/mtu3_core.c | 2 +-
 drivers/usb/mtu3/mtu3_qmu.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
index 520e55a..603b7f84 100644
--- a/drivers/usb/mtu3/mtu3_core.c
+++ b/drivers/usb/mtu3/mtu3_core.c
@@ -696,7 +696,7 @@ static irqreturn_t mtu3_u2_common_isr(struct mtu3 *mtu)
return IRQ_HANDLED;
 }
 
-irqreturn_t mtu3_irq(int irq, void *data)
+static irqreturn_t mtu3_irq(int irq, void *data)
 {
struct mtu3 *mtu = (struct mtu3 *)data;
unsigned long flags;
diff --git a/drivers/usb/mtu3/mtu3_qmu.c b/drivers/usb/mtu3/mtu3_qmu.c
index a6dd292..7d9ba8a 100644
--- a/drivers/usb/mtu3/mtu3_qmu.c
+++ b/drivers/usb/mtu3/mtu3_qmu.c
@@ -168,7 +168,7 @@ static struct qmu_gpd *advance_deq_gpd(struct mtu3_gpd_ring 
*ring)
 }
 
 /* check if a ring is emtpy */
-int gpd_ring_empty(struct mtu3_gpd_ring *ring)
+static int gpd_ring_empty(struct mtu3_gpd_ring *ring)
 {
struct qmu_gpd *enq = ring->enqueue;
struct qmu_gpd *next;
-- 
1.9.1

--
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] HID: usbkbd: return proper error code

[Sudip Mukherjee]

On Thursday 01 September 2016 04:51 PM, Jiri Kosina wrote:

On Wed, 31 Aug 2016, Sudip Mukherjee wrote:


Use proper error code instead of using -1 on failure to allocate
memory. We may use the error code later in the caller.


But we don't. usb_kbd_probe() returns -ENOMEM in case usb_kbd_alloc_mem()
fails anyway, so I fail to see the point of the change really.



Well, yes, we don't as of now.
When I was reading the code for something related to my day job I was a 
bit confused with -1 instead of a proper error code. I am sure there 
will be many others like me.

Its fine if you think the change is not needed.

regards
sudip
--
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] HID: usbkbd: return proper error code

[Sudip Mukherjee]

On Wednesday 31 August 2016 10:03 PM, Fabio Estevam wrote:

On Wed, Aug 31, 2016 at 1:28 PM, Sudip Mukherjee
<sudipm.mukher...@gmail.com> wrote:

Use proper error code instead of using -1 on failure to allocate
memory. We may use the error code later in the caller.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
  drivers/hid/usbhid/usbkbd.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
index 9a332e6..ee53359 100644
--- a/drivers/hid/usbhid/usbkbd.c
+++ b/drivers/hid/usbhid/usbkbd.c
@@ -249,15 +249,15 @@ static void usb_kbd_close(struct input_dev *dev)
  static int usb_kbd_alloc_mem(struct usb_device *dev, struct usb_kbd *kbd)
  {
 if (!(kbd->irq = usb_alloc_urb(0, GFP_KERNEL)))
-   return -1;
+   return -ENOMEM;


While you are it, the code would look better like this:

kbd->irq = usb_alloc_urb(0, GFP_KERNEL)
if (!kbd->irq)
 return -ENOMEM;



Yes, it will. But that will become two changes in one patch. I will send 
a series with this sent patch and another patch to reorder the assignment.


regards
sudip
--
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] HID: usbkbd: return proper error code

[Sudip Mukherjee]

Use proper error code instead of using -1 on failure to allocate
memory. We may use the error code later in the caller.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/hid/usbhid/usbkbd.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
index 9a332e6..ee53359 100644
--- a/drivers/hid/usbhid/usbkbd.c
+++ b/drivers/hid/usbhid/usbkbd.c
@@ -249,15 +249,15 @@ static void usb_kbd_close(struct input_dev *dev)
 static int usb_kbd_alloc_mem(struct usb_device *dev, struct usb_kbd *kbd)
 {
if (!(kbd->irq = usb_alloc_urb(0, GFP_KERNEL)))
-   return -1;
+   return -ENOMEM;
if (!(kbd->led = usb_alloc_urb(0, GFP_KERNEL)))
-   return -1;
+   return -ENOMEM;
if (!(kbd->new = usb_alloc_coherent(dev, 8, GFP_ATOMIC, >new_dma)))
-   return -1;
+   return -ENOMEM;
if (!(kbd->cr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL)))
-   return -1;
+   return -ENOMEM;
if (!(kbd->leds = usb_alloc_coherent(dev, 1, GFP_ATOMIC, 
>leds_dma)))
-   return -1;
+   return -ENOMEM;
 
return 0;
 }
-- 
1.9.1

--
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: USB broken on Banana Pi in Linux 4.6

[Sudip Mukherjee]

On Sun, Jun 12, 2016 at 06:47:56PM +0200, Marc Haber wrote:
> On Sat, Jun 11, 2016 at 02:55:04PM +0200, Marc Haber wrote:
> > On Tue, Jun 07, 2016 at 10:30:17AM -0700, Greg KH wrote:
> > > Nothing obvious, can you use 'git bisect' to go from 4.5.0 to 4.6.0 to
> > > find the offending commit?
> > 
> > I can. The first round of bisecting let me end up with
> > c8b710b3e4348119924051551b836c94835331b1 as the first bad commit,
> > which is wrong, since c8b710b3e4348119924051551b836c94835331b1^ is bad
> > as well. I am not sure whether things went well since I had to use git
> > bisect skip twice because the resulting kernel wouldn't boot on the pi.
> 
> The kernel panic on boot is caused by bugs in the parport part. I
> worked around these by disabling PARPORT in the kernel configuration.

May I ask which bug in parport are you reffering to? There was a bad
commit in ppdev which caused boot failure and that was reverted.
But in parport??

regards
sudip
--
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] usb: usbip: remove null check

[Sudip Mukherjee]

The only caller of get_gadget_descs() has already dereferenced udc
before calling this function, so udc can not be NULL at this point of
the code and hence no use of checking it.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/usbip/vudc_sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c
index 99397fa..0f98f2c 100644
--- a/drivers/usb/usbip/vudc_sysfs.c
+++ b/drivers/usb/usbip/vudc_sysfs.c
@@ -40,7 +40,7 @@ int get_gadget_descs(struct vudc *udc)
struct usb_ctrlrequest req;
int ret;
 
-   if (!udc || !udc->driver || !udc->pullup)
+   if (!udc->driver || !udc->pullup)
return -EINVAL;
 
req.bRequestType = USB_DIR_IN | USB_TYPE_STANDARD | USB_RECIP_DEVICE;
-- 
1.9.1

--
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] usb: usbip: fix null pointer dereference

[Sudip Mukherjee]

On Mon, Jun 06, 2016 at 10:20:37AM +0200, Krzysztof Opasiak wrote:
> 
> 
> On 06/05/2016 07:54 PM, Sudip Mukherjee wrote:

> > 
> > Yes, I should have seen earlier that the only caller has already
> > dereferenced udc. So maybe the following will be appropriate in this
> > situation.
> >
> 
> Your patch does exactly what Alan suggested and I agree with it;)

yes, i saw those mails after i sent the reply with this proposed patch.

> 
> Could you please resend this properly so Greg can easily pick it up?

will send it tonight.

regards
sudip
--
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] usb: usbip: fix null pointer dereference

[Sudip Mukherjee]

On Friday 03 June 2016 09:29 AM, Krzysztof Opasiak wrote:



On 06/02/2016 03:22 PM, Sudip Mukherjee wrote:

We have been dereferencing udc before checking it. Lets use it after it
has been checked.



To be honest I have mixed feelings about this patch.

On one hand it prevents us from dereferencing potential NULL ptr what is
generally good. But on the other hand it seems to be a little bit
pointless overhead. This function is called only in one place, it's
internal function of vudc driver and in addition generally it is
currently impossible that this function will get NULL ptr as parameter
as it's value is taken from container_of(). Not to mention that if this
is NULL or garbage we will end up in NULL ptr dereference much earlier
before calling this function.

So if there is something that you would like to fix with this patch and
you have a real problem with this function could you please provide us
some more details (for example stack trace)? If this patch is just to
prevent us from something that will never happen then I would rather to
not submit this. In my opinion if we get a NULL in this function this
means that we have some serious problem in UDC core and this check will
just mask this error.


Yes, I should have seen earlier that the only caller has already 
dereferenced udc. So maybe the following will be appropriate in this 
situation.


Regards
Sudip

diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c
index 99397fa..0f98f2c 100644
--- a/drivers/usb/usbip/vudc_sysfs.c
+++ b/drivers/usb/usbip/vudc_sysfs.c
@@ -40,7 +40,7 @@ int get_gadget_descs(struct vudc *udc)
struct usb_ctrlrequest req;
int ret;
 
-   if (!udc || !udc->driver || !udc->pullup)
+   if (!udc->driver || !udc->pullup)
return -EINVAL;
 
req.bRequestType = USB_DIR_IN | USB_TYPE_STANDARD | USB_RECIP_DEVICE;

[PATCH] usb: usbip: fix null pointer dereference

[Sudip Mukherjee]

We have been dereferencing udc before checking it. Lets use it after it
has been checked.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/usbip/vudc_sysfs.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c
index 99397fa..0ba6a06 100644
--- a/drivers/usb/usbip/vudc_sysfs.c
+++ b/drivers/usb/usbip/vudc_sysfs.c
@@ -35,14 +35,17 @@
 int get_gadget_descs(struct vudc *udc)
 {
struct vrequest *usb_req;
-   struct vep *ep0 = to_vep(udc->gadget.ep0);
-   struct usb_device_descriptor *ddesc = >dev_desc;
+   struct vep *ep0;
+   struct usb_device_descriptor *ddesc;
struct usb_ctrlrequest req;
int ret;
 
if (!udc || !udc->driver || !udc->pullup)
return -EINVAL;
 
+   ep0 = to_vep(udc->gadget.ep0);
+   ddesc = >dev_desc;
+
req.bRequestType = USB_DIR_IN | USB_TYPE_STANDARD | USB_RECIP_DEVICE;
req.bRequest = USB_REQ_GET_DESCRIPTOR;
req.wValue = cpu_to_le16(USB_DT_DEVICE << 8);
-- 
1.9.1

--
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: Problems with MCS7715 USB-attached parallel port

[Sudip Mukherjee]

On Tuesday 19 April 2016 10:26 PM, Alex Henrie wrote:

2016-04-19 10:44 GMT-06:00 Sudip Mukherjee <sudipm.mukher...@gmail.com>:

On Tuesday 19 April 2016 06:52 AM, Greg KH wrote:


On Mon, Apr 18, 2016 at 06:11:51PM -0600, Alex Henrie wrote:


Hi,

I recently bought an MCS7715 USB-attached parallel port,[1] but there
seem to be a couple of problems using it with Linux:

1. The lp, parport, and parport_pc kernel modules are not loaded when
the device is plugged in.
2. After manually loading the kernel modules, /dev/lp0 is not deleted
when the device is unplugged.

I'm using a fully updated copy of Arch Linux, and at first I thought
it was a udev rules problem, but the systemd guys think that it's
actually a kernel bug.[2] Is this something that you could fix, or
guide me thorough fixing? I'd be happy to donate one of these devices
if you don't have one handy for debugging.



If you have a spare one I will really appreciate having one for debugging.


Sure, just tell me what address to mail it to and it's yours. I'll
send more than one if other people want to collaborate with you.


I am not much familiar with Arch-Linux. which kernel version is it having?


4.5.1-1-ARCH


Just an update:
Two patches have been submitted which should solve part of the problem. 
lp needs more work. But I am afraid module autoloading will not work at 
the moment or in near future. That is last in the list after parport is 
fully converted to device model.


Regards
Sudip
--
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] USB: mos7720: delete parport

[Sudip Mukherjee]

parport subsystem has introduced parport_del_port() to delete a port
when it is going away. Without parport_del_port() the registered port
will not be unregistered.
To reproduce and verify the error:
Command to be used is : ls /sys/bus/parport/devices
1) without the device attached there is no output as there is no
registered parport.
2) Attach the device, and the command will show "parport0".
3) Remove the device and the command still shows "parport0".
4) Attach the device again and we get "parport1".

With the patch applied:
1) without the device attached there is no output as there is no
registered parport.
2) Attach the device, and the command will show "parport0".
3) Remove the device and there is no output as "parport0" is now
removed.
4) Attach device again to get "parport0" again.

Cc: <sta...@vger.kernel.org> # 4.2+
Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---

Tested with MosChip Semiconductor MCS7715 Parallel and serial port
adapter.

Just a minor hitch.
Please perform the above test after removing ppdev and lp modules. They
are having some problems in the release functions and are not releasing
the parport on which they are attached. Separate patches are getting
ready for them.

 drivers/usb/serial/mos7720.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index 2eddbe5..5608af4 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -2007,6 +2007,7 @@ static void mos7720_release(struct usb_serial *serial)
urblist_entry)
usb_unlink_urb(urbtrack->urb);
spin_unlock_irqrestore(_parport->listlock, flags);
+   parport_del_port(mos_parport->pp);
 
kref_put(_parport->ref_count, destroy_mos_parport);
}
-- 
1.9.1

--
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 v2] usb: renesas_usbhs: fix signed-unsigned return

[Sudip Mukherjee]

The return type of usbhsp_setup_pipecfg() was u16 but it was returning
a negative value (-EINVAL). Lets have an additional argument which will
have pipecfg and just return the status (success or error) as the return
from the function.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---

v2: added pipecfg as an argument.

 drivers/usb/renesas_usbhs/pipe.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/pipe.c b/drivers/usb/renesas_usbhs/pipe.c
index 77b615c..c238772 100644
--- a/drivers/usb/renesas_usbhs/pipe.c
+++ b/drivers/usb/renesas_usbhs/pipe.c
@@ -391,9 +391,8 @@ void usbhs_pipe_set_trans_count_if_bulk(struct usbhs_pipe 
*pipe, int len)
 /*
  * pipe setup
  */
-static u16 usbhsp_setup_pipecfg(struct usbhs_pipe *pipe,
-   int is_host,
-   int dir_in)
+static int usbhsp_setup_pipecfg(struct usbhs_pipe *pipe, int is_host,
+   int dir_in, u16 *pipecfg)
 {
u16 type = 0;
u16 bfre = 0;
@@ -451,14 +450,14 @@ static u16 usbhsp_setup_pipecfg(struct usbhs_pipe *pipe,
 
/* EPNUM */
epnum = 0; /* see usbhs_pipe_config_update() */
-
-   return  type|
-   bfre|
-   dblb|
-   cntmd   |
-   dir |
-   shtnak  |
-   epnum;
+   *pipecfg = type |
+  bfre |
+  dblb |
+  cntmd|
+  dir  |
+  shtnak   |
+  epnum;
+   return 0;
 }
 
 static u16 usbhsp_setup_pipebuff(struct usbhs_pipe *pipe)
@@ -703,7 +702,11 @@ struct usbhs_pipe *usbhs_pipe_malloc(struct usbhs_priv 
*priv,
return NULL;
}
 
-   pipecfg  = usbhsp_setup_pipecfg(pipe, is_host, dir_in);
+   if (usbhsp_setup_pipecfg(pipe, is_host, dir_in, )) {
+   dev_err(dev, "can't setup pipe\n");
+   return NULL;
+   }
+
pipebuf  = usbhsp_setup_pipebuff(pipe);
 
usbhsp_pipe_select(pipe);
-- 
1.9.1

--
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: Problems with MCS7715 USB-attached parallel port

[Sudip Mukherjee]

On Tuesday 19 April 2016 10:26 PM, Alex Henrie wrote:

2016-04-19 10:44 GMT-06:00 Sudip Mukherjee <sudipm.mukher...@gmail.com>:

On Tuesday 19 April 2016 06:52 AM, Greg KH wrote:


On Mon, Apr 18, 2016 at 06:11:51PM -0600, Alex Henrie wrote:


Hi,

I recently bought an MCS7715 USB-attached parallel port,[1] but there
seem to be a couple of problems using it with Linux:

1. The lp, parport, and parport_pc kernel modules are not loaded when
the device is plugged in.
2. After manually loading the kernel modules, /dev/lp0 is not deleted
when the device is unplugged.

I'm using a fully updated copy of Arch Linux, and at first I thought
it was a udev rules problem, but the systemd guys think that it's
actually a kernel bug.[2] Is this something that you could fix, or
guide me thorough fixing? I'd be happy to donate one of these devices
if you don't have one handy for debugging.



If you have a spare one I will really appreciate having one for debugging.


Sure, just tell me what address to mail it to and it's yours. I'll
send more than one if other people want to collaborate with you.


I will be in UK for some time from next week so better to send there 
than my address in India.


Sudip Mukherjee
Codethink Ltd.
Unit 302, Ducie House,
37 Ducie Street,
MANCHESTER,
M1 2JW,
United Kingdom.

Thanks in advance.

Regards
Sudip
--
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: Problems with MCS7715 USB-attached parallel port

[Sudip Mukherjee]

On Tuesday 19 April 2016 06:52 AM, Greg KH wrote:

On Mon, Apr 18, 2016 at 06:11:51PM -0600, Alex Henrie wrote:

Hi,

I recently bought an MCS7715 USB-attached parallel port,[1] but there
seem to be a couple of problems using it with Linux:

1. The lp, parport, and parport_pc kernel modules are not loaded when
the device is plugged in.
2. After manually loading the kernel modules, /dev/lp0 is not deleted
when the device is unplugged.

I'm using a fully updated copy of Arch Linux, and at first I thought
it was a udev rules problem, but the systemd guys think that it's
actually a kernel bug.[2] Is this something that you could fix, or
guide me thorough fixing? I'd be happy to donate one of these devices
if you don't have one handy for debugging.


If you have a spare one I will really appreciate having one for debugging.

I am not much familiar with Arch-Linux. which kernel version is it having?



Ah, yeah, this isn't going to work well, as you have found out.


Well, I am afraid its true at the moment. But i do hope in next couple 
of kernel release the situation should improve.




The parallel port subsystem is one of the last ones to be moved to the
driver core, it just now started for the 4.4 kernel release, and isn't
done yet at all.


It is a long process as the individual drivers using parallel port has 
to be converted first, while hoping that no one reports any regression. 
and we just recently had one :( .
BTW, Greg, that patch about subsys_initcall for parport is waiting for 
your review.




Sudip is the new maintainer of this, he might be able to answer this
much better than I can.  Sudip?


-Alex

[1] http://www.newegg.com/Product/Product.aspx?Item=N82E16812709411
[2] 
https://lists.freedesktop.org/archives/systemd-devel/2016-February/035937.html




Regards
Sudip
--
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] usb: renesas_usbhs: fix signed-unsigned return

[Sudip Mukherjee]

On Thursday 14 April 2016 04:25 PM, Felipe Balbi wrote:


Hi,

Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com> writes:

From: Sudip Mukherjee
Sent: Saturday, April 09, 2016 12:05 AM

The return type of usbhsp_setup_pipecfg() was u16 but it was returning
a negative value (-EINVAL). Instead lets return a pointer to u16 which
will hold the value to be returned or in case of error, return the
error code in ERR_PTR.


Thank you for the patch!
I also think this usbhsp_setup_pipecfg() should return error code using correct 
variable type.

However, I would like to avoid to use ERR_PTR and kmalloc() somehow because
I feel this patch is complex a little.
How about the usbhsp_setup_pipecfg() prototype is changed like the following?

static int usbhsp_setup_pipecfg(struct usbhs_pipe *pipe,
int is_host, int dir_in, u16 *pipecfg);


IMO, this makes much more sense.


Infact, I thought about both these ways while making the patch but 
somehow I thought this one is a better but ofcourse the other way is 
much simpler.

I will post v2.

regards
sudip

--
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] usb: renesas_usbhs: fix signed-unsigned return

[Sudip Mukherjee]

The return type of usbhsp_setup_pipecfg() was u16 but it was returning
a negative value (-EINVAL). Instead lets return a pointer to u16 which
will hold the value to be returned or in case of error, return the
error code in ERR_PTR.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/renesas_usbhs/pipe.c | 38 +-
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/pipe.c b/drivers/usb/renesas_usbhs/pipe.c
index 78e9dba..00d595c 100644
--- a/drivers/usb/renesas_usbhs/pipe.c
+++ b/drivers/usb/renesas_usbhs/pipe.c
@@ -391,9 +391,9 @@ void usbhs_pipe_set_trans_count_if_bulk(struct usbhs_pipe 
*pipe, int len)
 /*
  * pipe setup
  */
-static u16 usbhsp_setup_pipecfg(struct usbhs_pipe *pipe,
-   int is_host,
-   int dir_in)
+static u16 *usbhsp_setup_pipecfg(struct usbhs_pipe *pipe,
+int is_host,
+int dir_in)
 {
u16 type = 0;
u16 bfre = 0;
@@ -407,9 +407,13 @@ static u16 usbhsp_setup_pipecfg(struct usbhs_pipe *pipe,
[USB_ENDPOINT_XFER_INT]  = TYPE_INT,
[USB_ENDPOINT_XFER_ISOC] = TYPE_ISO,
};
+   u16 *result;
 
if (usbhs_pipe_is_dcp(pipe))
-   return -EINVAL;
+   return ERR_PTR(-EINVAL);
+   result = kmalloc(sizeof(u16), GFP_KERNEL);
+   if (!result)
+   return ERR_PTR(-ENOMEM);
 
/*
 * PIPECFG
@@ -451,14 +455,14 @@ static u16 usbhsp_setup_pipecfg(struct usbhs_pipe *pipe,
 
/* EPNUM */
epnum = 0; /* see usbhs_pipe_config_update() */
-
-   return  type|
-   bfre|
-   dblb|
-   cntmd   |
-   dir |
-   shtnak  |
-   epnum;
+   *result = type   |
+ bfre   |
+ dblb   |
+ cntmd  |
+ dir|
+ shtnak |
+ epnum;
+   return result;
 }
 
 static u16 usbhsp_setup_pipebuff(struct usbhs_pipe *pipe)
@@ -683,6 +687,7 @@ struct usbhs_pipe *usbhs_pipe_malloc(struct usbhs_priv 
*priv,
int is_host = usbhs_mod_is_host(priv);
int ret;
u16 pipecfg, pipebuf;
+   u16 *result;
 
pipe = usbhsp_get_pipe(priv, endpoint_type);
if (!pipe) {
@@ -702,7 +707,14 @@ struct usbhs_pipe *usbhs_pipe_malloc(struct usbhs_priv 
*priv,
return NULL;
}
 
-   pipecfg  = usbhsp_setup_pipecfg(pipe, is_host, dir_in);
+   result = usbhsp_setup_pipecfg(pipe, is_host, dir_in);
+   if (IS_ERR(result)) {
+   dev_err(dev, "can't setup pipe\n");
+   return NULL;
+   }
+   pipecfg = *result;
+   kfree(result);
+
pipebuf  = usbhsp_setup_pipebuff(pipe);
 
usbhsp_pipe_select(pipe);
-- 
1.9.1

--
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] usb: wusbcore: remove unreachable code

[Sudip Mukherjee]

The call to wusb_dev_sysfs_rm() which is just after return will never
be executed. On checking the code, wusb_dev_sysfs_add() is the last one
to be executed so even if that fails we do not need wusb_dev_sysfs_rm()
in the error path.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
---
 drivers/usb/wusbcore/devconnect.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/wusbcore/devconnect.c 
b/drivers/usb/wusbcore/devconnect.c
index 3f4f5fb..bf95517 100644
--- a/drivers/usb/wusbcore/devconnect.c
+++ b/drivers/usb/wusbcore/devconnect.c
@@ -893,7 +893,6 @@ out:
 error_nodev:
return;
 
-   wusb_dev_sysfs_rm(wusb_dev);
 error_add_sysfs:
wusb_dev_bos_rm(wusb_dev);
 error_bos_add:
-- 
1.9.1

--
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] usb: xhci: fix build warning

[Sudip Mukherjee]

We were getting build warning about:

drivers/usb/host/xhci.c: In function ‘xhci_add_ep_to_interval_table’:
drivers/usb/host/xhci.c:2499:2: warning: enumeration value
‘USB_SPEED_SUPER_PLUS’ not handled in switch

Fix it by adding SuperSpeedPlus USB3.1 devices as the behaviour is same
as with USB_SPEED_SUPER SuperSpeed devices.

Fixes: 8a1b2725a60d ("usb: define USB_SPEED_SUPER_PLUS speed for SuperSpeedPlus 
USB3.1 devices")
Cc: Mathias Nyman <mathias.ny...@linux.intel.com>
Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/host/xhci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 26a44c0..5bde15b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2410,7 +2410,7 @@ void xhci_drop_ep_from_interval_table(struct xhci_hcd 
*xhci,
if (xhci_is_async_ep(ep_bw->type))
return;
 
-   if (udev->speed == USB_SPEED_SUPER) {
+   if (udev->speed >= USB_SPEED_SUPER) {
if (xhci_is_sync_in_ep(ep_bw->type))
xhci->devs[udev->slot_id]->bw_table->ss_bw_in -=
xhci_get_ss_bw_consumed(ep_bw);
@@ -2448,6 +2448,7 @@ void xhci_drop_ep_from_interval_table(struct xhci_hcd 
*xhci,
interval_bw->overhead[HS_OVERHEAD_TYPE] -= 1;
break;
case USB_SPEED_SUPER:
+   case USB_SPEED_SUPER_PLUS:
case USB_SPEED_UNKNOWN:
case USB_SPEED_WIRELESS:
/* Should never happen because only LS/FS/HS endpoints will get
@@ -2474,7 +2475,7 @@ static void xhci_add_ep_to_interval_table(struct xhci_hcd 
*xhci,
if (xhci_is_async_ep(ep_bw->type))
return;
 
-   if (udev->speed == USB_SPEED_SUPER) {
+   if (udev->speed >= USB_SPEED_SUPER) {
if (xhci_is_sync_in_ep(ep_bw->type))
xhci->devs[udev->slot_id]->bw_table->ss_bw_in +=
xhci_get_ss_bw_consumed(ep_bw);
@@ -2507,6 +2508,7 @@ static void xhci_add_ep_to_interval_table(struct xhci_hcd 
*xhci,
interval_bw->overhead[HS_OVERHEAD_TYPE] += 1;
break;
case USB_SPEED_SUPER:
+   case USB_SPEED_SUPER_PLUS:
case USB_SPEED_UNKNOWN:
case USB_SPEED_WIRELESS:
/* Should never happen because only LS/FS/HS endpoints will get
-- 
1.9.1

--
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] usb: gadget: fix error handling

[Sudip Mukherjee]

We are doing PTR_ERR() of NULL, and that will actually make ret = 0.
So incase of both error and success we are actually returning the
success code.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/composite.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 8b14c2a..17ce6b5 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2017,14 +2017,14 @@ int composite_os_desc_req_prepare(struct 
usb_composite_dev *cdev,
 
cdev->os_desc_req = usb_ep_alloc_request(ep0, GFP_KERNEL);
if (!cdev->os_desc_req) {
-   ret = PTR_ERR(cdev->os_desc_req);
+   ret = -ENOMEM;
goto end;
}
 
/* OS feature descriptor length <= 4kB */
cdev->os_desc_req->buf = kmalloc(4096, GFP_KERNEL);
if (!cdev->os_desc_req->buf) {
-   ret = PTR_ERR(cdev->os_desc_req->buf);
+   ret = -ENOMEM;
kfree(cdev->os_desc_req);
goto end;
}
-- 
1.9.1

--
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 2/3] usb: gadget: at91_udc: mention proper dependency

[Sudip Mukherjee]

On Wed, Sep 30, 2015 at 07:12:18PM +0200, Nicolas Ferre wrote:
> 

> > Sorry that I missed ccing you while sending the patch. We should not
> > always depend on getmaintainer.pl.
> 
> Well, I'm marked as maintainer for this drivers actually and
> get_maintainer.pl shouldn't lie this time...

For the driver - yes, but for the Kconfig - no. Thatswhy I missed you in
my initial patch.

regards
sudip
--
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 2/3] usb: gadget: at91_udc: mention proper dependency

[Sudip Mukherjee]

On Wed, Sep 30, 2015 at 06:34:28PM +0200, Nicolas Ferre wrote:
> Le 30/09/2015 18:24, Sudip Mukherjee a écrit :
> > On Wed, Sep 30, 2015 at 11:04:54AM -0500, Felipe Balbi wrote:
> >> On Wed, Sep 23, 2015 at 09:22:48PM +0530, Sudip Mukherjee wrote:
> >>> On Mon, Sep 21, 2015 at 04:40:57PM +0530, Sudip Mukherjee wrote:
> >>>> On Sun, Sep 20, 2015 at 11:15:28AM -0500, Felipe Balbi wrote:
> >>>>> On Sat, Sep 19, 2015 at 10:42:58PM +0530, Sudip Mukherjee wrote:
> >>>>>> While building allmodconfig on avr32 the build failed with the error:
> >>>>>> "at91_pmc_base" [drivers/usb/gadget/udc/atmel_usba_udc.ko] undefined!
> >>>>>>
> >>>>>> On checking the code it turned out that if CONFIG_OF is defined then it
> >>>>>> is using at91_pmc_read() which is using at91_pmc_base. And unless
> >>>>>> COMMON_CLK_AT91 is defined we donot have at91_pmc_base. And
> >>>>>> COMMON_CLK_AT91 is available with AT91 architecture.
> >>>>>> Mention the dependency such that this driver builds with avr32 only if
> >>>>>> OF is not enabled.
> >>>>>>
> >>>>>> Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> >>>>>> ---
> >>>>>>
> >>>>>> Tested build with at91_dt_defconfig and allmodconfig of avr32. Build 
> >>>>>> log
> >>>>>> at:
> >>>>>> https://travis-ci.org/sudipm-mukherjee/parport/builds/81168845
> >>>>>>
> >>>>>>  drivers/usb/gadget/udc/Kconfig | 2 +-
> >>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/usb/gadget/udc/Kconfig 
> >>>>>> b/drivers/usb/gadget/udc/Kconfig
> >>>>>> index 9a3a6b0..cdbff54 100644
> >>>>>> --- a/drivers/usb/gadget/udc/Kconfig
> >>>>>> +++ b/drivers/usb/gadget/udc/Kconfig
> >>>>>> @@ -55,7 +55,7 @@ config USB_LPC32XX
> >>>>>>  
> >>>>>>  config USB_ATMEL_USBA
> >>>>>>tristate "Atmel USBA"
> >>>>>> -  depends on AVR32 || ARCH_AT91
> >>>>>> +  depends on ((AVR32 && !OF) || ARCH_AT91)
> >>>>>
> >>>>> any chance you can add || COMPILE_TEST here ? I'd like to make
> >>>>> sure this builds on my end too.
> >>>> With "depends on ((AVR32 && !OF) || ARCH_AT91 || COMPILE_TEST)"
> >>>> normal allmodconfig builiding for x86_64 failed with:
> >>>>
> >>>> drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_start’:
> >>>> drivers/usb/gadget/udc/atmel_usba_udc.c:1783:25: error: 
> >>>> ‘USBA_ENABLE_MASK’ undeclared (first use in this function)
> >>>>   usba_writel(udc, CTRL, USBA_ENABLE_MASK);
> >>>>  ^
> >>>> drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_stop’:
> >>>> drivers/usb/gadget/udc/atmel_usba_udc.c:1800:25: error: 
> >>>> ‘USBA_DISABLE_MASK’ undeclared (first use in this function)
> >>>>   usba_writel(udc, CTRL, USBA_DISABLE_MASK);
> >>>>  ^
> >>>>
> >>>> Looks like USBA_DISABLE_MASK and USBA_ENABLE_MASK is defined under
> >>>> #if defined(CONFIG_AVR32). :(
> >>> Can i check anything else here? Like I said with COMPILE_TEST
> >>> allmodconfig on x86_64 is failing.
> >>
> >> then keep it as is, but it would be nice to get that sorted out
> >> so I can do compile tests on my end too.
> > 
> > Maybe Nicolas can give some idea. Adding Nicolas Ferre to CC.
> 
> Hi,
> 
> I'm thinking about something like this:
> 
> 8<--
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.h
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
> @@ -46,10 +46,10 @@
>  #if defined(CONFIG_AVR32)
>  #define USBA_ENABLE_MASK   USBA_EN_USBA
>  #define USBA_DISABLE_MASK  0
> -#elif defined(CONFIG_ARCH_AT91)
> +#else
>  #define USBA_ENABLE_MASK   (USBA_EN_USBA | 
> USBA_PULLD_DIS)
>  #define USBA_DISABLE_MASK  USBA_DETACH
> -#endif /* CONFIG_ARCH_AT91 */
> +#endif
>  
>  /* Bitfields in FNUM */
>  #define USBA_MICRO_FRAME_NUM_OFFSET0
> 
> it can be sensible and will all to compile with the COMPILE_TEST directive.

OOPS... with this, allmodconfig on x86_64 fails build with:
ERROR: "at91_pmc_base" [drivers/usb/gadget/udc/atmel_usba_udc.ko] undefined!

Infact, this is the error I reported in my original patch.

regards
sudip
--
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] HID: hiddev: fix returned errno code in hiddev_connect()

[Sudip Mukherjee]

On Wed, Sep 30, 2015 at 10:56:26AM +0100, Luis de Bethencourt wrote:
> On 30/09/15 10:52, Luis de Bethencourt wrote:
> > The driver is using -1 instead of the -ENOMEM defined macro to specify
> > that a buffer allocation failed. Since the error number is propagated,
> > the caller will get a -EPERM which is the wrong error condition.
> > 
> > Also, the smatch tool complains with the following warning:
> > hiddev_connect() warn: returning -1 instead of -ENOMEM is sloppy
> > 
> > Signed-off-by: Luis de Bethencourt 
> > ---
> >  drivers/hid/usbhid/hiddev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> > index 2f1ddca..c5290ff 100644
> > --- a/drivers/hid/usbhid/hiddev.c
> > +++ b/drivers/hid/usbhid/hiddev.c
> > @@ -894,7 +894,7 @@ int hiddev_connect(struct hid_device *hid, unsigned int 
> > force)
> > }
> >  
> > if (!(hiddev = kzalloc(sizeof(struct hiddev), GFP_KERNEL)))
> > -   return -1;
> > +   return -ENOMEM;
> >  
> > init_waitqueue_head(>wait);
> > INIT_LIST_HEAD(>list);
> > 
> 
> Hello,
> 
> I got an "Undelivered Mail Returned to Sender" from Jiri Kosina's
> ji...@kernel.com email address. This email is listed multiple times
> in the MAINTAINERS file, does he have a new address to update this
> file?
Its ji...@kernel.org

regards
sudip
--
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 v2 02/12] usb: gadget: amd5536udc: fix error path

[Sudip Mukherjee]

On Tue, Sep 22, 2015 at 10:19:46AM -0500, Felipe Balbi wrote:
> On Tue, Sep 22, 2015 at 08:29:52PM +0530, Sudip Mukherjee wrote:
> > On Tue, Sep 22, 2015 at 08:12:29PM +0530, Sudip Mukherjee wrote:
> > > On Tue, Sep 22, 2015 at 09:37:45AM -0500, Felipe Balbi wrote:
> > > > On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> > > > > Handle the error properly instead of calling the pci remove function.
> > > > > 
> > > > > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> > > > 
> > > > this doesn't apply. Where did you rebase this series ? Please rebase on 
> > > > my
> > > > testing/next
> > > This was done on next-20150922. I will rebase on your tree.
> > Looks like today is a day of confusion for me.
> > I asked you to discard my v1 as I saw some part of the change was done
> > by:  6527cc27761a ("usb: gadget: amd5536udc: fix error handling in 
> > udc_pci_probe()")
> > 
> > But now I dont see that patch in your testing/next , but I can see that
> > patch in next-20150922. Confused. :(
> > Now?
> 
> heh, let's wait a bit then, seems like I need to wait for -rc3 and merge that
> in my testing/next.
Hi Felipe,
A gentle reminder. This series was waiting for -rc3 to be merged.

regards
sudip
--
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 2/3] usb: gadget: at91_udc: mention proper dependency

[Sudip Mukherjee]

On Wed, Sep 30, 2015 at 11:04:54AM -0500, Felipe Balbi wrote:
> On Wed, Sep 23, 2015 at 09:22:48PM +0530, Sudip Mukherjee wrote:
> > On Mon, Sep 21, 2015 at 04:40:57PM +0530, Sudip Mukherjee wrote:
> > > On Sun, Sep 20, 2015 at 11:15:28AM -0500, Felipe Balbi wrote:
> > > > On Sat, Sep 19, 2015 at 10:42:58PM +0530, Sudip Mukherjee wrote:
> > > > > While building allmodconfig on avr32 the build failed with the error:
> > > > > "at91_pmc_base" [drivers/usb/gadget/udc/atmel_usba_udc.ko] undefined!
> > > > > 
> > > > > On checking the code it turned out that if CONFIG_OF is defined then 
> > > > > it
> > > > > is using at91_pmc_read() which is using at91_pmc_base. And unless
> > > > > COMMON_CLK_AT91 is defined we donot have at91_pmc_base. And
> > > > > COMMON_CLK_AT91 is available with AT91 architecture.
> > > > > Mention the dependency such that this driver builds with avr32 only if
> > > > > OF is not enabled.
> > > > > 
> > > > > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> > > > > ---
> > > > > 
> > > > > Tested build with at91_dt_defconfig and allmodconfig of avr32. Build 
> > > > > log
> > > > > at:
> > > > > https://travis-ci.org/sudipm-mukherjee/parport/builds/81168845
> > > > > 
> > > > >  drivers/usb/gadget/udc/Kconfig | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/usb/gadget/udc/Kconfig 
> > > > > b/drivers/usb/gadget/udc/Kconfig
> > > > > index 9a3a6b0..cdbff54 100644
> > > > > --- a/drivers/usb/gadget/udc/Kconfig
> > > > > +++ b/drivers/usb/gadget/udc/Kconfig
> > > > > @@ -55,7 +55,7 @@ config USB_LPC32XX
> > > > >  
> > > > >  config USB_ATMEL_USBA
> > > > >   tristate "Atmel USBA"
> > > > > - depends on AVR32 || ARCH_AT91
> > > > > + depends on ((AVR32 && !OF) || ARCH_AT91)
> > > > 
> > > > any chance you can add || COMPILE_TEST here ? I'd like to make
> > > > sure this builds on my end too.
> > > With "depends on ((AVR32 && !OF) || ARCH_AT91 || COMPILE_TEST)"
> > > normal allmodconfig builiding for x86_64 failed with:
> > > 
> > > drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_start’:
> > > drivers/usb/gadget/udc/atmel_usba_udc.c:1783:25: error: 
> > > ‘USBA_ENABLE_MASK’ undeclared (first use in this function)
> > >   usba_writel(udc, CTRL, USBA_ENABLE_MASK);
> > >  ^
> > > drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_stop’:
> > > drivers/usb/gadget/udc/atmel_usba_udc.c:1800:25: error: 
> > > ‘USBA_DISABLE_MASK’ undeclared (first use in this function)
> > >   usba_writel(udc, CTRL, USBA_DISABLE_MASK);
> > >  ^
> > > 
> > > Looks like USBA_DISABLE_MASK and USBA_ENABLE_MASK is defined under
> > > #if defined(CONFIG_AVR32). :(
> > Can i check anything else here? Like I said with COMPILE_TEST
> > allmodconfig on x86_64 is failing.
> 
> then keep it as is, but it would be nice to get that sorted out
> so I can do compile tests on my end too.

Maybe Nicolas can give some idea. Adding Nicolas Ferre to CC.

regards
sudip
--
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 2/3] usb: gadget: at91_udc: mention proper dependency

[Sudip Mukherjee]

On Wed, Sep 30, 2015 at 06:34:28PM +0200, Nicolas Ferre wrote:
> Le 30/09/2015 18:24, Sudip Mukherjee a écrit :
> > On Wed, Sep 30, 2015 at 11:04:54AM -0500, Felipe Balbi wrote:
> >> On Wed, Sep 23, 2015 at 09:22:48PM +0530, Sudip Mukherjee wrote:
> >>> On Mon, Sep 21, 2015 at 04:40:57PM +0530, Sudip Mukherjee wrote:
> >>>> On Sun, Sep 20, 2015 at 11:15:28AM -0500, Felipe Balbi wrote:
> >>>>> On Sat, Sep 19, 2015 at 10:42:58PM +0530, Sudip Mukherjee wrote:
> >>>>>> While building allmodconfig on avr32 the build failed with the error:
> >>>>>> "at91_pmc_base" [drivers/usb/gadget/udc/atmel_usba_udc.ko] undefined!
> >>>>>>
> >>>>>> On checking the code it turned out that if CONFIG_OF is defined then it
> >>>>>> is using at91_pmc_read() which is using at91_pmc_base. And unless
> >>>>>> COMMON_CLK_AT91 is defined we donot have at91_pmc_base. And
> >>>>>> COMMON_CLK_AT91 is available with AT91 architecture.
> >>>>>> Mention the dependency such that this driver builds with avr32 only if
> >>>>>> OF is not enabled.
> >>>>>>
> >>>>>> Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> >>>>>> ---
> >>>>>>
> >>>>>> Tested build with at91_dt_defconfig and allmodconfig of avr32. Build 
> >>>>>> log
> >>>>>> at:
> >>>>>> https://travis-ci.org/sudipm-mukherjee/parport/builds/81168845
> >>>>>>
> >>>>>>  drivers/usb/gadget/udc/Kconfig | 2 +-
> >>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/usb/gadget/udc/Kconfig 
> >>>>>> b/drivers/usb/gadget/udc/Kconfig
> >>>>>> index 9a3a6b0..cdbff54 100644
> >>>>>> --- a/drivers/usb/gadget/udc/Kconfig
> >>>>>> +++ b/drivers/usb/gadget/udc/Kconfig
> >>>>>> @@ -55,7 +55,7 @@ config USB_LPC32XX
> >>>>>>  
> >>>>>>  config USB_ATMEL_USBA
> >>>>>>tristate "Atmel USBA"
> >>>>>> -  depends on AVR32 || ARCH_AT91
> >>>>>> +  depends on ((AVR32 && !OF) || ARCH_AT91)
> >>>>>
> >>>>> any chance you can add || COMPILE_TEST here ? I'd like to make
> >>>>> sure this builds on my end too.
> >>>> With "depends on ((AVR32 && !OF) || ARCH_AT91 || COMPILE_TEST)"
> >>>> normal allmodconfig builiding for x86_64 failed with:
> >>>>
> >>>> drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_start’:
> >>>> drivers/usb/gadget/udc/atmel_usba_udc.c:1783:25: error: 
> >>>> ‘USBA_ENABLE_MASK’ undeclared (first use in this function)
> >>>>   usba_writel(udc, CTRL, USBA_ENABLE_MASK);
> >>>>  ^
> >>>> drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_stop’:
> >>>> drivers/usb/gadget/udc/atmel_usba_udc.c:1800:25: error: 
> >>>> ‘USBA_DISABLE_MASK’ undeclared (first use in this function)
> >>>>   usba_writel(udc, CTRL, USBA_DISABLE_MASK);
> >>>>  ^
> >>>>
> >>>> Looks like USBA_DISABLE_MASK and USBA_ENABLE_MASK is defined under
> >>>> #if defined(CONFIG_AVR32). :(
> >>> Can i check anything else here? Like I said with COMPILE_TEST
> >>> allmodconfig on x86_64 is failing.
> >>
> >> then keep it as is, but it would be nice to get that sorted out
> >> so I can do compile tests on my end too.
> > 
> > Maybe Nicolas can give some idea. Adding Nicolas Ferre to CC.
> 
> Hi,
> 
> I'm thinking about something like this:
> 
> 8<--
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.h
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
> @@ -46,10 +46,10 @@
>  #if defined(CONFIG_AVR32)
>  #define USBA_ENABLE_MASK   USBA_EN_USBA
>  #define USBA_DISABLE_MASK  0
> -#elif defined(CONFIG_ARCH_AT91)
> +#else
>  #define USBA_ENABLE_MASK   (USBA_EN_USBA | 
> USBA_PULLD_DIS)
>  #define USBA_DISABLE_MASK  USBA_DETACH
> -#endif /* CONFIG_ARCH_AT91 */
> +#endif
>  
>  /* Bitfields in FNUM */
>  #define USBA_MICRO_FRAME_NUM_OFFSET0
> 
> it can be sensible and will all to compile with the COMPILE_TEST directive.

Thanks. Will test tomorrow. And my original patch of depending on
((AVR32 && !OF) || ARCH_AT91), is that correct?
Sorry that I missed ccing you while sending the patch. We should not
always depend on getmaintainer.pl.

regards
sudip
--
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] usb: gadget: bdc: fix memory leak

[Sudip Mukherjee]

If dma_pool_alloc() fails we are jumping to fail and releasing all the
bd_tables which have been added to the chain but we missed freeing this
bd_table which was just allocated and still not added to the chain of
bd_table.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/bdc/bdc_ep.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c 
b/drivers/usb/gadget/udc/bdc/bdc_ep.c
index d1b8153..97b7648 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_ep.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c
@@ -159,8 +159,10 @@ static int ep_bd_list_alloc(struct bdc_ep *ep)
bd_table->start_bd = dma_pool_alloc(bdc->bd_table_pool,
GFP_ATOMIC,
);
-   if (!bd_table->start_bd)
+   if (!bd_table->start_bd) {
+   kfree(bd_table);
goto fail;
+   }
 
bd_table->dma = dma;
 
-- 
1.9.1

--
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 1/3] avr32: fix build failure

[Sudip Mukherjee]

On Wed, Sep 23, 2015 at 07:15:16PM +0200, Hans-Christian Egtvedt wrote:
> Around Wed 23 Sep 2015 21:26:01 +0530 or thereabout, Sudip Mukherjee wrote:
> > On Mon, Sep 21, 2015 at 01:31:44PM +0530, Sudip Mukherjee wrote:
> >> On Mon, Sep 21, 2015 at 09:33:00AM +0200, Hans-Christian Egtvedt wrote:
> >> > Around Mon 21 Sep 2015 12:09:01 +0530 or thereabout, Sudip Mukherjee 
> >> > wrote:
> >> > > On Mon, Sep 21, 2015 at 08:09:42AM +0200, Hans-Christian Egtvedt wrote:
> >> > >> Around Sat 19 Sep 2015 22:42:57 +0530 or thereabout, Sudip Mukherjee 
> >> > >> wrote:
> >> > >> > While building avr32 with allmodconfig, the build used to fail with 
> >> > >> > the
> >> > >> > message:
> >> > >> > error: implicit declaration of function 'pci_iomap'
> >> > >> > error: implicit declaration of function 'pci_iounmap'
> >> > >> 
> >> > >> What has changed recently that start pulling in these? AVR32 does not 
> >> > >> have
> >> > >> PCI at all, and will never have it either. Is this exposing a bug 
> >> > >> somewhere
> >> > >> else?
> >> > > It looks like pci_iomap and pci_iounmap doesnot depend on CONFIG_PCI.
> >> > > So drivers/net/ethernet/via/via-rhine.c is calling these functions even
> >> > > if PCI is disabled. The build log is at:
> >> > > https://travis-ci.org/sudipm-mukherjee/parport/jobs/81127188
> >> > > 
> >> > > You can find a similar discussion at:
> >> > > http://www.linux-mips.org/archives/linux-mips/2013-06/msg00510.html
> >> > 
> >> > If multiple architectures have similar problems, then a more global
> >> > definition of these unused functions could be added.
> >> > 
> >> > Just move the implementation for MIPS into include/asm-generic/io.h ?
> >> > 
> >> > That way we do not have to implement and identical stub for every device 
> >> > not
> >> > supporting PCI.
> >> 
> >> include/asm-generic/io.h is having:
> >> #ifndef CONFIG_GENERIC_IOMAP
> >> struct pci_dev;
> >> extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned
> >>long max);
> >> 
> >> #ifndef pci_iounmap
> >> #define pci_iounmap pci_iounmap
> >> static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p)
> >> {
> >> }
> >> #endif
> >> #endif /* CONFIG_GENERIC_IOMAP */
> >> 
> >> and CONFIG_GENERIC_IOMAP is not defined when I do allmodconfig with
> >> avr32. I have not seen this earlier, maybe in my patch pci_iounmap() was
> >> not required. But pci_iomap is declared as extern so we need have it
> >> somewhere.
> > Hi Hans,
> > Please suggest.
> 
> I can ack the original change, but it will only lead to a cluttered code
> base. The real problem is somewhere else, where it looks like the
> CONFIG_GENERIC_IOMAP symbol assumes PCI is for everybody. I would like if
> somebody addressed that problem instead.

I was having a look and it looks like generic empty pci_iomap and
pci_iounmap are already defined and is used only if CONFIG_PCI is not
defined and CONFIG_GENERIC_PCI_IOMAP is defined. So instead of adding
empty functions to each architecture where PCI is not supported maybe
the driver can have a dependency on CONFIG_GENERIC_PCI_IOMAP.
I have submitted the patch and you are in CC list. arch/sh had a similar
problem with the same driver.

regards
sudip
--
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 3/3] page-flags: rectify forward declaration

[Sudip Mukherjee]

> > 
> > Also, I'm finding that the patch series introduces a pretty large
> > bisection hole:
> > 
> > include/linux/page-flags.h: In function 'PageYoung':
> > include/linux/page-flags.h:327: error: implicit declaration of function 
> > 'PF_ANY'
> > include/linux/page-flags.h:327: error: invalid type argument of '->' (have 
> > 'int')
> > include/linux/page-flags.h:327: error: invalid type argument of '->' (have 
> > 'int')
> > 
> > which later gets fixed up by
> > page-flags-rectify-forward-declaration.patch.
> How to test this? Should I apply them on top of v4.2 and bisect? And I
> don't see any relation between the first two patches and this patch of
> the series, then how does it fail in bisect? Am I missing something?
> Confused.. :(
> > 
> > Maybe it's time to do a wholesale refactoring of the patchset?
> If this patch is the first in the series will that help?
> And besides I got the auto mail from you that the patch is applied.
> Now totally confused.. :(
Ohhh ..all the time you were talking about Kirill's patch set and I kept
thinking you are mentioning about this patchset. :)

regards
sudip
--
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 1/3] avr32: fix build failure

[Sudip Mukherjee]

On Mon, Sep 21, 2015 at 01:31:44PM +0530, Sudip Mukherjee wrote:
> On Mon, Sep 21, 2015 at 09:33:00AM +0200, Hans-Christian Egtvedt wrote:
> > Around Mon 21 Sep 2015 12:09:01 +0530 or thereabout, Sudip Mukherjee wrote:
> > > On Mon, Sep 21, 2015 at 08:09:42AM +0200, Hans-Christian Egtvedt wrote:
> > >> Around Sat 19 Sep 2015 22:42:57 +0530 or thereabout, Sudip Mukherjee 
> > >> wrote:
> > >> > While building avr32 with allmodconfig, the build used to fail with the
> > >> > message:
> > >> > error: implicit declaration of function 'pci_iomap'
> > >> > error: implicit declaration of function 'pci_iounmap'
> > >> 
> > >> What has changed recently that start pulling in these? AVR32 does not 
> > >> have
> > >> PCI at all, and will never have it either. Is this exposing a bug 
> > >> somewhere
> > >> else?
> > > It looks like pci_iomap and pci_iounmap doesnot depend on CONFIG_PCI.
> > > So drivers/net/ethernet/via/via-rhine.c is calling these functions even
> > > if PCI is disabled. The build log is at:
> > > https://travis-ci.org/sudipm-mukherjee/parport/jobs/81127188
> > > 
> > > You can find a similar discussion at:
> > > http://www.linux-mips.org/archives/linux-mips/2013-06/msg00510.html
> > 
> > If multiple architectures have similar problems, then a more global
> > definition of these unused functions could be added.
> > 
> > Just move the implementation for MIPS into include/asm-generic/io.h ?
> > 
> > That way we do not have to implement and identical stub for every device not
> > supporting PCI.
> 
> include/asm-generic/io.h is having:
> #ifndef CONFIG_GENERIC_IOMAP
> struct pci_dev;
> extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned
>   long max);
> 
> #ifndef pci_iounmap
> #define pci_iounmap pci_iounmap
> static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p)
> {
> }
> #endif
> #endif /* CONFIG_GENERIC_IOMAP */
> 
> and CONFIG_GENERIC_IOMAP is not defined when I do allmodconfig with
> avr32. I have not seen this earlier, maybe in my patch pci_iounmap() was
> not required. But pci_iomap is declared as extern so we need have it
> somewhere.
Hi Hans,
Please suggest.

regards
sudip
--
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 2/3] usb: gadget: at91_udc: mention proper dependency

[Sudip Mukherjee]

On Mon, Sep 21, 2015 at 04:40:57PM +0530, Sudip Mukherjee wrote:
> On Sun, Sep 20, 2015 at 11:15:28AM -0500, Felipe Balbi wrote:
> > On Sat, Sep 19, 2015 at 10:42:58PM +0530, Sudip Mukherjee wrote:
> > > While building allmodconfig on avr32 the build failed with the error:
> > > "at91_pmc_base" [drivers/usb/gadget/udc/atmel_usba_udc.ko] undefined!
> > > 
> > > On checking the code it turned out that if CONFIG_OF is defined then it
> > > is using at91_pmc_read() which is using at91_pmc_base. And unless
> > > COMMON_CLK_AT91 is defined we donot have at91_pmc_base. And
> > > COMMON_CLK_AT91 is available with AT91 architecture.
> > > Mention the dependency such that this driver builds with avr32 only if
> > > OF is not enabled.
> > > 
> > > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> > > ---
> > > 
> > > Tested build with at91_dt_defconfig and allmodconfig of avr32. Build log
> > > at:
> > > https://travis-ci.org/sudipm-mukherjee/parport/builds/81168845
> > > 
> > >  drivers/usb/gadget/udc/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/gadget/udc/Kconfig 
> > > b/drivers/usb/gadget/udc/Kconfig
> > > index 9a3a6b0..cdbff54 100644
> > > --- a/drivers/usb/gadget/udc/Kconfig
> > > +++ b/drivers/usb/gadget/udc/Kconfig
> > > @@ -55,7 +55,7 @@ config USB_LPC32XX
> > >  
> > >  config USB_ATMEL_USBA
> > >   tristate "Atmel USBA"
> > > - depends on AVR32 || ARCH_AT91
> > > + depends on ((AVR32 && !OF) || ARCH_AT91)
> > 
> > any chance you can add || COMPILE_TEST here ? I'd like to make
> > sure this builds on my end too.
> With "depends on ((AVR32 && !OF) || ARCH_AT91 || COMPILE_TEST)"
> normal allmodconfig builiding for x86_64 failed with:
> 
> drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_start’:
> drivers/usb/gadget/udc/atmel_usba_udc.c:1783:25: error: ‘USBA_ENABLE_MASK’ 
> undeclared (first use in this function)
>   usba_writel(udc, CTRL, USBA_ENABLE_MASK);
>  ^
> drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_stop’:
> drivers/usb/gadget/udc/atmel_usba_udc.c:1800:25: error: ‘USBA_DISABLE_MASK’ 
> undeclared (first use in this function)
>   usba_writel(udc, CTRL, USBA_DISABLE_MASK);
>  ^
> 
> Looks like USBA_DISABLE_MASK and USBA_ENABLE_MASK is defined under
> #if defined(CONFIG_AVR32). :(
Can i check anything else here? Like I said with COMPILE_TEST
allmodconfig on x86_64 is failing.

regards
sudip
--
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 v2 02/12] usb: gadget: amd5536udc: fix error path

[Sudip Mukherjee]

On Tue, Sep 22, 2015 at 10:19:46AM -0500, Felipe Balbi wrote:
> On Tue, Sep 22, 2015 at 08:29:52PM +0530, Sudip Mukherjee wrote:
> > On Tue, Sep 22, 2015 at 08:12:29PM +0530, Sudip Mukherjee wrote:
> > > On Tue, Sep 22, 2015 at 09:37:45AM -0500, Felipe Balbi wrote:
> > > > On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> > > > > Handle the error properly instead of calling the pci remove function.
> > > > > 
> > > > > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> > > > 
> > > > this doesn't apply. Where did you rebase this series ? Please rebase on 
> > > > my
> > > > testing/next
> > > This was done on next-20150922. I will rebase on your tree.
> > Looks like today is a day of confusion for me.
> > I asked you to discard my v1 as I saw some part of the change was done
> > by:  6527cc27761a ("usb: gadget: amd5536udc: fix error handling in 
> > udc_pci_probe()")
> > 
> > But now I dont see that patch in your testing/next , but I can see that
> > patch in next-20150922. Confused. :(
> > Now?
> 
> heh, let's wait a bit then, seems like I need to wait for -rc3 and merge that
> in my testing/next.
It is in your usb/fixes tree. I am not rebasing then.

regards
sudip
--
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 3/3] page-flags: rectify forward declaration

[Sudip Mukherjee]

On Mon, Sep 21, 2015 at 03:35:09PM -0700, Andrew Morton wrote:
> On Sat, 19 Sep 2015 22:42:59 +0530 Sudip Mukherjee 
> <sudipm.mukher...@gmail.com> wrote:
> 

> 
> Is it fixable?  Can we use the traditional define-before-using structure?
How about this:

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index ab1a0e9..d7a1055 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -133,6 +133,19 @@ enum pageflags {
 
 #ifndef __GENERATING_BOUNDS_H
 
+/* Forward declarations */
+struct page;
+static inline int PageCompound(struct page *page);
+static inline int PageTail(struct page *page);
+static inline struct page *compound_head(struct page *page)
+{
+   unsigned long head = READ_ONCE(page->compound_head);
+
+   if (unlikely(head & 1))
+   return (struct page *) (head - 1);
+   return page;
+}
+
 /* Page flags policies wrt compound pages */
 #define PF_ANY(page, enforce)  page
 #define PF_HEAD(page, enforce) compound_head(page)
@@ -223,12 +236,6 @@ static inline int __TestClearPage##uname(struct page 
*page) { return 0; }
 #define TESTSCFLAG_FALSE(uname)
\
TESTSETFLAG_FALSE(uname) TESTCLEARFLAG_FALSE(uname)
 
-/* Forward declarations */
-struct page;
-static inline int PageCompound(struct page *page);
-static inline int PageTail(struct page *page);
-static struct page *compound_head(struct page *page);
-
 __PAGEFLAG(Locked, locked, PF_NO_TAIL)
 PAGEFLAG(Error, error, PF_NO_COMPOUND) TESTCLEARFLAG(Error, error, 
PF_NO_COMPOUND)
 PAGEFLAG(Referenced, referenced, PF_HEAD)
@@ -450,15 +457,6 @@ static inline void clear_compound_head(struct page *page)
WRITE_ONCE(page->compound_head, 0);
 }
 
-static inline struct page *compound_head(struct page *page)
-{
-   unsigned long head = READ_ONCE(page->compound_head);
-
-   if (unlikely(head & 1))
-   return (struct page *) (head - 1);
-   return page;
-}
-
 static inline int PageCompound(struct page *page)
 {
return PageHead(page) || PageTail(page);


---
It builds properly. Tested with allmodconfig of x86_64 and avr32.

> 
> Also, I'm finding that the patch series introduces a pretty large
> bisection hole:
> 
> include/linux/page-flags.h: In function 'PageYoung':
> include/linux/page-flags.h:327: error: implicit declaration of function 
> 'PF_ANY'
> include/linux/page-flags.h:327: error: invalid type argument of '->' (have 
> 'int')
> include/linux/page-flags.h:327: error: invalid type argument of '->' (have 
> 'int')
> 
> which later gets fixed up by
> page-flags-rectify-forward-declaration.patch.
How to test this? Should I apply them on top of v4.2 and bisect? And I
don't see any relation between the first two patches and this patch of
the series, then how does it fail in bisect? Am I missing something?
Confused.. :(
> 
> Maybe it's time to do a wholesale refactoring of the patchset?
If this patch is the first in the series will that help?
And besides I got the auto mail from you that the patch is applied.
Now totally confused.. :(

regards
sudip
--
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 v2 09/12] usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain

[Sudip Mukherjee]

Rearrange udc_free_dma_chain to remove the forward declaration. While at
it fixed all the relevant checkpatch warnings.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 51 +
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 2602173..6d64129 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -70,7 +70,6 @@ static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
 static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
 static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
 static void udc_pci_remove(struct pci_dev *pdev);
 
@@ -611,6 +610,30 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
return >req;
 }
 
+/* frees pci pool descriptors of a DMA chain */
+static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+{
+   int ret_val = 0;
+   struct udc_data_dma *td;
+   struct udc_data_dma *td_last = NULL;
+   unsigned int i;
+
+   DBG(dev, "free chain req = %p\n", req);
+
+   /* do not free first desc., will be done by free for request */
+   td_last = req->td_data;
+   td = phys_to_virt(td_last->next);
+
+   for (i = 1; i < req->chain_len; i++) {
+   pci_pool_free(dev->data_requests, td,
+ (dma_addr_t)td_last->next);
+   td_last = td;
+   td = phys_to_virt(td_last->next);
+   }
+
+   return ret_val;
+}
+
 /* Frees request packet, called by gadget driver */
 static void
 udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq)
@@ -1026,32 +1049,6 @@ __acquires(ep->dev->lock)
ep->halted = halted;
 }
 
-/* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
-{
-
-   int ret_val = 0;
-   struct udc_data_dma *td;
-   struct udc_data_dma *td_last = NULL;
-   unsigned int i;
-
-   DBG(dev, "free chain req = %p\n", req);
-
-   /* do not free first desc., will be done by free for request */
-   td_last = req->td_data;
-   td = phys_to_virt(td_last->next);
-
-   for (i = 1; i < req->chain_len; i++) {
-
-   pci_pool_free(dev->data_requests, td,
-   (dma_addr_t) td_last->next);
-   td_last = td;
-   td = phys_to_virt(td_last->next);
-   }
-
-   return ret_val;
-}
-
 /* Iterates to the end of a DMA chain and returns last descriptor */
 static struct udc_data_dma *udc_get_last_dma_desc(struct udc_request *req)
 {
-- 
1.9.1

--
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 v2 11/12] usb: gadget: amd5536udc: remove forward declaration of udc_basic_init

[Sudip Mukherjee]

Rearrange the udc_basic_init function to remove the forward declaration.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 55 ++---
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 00ae069..157bff1 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -65,7 +65,6 @@
 
 static void udc_tasklet_disconnect(unsigned long);
 static void empty_req_queue(struct udc_ep *);
-static void udc_basic_init(struct udc *dev);
 static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
@@ -1507,33 +1506,6 @@ static void make_ep_lists(struct udc *dev)
dev->ep[UDC_EPOUT_IX].fifo_depth = UDC_RXFIFO_SIZE;
 }
 
-/* init registers at driver load time */
-static int startup_registers(struct udc *dev)
-{
-   u32 tmp;
-
-   /* init controller by soft reset */
-   udc_soft_reset(dev);
-
-   /* mask not needed interrupts */
-   udc_mask_unused_interrupts(dev);
-
-   /* put into initial config */
-   udc_basic_init(dev);
-   /* link up all endpoints */
-   udc_setup_endpoints(dev);
-
-   /* program speed */
-   tmp = readl(>regs->cfg);
-   if (use_fullspeed)
-   tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_FS, UDC_DEVCFG_SPD);
-   else
-   tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_HS, UDC_DEVCFG_SPD);
-   writel(tmp, >regs->cfg);
-
-   return 0;
-}
-
 /* Inits UDC context */
 static void udc_basic_init(struct udc *dev)
 {
@@ -1572,6 +1544,33 @@ static void udc_basic_init(struct udc *dev)
dev->data_ep_queued = 0;
 }
 
+/* init registers at driver load time */
+static int startup_registers(struct udc *dev)
+{
+   u32 tmp;
+
+   /* init controller by soft reset */
+   udc_soft_reset(dev);
+
+   /* mask not needed interrupts */
+   udc_mask_unused_interrupts(dev);
+
+   /* put into initial config */
+   udc_basic_init(dev);
+   /* link up all endpoints */
+   udc_setup_endpoints(dev);
+
+   /* program speed */
+   tmp = readl(>regs->cfg);
+   if (use_fullspeed)
+   tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_FS, UDC_DEVCFG_SPD);
+   else
+   tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_HS, UDC_DEVCFG_SPD);
+   writel(tmp, >regs->cfg);
+
+   return 0;
+}
+
 /* Sets initial endpoint parameters */
 static void udc_setup_endpoints(struct udc *dev)
 {
-- 
1.9.1

--
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 v2 12/12] usb: gadget: amd5536udc: NULL comparison

[Sudip Mukherjee]

A NULL comparison can be written as if (var) or if (!var).

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 157bff1..cd87641 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -2185,7 +2185,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int 
ep_ix)
}
 
/* DMA */
-   } else if (!ep->cancel_transfer && req != NULL) {
+   } else if (!ep->cancel_transfer && req) {
ret_val = IRQ_HANDLED;
 
/* check for DMA done */
@@ -3189,7 +3189,7 @@ static int init_dma_pools(struct udc *dev)
/* setup */
td_stp = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
>ep[UDC_EP0OUT_IX].td_stp_dma);
-   if (td_stp == NULL) {
+   if (!td_stp) {
retval = -ENOMEM;
goto err_alloc_dma;
}
@@ -3198,7 +3198,7 @@ static int init_dma_pools(struct udc *dev)
/* data: 0 packets !? */
td_data = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
>ep[UDC_EP0OUT_IX].td_phys);
-   if (td_data == NULL) {
+   if (!td_data) {
retval = -ENOMEM;
goto err_alloc_phys;
}
@@ -3322,7 +3322,7 @@ static int udc_pci_probe(
}
 
dev->virt_addr = ioremap_nocache(resource, len);
-   if (dev->virt_addr == NULL) {
+   if (!dev->virt_addr) {
dev_dbg(>dev, "start address cannot be mapped\n");
retval = -EFAULT;
goto err_ioremap;
-- 
1.9.1

--
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 v2 03/12] usb: gadget: amd5536udc: use WARN_ON

[Sudip Mukherjee]

Use WARN_ON() instead of halting the kernel with BUG_ON() and also fix
the checkpatch warning.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 3f85044..6223b1b 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3127,7 +3127,8 @@ static void udc_pci_remove(struct pci_dev *pdev)
 
usb_del_gadget_udc(>gadget);
/* gadget driver must not be registered */
-   BUG_ON(dev->driver != NULL);
+   if (WARN_ON(dev->driver))
+   return;
 
/* dma pool cleanup */
if (dev->data_requests)
-- 
1.9.1

--
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 v2 06/12] usb: gadget: amd5536udc: remove forward declaration of udc_probe

[Sudip Mukherjee]

Rearrange the udc_probe function to remove the forward declarations.
While rearranging also fixed the relevant checkpatch warnings.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 133 ++--
 1 file changed, 66 insertions(+), 67 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 89e83e4..6c16737 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -65,7 +65,6 @@
 
 static void udc_tasklet_disconnect(unsigned long);
 static void empty_req_queue(struct udc_ep *);
-static int udc_probe(struct udc *dev);
 static void udc_basic_init(struct udc *dev);
 static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
@@ -3209,6 +3208,72 @@ err_create_dma_pool:
return retval;
 }
 
+/* general probe */
+static int udc_probe(struct udc *dev)
+{
+   chartmp[128];
+   u32 reg;
+   int retval;
+
+   /* mark timer as not initialized */
+   udc_timer.data = 0;
+   udc_pollstall_timer.data = 0;
+
+   /* device struct setup */
+   dev->gadget.ops = _ops;
+
+   dev_set_name(>gadget.dev, "gadget");
+   dev->gadget.name = name;
+   dev->gadget.max_speed = USB_SPEED_HIGH;
+
+   /* init registers, interrupts, ... */
+   startup_registers(dev);
+
+   dev_info(>pdev->dev, "%s\n", mod_desc);
+
+   snprintf(tmp, sizeof(tmp), "%d", dev->irq);
+   dev_info(>pdev->dev,
+"irq %s, pci mem %08lx, chip rev %02x(Geode5536 %s)\n",
+tmp, dev->phys_addr, dev->chiprev,
+(dev->chiprev == UDC_HSA0_REV) ? "A0" : "B1");
+   strcpy(tmp, UDC_DRIVER_VERSION_STRING);
+   if (dev->chiprev == UDC_HSA0_REV) {
+   dev_err(>pdev->dev, "chip revision is A0; too old\n");
+   retval = -ENODEV;
+   goto finished;
+   }
+   dev_info(>pdev->dev,
+"driver version: %s(for Geode5536 B1)\n", tmp);
+   udc = dev;
+
+   retval = usb_add_gadget_udc_release(>pdev->dev, >gadget,
+   gadget_release);
+   if (retval)
+   goto finished;
+
+   /* timer init */
+   init_timer(_timer);
+   udc_timer.function = udc_timer_function;
+   udc_timer.data = 1;
+   /* timer pollstall init */
+   init_timer(_pollstall_timer);
+   udc_pollstall_timer.function = udc_pollstall_timer_function;
+   udc_pollstall_timer.data = 1;
+
+   /* set SD */
+   reg = readl(>regs->ctl);
+   reg |= AMD_BIT(UDC_DEVCTL_SD);
+   writel(reg, >regs->ctl);
+
+   /* print dev register info */
+   print_regs(dev);
+
+   return 0;
+
+finished:
+   return retval;
+}
+
 /* Called by pci bus driver to init pci context */
 static int udc_pci_probe(
struct pci_dev *pdev,
@@ -3319,72 +3384,6 @@ err_pcidev:
return retval;
 }
 
-/* general probe */
-static int udc_probe(struct udc *dev)
-{
-   chartmp[128];
-   u32 reg;
-   int retval;
-
-   /* mark timer as not initialized */
-   udc_timer.data = 0;
-   udc_pollstall_timer.data = 0;
-
-   /* device struct setup */
-   dev->gadget.ops = _ops;
-
-   dev_set_name(>gadget.dev, "gadget");
-   dev->gadget.name = name;
-   dev->gadget.max_speed = USB_SPEED_HIGH;
-
-   /* init registers, interrupts, ... */
-   startup_registers(dev);
-
-   dev_info(>pdev->dev, "%s\n", mod_desc);
-
-   snprintf(tmp, sizeof tmp, "%d", dev->irq);
-   dev_info(>pdev->dev,
-   "irq %s, pci mem %08lx, chip rev %02x(Geode5536 %s)\n",
-   tmp, dev->phys_addr, dev->chiprev,
-   (dev->chiprev == UDC_HSA0_REV) ? "A0" : "B1");
-   strcpy(tmp, UDC_DRIVER_VERSION_STRING);
-   if (dev->chiprev == UDC_HSA0_REV) {
-   dev_err(>pdev->dev, "chip revision is A0; too old\n");
-   retval = -ENODEV;
-   goto finished;
-   }
-   dev_info(>pdev->dev,
-   "driver version: %s(for Geode5536 B1)\n", tmp);
-   udc = dev;
-
-   retval = usb_add_gadget_udc_release(>pdev->dev, >gadget,
-   gadget_release);
-   if (retval)
-   goto finished;
-
-   /* timer init */
-   init_timer(_timer);
-   udc_timer.function = udc_timer_function;
-   udc_timer.data = 1;
-   /* timer pollstall init */
-   init_timer(_pollstall_timer);
-   udc_pollstall_timer.function = udc_pollstall_timer_function;
-   udc_poll

[PATCH v2 04/12] usb: gadget: amd5536udc: use free_dma_pools

[Sudip Mukherjee]

We have the function free_dma_pools() which frees all the dma pools. Use
it instead of calling all the functions separately. The if conditions
for data_requests and stp_requests are also not required here as this is
the remove function and we are here means probe has succeeded and dma
has been successfully allocated, so they cannot be NULL here.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 15 +--
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 6223b1b..7805b29 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3131,20 +3131,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
return;
 
/* dma pool cleanup */
-   if (dev->data_requests)
-   pci_pool_destroy(dev->data_requests);
-
-   if (dev->stp_requests) {
-   /* cleanup DMA desc's for ep0in */
-   pci_pool_free(dev->stp_requests,
-   dev->ep[UDC_EP0OUT_IX].td_stp,
-   dev->ep[UDC_EP0OUT_IX].td_stp_dma);
-   pci_pool_free(dev->stp_requests,
-   dev->ep[UDC_EP0OUT_IX].td,
-   dev->ep[UDC_EP0OUT_IX].td_phys);
-
-   pci_pool_destroy(dev->stp_requests);
-   }
+   free_dma_pools(dev);
 
/* reset controller */
writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
-- 
1.9.1

--
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 v2 07/12] usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup

[Sudip Mukherjee]

Rearrange the udc_remote_wakeup function to remove the forward
declaration.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 41 ++---
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 6c16737..f218520 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -73,7 +73,6 @@ static void udc_free_request(struct usb_ep *usbep, struct 
usb_request *usbreq);
 static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
 static int udc_create_dma_chain(struct udc_ep *ep, struct udc_request *req,
unsigned long buf_len, gfp_t gfp_flags);
-static int udc_remote_wakeup(struct udc *dev);
 static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
 static void udc_pci_remove(struct pci_dev *pdev);
 
@@ -1452,6 +1451,26 @@ static int udc_get_frame(struct usb_gadget *gadget)
return -EOPNOTSUPP;
 }
 
+/* Initiates a remote wakeup */
+static int udc_remote_wakeup(struct udc *dev)
+{
+   unsigned long flags;
+   u32 tmp;
+
+   DBG(dev, "UDC initiates remote wakeup\n");
+
+   spin_lock_irqsave(>lock, flags);
+
+   tmp = readl(>regs->ctl);
+   tmp |= AMD_BIT(UDC_DEVCTL_RES);
+   writel(tmp, >regs->ctl);
+   tmp &= AMD_CLEAR_BIT(UDC_DEVCTL_RES);
+   writel(tmp, >regs->ctl);
+
+   spin_unlock_irqrestore(>lock, flags);
+   return 0;
+}
+
 /* Remote wakeup gadget interface */
 static int udc_wakeup(struct usb_gadget *gadget)
 {
@@ -3384,26 +3403,6 @@ err_pcidev:
return retval;
 }
 
-/* Initiates a remote wakeup */
-static int udc_remote_wakeup(struct udc *dev)
-{
-   unsigned long flags;
-   u32 tmp;
-
-   DBG(dev, "UDC initiates remote wakeup\n");
-
-   spin_lock_irqsave(>lock, flags);
-
-   tmp = readl(>regs->ctl);
-   tmp |= AMD_BIT(UDC_DEVCTL_RES);
-   writel(tmp, >regs->ctl);
-   tmp &= AMD_CLEAR_BIT(UDC_DEVCTL_RES);
-   writel(tmp, >regs->ctl);
-
-   spin_unlock_irqrestore(>lock, flags);
-   return 0;
-}
-
 /* PCI device parameters */
 static const struct pci_device_id pci_id[] = {
{
-- 
1.9.1

--
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 v2 02/12] usb: gadget: amd5536udc: fix error path

[Sudip Mukherjee]

Handle the error properly instead of calling the pci remove function.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 29 ++---
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 38a6858..3f85044 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3107,6 +3107,17 @@ static void udc_remove(struct udc *dev)
udc = NULL;
 }
 
+/* free all the dma pools */
+static void free_dma_pools(struct udc *dev)
+{
+   dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td,
+ dev->ep[UDC_EP0OUT_IX].td_phys);
+   dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td_stp,
+ dev->ep[UDC_EP0OUT_IX].td_stp_dma);
+   dma_pool_destroy(dev->stp_requests);
+   dma_pool_destroy(dev->data_requests);
+}
+
 /* Reset all pci context */
 static void udc_pci_remove(struct pci_dev *pdev)
 {
@@ -3297,7 +3308,7 @@ static int udc_pci_probe(
if (use_dma) {
retval = init_dma_pools(dev);
if (retval != 0)
-   goto finished;
+   goto err_dma;
}
 
dev->phys_addr = resource;
@@ -3305,13 +3316,17 @@ static int udc_pci_probe(
dev->pdev = pdev;
 
/* general probing */
-   if (udc_probe(dev) == 0)
-   return 0;
-
-finished:
-   udc_pci_remove(pdev);
-   return retval;
+   if (udc_probe(dev)) {
+   retval = -ENODEV;
+   goto err_probe;
+   }
+   return 0;
 
+err_probe:
+   if (use_dma)
+   free_dma_pools(dev);
+err_dma:
+   free_irq(pdev->irq, dev);
 err_irq:
iounmap(dev->virt_addr);
 err_ioremap:
-- 
1.9.1

--
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 v2 08/12] usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain

[Sudip Mukherjee]

Rearrange udc_create_dma_chain to remove the forward declaration. While
rearranging fixed the relevant checkpatch warnings.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 238 ++--
 1 file changed, 117 insertions(+), 121 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index f218520..2602173 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -71,8 +71,6 @@ static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
 static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
 static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
-static int udc_create_dma_chain(struct udc_ep *ep, struct udc_request *req,
-   unsigned long buf_len, gfp_t gfp_flags);
 static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
 static void udc_pci_remove(struct pci_dev *pdev);
 
@@ -787,6 +785,123 @@ udc_rxfifo_read(struct udc_ep *ep, struct udc_request 
*req)
return finished;
 }
 
+/* Creates or re-inits a DMA chain */
+static int udc_create_dma_chain(
+   struct udc_ep *ep,
+   struct udc_request *req,
+   unsigned long buf_len, gfp_t gfp_flags
+)
+{
+   unsigned long bytes = req->req.length;
+   unsigned int i;
+   dma_addr_t dma_addr;
+   struct udc_data_dma *td = NULL;
+   struct udc_data_dma *last = NULL;
+   unsigned long txbytes;
+   unsigned create_new_chain = 0;
+   unsigned len;
+
+   VDBG(ep->dev, "udc_create_dma_chain: bytes=%ld buf_len=%ld\n",
+bytes, buf_len);
+   dma_addr = DMA_DONT_USE;
+
+   /* unset L bit in first desc for OUT */
+   if (!ep->in)
+   req->td_data->status &= AMD_CLEAR_BIT(UDC_DMA_IN_STS_L);
+
+   /* alloc only new desc's if not already available */
+   len = req->req.length / ep->ep.maxpacket;
+   if (req->req.length % ep->ep.maxpacket)
+   len++;
+
+   if (len > req->chain_len) {
+   /* shorter chain already allocated before */
+   if (req->chain_len > 1)
+   udc_free_dma_chain(ep->dev, req);
+   req->chain_len = len;
+   create_new_chain = 1;
+   }
+
+   td = req->td_data;
+   /* gen. required number of descriptors and buffers */
+   for (i = buf_len; i < bytes; i += buf_len) {
+   /* create or determine next desc. */
+   if (create_new_chain) {
+   td = pci_pool_alloc(ep->dev->data_requests,
+   gfp_flags, _addr);
+   if (!td)
+   return -ENOMEM;
+
+   td->status = 0;
+   } else if (i == buf_len) {
+   /* first td */
+   td = (struct udc_data_dma *)phys_to_virt(
+   req->td_data->next);
+   td->status = 0;
+   } else {
+   td = (struct udc_data_dma *)phys_to_virt(last->next);
+   td->status = 0;
+   }
+
+   if (td)
+   td->bufptr = req->req.dma + i; /* assign buffer */
+   else
+   break;
+
+   /* short packet ? */
+   if ((bytes - i) >= buf_len) {
+   txbytes = buf_len;
+   } else {
+   /* short packet */
+   txbytes = bytes - i;
+   }
+
+   /* link td and assign tx bytes */
+   if (i == buf_len) {
+   if (create_new_chain)
+   req->td_data->next = dma_addr;
+   /*
+* else
+*  req->td_data->next = virt_to_phys(td);
+*/
+   /* write tx bytes */
+   if (ep->in) {
+   /* first desc */
+   req->td_data->status =
+   AMD_ADDBITS(req->td_data->status,
+   ep->ep.maxpacket,
+   UDC_DMA_IN_STS_TXBYTES);
+   /* second desc */
+   td->status = AMD_ADDBITS(td->status,
+   txbytes,
+   UDC_DMA_IN_STS_TXBYTES);
+   }
+   } else {
+  

[PATCH v2 10/12] usb: gadget: amd5536udc: remove forward declaration of udc_pci_*

[Sudip Mukherjee]

Remove the forward declarations of udc_pci_probe and udc_pci_remove.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 6d64129..00ae069 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -70,8 +70,6 @@ static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
 static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
-static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
-static void udc_pci_remove(struct pci_dev *pdev);
 
 /* description */
 static const char mod_desc[] = UDC_MOD_DESCRIPTION;
-- 
1.9.1

--
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 v2 05/12] usb: gadget: amd5536udc: remove unnecessary conditions

[Sudip Mukherjee]

The condition checking for irq_registered, regs, mem_region and active
are not required as this is the remove function. And we are in the
remove means that probe was successful and they can never be NULL at
this point of code.
It was required in the original code as the remove function was part of
the error handler of probe function.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 17 +
 drivers/usb/gadget/udc/amd5536udc.h |  5 +
 2 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 7805b29..89e83e4 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3135,15 +3135,11 @@ static void udc_pci_remove(struct pci_dev *pdev)
 
/* reset controller */
writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
-   if (dev->irq_registered)
-   free_irq(pdev->irq, dev);
-   if (dev->virt_addr)
-   iounmap(dev->virt_addr);
-   if (dev->mem_region)
-   release_mem_region(pci_resource_start(pdev, 0),
-   pci_resource_len(pdev, 0));
-   if (dev->active)
-   pci_disable_device(pdev);
+   free_irq(pdev->irq, dev);
+   iounmap(dev->virt_addr);
+   release_mem_region(pci_resource_start(pdev, 0),
+  pci_resource_len(pdev, 0));
+   pci_disable_device(pdev);
 
udc_remove(dev);
 }
@@ -3240,7 +3236,6 @@ static int udc_pci_probe(
retval = -ENODEV;
goto err_pcidev;
}
-   dev->active = 1;
 
/* PCI resource allocation */
resource = pci_resource_start(pdev, 0);
@@ -3251,7 +3246,6 @@ static int udc_pci_probe(
retval = -EBUSY;
goto err_memreg;
}
-   dev->mem_region = 1;
 
dev->virt_addr = ioremap_nocache(resource, len);
if (dev->virt_addr == NULL) {
@@ -3282,7 +3276,6 @@ static int udc_pci_probe(
retval = -EBUSY;
goto err_irq;
}
-   dev->irq_registered = 1;
 
pci_set_drvdata(pdev, dev);
 
diff --git a/drivers/usb/gadget/udc/amd5536udc.h 
b/drivers/usb/gadget/udc/amd5536udc.h
index 6744d3b..4638d70 100644
--- a/drivers/usb/gadget/udc/amd5536udc.h
+++ b/drivers/usb/gadget/udc/amd5536udc.h
@@ -526,14 +526,11 @@ struct udc {
struct udc_ep   ep[UDC_EP_NUM];
struct usb_gadget_driver*driver;
/* operational flags */
-   unsignedactive : 1,
-   stall_ep0in : 1,
+   unsignedstall_ep0in : 1,
waiting_zlp_ack_ep0in : 1,
set_cfg_not_acked : 1,
-   irq_registered : 1,
data_ep_enabled : 1,
data_ep_queued : 1,
-   mem_region : 1,
sys_suspended : 1,
connected;
 
-- 
1.9.1

--
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 v2 00/12] usb: gadget: amd5536udc: fix memory leaks

[Sudip Mukherjee]

Proper error handling was done in the probe and some unwanted comparison
was removed. Also some forward declaration was removed.

regards
sudip

Sudip Mukherjee (12):
  usb: gadget: amd5536udc: rewrite init_dma_pools
  usb: gadget: amd5536udc: fix error path
  usb: gadget: amd5536udc: use WARN_ON
  usb: gadget: amd5536udc: use free_dma_pools
  usb: gadget: amd5536udc: remove unnecessary conditions
  usb: gadget: amd5536udc: remove forward declaration of udc_probe
  usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup
  usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain
  usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain
  usb: gadget: amd5536udc: remove forward declaration of udc_pci_*
  usb: gadget: amd5536udc: remove forward declaration of udc_basic_init
  usb: gadget: amd5536udc: NULL comparison

 drivers/usb/gadget/udc/amd5536udc.c | 609 ++--
 drivers/usb/gadget/udc/amd5536udc.h |   5 +-
 2 files changed, 301 insertions(+), 313 deletions(-)

-- 
1.9.1

--
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 v2 01/12] usb: gadget: amd5536udc: rewrite init_dma_pools

[Sudip Mukherjee]

A rewrite of init_dma_pools() with proper error handling.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 175ca93..38a6858 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3169,8 +3169,7 @@ static int init_dma_pools(struct udc *dev)
sizeof(struct udc_data_dma), 0, 0);
if (!dev->data_requests) {
DBG(dev, "can't get request data pool\n");
-   retval = -ENOMEM;
-   goto finished;
+   return -ENOMEM;
}
 
/* EP0 in dma regs = dev control regs */
@@ -3182,14 +3181,14 @@ static int init_dma_pools(struct udc *dev)
if (!dev->stp_requests) {
DBG(dev, "can't get stp request pool\n");
retval = -ENOMEM;
-   goto finished;
+   goto err_create_dma_pool;
}
/* setup */
td_stp = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
>ep[UDC_EP0OUT_IX].td_stp_dma);
if (td_stp == NULL) {
retval = -ENOMEM;
-   goto finished;
+   goto err_alloc_dma;
}
dev->ep[UDC_EP0OUT_IX].td_stp = td_stp;
 
@@ -3198,12 +3197,20 @@ static int init_dma_pools(struct udc *dev)
>ep[UDC_EP0OUT_IX].td_phys);
if (td_data == NULL) {
retval = -ENOMEM;
-   goto finished;
+   goto err_alloc_phys;
}
dev->ep[UDC_EP0OUT_IX].td = td_data;
return 0;
 
-finished:
+err_alloc_phys:
+   dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td_stp,
+ dev->ep[UDC_EP0OUT_IX].td_stp_dma);
+err_alloc_dma:
+   dma_pool_destroy(dev->stp_requests);
+   dev->stp_requests = NULL;
+err_create_dma_pool:
+   dma_pool_destroy(dev->data_requests);
+   dev->data_requests = NULL;
return retval;
 }
 
-- 
1.9.1

--
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 v2 02/12] usb: gadget: amd5536udc: fix error path

[Sudip Mukherjee]

On Tue, Sep 22, 2015 at 08:12:29PM +0530, Sudip Mukherjee wrote:
> On Tue, Sep 22, 2015 at 09:37:45AM -0500, Felipe Balbi wrote:
> > On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> > > Handle the error properly instead of calling the pci remove function.
> > > 
> > > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> > 
> > this doesn't apply. Where did you rebase this series ? Please rebase on my
> > testing/next
> This was done on next-20150922. I will rebase on your tree.
Looks like today is a day of confusion for me.
I asked you to discard my v1 as I saw some part of the change was done
by:  6527cc27761a ("usb: gadget: amd5536udc: fix error handling in 
udc_pci_probe()")

But now I dont see that patch in your testing/next , but I can see that
patch in next-20150922. Confused. :(
Now?
 
regards
sudip
--
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 v2 02/12] usb: gadget: amd5536udc: fix error path

[Sudip Mukherjee]

On Tue, Sep 22, 2015 at 09:37:45AM -0500, Felipe Balbi wrote:
> On Tue, Sep 22, 2015 at 06:54:27PM +0530, Sudip Mukherjee wrote:
> > Handle the error properly instead of calling the pci remove function.
> > 
> > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> 
> this doesn't apply. Where did you rebase this series ? Please rebase on my
> testing/next
This was done on next-20150922. I will rebase on your tree.

regards
sudip
--
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 1/3] avr32: fix build failure

[Sudip Mukherjee]

On Mon, Sep 21, 2015 at 08:09:42AM +0200, Hans-Christian Egtvedt wrote:
> Around Sat 19 Sep 2015 22:42:57 +0530 or thereabout, Sudip Mukherjee wrote:
> > While building avr32 with allmodconfig, the build used to fail with the
> > message:
> > error: implicit declaration of function 'pci_iomap'
> > error: implicit declaration of function 'pci_iounmap'
> 
> What has changed recently that start pulling in these? AVR32 does not have
> PCI at all, and will never have it either. Is this exposing a bug somewhere
> else?
It looks like pci_iomap and pci_iounmap doesnot depend on CONFIG_PCI.
So drivers/net/ethernet/via/via-rhine.c is calling these functions even
if PCI is disabled. The build log is at:
https://travis-ci.org/sudipm-mukherjee/parport/jobs/81127188

You can find a similar discussion at:
http://www.linux-mips.org/archives/linux-mips/2013-06/msg00510.html

regards
sudip
--
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 1/3] avr32: fix build failure

[Sudip Mukherjee]

On Mon, Sep 21, 2015 at 09:33:00AM +0200, Hans-Christian Egtvedt wrote:
> Around Mon 21 Sep 2015 12:09:01 +0530 or thereabout, Sudip Mukherjee wrote:
> > On Mon, Sep 21, 2015 at 08:09:42AM +0200, Hans-Christian Egtvedt wrote:
> >> Around Sat 19 Sep 2015 22:42:57 +0530 or thereabout, Sudip Mukherjee wrote:
> >> > While building avr32 with allmodconfig, the build used to fail with the
> >> > message:
> >> > error: implicit declaration of function 'pci_iomap'
> >> > error: implicit declaration of function 'pci_iounmap'
> >> 
> >> What has changed recently that start pulling in these? AVR32 does not have
> >> PCI at all, and will never have it either. Is this exposing a bug somewhere
> >> else?
> > It looks like pci_iomap and pci_iounmap doesnot depend on CONFIG_PCI.
> > So drivers/net/ethernet/via/via-rhine.c is calling these functions even
> > if PCI is disabled. The build log is at:
> > https://travis-ci.org/sudipm-mukherjee/parport/jobs/81127188
> > 
> > You can find a similar discussion at:
> > http://www.linux-mips.org/archives/linux-mips/2013-06/msg00510.html
> 
> If multiple architectures have similar problems, then a more global
> definition of these unused functions could be added.
> 
> Just move the implementation for MIPS into include/asm-generic/io.h ?
> 
> That way we do not have to implement and identical stub for every device not
> supporting PCI.

include/asm-generic/io.h is having:
#ifndef CONFIG_GENERIC_IOMAP
struct pci_dev;
extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned
long max);

#ifndef pci_iounmap
#define pci_iounmap pci_iounmap
static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p)
{
}
#endif
#endif /* CONFIG_GENERIC_IOMAP */

and CONFIG_GENERIC_IOMAP is not defined when I do allmodconfig with
avr32. I have not seen this earlier, maybe in my patch pci_iounmap() was
not required. But pci_iomap is declared as extern so we need have it
somewhere.

regards
sudip
--
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 2/3] usb: gadget: at91_udc: mention proper dependency

[Sudip Mukherjee]

On Sun, Sep 20, 2015 at 11:15:28AM -0500, Felipe Balbi wrote:
> On Sat, Sep 19, 2015 at 10:42:58PM +0530, Sudip Mukherjee wrote:
> > While building allmodconfig on avr32 the build failed with the error:
> > "at91_pmc_base" [drivers/usb/gadget/udc/atmel_usba_udc.ko] undefined!
> > 
> > On checking the code it turned out that if CONFIG_OF is defined then it
> > is using at91_pmc_read() which is using at91_pmc_base. And unless
> > COMMON_CLK_AT91 is defined we donot have at91_pmc_base. And
> > COMMON_CLK_AT91 is available with AT91 architecture.
> > Mention the dependency such that this driver builds with avr32 only if
> > OF is not enabled.
> > 
> > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> > ---
> > 
> > Tested build with at91_dt_defconfig and allmodconfig of avr32. Build log
> > at:
> > https://travis-ci.org/sudipm-mukherjee/parport/builds/81168845
> > 
> >  drivers/usb/gadget/udc/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> > index 9a3a6b0..cdbff54 100644
> > --- a/drivers/usb/gadget/udc/Kconfig
> > +++ b/drivers/usb/gadget/udc/Kconfig
> > @@ -55,7 +55,7 @@ config USB_LPC32XX
> >  
> >  config USB_ATMEL_USBA
> > tristate "Atmel USBA"
> > -   depends on AVR32 || ARCH_AT91
> > +   depends on ((AVR32 && !OF) || ARCH_AT91)
> 
> any chance you can add || COMPILE_TEST here ? I'd like to make
> sure this builds on my end too.
With "depends on ((AVR32 && !OF) || ARCH_AT91 || COMPILE_TEST)"
normal allmodconfig builiding for x86_64 failed with:

drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_start’:
drivers/usb/gadget/udc/atmel_usba_udc.c:1783:25: error: ‘USBA_ENABLE_MASK’ 
undeclared (first use in this function)
  usba_writel(udc, CTRL, USBA_ENABLE_MASK);
 ^
drivers/usb/gadget/udc/atmel_usba_udc.c: In function ‘usba_stop’:
drivers/usb/gadget/udc/atmel_usba_udc.c:1800:25: error: ‘USBA_DISABLE_MASK’ 
undeclared (first use in this function)
  usba_writel(udc, CTRL, USBA_DISABLE_MASK);
 ^

Looks like USBA_DISABLE_MASK and USBA_ENABLE_MASK is defined under
#if defined(CONFIG_AVR32). :(

regards
sudip
--
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 00/16] usb: gadget: amd5536udc: fix memory leaks

[Sudip Mukherjee]

On Sun, Sep 20, 2015 at 11:17:36AM -0500, Felipe Balbi wrote:
> On Sun, Sep 20, 2015 at 01:42:42PM +0530, Sudip Mukherjee wrote:
> > On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> > > On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > > > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > > > This amd5536udc was a complete mess. The major problems that i could
> > > > > find are:
> > > > > 
> > 
> > > > 
> > > > run checkpatch.pl and try again
> > 
> > > Anyways, I will fix up all the warnings and send v2.
> > 
> > I guess v2 is not required any more. The main thing that this series was
> > trying to do has already been done by:
> > 6527cc27761a ("usb: gadget: amd5536udc: fix error handling in 
> > udc_pci_probe()")
> 
> all right, see if there's anything missing, please.
I think something still needs to be done there. I will send a v2 for
your review.

regards
sudip
--
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 00/16] usb: gadget: amd5536udc: fix memory leaks

[Sudip Mukherjee]

On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > This amd5536udc was a complete mess. The major problems that i could
> > > find are:
> > > 

> > 
> > run checkpatch.pl and try again

> Anyways, I will fix up all the warnings and send v2.

I guess v2 is not required any more. The main thing that this series was
trying to do has already been done by:
6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")

regards
sudip
--
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] USB: EHCI: fix dereference of ERR_PTR

[Sudip Mukherjee]

On Mon, Sep 21, 2015 at 10:48:52AM +0800, Lu, Baolu wrote:
> 
> 
> On 09/16/2015 10:08 PM, Sudip Mukherjee wrote:
> >On error find_tt() returns either a NULL pointer or the error value in
> >ERR_PTR. But we were dereferencing it directly without even checking if
> >find_tt() returned a valid pointer or not.
> >
> >Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> >---
> >  drivers/usb/host/ehci-sched.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> >diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
> >index f9a3327..27bced7 100644
> >--- a/drivers/usb/host/ehci-sched.c
> >+++ b/drivers/usb/host/ehci-sched.c
> >@@ -257,6 +257,8 @@ static void reserve_release_intr_bandwidth(struct 
> >ehci_hcd *ehci,
> > /* FS/LS bus bandwidth */
> > if (tt_usecs) {
> > tt = find_tt(qh->ps.udev);
> >+if (!tt || IS_ERR(tt))
> 
> Why not IS_ERR_OR_NULL()?
This was v1, corrected in v2. And Alan has already explained why this
patch is not required.

regards
sudip
--
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 0/3] fix allmodconfig failure of avr32

[Sudip Mukherjee]

Build of allmodconfig with avr32 used to fail with the errors:
error: implicit declaration of function 'pci_iomap'
error: implicit declaration of function 'pci_iounmap'
and
"at91_pmc_base" [drivers/usb/gadget/udc/atmel_usba_udc.ko] undefined!

First two patches fixes these two problems. Third patch is related to a
build warning:
warning: 'compound_head' declared inline after being called.
warning: previous declaration of 'compound_head' was here.

regards
sudip

Sudip Mukherjee (3):
  avr32: fix build failure
  usb: gadget: at91_udc: mention proper dependency
  page-flags: rectify forward declaration

 arch/avr32/include/asm/io.h| 13 +
 drivers/usb/gadget/udc/Kconfig |  2 +-
 include/linux/page-flags.h |  2 +-
 3 files changed, 15 insertions(+), 2 deletions(-)

-- 
1.9.1

--
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 2/3] usb: gadget: at91_udc: mention proper dependency

[Sudip Mukherjee]

While building allmodconfig on avr32 the build failed with the error:
"at91_pmc_base" [drivers/usb/gadget/udc/atmel_usba_udc.ko] undefined!

On checking the code it turned out that if CONFIG_OF is defined then it
is using at91_pmc_read() which is using at91_pmc_base. And unless
COMMON_CLK_AT91 is defined we donot have at91_pmc_base. And
COMMON_CLK_AT91 is available with AT91 architecture.
Mention the dependency such that this driver builds with avr32 only if
OF is not enabled.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---

Tested build with at91_dt_defconfig and allmodconfig of avr32. Build log
at:
https://travis-ci.org/sudipm-mukherjee/parport/builds/81168845

 drivers/usb/gadget/udc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 9a3a6b0..cdbff54 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -55,7 +55,7 @@ config USB_LPC32XX
 
 config USB_ATMEL_USBA
tristate "Atmel USBA"
-   depends on AVR32 || ARCH_AT91
+   depends on ((AVR32 && !OF) || ARCH_AT91)
help
  USBA is the integrated high-speed USB Device controller on
  the AT32AP700x, some AT91SAM9 and AT91CAP9 processors from Atmel.
-- 
1.9.1

--
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 3/3] page-flags: rectify forward declaration

[Sudip Mukherjee]

compound_head is defined as inline in page-flags.h but in the forward
declaration of compound_head in the same file missed "inline". As a result
we got plenty of build warnings while building for some architecture
like avr32. The warning showed as:
warning: 'compound_head' declared inline after being called.
warning: previous declaration of 'compound_head' was here

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---

tested build with avr32 and also with x86_64 allmodconfig to verify that
nothing breaks due to this change.

 include/linux/page-flags.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index ab1a0e9..2a2391c 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -227,7 +227,7 @@ static inline int __TestClearPage##uname(struct page *page) 
{ return 0; }
 struct page;
 static inline int PageCompound(struct page *page);
 static inline int PageTail(struct page *page);
-static struct page *compound_head(struct page *page);
+static inline struct page *compound_head(struct page *page);
 
 __PAGEFLAG(Locked, locked, PF_NO_TAIL)
 PAGEFLAG(Error, error, PF_NO_COMPOUND) TESTCLEARFLAG(Error, error, 
PF_NO_COMPOUND)
-- 
1.9.1

--
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 1/3] avr32: fix build failure

[Sudip Mukherjee]

While building avr32 with allmodconfig, the build used to fail with the
message:
error: implicit declaration of function 'pci_iomap'
error: implicit declaration of function 'pci_iounmap'

Create dummy pci_io{map,unmap} functions to fix the build.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---

Tested with defconfig, allmodconfig, allnoconfig and merisc_defconfig.
Build is at:
https://travis-ci.org/sudipm-mukherjee/parport/builds/81168845

Partial idea taken from:
78857614104a ("MIPS: Expose missing pci_io{map,unmap} declarations")
which solved a similar problem with mips.

 arch/avr32/include/asm/io.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
index f855646..1d8c4e4 100644
--- a/arch/avr32/include/asm/io.h
+++ b/arch/avr32/include/asm/io.h
@@ -276,6 +276,19 @@ extern void __iomem *__ioremap(unsigned long offset, 
size_t size,
   unsigned long flags);
 extern void __iounmap(void __iomem *addr);
 
+#ifndef CONFIG_PCI
+struct pci_dev;
+static inline void pci_iounmap(struct pci_dev *dev, void __iomem *addr)
+{
+}
+
+static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar,
+ unsigned long maxlen)
+{
+   return NULL;
+}
+#endif
+
 /*
  * ioremap -   map bus memory into CPU space
  * @offset bus address of the memory
-- 
1.9.1

--
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: First kernel patch (optimization)

[Sudip Mukherjee]

On Fri, Sep 18, 2015 at 10:26:24PM -0400, Theodore Ts'o wrote:
> On Fri, Sep 18, 2015 at 12:42:48AM -0700, Greg KH wrote:
>
> Rather, what concerns me is that we aren't pushing people to go
> *beyond* cleanup patches.  We have lots of tutorials about how to
> create perfectly formed patches; but we don't seem to have patches
> about how to do proper benchmarking.  Or how to check system call and
> ioctl interfaces to make sure the code is doing appropriate input
> checking.  So I don't want to pre-reject these people; but to rather
> push them and to help them to try their hand at more substantive work.
> 
> What surprises me is the apparent assumption that people need a huge
> amount of hand-holding on how to properly form a patch, but that
> people will be able to figure out how to do proper benchmarking, or
> design proper abstractions, etc., as skills that will magically appear
> full-formed into the new kernel programmer's mind without any help or
> work on our part.
Reading Ted's earlier mail I was thinking since I don't have some of the
skills mentioned, maybe I am in wrong place. But what Ted said now is
almost the same what I said in the Ksummit discussion about recruitment.
I will copy-paste here for reference:
"In my opinion the main problem is lack of direction or guidance. As a 
newbie I send my first patch, it gets accepted, I have a party to
celebrate and do more style correction and few more patches are accepted.
But by that time I am getting bored with just style correction and want
to do something more.
Now the problem starts. No one is there to guide me and I as a newbie
will not be that much capable enough to find things to do on my own. And
I start loosing the interest. Newbies who are coming from Eudyptula or
starting on their own will face this. But on the otherhand participants
of Outreachy will get a Mentor to guide them and gets a stipend to keep
them motivated. Stipend may not matter to the right candidate who has
interest but having a mentor is the big difference."

This is from my own experience as I have gone through that time phrase.
But now after one year I know there are numerous things to do. But still
I don't have some of the skills Ted mentioned.

I know I will get the skills which I don't have now but since I am on my
own that will be a time consuming process. Even more time consuming as
this is not part of my dayjob. But if I had a mentor/guide who could
have given some hint the process might have been much much faster.

regards
sudip

--
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 00/16] usb: gadget: amd5536udc: fix memory leaks

[Sudip Mukherjee]

On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > This amd5536udc was a complete mess. The major problems that i could
> > find are:
> > 
> > 1) if udc_pci_probe() fails in any stage then it just calls the
> > udc_pci_remove() to handle error. And udc_pci_remove() works with
> > struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
> > pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
> > incase of error we are handling the error by dereferencing a NULL
> > pointer.
> > 
> > 2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
> > will be set only if probe is success. So that means if probe fails then
> > probe will call udc_pci_remove() for error handling and udc_pci_remove()
> > will inturn halts the kernel by calling BUG().
> > 
> > And apart from these numerous memory leaks and not releasing of
> > resources. Here comes a rewrite of few of the functions in an
> > attempt to fix these.
> 
> run checkpatch.pl and try again
I know checkpatch gives warning on some of my patches but as the warning
was not related to the part I have modified so I have not done any thing
with them as they will become unrelated changes than what is mentioned
in the commit log.
Anyways, I will fix up all the warnings and send v2. But do you want me
to also fix the checkpatch warnings in those patch where functions are
rearranged? Because in those patches functions were just moved and there
was no change in the body of the function.

regards
sudip
--
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 v2] USB: EHCI: fix dereference of ERR_PTR

[Sudip Mukherjee]

On Wed, Sep 16, 2015 at 12:54:03PM -0400, Alan Stern wrote:
> On Wed, 16 Sep 2015, Sudip Mukherjee wrote:
> 
> > On error find_tt() returns either a NULL pointer or the error value in
> > ERR_PTR. But we were dereferencing it directly without even checking if
> > find_tt() returned a valid pointer or not.
> > 
> > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> > ---

> > @@ -1373,6 +1375,8 @@ static void reserve_release_iso_bandwidth(struct 
> > ehci_hcd *ehci,
> > }
> >  
> > tt = find_tt(stream->ps.udev);
> > +   if (IS_ERR_OR_NULL(tt))
> > +   return;
> > if (sign > 0)
> > list_add_tail(>ps.ps_list, >ps_list);
> > else
> 
> This patch isn't needed.  In both reserve_release_intr_bandwidth() and 
> reserve_release_iso_bandwidth() it is known that find_tt() will return 
> a valid pointer.
> 
> This is because each of those functions is called from only one place.  
> For example, reserve_release_intr_bandwidth() is called only at the end
> of qh_schedule().  But near the start of qh_schedule() there is earlier
> call to tt_find(), and there we do test for error pointers.  If the
> first call doesn't return an error then the second call won't either.
> 
> The same sort of thing happens in reserve_release_iso_bandwidth().
Yes, I should have looked more before sending. Sorry for the noise.
But in those checkes for find_tt() only IS_ERR is checked, shouldn't we
check for IS_ERR_OR_NULL as find_tt() can return NULL also?

regards
sudip
--
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] USB: EHCI: fix dereference of ERR_PTR

[Sudip Mukherjee]

On error find_tt() returns either a NULL pointer or the error value in
ERR_PTR. But we were dereferencing it directly without even checking if
find_tt() returned a valid pointer or not.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/host/ehci-sched.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index f9a3327..27bced7 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -257,6 +257,8 @@ static void reserve_release_intr_bandwidth(struct ehci_hcd 
*ehci,
/* FS/LS bus bandwidth */
if (tt_usecs) {
tt = find_tt(qh->ps.udev);
+   if (!tt || IS_ERR(tt))
+   return;
if (sign > 0)
list_add_tail(>ps.ps_list, >ps_list);
else
@@ -1373,6 +1375,8 @@ static void reserve_release_iso_bandwidth(struct ehci_hcd 
*ehci,
}
 
tt = find_tt(stream->ps.udev);
+   if (!tt || IS_ERR(tt))
+   return;
if (sign > 0)
list_add_tail(>ps.ps_list, >ps_list);
else
-- 
1.9.1

--
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 v2] USB: EHCI: fix dereference of ERR_PTR

[Sudip Mukherjee]

On error find_tt() returns either a NULL pointer or the error value in
ERR_PTR. But we were dereferencing it directly without even checking if
find_tt() returned a valid pointer or not.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---

v2: used IS_ERR_OR_NULL (didn't know it was there. thanks)


 drivers/usb/host/ehci-sched.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index f9a3327..de75343 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -257,6 +257,8 @@ static void reserve_release_intr_bandwidth(struct ehci_hcd 
*ehci,
/* FS/LS bus bandwidth */
if (tt_usecs) {
tt = find_tt(qh->ps.udev);
+   if (IS_ERR_OR_NULL(tt))
+   return;
if (sign > 0)
list_add_tail(>ps.ps_list, >ps_list);
else
@@ -1373,6 +1375,8 @@ static void reserve_release_iso_bandwidth(struct ehci_hcd 
*ehci,
}
 
tt = find_tt(stream->ps.udev);
+   if (IS_ERR_OR_NULL(tt))
+   return;
if (sign > 0)
list_add_tail(>ps.ps_list, >ps_list);
else
-- 
1.9.1

--
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 14/16] usb: gadget: amd5536udc: NULL comparison

[Sudip Mukherjee]

A NULL comparison can be written as if (var) or if (!var).

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index f6d6097..3657a66 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -2189,7 +2189,7 @@ static irqreturn_t udc_data_out_isr(struct udc *dev, int 
ep_ix)
}
 
/* DMA */
-   } else if (!ep->cancel_transfer && req != NULL) {
+   } else if (!ep->cancel_transfer && req) {
ret_val = IRQ_HANDLED;
 
/* check for DMA done */
@@ -3139,7 +3139,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
 
usb_del_gadget_udc(>gadget);
/* gadget driver must not be registered */
-   if (WARN_ON(dev->driver != NULL))
+   if (WARN_ON(dev->driver))
return;
 
/* dma pool cleanup */
@@ -3193,7 +3193,7 @@ static int init_dma_pools(struct udc *dev)
/* setup */
td_stp = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
>ep[UDC_EP0OUT_IX].td_stp_dma);
-   if (td_stp == NULL) {
+   if (!td_stp) {
retval = -ENOMEM;
goto err_alloc_dma;
}
@@ -3202,7 +3202,7 @@ static int init_dma_pools(struct udc *dev)
/* data: 0 packets !? */
td_data = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
>ep[UDC_EP0OUT_IX].td_phys);
-   if (td_data == NULL) {
+   if (!td_data) {
retval = -ENOMEM;
goto err_alloc_phys;
}
@@ -3328,7 +3328,7 @@ static int udc_pci_probe(
dev->mem_region = 1;
 
dev->virt_addr = ioremap_nocache(resource, len);
-   if (dev->virt_addr == NULL) {
+   if (!dev->virt_addr) {
dev_dbg(>dev, "start address cannot be mapped\n");
retval = -EFAULT;
goto err_ioremap;
-- 
1.9.1

--
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 16/16] usb: gadget: amd5536udc: match alignment

[Sudip Mukherjee]

checkpatch complains that the alignment should match with open
parenthesis.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 156 ++--
 1 file changed, 78 insertions(+), 78 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 98b841d..9f73de8 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -226,7 +226,7 @@ module_param(use_dma_ppb, bool, S_IRUGO);
 MODULE_PARM_DESC(use_dma_ppb, "true for DMA in packet per buffer mode");
 module_param(use_dma_ppb_du, bool, S_IRUGO);
 MODULE_PARM_DESC(use_dma_ppb_du,
-   "true for DMA in packet per buffer mode with descriptor update");
+"true for DMA in packet per buffer mode with descriptor 
update");
 module_param(use_fullspeed, bool, S_IRUGO);
 MODULE_PARM_DESC(use_fullspeed, "true for fullspeed only");
 
@@ -436,7 +436,7 @@ udc_ep_enable(struct usb_ep *usbep, const struct 
usb_endpoint_descriptor *desc)
/* set max packet size UDC CSR  */
tmp = readl(>csr->ne[ep->num - UDC_CSR_EP_OUT_IX_OFS]);
tmp = AMD_ADDBITS(tmp, maxpacket,
-   UDC_CSR_NE_MAX_PKT);
+ UDC_CSR_NE_MAX_PKT);
writel(tmp, >csr->ne[ep->num - UDC_CSR_EP_OUT_IX_OFS]);
 
if (use_dma && !ep->in) {
@@ -581,7 +581,7 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
if (ep->dma) {
/* ep0 in requests are allocated from data pool here */
dma_desc = pci_pool_alloc(ep->dev->data_requests, gfp,
-   >td_phys);
+ >td_phys);
if (!dma_desc) {
kfree(req);
return NULL;
@@ -622,7 +622,7 @@ static int udc_free_dma_chain(struct udc *dev, struct 
udc_request *req)
for (i = 1; i < req->chain_len; i++) {
 
pci_pool_free(dev->data_requests, td,
-   (dma_addr_t) td_last->next);
+ (dma_addr_t) td_last->next);
td_last = td;
td = phys_to_virt(td_last->next);
}
@@ -652,7 +652,7 @@ udc_free_request(struct usb_ep *usbep, struct usb_request 
*usbreq)
udc_free_dma_chain(ep->dev, req);
 
pci_pool_free(ep->dev->data_requests, req->td_data,
-   req->td_phys);
+ req->td_phys);
}
kfree(req);
 }
@@ -672,7 +672,7 @@ static void udc_init_bna_dummy(struct udc_request *req)
UDC_DMA_STP_STS_BS);
 #ifdef UDC_VERBOSE
pr_debug("bna desc = %p, sts = %08x\n",
-   req->td_data, req->td_data->status);
+req->td_data, req->td_data->status);
 #endif
}
 }
@@ -723,7 +723,7 @@ udc_txfifo_write(struct udc_ep *ep, struct usb_request *req)
/* remaining bytes must be written by byte access */
for (j = 0; j < bytes % UDC_DWORD_BYTES; j++) {
writeb((u8)(*(buf + i) >> (j << UDC_BITS_PER_BYTE_SHIFT)),
-   ep->txfifo);
+  ep->txfifo);
}
 
/* dummy write confirm */
@@ -784,7 +784,7 @@ udc_rxfifo_read(struct udc_ep *ep, struct udc_request *req)
if (bytes > buf_space) {
if ((buf_space % ep->ep.maxpacket) != 0) {
DBG(ep->dev,
-   "%s: rx %d bytes, rx-buf space = %d bytesn\n",
+   "%s: rx %d bytes, rx-buf space = %d bytesn\n",
ep->ep.name, bytes, buf_space);
req->req.status = -EOVERFLOW;
}
@@ -821,7 +821,7 @@ static int udc_create_dma_chain(
unsigned len;
 
VDBG(ep->dev, "udc_create_dma_chain: bytes=%ld buf_len=%ld\n",
-   bytes, buf_len);
+bytes, buf_len);
dma_addr = DMA_DONT_USE;
 
/* unset L bit in first desc for OUT */
@@ -848,7 +848,7 @@ static int udc_create_dma_chain(
if (create_new_chain) {
 
td = pci_pool_alloc(ep->dev->data_requests,
-   gfp_flags, _addr);
+   gfp_flags, _addr);
if (!td)
return -ENOMEM;
 
@@ -889,7 +889,7 @@ static int udc_create_dma_chain(
/* first desc */

[PATCH 05/16] usb: gadget: amd5536udc: use free_dma_pools

[Sudip Mukherjee]

We have the function free_dma_pools() which frees all the dma pools. Use
it instead of calling all the functions separately. The if conditions
for data_requests and stp_requests are also not required here as this is
the remove function and we are here means probe has succeeded and dma
has been successfully allocated, so they cannot be NULL here.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 15 +--
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 4edcfd4..3ae0bb8 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3131,20 +3131,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
return;
 
/* dma pool cleanup */
-   if (dev->data_requests)
-   pci_pool_destroy(dev->data_requests);
-
-   if (dev->stp_requests) {
-   /* cleanup DMA desc's for ep0in */
-   pci_pool_free(dev->stp_requests,
-   dev->ep[UDC_EP0OUT_IX].td_stp,
-   dev->ep[UDC_EP0OUT_IX].td_stp_dma);
-   pci_pool_free(dev->stp_requests,
-   dev->ep[UDC_EP0OUT_IX].td,
-   dev->ep[UDC_EP0OUT_IX].td_phys);
-
-   pci_pool_destroy(dev->stp_requests);
-   }
+   free_dma_pools(dev);
 
/* reset controller */
writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
-- 
1.9.1

--
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 03/16] usb: gadget: amd5536udc: rewrite udc_pci_probe

[Sudip Mukherjee]

A rewrite of udc_pci_probe() with proper error handling. We use here
free_dma_pools() in error handling.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 52 +++--
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 8b700de..8646f6c 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3244,17 +3244,13 @@ static int udc_pci_probe(
 
/* init */
dev = kzalloc(sizeof(struct udc), GFP_KERNEL);
-   if (!dev) {
-   retval = -ENOMEM;
-   goto finished;
-   }
+   if (!dev)
+   return -ENOMEM;
 
/* pci setup */
if (pci_enable_device(pdev) < 0) {
-   kfree(dev);
-   dev = NULL;
retval = -ENODEV;
-   goto finished;
+   goto err_enabledev;
}
dev->active = 1;
 
@@ -3264,28 +3260,22 @@ static int udc_pci_probe(
 
if (!request_mem_region(resource, len, name)) {
dev_dbg(>dev, "pci device used already\n");
-   kfree(dev);
-   dev = NULL;
retval = -EBUSY;
-   goto finished;
+   goto err_requestmem;
}
dev->mem_region = 1;
 
dev->virt_addr = ioremap_nocache(resource, len);
if (dev->virt_addr == NULL) {
dev_dbg(>dev, "start address cannot be mapped\n");
-   kfree(dev);
-   dev = NULL;
retval = -EFAULT;
-   goto finished;
+   goto err_ioremap;
}
 
if (!pdev->irq) {
dev_err(>dev, "irq not set\n");
-   kfree(dev);
-   dev = NULL;
retval = -ENODEV;
-   goto finished;
+   goto err_irqreq;
}
 
spin_lock_init(>lock);
@@ -3301,10 +3291,8 @@ static int udc_pci_probe(
 
if (request_irq(pdev->irq, udc_irq, IRQF_SHARED, name, dev) != 0) {
dev_dbg(>dev, "request_irq(%d) fail\n", pdev->irq);
-   kfree(dev);
-   dev = NULL;
retval = -EBUSY;
-   goto finished;
+   goto err_irqreq;
}
dev->irq_registered = 1;
 
@@ -3320,7 +3308,7 @@ static int udc_pci_probe(
if (use_dma) {
retval = init_dma_pools(dev);
if (retval != 0)
-   goto finished;
+   goto err_dma;
}
 
dev->phys_addr = resource;
@@ -3328,12 +3316,26 @@ static int udc_pci_probe(
dev->pdev = pdev;
 
/* general probing */
-   if (udc_probe(dev) == 0)
-   return 0;
+   if (udc_probe(dev) != 0) {
+   retval = -ENODEV;
+   goto err_probe;
+   }
+   return 0;
 
-finished:
-   if (dev)
-   udc_pci_remove(pdev);
+err_probe:
+   if (use_dma)
+   free_dma_pools(dev);
+err_dma:
+   free_irq(pdev->irq, dev);
+err_irqreq:
+   iounmap(dev->virt_addr);
+err_ioremap:
+   release_mem_region(resource, len);
+err_requestmem:
+   pci_disable_device(pdev);
+err_enabledev:
+   kfree(dev);
+   dev = NULL;
return retval;
 }
 
-- 
1.9.1

--
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 08/16] usb: gadget: amd5536udc: remove forward declaration of udc_probe

[Sudip Mukherjee]

Rearrange the udc_probe function to remove the forward declarations.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 133 ++--
 1 file changed, 66 insertions(+), 67 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index b7753b2..01a6183 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -65,7 +65,6 @@
 
 static void udc_tasklet_disconnect(unsigned long);
 static void empty_req_queue(struct udc_ep *);
-static int udc_probe(struct udc *dev);
 static void udc_basic_init(struct udc *dev);
 static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
@@ -3209,6 +3208,72 @@ err_create_dma_pool:
return retval;
 }
 
+/* general probe */
+static int udc_probe(struct udc *dev)
+{
+   chartmp[128];
+   u32 reg;
+   int retval;
+
+   /* mark timer as not initialized */
+   udc_timer.data = 0;
+   udc_pollstall_timer.data = 0;
+
+   /* device struct setup */
+   dev->gadget.ops = _ops;
+
+   dev_set_name(>gadget.dev, "gadget");
+   dev->gadget.name = name;
+   dev->gadget.max_speed = USB_SPEED_HIGH;
+
+   /* init registers, interrupts, ... */
+   startup_registers(dev);
+
+   dev_info(>pdev->dev, "%s\n", mod_desc);
+
+   snprintf(tmp, sizeof tmp, "%d", dev->irq);
+   dev_info(>pdev->dev,
+   "irq %s, pci mem %08lx, chip rev %02x(Geode5536 %s)\n",
+   tmp, dev->phys_addr, dev->chiprev,
+   (dev->chiprev == UDC_HSA0_REV) ? "A0" : "B1");
+   strcpy(tmp, UDC_DRIVER_VERSION_STRING);
+   if (dev->chiprev == UDC_HSA0_REV) {
+   dev_err(>pdev->dev, "chip revision is A0; too old\n");
+   retval = -ENODEV;
+   goto finished;
+   }
+   dev_info(>pdev->dev,
+   "driver version: %s(for Geode5536 B1)\n", tmp);
+   udc = dev;
+
+   retval = usb_add_gadget_udc_release(>pdev->dev, >gadget,
+   gadget_release);
+   if (retval)
+   goto finished;
+
+   /* timer init */
+   init_timer(_timer);
+   udc_timer.function = udc_timer_function;
+   udc_timer.data = 1;
+   /* timer pollstall init */
+   init_timer(_pollstall_timer);
+   udc_pollstall_timer.function = udc_pollstall_timer_function;
+   udc_pollstall_timer.data = 1;
+
+   /* set SD */
+   reg = readl(>regs->ctl);
+   reg |= AMD_BIT(UDC_DEVCTL_SD);
+   writel(reg, >regs->ctl);
+
+   /* print dev register info */
+   print_regs(dev);
+
+   return 0;
+
+finished:
+   return retval;
+}
+
 /* Called by pci bus driver to init pci context */
 static int udc_pci_probe(
struct pci_dev *pdev,
@@ -3323,72 +3388,6 @@ err_enabledev:
return retval;
 }
 
-/* general probe */
-static int udc_probe(struct udc *dev)
-{
-   chartmp[128];
-   u32 reg;
-   int retval;
-
-   /* mark timer as not initialized */
-   udc_timer.data = 0;
-   udc_pollstall_timer.data = 0;
-
-   /* device struct setup */
-   dev->gadget.ops = _ops;
-
-   dev_set_name(>gadget.dev, "gadget");
-   dev->gadget.name = name;
-   dev->gadget.max_speed = USB_SPEED_HIGH;
-
-   /* init registers, interrupts, ... */
-   startup_registers(dev);
-
-   dev_info(>pdev->dev, "%s\n", mod_desc);
-
-   snprintf(tmp, sizeof tmp, "%d", dev->irq);
-   dev_info(>pdev->dev,
-   "irq %s, pci mem %08lx, chip rev %02x(Geode5536 %s)\n",
-   tmp, dev->phys_addr, dev->chiprev,
-   (dev->chiprev == UDC_HSA0_REV) ? "A0" : "B1");
-   strcpy(tmp, UDC_DRIVER_VERSION_STRING);
-   if (dev->chiprev == UDC_HSA0_REV) {
-   dev_err(>pdev->dev, "chip revision is A0; too old\n");
-   retval = -ENODEV;
-   goto finished;
-   }
-   dev_info(>pdev->dev,
-   "driver version: %s(for Geode5536 B1)\n", tmp);
-   udc = dev;
-
-   retval = usb_add_gadget_udc_release(>pdev->dev, >gadget,
-   gadget_release);
-   if (retval)
-   goto finished;
-
-   /* timer init */
-   init_timer(_timer);
-   udc_timer.function = udc_timer_function;
-   udc_timer.data = 1;
-   /* timer pollstall init */
-   init_timer(_pollstall_timer);
-   udc_pollstall_timer.function = udc_pollstall_timer_function;
-   udc_pollstall_timer.data = 1;
-
-   /* set SD */
-   reg = readl(>r

[PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks

[Sudip Mukherjee]

This amd5536udc was a complete mess. The major problems that i could
find are:

1) if udc_pci_probe() fails in any stage then it just calls the
udc_pci_remove() to handle error. And udc_pci_remove() works with
struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
incase of error we are handling the error by dereferencing a NULL
pointer.

2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
will be set only if probe is success. So that means if probe fails then
probe will call udc_pci_remove() for error handling and udc_pci_remove()
will inturn halts the kernel by calling BUG().

And apart from these numerous memory leaks and not releasing of
resources. Here comes a rewrite of few of the functions in an
attempt to fix these.

regards
sudip

Sudip Mukherjee (16):
  usb: gadget: amd5536udc: introduce free_dma_pools
  usb: gadget: amd5536udc: rewrite init_dma_pools
  usb: gadget: amd5536udc: rewrite udc_pci_probe
  usb: gadget: amd5536udc: use WARN_ON
  usb: gadget: amd5536udc: use free_dma_pools
  usb: gadget: amd5536udc: remove unnecessary conditions
  usb: gadget: amd5536udc: unmap virt_addr
  usb: gadget: amd5536udc: remove forward declaration of udc_probe
  usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup
  usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain
  usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain
  usb: gadget: amd5536udc: remove forward declaration of udc_pci_*
  usb: gadget: amd5536udc: remove forward declaration of udc_basic_init
  usb: gadget: amd5536udc: NULL comparison
  usb: gadget: amd5536udc: remove multiple blank lines
  usb: gadget: amd5536udc: match alignment

 drivers/usb/gadget/udc/amd5536udc.c | 797 ++--
 1 file changed, 390 insertions(+), 407 deletions(-)

-- 
1.9.1

--
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 06/16] usb: gadget: amd5536udc: remove unnecessary conditions

[Sudip Mukherjee]

The condition checking for irq_registered, regs, mem_region and active
are not required as this is the remove function. And we are in the
remove means that probe was successful and they can never be NULL at
this point of code.
It was required in the original code as the remove function was part of
the error handler of probe function.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 3ae0bb8..e2f6128 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3135,15 +3135,11 @@ static void udc_pci_remove(struct pci_dev *pdev)
 
/* reset controller */
writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
-   if (dev->irq_registered)
-   free_irq(pdev->irq, dev);
-   if (dev->regs)
-   iounmap(dev->regs);
-   if (dev->mem_region)
-   release_mem_region(pci_resource_start(pdev, 0),
+   free_irq(pdev->irq, dev);
+   iounmap(dev->regs);
+   release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
-   if (dev->active)
-   pci_disable_device(pdev);
+   pci_disable_device(pdev);
 
udc_remove(dev);
 }
-- 
1.9.1

--
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 07/16] usb: gadget: amd5536udc: unmap virt_addr

[Sudip Mukherjee]

We have actually ioremapped dev->virt_addr and dev->regs is
dev->virt_addr + UDC_DEVCFG_ADDR so while unmapping we should unmap
dev->virt_addr.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index e2f6128..b7753b2 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3136,7 +3136,7 @@ static void udc_pci_remove(struct pci_dev *pdev)
/* reset controller */
writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
free_irq(pdev->irq, dev);
-   iounmap(dev->regs);
+   iounmap(dev->virt_addr);
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
pci_disable_device(pdev);
-- 
1.9.1

--
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 02/16] usb: gadget: amd5536udc: rewrite init_dma_pools

[Sudip Mukherjee]

A rewrite of init_dma_pools() with proper error handling.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 384b824..8b700de 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3180,8 +3180,7 @@ static int init_dma_pools(struct udc *dev)
sizeof(struct udc_data_dma), 0, 0);
if (!dev->data_requests) {
DBG(dev, "can't get request data pool\n");
-   retval = -ENOMEM;
-   goto finished;
+   return -ENOMEM;
}
 
/* EP0 in dma regs = dev control regs */
@@ -3193,14 +3192,14 @@ static int init_dma_pools(struct udc *dev)
if (!dev->stp_requests) {
DBG(dev, "can't get stp request pool\n");
retval = -ENOMEM;
-   goto finished;
+   goto err_create_dma_pool;
}
/* setup */
td_stp = dma_pool_alloc(dev->stp_requests, GFP_KERNEL,
>ep[UDC_EP0OUT_IX].td_stp_dma);
if (td_stp == NULL) {
retval = -ENOMEM;
-   goto finished;
+   goto err_alloc_dma;
}
dev->ep[UDC_EP0OUT_IX].td_stp = td_stp;
 
@@ -3209,12 +3208,20 @@ static int init_dma_pools(struct udc *dev)
>ep[UDC_EP0OUT_IX].td_phys);
if (td_data == NULL) {
retval = -ENOMEM;
-   goto finished;
+   goto err_alloc_phys;
}
dev->ep[UDC_EP0OUT_IX].td = td_data;
return 0;
 
-finished:
+err_alloc_phys:
+   dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td_stp,
+ dev->ep[UDC_EP0OUT_IX].td_stp_dma);
+err_alloc_dma:
+   dma_pool_destroy(dev->stp_requests);
+   dev->stp_requests = NULL;
+err_create_dma_pool:
+   dma_pool_destroy(dev->data_requests);
+   dev->data_requests = NULL;
return retval;
 }
 
-- 
1.9.1

--
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 04/16] usb: gadget: amd5536udc: use WARN_ON

[Sudip Mukherjee]

Use WARN_ON() instead of halting the kernel with BUG_ON().

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 8646f6c..4edcfd4 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3127,7 +3127,8 @@ static void udc_pci_remove(struct pci_dev *pdev)
 
usb_del_gadget_udc(>gadget);
/* gadget driver must not be registered */
-   BUG_ON(dev->driver != NULL);
+   if (WARN_ON(dev->driver != NULL))
+   return;
 
/* dma pool cleanup */
if (dev->data_requests)
-- 
1.9.1

--
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 12/16] usb: gadget: amd5536udc: remove forward declaration of udc_pci_*

[Sudip Mukherjee]

Remove the forward declarations of udc_pci_probe and udc_pci_remove.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 778a424..789441f 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -70,8 +70,6 @@ static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
 static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
-static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
-static void udc_pci_remove(struct pci_dev *pdev);
 
 /* description */
 static const char mod_desc[] = UDC_MOD_DESCRIPTION;
-- 
1.9.1

--
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 01/16] usb: gadget: amd5536udc: introduce free_dma_pools

[Sudip Mukherjee]

dma pools are being created by init_dma_pools() but there was no
function for freeing them. Introduce the function now so that we can use
it later.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index fdacddb..384b824 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3107,6 +3107,17 @@ static void udc_remove(struct udc *dev)
udc = NULL;
 }
 
+/* free all the dma pools */
+static void free_dma_pools(struct udc *dev)
+{
+   dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td,
+ dev->ep[UDC_EP0OUT_IX].td_phys);
+   dma_pool_free(dev->stp_requests, dev->ep[UDC_EP0OUT_IX].td_stp,
+ dev->ep[UDC_EP0OUT_IX].td_stp_dma);
+   dma_pool_destroy(dev->stp_requests);
+   dma_pool_destroy(dev->data_requests);
+}
+
 /* Reset all pci context */
 static void udc_pci_remove(struct pci_dev *pdev)
 {
-- 
1.9.1

--
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 13/16] usb: gadget: amd5536udc: remove forward declaration of udc_basic_init

[Sudip Mukherjee]

Rearrange the udc_basic_init function to remove the forward declaration.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 55 ++---
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 789441f..f6d6097 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -65,7 +65,6 @@
 
 static void udc_tasklet_disconnect(unsigned long);
 static void empty_req_queue(struct udc_ep *);
-static void udc_basic_init(struct udc *dev);
 static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
@@ -1511,33 +1510,6 @@ static void make_ep_lists(struct udc *dev)
dev->ep[UDC_EPOUT_IX].fifo_depth = UDC_RXFIFO_SIZE;
 }
 
-/* init registers at driver load time */
-static int startup_registers(struct udc *dev)
-{
-   u32 tmp;
-
-   /* init controller by soft reset */
-   udc_soft_reset(dev);
-
-   /* mask not needed interrupts */
-   udc_mask_unused_interrupts(dev);
-
-   /* put into initial config */
-   udc_basic_init(dev);
-   /* link up all endpoints */
-   udc_setup_endpoints(dev);
-
-   /* program speed */
-   tmp = readl(>regs->cfg);
-   if (use_fullspeed)
-   tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_FS, UDC_DEVCFG_SPD);
-   else
-   tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_HS, UDC_DEVCFG_SPD);
-   writel(tmp, >regs->cfg);
-
-   return 0;
-}
-
 /* Inits UDC context */
 static void udc_basic_init(struct udc *dev)
 {
@@ -1576,6 +1548,33 @@ static void udc_basic_init(struct udc *dev)
dev->data_ep_queued = 0;
 }
 
+/* init registers at driver load time */
+static int startup_registers(struct udc *dev)
+{
+   u32 tmp;
+
+   /* init controller by soft reset */
+   udc_soft_reset(dev);
+
+   /* mask not needed interrupts */
+   udc_mask_unused_interrupts(dev);
+
+   /* put into initial config */
+   udc_basic_init(dev);
+   /* link up all endpoints */
+   udc_setup_endpoints(dev);
+
+   /* program speed */
+   tmp = readl(>regs->cfg);
+   if (use_fullspeed)
+   tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_FS, UDC_DEVCFG_SPD);
+   else
+   tmp = AMD_ADDBITS(tmp, UDC_DEVCFG_SPD_HS, UDC_DEVCFG_SPD);
+   writel(tmp, >regs->cfg);
+
+   return 0;
+}
+
 /* Sets initial endpoint parameters */
 static void udc_setup_endpoints(struct udc *dev)
 {
-- 
1.9.1

--
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 11/16] usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain

[Sudip Mukherjee]

Rearrange udc_free_dma_chain to remove the forward declaration.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 53 ++---
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 34edb9b..778a424 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -70,7 +70,6 @@ static void udc_setup_endpoints(struct udc *dev);
 static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
 static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
 static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
 static void udc_pci_remove(struct pci_dev *pdev);
 
@@ -611,6 +610,32 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
return >req;
 }
 
+/* frees pci pool descriptors of a DMA chain */
+static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+{
+
+   int ret_val = 0;
+   struct udc_data_dma *td;
+   struct udc_data_dma *td_last = NULL;
+   unsigned int i;
+
+   DBG(dev, "free chain req = %p\n", req);
+
+   /* do not free first desc., will be done by free for request */
+   td_last = req->td_data;
+   td = phys_to_virt(td_last->next);
+
+   for (i = 1; i < req->chain_len; i++) {
+
+   pci_pool_free(dev->data_requests, td,
+   (dma_addr_t) td_last->next);
+   td_last = td;
+   td = phys_to_virt(td_last->next);
+   }
+
+   return ret_val;
+}
+
 /* Frees request packet, called by gadget driver */
 static void
 udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq)
@@ -1028,32 +1053,6 @@ __acquires(ep->dev->lock)
ep->halted = halted;
 }
 
-/* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
-{
-
-   int ret_val = 0;
-   struct udc_data_dma *td;
-   struct udc_data_dma *td_last = NULL;
-   unsigned int i;
-
-   DBG(dev, "free chain req = %p\n", req);
-
-   /* do not free first desc., will be done by free for request */
-   td_last = req->td_data;
-   td = phys_to_virt(td_last->next);
-
-   for (i = 1; i < req->chain_len; i++) {
-
-   pci_pool_free(dev->data_requests, td,
-   (dma_addr_t) td_last->next);
-   td_last = td;
-   td = phys_to_virt(td_last->next);
-   }
-
-   return ret_val;
-}
-
 /* Iterates to the end of a DMA chain and returns last descriptor */
 static struct udc_data_dma *udc_get_last_dma_desc(struct udc_request *req)
 {
-- 
1.9.1

--
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 09/16] usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup

[Sudip Mukherjee]

Rearrange the udc_remote_wakeup function to remove the forward declaration.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 40 ++---
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 01a6183..8b0ed74 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -1452,6 +1452,26 @@ static int udc_get_frame(struct usb_gadget *gadget)
return -EOPNOTSUPP;
 }
 
+/* Initiates a remote wakeup */
+static int udc_remote_wakeup(struct udc *dev)
+{
+   unsigned long flags;
+   u32 tmp;
+
+   DBG(dev, "UDC initiates remote wakeup\n");
+
+   spin_lock_irqsave(>lock, flags);
+
+   tmp = readl(>regs->ctl);
+   tmp |= AMD_BIT(UDC_DEVCTL_RES);
+   writel(tmp, >regs->ctl);
+   tmp &= AMD_CLEAR_BIT(UDC_DEVCTL_RES);
+   writel(tmp, >regs->ctl);
+
+   spin_unlock_irqrestore(>lock, flags);
+   return 0;
+}
+
 /* Remote wakeup gadget interface */
 static int udc_wakeup(struct usb_gadget *gadget)
 {
@@ -3388,26 +3408,6 @@ err_enabledev:
return retval;
 }
 
-/* Initiates a remote wakeup */
-static int udc_remote_wakeup(struct udc *dev)
-{
-   unsigned long flags;
-   u32 tmp;
-
-   DBG(dev, "UDC initiates remote wakeup\n");
-
-   spin_lock_irqsave(>lock, flags);
-
-   tmp = readl(>regs->ctl);
-   tmp |= AMD_BIT(UDC_DEVCTL_RES);
-   writel(tmp, >regs->ctl);
-   tmp &= AMD_CLEAR_BIT(UDC_DEVCTL_RES);
-   writel(tmp, >regs->ctl);
-
-   spin_unlock_irqrestore(>lock, flags);
-   return 0;
-}
-
 /* PCI device parameters */
 static const struct pci_device_id pci_id[] = {
{
-- 
1.9.1

--
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 10/16] usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain

[Sudip Mukherjee]

Rearrange udc_create_dma_chain to remove the forward declaration.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 241 ++--
 1 file changed, 119 insertions(+), 122 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 8b0ed74..34edb9b 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -71,9 +71,6 @@ static void udc_soft_reset(struct udc *dev);
 static struct udc_request *udc_alloc_bna_dummy(struct udc_ep *ep);
 static void udc_free_request(struct usb_ep *usbep, struct usb_request *usbreq);
 static int udc_free_dma_chain(struct udc *dev, struct udc_request *req);
-static int udc_create_dma_chain(struct udc_ep *ep, struct udc_request *req,
-   unsigned long buf_len, gfp_t gfp_flags);
-static int udc_remote_wakeup(struct udc *dev);
 static int udc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
 static void udc_pci_remove(struct pci_dev *pdev);
 
@@ -788,6 +785,125 @@ udc_rxfifo_read(struct udc_ep *ep, struct udc_request 
*req)
return finished;
 }
 
+/* Creates or re-inits a DMA chain */
+static int udc_create_dma_chain(
+   struct udc_ep *ep,
+   struct udc_request *req,
+   unsigned long buf_len, gfp_t gfp_flags
+)
+{
+   unsigned long bytes = req->req.length;
+   unsigned int i;
+   dma_addr_t dma_addr;
+   struct udc_data_dma *td = NULL;
+   struct udc_data_dma *last = NULL;
+   unsigned long txbytes;
+   unsigned create_new_chain = 0;
+   unsigned len;
+
+   VDBG(ep->dev, "udc_create_dma_chain: bytes=%ld buf_len=%ld\n",
+   bytes, buf_len);
+   dma_addr = DMA_DONT_USE;
+
+   /* unset L bit in first desc for OUT */
+   if (!ep->in)
+   req->td_data->status &= AMD_CLEAR_BIT(UDC_DMA_IN_STS_L);
+
+   /* alloc only new desc's if not already available */
+   len = req->req.length / ep->ep.maxpacket;
+   if (req->req.length % ep->ep.maxpacket)
+   len++;
+
+   if (len > req->chain_len) {
+   /* shorter chain already allocated before */
+   if (req->chain_len > 1)
+   udc_free_dma_chain(ep->dev, req);
+   req->chain_len = len;
+   create_new_chain = 1;
+   }
+
+   td = req->td_data;
+   /* gen. required number of descriptors and buffers */
+   for (i = buf_len; i < bytes; i += buf_len) {
+   /* create or determine next desc. */
+   if (create_new_chain) {
+
+   td = pci_pool_alloc(ep->dev->data_requests,
+   gfp_flags, _addr);
+   if (!td)
+   return -ENOMEM;
+
+   td->status = 0;
+   } else if (i == buf_len) {
+   /* first td */
+   td = (struct udc_data_dma *) phys_to_virt(
+   req->td_data->next);
+   td->status = 0;
+   } else {
+   td = (struct udc_data_dma *) phys_to_virt(last->next);
+   td->status = 0;
+   }
+
+
+   if (td)
+   td->bufptr = req->req.dma + i; /* assign buffer */
+   else
+   break;
+
+   /* short packet ? */
+   if ((bytes - i) >= buf_len) {
+   txbytes = buf_len;
+   } else {
+   /* short packet */
+   txbytes = bytes - i;
+   }
+
+   /* link td and assign tx bytes */
+   if (i == buf_len) {
+   if (create_new_chain)
+   req->td_data->next = dma_addr;
+   /*
+   else
+   req->td_data->next = virt_to_phys(td);
+   */
+   /* write tx bytes */
+   if (ep->in) {
+   /* first desc */
+   req->td_data->status =
+   AMD_ADDBITS(req->td_data->status,
+   ep->ep.maxpacket,
+   UDC_DMA_IN_STS_TXBYTES);
+   /* second desc */
+   td->status = AMD_ADDBITS(td->status,
+   txbytes,
+   UDC_DMA_IN_STS_TXBYTES);
+   }
+   } else {
+   

[PATCH 15/16] usb: gadget: amd5536udc: remove multiple blank lines

[Sudip Mukherjee]

checkpatch complains about multiple blank lines.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 3657a66..98b841d 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -62,7 +62,6 @@
 /* udc specific */
 #include "amd5536udc.h"
 
-
 static void udc_tasklet_disconnect(unsigned long);
 static void empty_req_queue(struct udc_ep *);
 static void udc_setup_endpoints(struct udc *dev);
@@ -127,7 +126,6 @@ static DECLARE_COMPLETION(on_pollstall_exit);
 static DECLARE_TASKLET(disconnect_tasklet, udc_tasklet_disconnect,
(unsigned long) );
 
-
 /* endpoint names used for print */
 static const char ep0_string[] = "ep0in";
 static const struct {
@@ -364,7 +362,6 @@ static void UDC_QUEUE_CNAK(struct udc_ep *ep, unsigned num)
cnak_pending = cnak_pending & (~(1 << (num)));
 }
 
-
 /* Enables endpoint, is called by gadget driver */
 static int
 udc_ep_enable(struct usb_ep *usbep, const struct usb_endpoint_descriptor *desc)
@@ -866,7 +863,6 @@ static int udc_create_dma_chain(
td->status = 0;
}
 
-
if (td)
td->bufptr = req->req.dma + i; /* assign buffer */
else
@@ -1000,7 +996,6 @@ static int prep_dma(struct udc_ep *ep, struct udc_request 
*req, gfp_t gfp)
UDC_DMA_STP_STS_BS_HOST_READY,
UDC_DMA_STP_STS_BS);
 
-
/* clear NAK by writing CNAK */
if (ep->naking) {
tmp = readl(>regs->ctl);
@@ -1728,7 +1723,6 @@ static void udc_tasklet_disconnect(unsigned long par)
ep_init(dev->regs,
>ep[UDC_EP0IN_IX]);
 
-
if (!soft_reset_occured) {
/* init controller by soft reset */
udc_soft_reset(dev);
@@ -2120,7 +2114,6 @@ static void udc_ep0_set_rde(struct udc *dev)
}
 }
 
-
 /* Interrupt handler for data OUT traffic */
 static irqreturn_t udc_data_out_isr(struct udc *dev, int ep_ix)
 {
@@ -2634,7 +2627,6 @@ __acquires(dev->lock)
} else
dev->waiting_zlp_ack_ep0in = 1;
 
-
/* clear NAK by writing CNAK in EP0_OUT */
if (!set) {
tmp = readl(>ep[UDC_EP0OUT_IX].regs->ctl);
@@ -2809,7 +2801,6 @@ static irqreturn_t udc_control_in_isr(struct udc *dev)
return ret_val;
 }
 
-
 /* Interrupt handler for global device events */
 static irqreturn_t udc_dev_isr(struct udc *dev, u32 dev_irq)
 __releases(dev->lock)
@@ -2846,7 +2837,6 @@ __acquires(dev->lock)
/* ep ix in UDC CSR register space */
udc_csr_epix = ep->num;
 
-
/* OUT ep */
} else {
/* ep ix in UDC CSR register space */
@@ -2899,7 +2889,6 @@ __acquires(dev->lock)
/* ep ix in UDC CSR register space */
udc_csr_epix = ep->num;
 
-
/* OUT ep */
} else {
/* ep ix in UDC CSR register space */
@@ -3080,7 +3069,6 @@ static irqreturn_t udc_irq(int irq, void *pdev)
 
}
 
-
/* check for dev irq */
reg = readl(>regs->irqsts);
if (reg) {
@@ -3089,7 +3077,6 @@ static irqreturn_t udc_irq(int irq, void *pdev)
ret_val |= udc_dev_isr(dev, reg);
}
 
-
spin_unlock(>lock);
return ret_val;
 }
-- 
1.9.1

--
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] usb: gadget: amd5536udc: fix NULL pointer dereference

[Sudip Mukherjee]

On Thu, Sep 10, 2015 at 11:03:34AM -0700, David Cohen wrote:
> Hi Sudip,
> 
> On Fri, Sep 04, 2015 at 05:12:23PM +0530, Sudip Mukherjee wrote:
> > We were checking if dev->regs is NULL but it was done after
> > dereferencing it. Lets reset the controller and iounmap dev->regs only
> > if it is not NULL.
> > free_irq() does not need dev->regs, so unmaping it before freeing the
> > irq should not matter.
> > 
> > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
> > ---
> >  drivers/usb/gadget/udc/amd5536udc.c | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
> > b/drivers/usb/gadget/udc/amd5536udc.c
> > index fdacddb..26066d3 100644
> > --- a/drivers/usb/gadget/udc/amd5536udc.c
> > +++ b/drivers/usb/gadget/udc/amd5536udc.c
> > @@ -3135,11 +3135,12 @@ static void udc_pci_remove(struct pci_dev *pdev)

> 
> I'm not familiar with the driver, but you're iounmap'ing before freeing
> irq. Looks fishy to me.
Well, I thought you will be able to give me some idea about how fix it. :)
Then I guess we should be on the safe side and what about the following:


diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index fdacddb..82f36f6 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3134,8 +3134,9 @@ static void udc_pci_remove(struct pci_dev *pdev)
pci_pool_destroy(dev->stp_requests);
}
 
-   /* reset controller */
-   writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
+   if (dev->regs)
+   /* reset controller */
+   writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
if (dev->irq_registered)
free_irq(pdev->irq, dev);
if (dev->regs)

And just for my information: for a device what might happen if I iounmap
before I free the irq? One thing I can think of is that after iounmap
just at that moment one interrupt comes and the driver tries to access
the io memory while servicing the irq.

regards
sudip
--
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] usb: gadget: amd5536udc: fix NULL pointer dereference

[Sudip Mukherjee]

On Fri, Sep 11, 2015 at 08:28:34AM -0500, Felipe Balbi wrote:
> Hi,
> 
> On Fri, Sep 11, 2015 at 03:32:56PM +0530, Sudip Mukherjee wrote:
> > On Thu, Sep 10, 2015 at 11:03:34AM -0700, David Cohen wrote:

> no, the check is pointless. Most of these are. Just look at your probe()
> and you'll see that if dev->virt_addr is NULL (meaning ioremap_nocache()
> failed) you exit from probe() with error. The driver doesn't probe at
> all. So you can be sure that by remove, dev->regs is valid.
> 
> BTW, if probe fails, you have a TON of leaked resources!! You don't kfree()
> dev, you don't pci_disable_device(), you don't release_mem_region(), you
> don't iounmap() virt_addr, you don't free_irq().
> 
> Also the iounmap() call in remove is wrong. You ioremapped
> dev->virt_addr but iounmap() dev->regs. Are you SURE that's ok ?
> 
> Man, what a mess! You gotta fix that up.
:(
Yes, total mess. I am on it.
BTW, while I am fixing it can i also include a patch which will
rearrange the functions and remove the forward declarations?

regards
sudip
--
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] usb: gadget: amd5536udc: fix NULL pointer dereference

[Sudip Mukherjee]

We were checking if dev->regs is NULL but it was done after
dereferencing it. Lets reset the controller and iounmap dev->regs only
if it is not NULL.
free_irq() does not need dev->regs, so unmaping it before freeing the
irq should not matter.

Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
---
 drivers/usb/gadget/udc/amd5536udc.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index fdacddb..26066d3 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3135,11 +3135,12 @@ static void udc_pci_remove(struct pci_dev *pdev)
}
 
/* reset controller */
-   writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
+   if (dev->regs) {
+   writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
+   iounmap(dev->regs);
+   }
if (dev->irq_registered)
free_irq(pdev->irq, dev);
-   if (dev->regs)
-   iounmap(dev->regs);
if (dev->mem_region)
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
-- 
1.9.1

--
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