[U-Boot] [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB erratum A-008751
Sriram Dash
sriram.dash at nxp.com
Wed Jun 8 06:12:48 CEST 2016
>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Monday, June 06, 2016 6:15 PM
>To: Sriram Dash <sriram.dash at nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun at nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
><rajesh.bhagat at nxp.com>
>Subject: Re: [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB erratum A-
>008751
>
>On 06/06/2016 06:24 AM, Sriram Dash wrote:
>>> -----Original Message-----
>>> From: Marek Vasut [mailto:marex at denx.de]
>>> Sent: Thursday, June 02, 2016 6:31 PM
>>> To: Sriram Dash <sriram.dash at nxp.com>; u-boot at lists.denx.de
>>> Cc: york sun <york.sun at nxp.com>; albert.u.boot at aribaud.net; Rajesh
>>> Bhagat <rajesh.bhagat at nxp.com>
>>> Subject: Re: [PATCH v2 5/5] usb: xhci: fsl: Add workaround for USB
>>> erratum A-
>>> 008751
>>>
>>> On 06/02/2016 08:54 AM, Sriram Dash wrote:
>>>> This patch is doing the following:
>>>> 1. Implementing the errata for LS2080.
>>>> 2. Adding fixup for fdt for LS2080.
>>>>
>>>> Signed-off-by: Sriram Dash <sriram.dash at nxp.com>
>>>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat at nxp.com>
>>>> ---
>>>> Changes in v2:
>>>> - Reworked for changes done in errata checking code.
>>>
>>> [...]
>>>
>>>> diff --git a/drivers/usb/common/fsl-dt-fixup.c
>>>> b/drivers/usb/common/fsl-dt-fixup.c
>>>> index 7c039cb..fd85439 100644
>>>> --- a/drivers/usb/common/fsl-dt-fixup.c
>>>> +++ b/drivers/usb/common/fsl-dt-fixup.c
>>>> @@ -125,6 +125,7 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
>>>> int usb_erratum_a007075_off = -1;
>>>> int usb_erratum_a007792_off = -1;
>>>> int usb_erratum_a005697_off = -1;
>>>> + int usb_erratum_a008751_off = -1;
>>>
>>> Will there be an ever-growing list of unused variables here ?
>>>
>>
>> Same as in Patch 2/5 code cleanup for device tree fixup for fsl usb controllers
>usb_erratum_aNNNNNN_off variable are keeping track of the device tree fixup for
>errata NNNNNN. The code checks errata applicability for each controller and tries
>to fix the device tree accordingly. During this time, the variable
>usb_erratum_aNNNNNN_off is updated to the offset of device tree, if the device
>tree is fixed. Now, for the second controller, when it tries to fix the device tree, it
>checks from the same offset obtained. As the API for fdt_setprop is such that the
>fixup will occur as soon as the API finds first match, if this variable
>usb_erratum_aNNNNNN_off is not maintained, the errata will be fixed always for
>the first usb controller it comes across the device tree. So, we need this variable.
>
>What will happen if you have two different controllers in the system and each of
>them has different set of erratas ? Will this code fail at handling them by applying
>wrong sets of erratas ?
>
Yes, you are right.
For the different controllers with different erratas, I am planning
to handle it by passing the controller type(ex: fsl-usb2-dr) from the
fdt_fixup_erratum function, for which , the errata would be applied,
and decided in fdt_fixup_usb_erratum function in V3.
>btw Can you please fix your mailer to break lines at 80 chars ?
>
>>>> int usb_mode_off = -1;
>>>> int usb_phy_off = -1;
>>>> char str[5];
>>>> @@ -190,6 +191,8 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
>>>> "a007792", has_erratum_a007792);
>>>> fdt_fixup_erratum(&usb_erratum_a005697_off, blob,
>>>> "a005697", has_erratum_a005697);
>>>> + fdt_fixup_erratum(&usb_erratum_a008751_off, blob,
>>>> + "a008751", has_erratum_a008751);
>>>>
>>>> }
>>>> }
>>>> diff --git a/drivers/usb/common/fsl-errata.c
>>>> b/drivers/usb/common/fsl-errata.c index 95918fc..ebe60a8 100644
>>>> --- a/drivers/usb/common/fsl-errata.c
>>>> +++ b/drivers/usb/common/fsl-errata.c
>>>> @@ -175,4 +175,19 @@ bool has_erratum_a004477(void)
>>>> return false;
>>>> }
>>>>
>>>> +bool has_erratum_a008751(void)
>>>> +{
>>>> + u32 svr = get_svr();
>>>> + u32 soc = SVR_SOC_VER(svr);
>>>> +
>>>> + switch (soc) {
>>>> +#ifdef CONFIG_ARM64
>>>> + case SVR_LS2080:
>>>> + case SVR_LS2085:
>>>> + return IS_SVR_REV(svr, 1, 0);
>>>> +#endif
>>>> + }
>>>> + return false;
>>>> +}
>>>> +
>>>> #endif
>>>> diff --git a/drivers/usb/host/xhci-fsl.c
>>>> b/drivers/usb/host/xhci-fsl.c index 05f09d7..d55ed87 100644
>>>> --- a/drivers/usb/host/xhci-fsl.c
>>>> +++ b/drivers/usb/host/xhci-fsl.c
>>>> @@ -15,6 +15,8 @@
>>>> #include <linux/usb/xhci-fsl.h>
>>>> #include <linux/usb/dwc3.h>
>>>> #include "xhci.h"
>>>> +#include <fsl_errata.h>
>>>> +#include <fsl_usb.h>
>>>>
>>>> /* Declare global data pointer */
>>>> DECLARE_GLOBAL_DATA_PTR;
>>>> @@ -27,6 +29,31 @@ __weak int __board_usb_init(int index, enum
>>>> usb_init_type
>>> init)
>>>> return 0;
>>>> }
>>>>
>>>> +static inline bool erratum_a008751(void)
>>>
>>> Drop the inline, let compiler decide. Also, make this an int instead
>>> of bool and return 0 on success, 1 on error.
>>>
>>
>> Ok. Will drop inline and make static int from static bool.
>> Return 0 on success instead of true.
>> Return 1 on error instead of false.
>>
>>>> +{
>>>> +#if defined(CONFIG_TARGET_LS2080AQDS) ||
>>> defined(CONFIG_TARGET_LS2080ARDB)
>>>> + u32 __iomem *scfg = (u32 __iomem *)SCFG_BASE;
>>>> + writel(SCFG_USB3PRM1CR_INIT, scfg + SCFG_USB3PRM1CR / 4);
>>>> + return true;
>>>> +#endif
>>>> + return false;
>>>> +}
>>>> +
>>>> +#define APPLY_ERRATUM(id) \
>>>> +do { \
>>>> + bool ret; \
>>>> + if (has_erratum_##id()) { \
>>>> + ret = erratum_##id(); \
>>>> + if (ret <= 0) \
>>>> + puts("Failed to apply erratum " #id "\n"); \
>>>> + } \
>>>> +} while (0)
>>>
>>> Turn this into a function.
>>>
>>
>> On a second thought, I guess I will drop the MACRO and directly call
>> the
>> has_erratum_a008751() etc from fsl_apply_xhci_errata, instead of going for a
>function.
>> What do you say?
>
>Fine
>
>>>> +static void fsl_apply_xhci_errata(void) {
>>>> + APPLY_ERRATUM(a008751);
>>>> +}
>>>> +
>>>> static int fsl_xhci_core_init(struct fsl_xhci *fsl_xhci) {
>>>> int ret = 0;
>>>> @@ -69,6 +96,8 @@ int xhci_hcd_init(int index, struct xhci_hccr
>>>> **hccr, struct
>>> xhci_hcor **hcor)
>>>> return ret;
>>>> }
>>>>
>>>> + fsl_apply_xhci_errata();
>>>> +
>>>> ret = fsl_xhci_core_init(ctx);
>>>> if (ret < 0) {
>>>> puts("Failed to initialize xhci\n"); diff --git
>>>> a/include/fsl_usb.h b/include/fsl_usb.h index d183349..fc72fb9
>>>> 100644
>>>> --- a/include/fsl_usb.h
>>>> +++ b/include/fsl_usb.h
>>>> @@ -94,5 +94,6 @@ bool has_erratum_a007798(void); bool
>>>> has_erratum_a007792(void); bool has_erratum_a005697(void); bool
>>>> has_erratum_a004477(void);
>>>> +bool has_erratum_a008751(void);
>>>> #endif
>>>> #endif /*_ASM_FSL_USB_H_ */
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Marek Vasut
>
>
>--
>Best regards,
>Marek Vasut
More information about the U-Boot
mailing list