Re: [PATCH 2/3 v3] usb: chipidea: Fix Internal error: : 808 [#1]ARM related to STS flag

From: Chris Ruehl
Date: Mon Dec 02 2013 - 00:18:07 EST




On Monday, December 02, 2013 01:10 PM, Peter Chen wrote:


If you have a look into the function hw_write() you will see that there
is no
effect if hw_write(...,sts) is called with sts=0/1, because the mask will
cut
off all bits beside BIT(29).

Yes, it is my careless. I thought sts is PORTCS_STS.

I used BIT(29) rather then PORTCS_STS to make it more clear what going on.

It is not a good coding style, you do need use MACRO to instead of raw number directly.

A write to PORTCS will always be "0" for the STS Register no matter if
sts is 1
or 0 within Patch v2. Patch v3 will take care of the registers bit
position and
set 1 or 0 to PORTCS_STS.


My suggestion like below:

if (ci->hw_bank.lpm) {
hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
- hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
+ if (sts)
+ hw_write(ci, OP_DEVLC, DEVLC_STS, DEVLC_STS);
} else {
hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
- hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
+ if (sts)
+ hw_write(ci, OP_PORTSC, PORTSC_STS, PORTSC_STS);
}


I think we just need to fix the original bug, and do not add any new fixes
since we don't know which one is correct for every platform. My proposal is
just set PORTSC_STS (DEVLC_STS is lpm) if it is serial PHY. For any other PHYS, just keep
the reset value.

Peter

I used the imx27 reference manual Capital 30.8.1.5.12 PORTSCx.


I can follow your arguments, ACK.
I prepare a patch set with this solution if not other people have better
ideas.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/