Re: omap2430 regression due to "usb: musb: fix incorrect usage of resource pointer"

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2761 bytes --]

On Fri, 26 Jul 2013 16:51:08 +0300 Felipe Balbi <balbi@ti.com> wrote:

> On Fri, Jul 26, 2013 at 10:24:58PM +1000, NeilBrown wrote:
> > On Fri, 26 Jul 2013 13:20:08 +0300 Felipe Balbi <balbi@ti.com> wrote:
> > 
> > > On Fri, Jul 26, 2013 at 07:14:45PM +1000, NeilBrown wrote:
> > > > 
> > > > Hi,
> > > >  the commit:
> > > > 
> > > > commit 09fc7d22b024692b2fe8a943b246de1af307132b
> > > > Author: Felipe Balbi <balbi@ti.com>
> > > > Date:   Wed Apr 24 17:21:42 2013 +0300
> > > > 
> > > >     usb: musb: fix incorrect usage of resource pointer
> > > >     
> > > >     We can't simply pass the resource pointer from our
> > > >     device down to our children, otherwise module
> > > >     reinsertion will not work as the resource will
> > > >     continue to be marked as busy.
> > > >     
> > > >     Fix it by building a proper struct resource for
> > > >     our child musb device.
> > > >  
> > > > 
> > > > (even with the subsequent compile fix applied) breaks my USB-OTG port on my
> > > > GTA04.
> > > > 
> > > > I must admit that I don't see the point of the patch at all.
> > > > Instead of passing one array to platform_device_add_resources()
> > > > which will copy it into malloced space, it copies bits of the array onto
> > > > the stack on passes that for platform_device_add_resources to copy.
> > > > What is the value of the second copy??
> > > 
> > > if you don't do that, the resource will already be busy and ioremap will
> > > fail on second modprobe.
> > 
> > Ahhh... the parent/sibling/child linkage - got it.
> > 
> > > 
> > > > But that isn't the real problem.  The real problem is that in omap2430_probe,
> > > > on my board at least, pdev->num_resources == 3.  But only the first 2 entries
> > > > in the array are copied.
> > > > 
> > > > If I make the array one entry larger and copy the extra entry it works.  Of
> > > > course that might break for someone else who only has 2 resources to copy....
> > > > 
> > > > Can we go back to just leaving it to platform_device_add_resources to do the
> > > > copying it have I missed something?  (I can't see where the new old code
> > > > would mark something as 'busy' that the new code won't mark as busy...)
> > > 
> > > patches already in Greg's queue
> > > 
> > 
> > Thanks. 
> > Doesn't seem to be in the usb-next or usb-linus branches of 
> > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
> > Can you give me a pointer?
> 
> hmmm, looks like this is not in greg/usb-linus indeed. I'll send him
> another pull request on monday. If you wanna try, use my 'fixes' branch
> on kernel.org.

Thanks.  I found that one, pulled it in, and can confirm that it fixed my USB
problem.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]