>From: Marek Vasut [mailto:ma...@denx.de] >On 09/16/2016 07:50 AM, Sriram Dash wrote: >>> From: Marek Vasut [mailto:ma...@denx.de] On 09/15/2016 08:31 AM, >>> Sriram Dash wrote: >>>>> From: Marek Vasut [mailto:ma...@denx.de] On 09/14/2016 07:15 AM, >>>>> Sriram Dash wrote: >>>>>> Currently the controller by default enables the Receive Detect >>>>>> feature in P3 mode in USB 3.0 PHY. However, USB 3.0 PHY does not >>>>>> reliably support receive detection in P3 mode. >>>>>> Enabling the USB3 controller to configure USB in P2 mode whenever >>>>>> the Receive Detect feature is required. >>>>>> >>>>>> Signed-off-by: Sriram Dash <sriram.d...@nxp.com> >>>>>> Signed-off-by: Rajesh Bhagat <rajesh.bha...@nxp.com> >>>>>> --- >>>>>> Changes in v3: >>>>>> - Fixing conflicts and repost >>>>> >>>>> Changelog of this verbosity is completely useless. >>>>> >>>> >>>> I will take care the next time. >>>> >>>>>> Changes in v2: >>>>>> - Do Soc ver checking for applying erratum >>>>>> >>>>>> drivers/usb/common/fsl-errata.c | 26 ++++++++++++++++++++++++++ >>>>>> drivers/usb/host/xhci-dwc3.c | 5 +++++ >>>>>> drivers/usb/host/xhci-fsl.c | 8 ++++++++ >>>>>> include/fsl_usb.h | 1 + >>>>>> include/linux/usb/dwc3.h | 2 ++ >>>>>> 5 files changed, 42 insertions(+) >>>>>> >>>>>> diff --git a/drivers/usb/common/fsl-errata.c >>>>>> b/drivers/usb/common/fsl-errata.c index 183bf2b..f2bffba 100644 >>>>>> --- a/drivers/usb/common/fsl-errata.c >>>>>> +++ b/drivers/usb/common/fsl-errata.c >>>>>> @@ -190,4 +190,30 @@ bool has_erratum_a008751(void) >>>>>> return false; >>>>>> } >>>>>> >>>>>> +bool has_erratum_a010151(void) >>>>>> +{ >>>>>> + u32 svr = get_svr(); >>>>>> + u32 soc = SVR_SOC_VER(svr); >>>>>> + >>>>>> + switch (soc) { >>>>>> +#ifdef CONFIG_ARM64 >>>>>> + case SVR_LS2080A: >>>>>> + case SVR_LS2085A: >>>>>> + case SVR_LS1046A: >>>>>> + case SVR_LS1012A: >>>>>> + return IS_SVR_REV(svr, 1, 0); >>>>>> + case SVR_LS1043A: >>>>>> + return IS_SVR_REV(svr, 1, 0) || IS_SVR_REV(svr, 1, 1); >>>>>> #endif >>>>>> +#ifdef CONFIG_LS102XA >>>>>> + case SOC_VER_LS1020: >>>>>> + case SOC_VER_LS1021: >>>>>> + case SOC_VER_LS1022: >>>>>> + case SOC_VER_SLS1020: >>>>>> + return IS_SVR_REV(svr, 2, 0); >>>>>> +#endif >>>>>> + } >>>>>> + return false; >>>>>> +} >>>>>> + >>>>>> #endif >>>>>> diff --git a/drivers/usb/host/xhci-dwc3.c >>>>>> b/drivers/usb/host/xhci-dwc3.c index 33961cd..adbd9b5 100644 >>>>>> --- a/drivers/usb/host/xhci-dwc3.c >>>>>> +++ b/drivers/usb/host/xhci-dwc3.c >>>>>> @@ -97,3 +97,8 @@ void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val) >>>>>> setbits_le32(&dwc3_reg->g_fladj, GFLADJ_30MHZ_REG_SEL | >>>>>> GFLADJ_30MHZ(val)); >>>>>> } >>>>>> + >>>>>> +void dwc3_set_rxdetect_power_mode(struct dwc3 *dwc3_reg, u32 val) { >>>>>> + setbits_le32(&dwc3_reg->g_usb3pipectl[0], val); >>>>> >>>>> So what would happen if I wanted to clean some bits instead ? >>>> >>>> Setting the Rx detect in P2 mode is a one time job, and it does not >>>> change. Hence, IMO, the clear bit functionality is not required. >>> >>> Until an SoC comes which has some bits set there and someone wants to >>> clear them . At which point, this code will serve as a trap . >>> >> >> The default value of the register bit g_usb3pipectl is 0. > >For this particular hardware revision. That can be changed in some later >revision >and thus any user will fall into this trap. >
Ok. I get it now. Anyways, I will be taking it up in the fsl file and removing the function. >> And the default value is >> not modified anytime during execution. For the unreliable rxdetect, it >> is set to P2 mode(by setting the bit to 1). So, if any Soc chooses the >> rxdetect as P3, they will not use the function >> dwc3_set_rxdetect_power_mode to set the bit. > >Bit or bits ? If you are setting exactly one bit, why does the function have >u32 >argument at all ? > We are modifying single bit. Now that we are planning to have it done with single command, as mentioned below, the function will be dropped. >And you still didn't explain why is the setbits_le32() here and not plain >writel() . > >>>>> Why do you even need a dedicated function to write a single register? >>>>> >>>> >>>> The dwc3 phy for the specific controller version >>> >>> This should be explicitly documented with a comment here. >>> >> >> OK. Will take care in the next patch v4. >> >>>> does not reliably >>>> support Rx Detect in P3 mode(P3 is the default setting). So, this >>>> setting can be used by any other Soc, apart from freescale. IMO, >>>> this function is required. >>> >>> So why won't such a system call single register write directly ? >>> >> >> I agree to your point. We can set the bit from fsl specific file with >> the function setbits_le32(fsl_xhci->dwc3_reg->g_usb3pipectl[0], >> DWC3_GUSB3PIPECTL_DISRXDETP3); >> If any other Soc, other than fsl/nxp wants the functionality, they >> will be using the same in their respective code. What do you say? > >Why do you use setbits_le32() instead of writel() ? > We will be modifying a single bit. So, better to use setbit_le32 and leave other bits unchanged. >>>>>> +} >>>>>> diff --git a/drivers/usb/host/xhci-fsl.c >>>>>> b/drivers/usb/host/xhci-fsl.c index 0e3e056..9297ced 100644 >>>>>> --- a/drivers/usb/host/xhci-fsl.c >>>>>> +++ b/drivers/usb/host/xhci-fsl.c >>>>>> @@ -84,6 +84,14 @@ static int fsl_xhci_core_init(struct fsl_xhci >>>>>> *fsl_xhci) >>>>>> /* Change beat burst and outstanding pipelined transfers >>>>>> requests */ >>>>>> fsl_xhci_set_beat_burst_length(fsl_xhci->dwc3_reg); >>>>>> >>>>>> + /* >>>>>> + * A-010151: USB controller to configure USB in P2 mode >>>>>> + * whenever the Receive Detect feature is required >>>>>> + */ >>>>>> + if (has_erratum_a010151()) >>>>>> + dwc3_set_rxdetect_power_mode(fsl_xhci->dwc3_reg, >>>>>> + >>>>>> DWC3_GUSB3PIPECTL_DISRXDETP3); >>>>> >>>>> Can't you parse these errata infos from device tree ? >>>>> >>>> >>>> Currently, all the freescale Socs using this controller are not using DM. >>> >>> I am asking about device tree, not driver model. These two are orthogonal. >>> >> >> Sorry. My bad. But all the socs are not using the device tree now for uboot. >> I am planning to modify the implementation when the device tree >> support is used by all the socs using xhci controller for fsl/nxp. What is >> your >opinion? > >That is fine. > OK. >>>> Shall we proceed with this solution currently, and then when the DM >>>> is supported by all Socs, modify the implementation according to device >>>> tree? >>>> What do you suggest? >>>> >>> [...] >>> >>> >>> -- >>> Best regards, >>> Marek Vasut > > >-- >Best regards, >Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
