[Cbe-oss-dev] [PATCH 1/5 v2] usb: Fix PS3 EHCI suspend

[Geert Uytterhoeven]

Hi Antonio,

On Fri, Dec 2, 2011 at 10:30, Antonio Ospite <ospite at studenti.unina.it> wrote:
> On Thu, 1 Dec 2011 16:19:18 -0800
> Geoff Levand <geoff at infradead.org> wrote:
>
> [...]
>> @@ -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
>
> Sometime I still have these doubts, forgive me:
> Geoff is this done this way to keep the #endif as near as possible to
> the #if? I mean wouldn't it be equivalent (even if one would be left out
> by the preprocessor and the other by the compiler) to put the whole
> conditional block inside the #if defined(CONFIG_PPC_PS3)?

The advantage of the way Geoff did it is that the compiler will detect if
someone breaks compilation for PS3 inside the block. With a big #ifdef
around the block, only PS3 users (and linux-next) will notice.

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds