[U-Boot] [PATCH 5/6] usb: add Marvell Kirkwood ehci host controller driver

Prafulla Wadaskar
2009-07-10T04:15:11Z

> -----Original Message-----
> From: Michael Trimarchi [mailto:trimarchi at gandalf.sssup.it]
> Sent: Thursday, July 09, 2009 9:54 PM
> To: Prafulla Wadaskar
> Cc: Remy Bohmer; u-boot at lists.denx.de; Prabhanjan Sarnaik;
> Ronen Shitrit; Ashish Karkare
> Subject: Re: [PATCH 5/6] usb: add Marvell Kirkwood ehci host
> controller driver
>
> Prafulla Wadaskar wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: l.pinguin at gmail.com [mailto:l.pinguin at gmail.com] On
> Behalf Of
> >> Remy Bohmer
> >> Sent: Thursday, July 09, 2009 1:00 PM
> >> To: Prafulla Wadaskar; Michael Trimarchi
> >> Cc: u-boot at lists.denx.de; Prabhanjan Sarnaik; Ronen
> Shitrit; Ashish
> >> Karkare
> >> Subject: Re: [PATCH 5/6] usb: add Marvell Kirkwood ehci host
> >> controller driver
> >>
> >> Hello Prafulla,
> >>
> >>
> >>> As stated in other email thread too.
> >>> Now all dependencies for Sheevaplug board support are in accepted
> >>> state except this patch
> >>> http://lists.denx.de/pipermail/u-boot/2009-June/055112.html
> >>>
> >> I know that but I cannot test it because I do not have the
> hardware,
> >> so I depend on others here. (it might be even you).
> >> I already mentioned that I would prefer the cleanup patch from
> >> Michael Trimarchi, if it is correct.
> >>
> >> At least there is 1 significant difference between your
> patch and the
> >> patch of Michael
> >>
> >> Your patch contains this:
> >> --------------------------------------------------------
> >> - typeReq = req->request << 8 | req->requesttype;
> >> + typeReq = req->request | req->requesttype << 8;
> >>
> >> switch (le16_to_cpu(typeReq)) {
> >> --------------------------------------------------------
> >>
> >> and Michael patch contains this:
> >> --------------------------------------------------------
> >> - typeReq = req->request << 8 | req->requesttype;
> >> + typeReq = req->request | req->requesttype << 8;
> >>
> >> - switch (le16_to_cpu(typeReq)) {
> >> + switch (typeReq) {
> >>
> > Dear Remy
> > These two lines makes no difference for me since le16_to_cpu macro
> > doesn't change anything for arm little-endian machines
> (i.e. Sheevaplug) Well.. for big endian machines someone need
> to check since it swaps bytes.
> >
> > I think we should not remove this macro since it provides
> arch specific abstraction.
> >
> If you don't remove the macro I think that you have trouble
> with big endian machines.
Dear Michael
At least common change in both the patch can be applied, this will at least resolve fix for little endian machines. We can defer second change until somebody tests it

Regards..
Prafulla . .

>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.g
it;a=blob;f=drivers/usb/core/hcd.c;h=95ccfa0b9fc52876f5166b3edea039409adf9a33;hb=HEAD
>
> line 442
>
> Michael
> > Regards..
> > Prafulla . .
> >
> >
> >> --------------------------------------------------------
> >>
> >> So, at least one of them is wrong.
> >> This difference is reason enough for me to not pull either
> patch in
> >> until this is clear.
> >>
> >> Kind Regards,
> >>
> >> Remy
> >>
> >>
>
>