[U-Boot] [PATCH v3] drivers: usb: xhci-fsl: Implement Erratum A-010151 for FSL USB3 controller
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.dash@nxp.com Signed-off-by: Rajesh Bhagat rajesh.bhagat@nxp.com --- Changes in v3: - Fixing conflicts and repost 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); +} 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); + return ret; }
diff --git a/include/fsl_usb.h b/include/fsl_usb.h index fc72fb9..73235b8 100644 --- a/include/fsl_usb.h +++ b/include/fsl_usb.h @@ -95,5 +95,6 @@ bool has_erratum_a007792(void); bool has_erratum_a005697(void); bool has_erratum_a004477(void); bool has_erratum_a008751(void); +bool has_erratum_a010151(void); #endif #endif /*_ASM_FSL_USB_H_ */ diff --git a/include/linux/usb/dwc3.h b/include/linux/usb/dwc3.h index 6d1e365..f68cdd2 100644 --- a/include/linux/usb/dwc3.h +++ b/include/linux/usb/dwc3.h @@ -184,6 +184,7 @@ struct dwc3 { /* offset: 0xC100 */
/* Global USB3 PIPE Control Register */ #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) +#define DWC3_GUSB3PIPECTL_DISRXDETP3 (1 << 28) #define DWC3_GUSB3PIPECTL_SUSPHY (1 << 17)
/* Global TX Fifo Size Register */ @@ -205,5 +206,6 @@ void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode); void dwc3_core_soft_reset(struct dwc3 *dwc3_reg); int dwc3_core_init(struct dwc3 *dwc3_reg); void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val); +void dwc3_set_rxdetect_power_mode(struct dwc3 *dwc3_reg, u32 val); #endif #endif /* __DWC3_H_ */
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.dash@nxp.com Signed-off-by: Rajesh Bhagat rajesh.bhagat@nxp.com
Changes in v3:
- Fixing conflicts and repost
Changelog of this verbosity is completely useless.
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 ? Why do you even need a dedicated function to write a single register?
+} 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 ?
return ret; }
diff --git a/include/fsl_usb.h b/include/fsl_usb.h index fc72fb9..73235b8 100644 --- a/include/fsl_usb.h +++ b/include/fsl_usb.h @@ -95,5 +95,6 @@ bool has_erratum_a007792(void); bool has_erratum_a005697(void); bool has_erratum_a004477(void); bool has_erratum_a008751(void); +bool has_erratum_a010151(void); #endif #endif /*_ASM_FSL_USB_H_ */ diff --git a/include/linux/usb/dwc3.h b/include/linux/usb/dwc3.h index 6d1e365..f68cdd2 100644 --- a/include/linux/usb/dwc3.h +++ b/include/linux/usb/dwc3.h @@ -184,6 +184,7 @@ struct dwc3 { /* offset: 0xC100 */
/* Global USB3 PIPE Control Register */ #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) +#define DWC3_GUSB3PIPECTL_DISRXDETP3 (1 << 28) #define DWC3_GUSB3PIPECTL_SUSPHY (1 << 17)
/* Global TX Fifo Size Register */ @@ -205,5 +206,6 @@ void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode); void dwc3_core_soft_reset(struct dwc3 *dwc3_reg); int dwc3_core_init(struct dwc3 *dwc3_reg); void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val); +void dwc3_set_rxdetect_power_mode(struct dwc3 *dwc3_reg, u32 val); #endif #endif /* __DWC3_H_ */
From: Marek Vasut [mailto:marex@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.dash@nxp.com Signed-off-by: Rajesh Bhagat rajesh.bhagat@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.
Why do you even need a dedicated function to write a single register?
The dwc3 phy for the specific controller version 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.
+} 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. 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?
return ret; }
diff --git a/include/fsl_usb.h b/include/fsl_usb.h index fc72fb9..73235b8 100644 --- a/include/fsl_usb.h +++ b/include/fsl_usb.h @@ -95,5 +95,6 @@ bool has_erratum_a007792(void); bool has_erratum_a005697(void); bool has_erratum_a004477(void); bool has_erratum_a008751(void); +bool has_erratum_a010151(void); #endif #endif /*_ASM_FSL_USB_H_ */ diff --git a/include/linux/usb/dwc3.h b/include/linux/usb/dwc3.h index 6d1e365..f68cdd2 100644 --- a/include/linux/usb/dwc3.h +++ b/include/linux/usb/dwc3.h @@ -184,6 +184,7 @@ struct dwc3 { /* offset:
0xC100 */
/* Global USB3 PIPE Control Register */ #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) +#define DWC3_GUSB3PIPECTL_DISRXDETP3 (1 << 28) #define DWC3_GUSB3PIPECTL_SUSPHY (1 << 17)
/* Global TX Fifo Size Register */ @@ -205,5 +206,6 @@ void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode); void dwc3_core_soft_reset(struct dwc3 *dwc3_reg); int dwc3_core_init(struct dwc3 *dwc3_reg); void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val); +void dwc3_set_rxdetect_power_mode(struct dwc3 *dwc3_reg, u32 val); #endif #endif /* __DWC3_H_ */
-- Best regards, Marek Vasut
On 09/15/2016 08:31 AM, Sriram Dash wrote:
From: Marek Vasut [mailto:marex@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.dash@nxp.com Signed-off-by: Rajesh Bhagat rajesh.bhagat@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 .
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.
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 ?
+} 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.
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?
[...]
From: Marek Vasut [mailto:marex@denx.de] On 09/15/2016 08:31 AM, Sriram Dash wrote:
From: Marek Vasut [mailto:marex@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.dash@nxp.com Signed-off-by: Rajesh Bhagat rajesh.bhagat@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. 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.
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?
+} 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?
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
On 09/16/2016 07:50 AM, Sriram Dash wrote:
From: Marek Vasut [mailto:marex@denx.de] On 09/15/2016 08:31 AM, Sriram Dash wrote:
From: Marek Vasut [mailto:marex@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.dash@nxp.com Signed-off-by: Rajesh Bhagat rajesh.bhagat@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.
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 ?
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() ?
+} 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.
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
From: Marek Vasut [mailto:marex@denx.de] On 09/16/2016 07:50 AM, Sriram Dash wrote:
From: Marek Vasut [mailto:marex@denx.de] On 09/15/2016 08:31 AM, Sriram Dash wrote:
From: Marek Vasut [mailto:marex@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.dash@nxp.com Signed-off-by: Rajesh Bhagat rajesh.bhagat@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
On 09/16/2016 11:35 AM, Sriram Dash wrote:
[...]
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.
In that case, you should use clrsetbits_le32() to clear the bit first in case someone decided to clear it instead.
[...]
From: Marek Vasut [mailto:marex@denx.de] On 09/16/2016 11:35 AM, Sriram Dash wrote:
[...]
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.
In that case, you should use clrsetbits_le32() to clear the bit first in case someone decided to clear it instead.
OK. Will modify in v4.
[...]
-- Best regards, Marek Vasut
participants (2)
-
Marek Vasut -
Sriram Dash