Hi Alan,
On Mon, 2011-12-05 at 11:57 -0500, Alan Stern wrote:
> On Thu, 1 Dec 2011, Geoff Levand wrote:
> > drivers/usb/host/ehci-hcd.c | 28 ++++++++++++++++++++++++++++
> > 1 files changed, 28 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > index 46dccbf..4a8391c 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -48,6 +48,10 @@
> > #include <asm/system.h>
> > #include <asm/unaligned.h>
> >
> > +#if defined(CONFIG_PPC_PS3)
> > +#include <asm/firmware.h>
> > +#endif
>
> That's okay; you don't need the #if here -- there's nothing wrong with
> #include-ing the header unconditionally.
This is asm/firmware.h, and only powerpc has it. Without the conditionals
the build will fail for non-powerpc. I need this included for the
firmware_has_feature() test.
> > +
> > /*-------------------------------------------------------------------------*/
> >
> > /*
> > @@ -236,6 +240,30 @@ static int handshake_on_error_set_halt(struct ehci_hcd *ehci, void __iomem *ptr,
> > int error;
> >
> > error = handshake(ehci, ptr, mask, done, usec);
> > +
> > + /*
> > + * The EHCI controller of the Cell Super Companion Chip used in the
> > + * PS3 will stop the root hub after all root hub ports are suspended.
> > + * When in this condition handshake will return -ETIMEDOUT. The
> > + * STS_HLT bit will not be set, so inspection of the frame index is
> > + * used here to test for the condition. If the condition is found
> > + * return success to allow the USB suspend to complete.
> > + */
> > +
> > +#if defined(CONFIG_USB_SUSPEND) && defined(CONFIG_PPC_PS3)
> > + if (firmware_has_feature(FW_FEATURE_PS3_LV1) && error == -ETIMEDOUT) {
> > +#else
> > + if (0) {
> > +#endif
> > + unsigned int old_index = ehci_read_frame_index(ehci);
> > +
> > + error = handshake(ehci, ptr, mask, done, usec);
> > +
> > + if (error == -ETIMEDOUT
> > + && (ehci_read_frame_index(ehci) == old_index))
> > + return 0;
> > + }
> > +
> > if (error) {
> > ehci_halt(ehci);
> > ehci->rh_state = EHCI_RH_HALTED;
> >
>
> This is a little ugly, with the preprocessor stuff inside the function.
> The generally preferred approach puts functions inside preprocessor
> blocks instead of the other way around, like this:
>
> #if defined(...)
OK, a new patch will follow.
-Geoff
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
