Heiko,
On Fri, Dec 18, 2015 at 3:17 PM, Heiko Stübner
<heiko.stuebner@xxxxxxxxxxxxx> wrote:
> Hi Doug,
>
> Am Freitag, 18. Dezember 2015, 14:50:14 schrieb Doug Anderson:
>> On Fri, Dec 18, 2015 at 10:30 AM, Heiko Stübner
>> <heiko.stuebner@xxxxxxxxxxxxx> wrote:
>> > In specific conditions (involving usb hubs) dwc2 devices can create a
>> > lot of interrupts, even to the point of overwhelming devices running
>> > at low frequencies. Some devices need to do special clock handling
>> > at shutdown-time which may bring the system clock below the threshold
>> > of being able to handle the dwc2 interrupts. Disabling dwc2-irqs
>> > in a shutdown callbacks prevents reboots/poweroffs from getting stuck
>> > in such cases.
>> >
>> > The hsotg struct already contains an unused irq element, so we can
>> > just use it to store the irq number for the shutdown callback.
>> >
>> > Signed-off-by: Heiko Stuebner <heiko.stuebner@xxxxxxxxxxxxx>
>> > ---
>> > I'm also adapting the code on the clock-side to lessen the effects of
>> > the slow clock (see clk: rockchip: only enter pll slow-mode directly
>> > before reboots on rk3288), but this patch also fixes the issue of the
>> > overwhelming irq-number in itself and may be interesting for other/future
>> > platforms using the dwc2.
>> >
>> > drivers/usb/dwc2/platform.c | 35 +++++++++++++++++++++++++++--------
>> > 1 file changed, 27 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>> > index 39c1cbf..5510d07 100644
>> > --- a/drivers/usb/dwc2/platform.c
>> > +++ b/drivers/usb/dwc2/platform.c
>> > @@ -306,6 +306,25 @@ static int dwc2_driver_remove(struct platform_device
>> > *dev)>
>> > return 0;
>> >
>> > }
>> >
>> > +/**
>> > + * dwc2_driver_shutdown() - Called on device shutdown
>> > + *
>> > + * @dev: Platform device
>> > + *
>> > + * In specific conditions (involving usb hubs) dwc2 devices can create a
>> > + * lot of interrupts, even to the point of overwhelming devices running
>> > + * at low frequencies. Some devices need to do special clock handling
>> > + * at shutdown-time which may bring the system clock below the threshold
>> > + * of being able to handle the dwc2 interrupts. Disabling dwc2-irqs
>> > + * prevents reboots/poweroffs from getting stuck in such cases.
>> > + */
>> > +static void dwc2_driver_shutdown(struct platform_device *dev)
>> > +{
>> > + struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
>> > +
>> > + disable_irq(hsotg->irq);
>>
>> In one other place I see dwc2 getting the IRQ from the USB HCD
>> structure. That is:
>>
>> dwc2_hsotg_to_hcd(hsotg)->irq
>>
>> I wonder if that would be a good idea to do? Then a future patch
>> could just remove the unused (and redundant) irq from the hsotg
>> structure?
>
> The hcd-part as well as the gadget equivalent only gets enabled when the right
> dr-mode is set:
> if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
> retval = dwc2_hcd_init(hsotg, hsotg->irq);
> ...
>
> Additionally the hcd/gadget part can also be compiled out, making that init
> function a stub. Also I think dwc2_hsotg_to_hcd is only defined in the hcd-
> scope and in the platform code you cannot really be sure if that is actually
> available - or would need to check for hcd-existence + gadget-existence as
> fallback.
>
> So I'd think accessing a generic irq-value might be preferrable :-) .
Oh right. Duh. Now that I've made a fool of myself, you can decide
if my reviewed-by is worth anything, but feel free to it. :-P
Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
--
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
