[U-Boot] [PATCH] drivers: usb: fsl: Workaround for Erratum A004477

[York Sun <yorksun@freescale.com>]

On 04/09/2015 08:29 PM, nikhil.badola at freescale.com wrote:
>> -----Original Message-----
>> From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
>> Sent: Friday, November 21, 2014 6:00 PM
>> To: Badola Nikhil-B46172
>> Cc: u-boot at lists.denx.de
>> Subject: Re: [U-Boot] [PATCH] drivers: usb: fsl: Workaround for Erratum
>> A004477
>>
>> Hello Nikhil,
>>
>> On Fri, 21 Nov 2014 17:25:21 +0530, Nikhil Badola
>> <nikhil.badola@freescale.com> wrote:
>>> Add a delay of 1 microsecond before issuing soft reset to the
>>> controller to let ongoing ULPI transaction complete.
>>> This prevents corruption of ULPI Function Control Register which
>>> eventually prevents phy clock from entering to low power mode
>>>
>>> Signed-off-by: Nikhil Badola <nikhil.badola@freescale.com>
>>> ---
>>> 	Depends on patch https://patchwork.ozlabs.org/patch/404879/
>>>
>>>  arch/powerpc/cpu/mpc85xx/cmd_errata.c     |  4 ++++
>>>  arch/powerpc/include/asm/config_mpc85xx.h |  6 ++++++
>>>  drivers/usb/host/ehci-fsl.c               | 10 ++++++++++
>>>  include/fsl_usb.h                         | 29 +++++++++++++++++++++++++++++
>>>  4 files changed, 49 insertions(+)
>>>
>>> diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c
>>> b/arch/powerpc/cpu/mpc85xx/cmd_errata.c
>>> index fe3eb06..bc59352 100644
>>> --- a/arch/powerpc/cpu/mpc85xx/cmd_errata.c
>>> +++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c
>>> @@ -298,6 +298,10 @@ static int do_errata(cmd_tbl_t *cmdtp, int flag, int
>> argc, char * const argv[])
>>>  	if (has_erratum_a007798())
>>>  		puts("Work-around for Erratum A007798 enabled\n");
>> #endif
>>> +#ifdef CONFIG_SYS_FSL_ERRATUM_A004477
>>> +	if (has_erratum_a004477())
>>> +		puts("Work-around for Erratum A004477 enabled\n"); #endif
>>>  #ifdef CONFIG_SYS_FSL_ERRATUM_I2C_A004447
>>>  	if ((SVR_SOC_VER(svr) == SVR_8548 && IS_SVR_REV(svr, 3, 1)) ||
>>>  	    (SVR_REV(svr) <= CONFIG_SYS_FSL_A004447_SVR_REV)) diff --git
>>> a/arch/powerpc/include/asm/config_mpc85xx.h
>>> b/arch/powerpc/include/asm/config_mpc85xx.h
>>> index 7860b40..fe9e9c1 100644
>>> --- a/arch/powerpc/include/asm/config_mpc85xx.h
>>> +++ b/arch/powerpc/include/asm/config_mpc85xx.h
>>> @@ -161,6 +161,7 @@
>>>  #define CONFIG_SYS_FSL_ERRATUM_A004508  #define
>>> CONFIG_SYS_FSL_ERRATUM_A007075  #define
>> CONFIG_SYS_FSL_ERRATUM_A006261
>>> +#define CONFIG_SYS_FSL_ERRATUM_A004477
>>>  #define CONFIG_SYS_FSL_A004447_SVR_REV	0x10
>>>  #define CONFIG_ESDHC_HC_BLK_ADDR
>>>
>>> @@ -292,6 +293,7 @@
>>>  #define CONFIG_FSL_SATA_ERRATUM_A001
>>>  #define CONFIG_SYS_FSL_ERRATUM_A004508  #define
>>> CONFIG_SYS_FSL_ERRATUM_A005125
>>> +#define CONFIG_SYS_FSL_ERRATUM_A004477
>>>
>>>  #elif defined(CONFIG_P1023)
>>>  #define CONFIG_MAX_CPUS			2
>>> @@ -372,6 +374,7 @@
>>>  #define CONFIG_SYS_FSL_SRIO_MSG_UNIT_NUM	2
>>>  #define CONFIG_SYS_FSL_ERRATUM_A004508  #define
>>> CONFIG_SYS_FSL_ERRATUM_A005125
>>> +#define CONFIG_SYS_FSL_ERRATUM_A004477
>>>  #define CONFIG_USB_MAX_CONTROLLER_COUNT	1
>>>
>>>  #elif defined(CONFIG_PPC_P2041) /* also supports P2040 */ @@ -589,6
>>> +592,7 @@  #define CONFIG_NAND_FSL_IFC  #define
>>> CONFIG_SYS_FSL_ERRATUM_ESDHC111  #define
>>> CONFIG_SYS_FSL_ERRATUM_A005125
>>> +#define CONFIG_SYS_FSL_ERRATUM_A004477
>>>  #define CONFIG_ESDHC_HC_BLK_ADDR
>>>
>>>  #elif defined(CONFIG_BSC9132)
>>> @@ -613,6 +617,7 @@
>>>  #define CONFIG_SYS_FSL_PCIE_COMPAT	"fsl,qoriq-pcie-v2.2"
>>>  #define CONFIG_SYS_FSL_ERRATUM_A005125  #define
>>> CONFIG_SYS_FSL_ERRATUM_A005434
>>> +#define CONFIG_SYS_FSL_ERRATUM_A004477
>>>  #define CONFIG_SYS_FSL_ERRATUM_I2C_A004447
>>>  #define CONFIG_SYS_FSL_A004447_SVR_REV	0x11
>>>  #define CONFIG_ESDHC_HC_BLK_ADDR
>>> @@ -712,6 +717,7 @@
>>>  #define CONFIG_SYS_FSL_ERRATUM_A006475  #define
>>> CONFIG_SYS_FSL_ERRATUM_A006384  #define
>> CONFIG_SYS_FSL_ERRATUM_A007212
>>> +#define CONFIG_SYS_FSL_ERRATUM_A004477
>>>  #define CONFIG_SYS_CCSRBAR_DEFAULT	0xfe000000
>>>  #define CONFIG_SYS_FSL_SFP_VER_3_0
>>>
>>> diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
>>> index 5d4288d..41ff1a7 100644
>>> --- a/drivers/usb/host/ehci-fsl.c
>>> +++ b/drivers/usb/host/ehci-fsl.c
>>> @@ -138,6 +138,16 @@ int ehci_hcd_init(int index, enum usb_init_type
>> init,
>>>  	if (has_erratum_a007798())
>>>  		set_txfifothresh(ehci, TXFIFOTHRESH);
>>>
>>> +	if (has_erratum_a004477()) {
>>> +		/*
>>> +		 * When reset is issued while any ULPI transaction is ongoing
>>> +		 * then it may result to corruption of ULPI Function Control
>>> +		 * Register which eventually causes phy clock to enter low
>>> +		 * power mode which stops the clock. Thus delay is required
>>> +		 * before reset to let ongoing ULPI transaction complete.
>>
>> Actually the erratum suggests checking transaction status before issuing a
>> soft reset. Here, could the code do this check and wait for the transation to
>> finish, rather than wait for an arbitrary duration?
> 
> There have been discussions with IP team for changing the suggested workaround.
> The erratum talks about the ULPI transactions i.e from controller to ULPI phy and not 
> the USB schedule transactions .Hence halting the controller before soft reset isn't the 
> correct workaround.
> In addition, as per the current flow of usb start/reset, the controller is always in halt set 
> before issuing soft reset. This is taken care by usb_stop().
> 
> Regarding the delay of 1 microsecond( minimum delay), it seems to be sufficient for ULPI 
> Transactions to complete as ULPI interface runs on 60Mhz clock.
> 

Guys,

Is there any open comment on this patch? Do we all agree on this approach? Are
we still waiting for IP team for the workaround?

York