* [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes
@ 2024-10-04 3:41 Sam Edwards
2024-10-04 3:41 ` [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array Sam Edwards
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Sam Edwards @ 2024-10-04 3:41 UTC (permalink / raw)
To: Justin Chen, Al Cooper
Cc: Broadcom internal kernel review list, Vinod Koul,
Kishon Vijay Abraham I, Florian Fainelli, linux-kernel, linux-phy,
Sam Edwards
Greetings list,
This is v2 of my previous patch [1] targeted at resolving a crash-on-boot on
the BCM4908 family due to a missized table.
Changes v1->v2:
- Florian requested this change be broken into an immediate bugfix (w/ the
'fixes' tag) and a second patch containing the coding change aimed at
preventing this problem from happening again
Best,
Sam
[1]: https://lore.kernel.org/lkml/20241003211720.1339468-1-CFSworks@gmail.com/T/
Sam Edwards (2):
phy: usb: Fix missing elements in BCM4908 USB init array
phy: usb: update Broadcom driver table to use designated initializers
drivers/phy/broadcom/phy-brcm-usb-init.c | 433 +++++++++++------------
1 file changed, 215 insertions(+), 218 deletions(-)
--
2.44.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array
2024-10-04 3:41 [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes Sam Edwards
@ 2024-10-04 3:41 ` Sam Edwards
2024-10-04 16:14 ` Florian Fainelli
2024-10-04 23:34 ` Justin Chen
2024-10-04 3:41 ` [PATCH v2 2/2] phy: usb: update Broadcom driver table to use designated initializers Sam Edwards
2024-10-07 15:48 ` (subset) [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes Vinod Koul
2 siblings, 2 replies; 13+ messages in thread
From: Sam Edwards @ 2024-10-04 3:41 UTC (permalink / raw)
To: Justin Chen, Al Cooper
Cc: Broadcom internal kernel review list, Vinod Koul,
Kishon Vijay Abraham I, Florian Fainelli, linux-kernel, linux-phy,
Sam Edwards
The Broadcom USB PHY driver contains a lookup table
(`reg_bits_map_tables`) to resolve register bitmaps unique to certain
versions of the USB PHY as found in various Broadcom chip families. A
recent commit (see 'fixes' tag) introduced two new elements to each chip
family in this table -- except for one: BCM4908. This resulted in the
xHCI controller not being initialized correctly, causing a panic on
boot.
The next patch will update this table to use designated initializers in
order to prevent this from happening again. For now, just add back the
missing array elements to resolve the regression.
Fixes: 4536fe9640b6 ("phy: usb: suppress OC condition for 7439b2")
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
drivers/phy/broadcom/phy-brcm-usb-init.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.c b/drivers/phy/broadcom/phy-brcm-usb-init.c
index 39536b6d96a9..5ebb3a616115 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init.c
@@ -220,6 +220,8 @@ usb_reg_bits_map_table[BRCM_FAMILY_COUNT][USB_CTRL_SELECTOR_COUNT] = {
0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
0, /* USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK */
0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
+ 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK */
+ 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK */
0, /* USB_CTRL_SETUP_OC3_DISABLE_MASK */
0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
--
2.44.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/2] phy: usb: update Broadcom driver table to use designated initializers
2024-10-04 3:41 [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes Sam Edwards
2024-10-04 3:41 ` [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array Sam Edwards
@ 2024-10-04 3:41 ` Sam Edwards
2024-10-04 17:21 ` Florian Fainelli
2024-10-04 23:35 ` Justin Chen
2024-10-07 15:48 ` (subset) [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes Vinod Koul
2 siblings, 2 replies; 13+ messages in thread
From: Sam Edwards @ 2024-10-04 3:41 UTC (permalink / raw)
To: Justin Chen, Al Cooper
Cc: Broadcom internal kernel review list, Vinod Koul,
Kishon Vijay Abraham I, Florian Fainelli, linux-kernel, linux-phy,
Sam Edwards
The Broadcom USB PHY driver contains a lookup table
(`reg_bits_map_tables`) to resolve register bitmaps unique to certain
versions of the USB PHY as found in various Broadcom chip families.
Historically, this table was just kept carefully in sync with the
"selector" enum every time the latter changed to ensure consistency.
However, a recent commit [1] introduced two new enumerators but did not
adjust the array for BCM4908, thus breaking the xHCI controller (and
boot process) on this platform and revealing the fragility of this
approach.
Since these arrays are a little sparse (many elements are zero) and the
position of the array elements is significant only insofar as they agree
with the enumerators, designated initializers are a better fit than
positional initializers here. Convert this table accordingly.
[1] 4536fe9640b6 ("phy: usb: suppress OC condition for 7439b2")
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
drivers/phy/broadcom/phy-brcm-usb-init.c | 435 +++++++++++------------
1 file changed, 215 insertions(+), 220 deletions(-)
diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.c b/drivers/phy/broadcom/phy-brcm-usb-init.c
index 5ebb3a616115..da23078878a9 100644
--- a/drivers/phy/broadcom/phy-brcm-usb-init.c
+++ b/drivers/phy/broadcom/phy-brcm-usb-init.c
@@ -193,256 +193,251 @@ static const u32
usb_reg_bits_map_table[BRCM_FAMILY_COUNT][USB_CTRL_SELECTOR_COUNT] = {
/* 3390B0 */
[BRCM_FAMILY_3390A0] = {
- USB_CTRL_SETUP_SCB1_EN_MASK,
- USB_CTRL_SETUP_SCB2_EN_MASK,
- USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
- USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_MASK,
- 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
- 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
- USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_USB_PWRDN_MASK,
- 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
- 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
- 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
- USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
- ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB1_EN_MASK,
+ [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB2_EN_MASK,
+ [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
+ USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
+ [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
+ USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_MASK,
+ [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
+ USB_CTRL_USB_PM_USB_PWRDN_MASK,
+ [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
+ USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
+ [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
+ [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
},
/* 4908 */
[BRCM_FAMILY_4908] = {
- 0, /* USB_CTRL_SETUP_SCB1_EN_MASK */
- 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
- 0, /* USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
- 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK */
- 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK */
- 0, /* USB_CTRL_SETUP_OC3_DISABLE_MASK */
- 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
- 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
- USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_USB_PWRDN_MASK,
- 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
- 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
- 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
- 0, /* USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK */
- 0, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
+ USB_CTRL_USB_PM_USB_PWRDN_MASK,
},
/* 7250b0 */
[BRCM_FAMILY_7250B0] = {
- USB_CTRL_SETUP_SCB1_EN_MASK,
- USB_CTRL_SETUP_SCB2_EN_MASK,
- USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
- 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
- USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_MASK,
- USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
- 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
- USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
- 0, /* USB_CTRL_USB_PM_USB_PWRDN_MASK */
- 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
- 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
- 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
- USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
- ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB1_EN_MASK,
+ [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB2_EN_MASK,
+ [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
+ USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_MASK,
+ [USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_SELECTOR] =
+ USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
+ [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
+ [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
+ [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
},
/* 7271a0 */
[BRCM_FAMILY_7271A0] = {
- 0, /* USB_CTRL_SETUP_SCB1_EN_MASK */
- 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
- USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
- USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_MASK,
- 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
- USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_USB_PWRDN_MASK,
- 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
- USB_CTRL_USB_PM_SOFT_RESET_MASK,
- USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK,
- USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK,
- USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
- ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
+ USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
+ [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
+ USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_MASK,
+ [USB_CTRL_USB_PM_BDC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
+ USB_CTRL_USB_PM_USB_PWRDN_MASK,
+ [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
+ USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
+ [USB_CTRL_USB_PM_SOFT_RESET_SELECTOR] =
+ USB_CTRL_USB_PM_SOFT_RESET_MASK,
+ [USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_SELECTOR] =
+ USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK,
+ [USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_SELECTOR] =
+ USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK,
+ [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
+ [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
},
/* 7364a0 */
[BRCM_FAMILY_7364A0] = {
- USB_CTRL_SETUP_SCB1_EN_MASK,
- USB_CTRL_SETUP_SCB2_EN_MASK,
- USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
- 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
- USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_MASK,
- USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
- 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
- USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
- 0, /* USB_CTRL_USB_PM_USB_PWRDN_MASK */
- 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
- 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
- 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
- USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
- ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB1_EN_MASK,
+ [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB2_EN_MASK,
+ [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
+ USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_MASK,
+ [USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_SELECTOR] =
+ USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
+ [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
+ [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
+ [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
},
/* 7366c0 */
[BRCM_FAMILY_7366C0] = {
- USB_CTRL_SETUP_SCB1_EN_MASK,
- USB_CTRL_SETUP_SCB2_EN_MASK,
- USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
- 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
- USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_MASK,
- 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
- 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
- USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
- USB_CTRL_USB_PM_USB_PWRDN_MASK,
- 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
- 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
- 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
- USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
- ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB1_EN_MASK,
+ [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB2_EN_MASK,
+ [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
+ USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_MASK,
+ [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
+ [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
+ USB_CTRL_USB_PM_USB_PWRDN_MASK,
+ [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
+ [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
},
/* 74371A0 */
[BRCM_FAMILY_74371A0] = {
- USB_CTRL_SETUP_SCB1_EN_MASK,
- USB_CTRL_SETUP_SCB2_EN_MASK,
- USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK,
- 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
- 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK */
- 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK */
- 0, /* USB_CTRL_SETUP_OC3_DISABLE_MASK */
- USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
- 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB_PM_USB_PWRDN_MASK */
- USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK,
- USB_CTRL_USB30_CTL1_USB3_IOC_MASK,
- USB_CTRL_USB30_CTL1_USB3_IPP_MASK,
- 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
- 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
- 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
- 0, /* USB_CTRL_USB_PM_USB20_HC_RESETB_MASK */
- ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB1_EN_MASK,
+ [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB2_EN_MASK,
+ [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
+ USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK,
+ [USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_SELECTOR] =
+ USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
+ [USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB30_CTL1_USB3_IOC_SELECTOR] =
+ USB_CTRL_USB30_CTL1_USB3_IOC_MASK,
+ [USB_CTRL_USB30_CTL1_USB3_IPP_SELECTOR] =
+ USB_CTRL_USB30_CTL1_USB3_IPP_MASK,
+ [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
},
/* 7439B0 */
[BRCM_FAMILY_7439B0] = {
- USB_CTRL_SETUP_SCB1_EN_MASK,
- USB_CTRL_SETUP_SCB2_EN_MASK,
- USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
- USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_MASK,
- 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
- USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_USB_PWRDN_MASK,
- 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
- 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
- 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
- USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
- ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB1_EN_MASK,
+ [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB2_EN_MASK,
+ [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
+ USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
+ [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
+ USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_MASK,
+ [USB_CTRL_USB_PM_BDC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
+ USB_CTRL_USB_PM_USB_PWRDN_MASK,
+ [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
+ USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
+ [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
+ [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
},
/* 7445d0 */
[BRCM_FAMILY_7445D0] = {
- USB_CTRL_SETUP_SCB1_EN_MASK,
- USB_CTRL_SETUP_SCB2_EN_MASK,
- USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK,
- 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
- USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_MASK,
- USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
- 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB_PM_USB_PWRDN_MASK */
- USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK,
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
- 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
- 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
- USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
- ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB1_EN_MASK,
+ [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
+ USB_CTRL_SETUP_SCB2_EN_MASK,
+ [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
+ USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_MASK,
+ [USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_SELECTOR] =
+ USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
+ [USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
+ [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
},
/* 7260a0 */
[BRCM_FAMILY_7260A0] = {
- 0, /* USB_CTRL_SETUP_SCB1_EN_MASK */
- 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
- USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
- USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_MASK,
- 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
- USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_USB_PWRDN_MASK,
- 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
- USB_CTRL_USB_PM_SOFT_RESET_MASK,
- USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK,
- USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK,
- USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
- ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
+ USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
+ [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
+ USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_MASK,
+ [USB_CTRL_USB_PM_BDC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
+ USB_CTRL_USB_PM_USB_PWRDN_MASK,
+ [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
+ USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
+ [USB_CTRL_USB_PM_SOFT_RESET_SELECTOR] =
+ USB_CTRL_USB_PM_SOFT_RESET_MASK,
+ [USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_SELECTOR] =
+ USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK,
+ [USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_SELECTOR] =
+ USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK,
+ [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
+ [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
},
/* 7278a0 */
[BRCM_FAMILY_7278A0] = {
- 0, /* USB_CTRL_SETUP_SCB1_EN_MASK */
- 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
- 0, /*USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK */
- USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
- USB_CTRL_SETUP_OC3_DISABLE_MASK,
- 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
- USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
- USB_CTRL_USB_PM_USB_PWRDN_MASK,
- 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
- 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
- USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
- USB_CTRL_USB_PM_SOFT_RESET_MASK,
- 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
- 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
- 0, /* USB_CTRL_USB_PM_USB20_HC_RESETB_MASK */
- 0, /* USB_CTRL_SETUP ENDIAN bits */
+ [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
+ USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
+ [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
+ USB_CTRL_SETUP_OC3_DISABLE_MASK,
+ [USB_CTRL_USB_PM_BDC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
+ USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
+ [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
+ USB_CTRL_USB_PM_USB_PWRDN_MASK,
+ [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
+ USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
+ [USB_CTRL_USB_PM_SOFT_RESET_SELECTOR] =
+ USB_CTRL_USB_PM_SOFT_RESET_MASK,
},
};
--
2.44.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array
2024-10-04 3:41 ` [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array Sam Edwards
@ 2024-10-04 16:14 ` Florian Fainelli
2024-10-04 23:35 ` Sam Edwards
2024-10-04 23:34 ` Justin Chen
1 sibling, 1 reply; 13+ messages in thread
From: Florian Fainelli @ 2024-10-04 16:14 UTC (permalink / raw)
To: Sam Edwards, Justin Chen, Al Cooper
Cc: Broadcom internal kernel review list, Vinod Koul,
Kishon Vijay Abraham I, linux-kernel, linux-phy
On 10/3/24 20:41, Sam Edwards wrote:
> The Broadcom USB PHY driver contains a lookup table
> (`reg_bits_map_tables`) to resolve register bitmaps unique to certain
> versions of the USB PHY as found in various Broadcom chip families. A
> recent commit (see 'fixes' tag) introduced two new elements to each chip
> family in this table -- except for one: BCM4908. This resulted in the
> xHCI controller not being initialized correctly, causing a panic on
> boot.
Yes, I think I see what happened here, we took the patch in the "fixes"
tag from the our downstream tree, and it applied just fine, we will keep
a closer eye on other entries in the future.
>
> The next patch will update this table to use designated initializers in
> order to prevent this from happening again. For now, just add back the
> missing array elements to resolve the regression.
Out of curiosity, can you check whether building with
-Wmissing-field-initializers would have caught this?
>
> Fixes: 4536fe9640b6 ("phy: usb: suppress OC condition for 7439b2")
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
> drivers/phy/broadcom/phy-brcm-usb-init.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.c b/drivers/phy/broadcom/phy-brcm-usb-init.c
> index 39536b6d96a9..5ebb3a616115 100644
> --- a/drivers/phy/broadcom/phy-brcm-usb-init.c
> +++ b/drivers/phy/broadcom/phy-brcm-usb-init.c
> @@ -220,6 +220,8 @@ usb_reg_bits_map_table[BRCM_FAMILY_COUNT][USB_CTRL_SELECTOR_COUNT] = {
> 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
> 0, /* USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK */
> 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
> + 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK */
> + 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK */
> 0, /* USB_CTRL_SETUP_OC3_DISABLE_MASK */
> 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
--
Florian
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] phy: usb: update Broadcom driver table to use designated initializers
2024-10-04 3:41 ` [PATCH v2 2/2] phy: usb: update Broadcom driver table to use designated initializers Sam Edwards
@ 2024-10-04 17:21 ` Florian Fainelli
2024-10-04 23:35 ` Justin Chen
1 sibling, 0 replies; 13+ messages in thread
From: Florian Fainelli @ 2024-10-04 17:21 UTC (permalink / raw)
To: Sam Edwards, Justin Chen, Al Cooper
Cc: Broadcom internal kernel review list, Vinod Koul,
Kishon Vijay Abraham I, linux-kernel, linux-phy
On 10/3/24 20:41, Sam Edwards wrote:
> The Broadcom USB PHY driver contains a lookup table
> (`reg_bits_map_tables`) to resolve register bitmaps unique to certain
> versions of the USB PHY as found in various Broadcom chip families.
> Historically, this table was just kept carefully in sync with the
> "selector" enum every time the latter changed to ensure consistency.
> However, a recent commit [1] introduced two new enumerators but did not
> adjust the array for BCM4908, thus breaking the xHCI controller (and
> boot process) on this platform and revealing the fragility of this
> approach.
>
> Since these arrays are a little sparse (many elements are zero) and the
> position of the array elements is significant only insofar as they agree
> with the enumerators, designated initializers are a better fit than
> positional initializers here. Convert this table accordingly.
>
> [1] 4536fe9640b6 ("phy: usb: suppress OC condition for 7439b2")
>
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
--
Florian
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array
2024-10-04 3:41 ` [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array Sam Edwards
2024-10-04 16:14 ` Florian Fainelli
@ 2024-10-04 23:34 ` Justin Chen
1 sibling, 0 replies; 13+ messages in thread
From: Justin Chen @ 2024-10-04 23:34 UTC (permalink / raw)
To: Sam Edwards, Al Cooper
Cc: Broadcom internal kernel review list, Vinod Koul,
Kishon Vijay Abraham I, Florian Fainelli, linux-kernel, linux-phy
On 10/3/24 8:41 PM, Sam Edwards wrote:
> The Broadcom USB PHY driver contains a lookup table
> (`reg_bits_map_tables`) to resolve register bitmaps unique to certain
> versions of the USB PHY as found in various Broadcom chip families. A
> recent commit (see 'fixes' tag) introduced two new elements to each chip
> family in this table -- except for one: BCM4908. This resulted in the
> xHCI controller not being initialized correctly, causing a panic on
> boot.
>
> The next patch will update this table to use designated initializers in
> order to prevent this from happening again. For now, just add back the
> missing array elements to resolve the regression.
>
> Fixes: 4536fe9640b6 ("phy: usb: suppress OC condition for 7439b2")
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>
Woops! Sorry I broke it.
Reviewed-by: Justin Chen <justin.chen@broadcom.com>
> ---
> drivers/phy/broadcom/phy-brcm-usb-init.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.c b/drivers/phy/broadcom/phy-brcm-usb-init.c
> index 39536b6d96a9..5ebb3a616115 100644
> --- a/drivers/phy/broadcom/phy-brcm-usb-init.c
> +++ b/drivers/phy/broadcom/phy-brcm-usb-init.c
> @@ -220,6 +220,8 @@ usb_reg_bits_map_table[BRCM_FAMILY_COUNT][USB_CTRL_SELECTOR_COUNT] = {
> 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
> 0, /* USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK */
> 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
> + 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK */
> + 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK */
> 0, /* USB_CTRL_SETUP_OC3_DISABLE_MASK */
> 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] phy: usb: update Broadcom driver table to use designated initializers
2024-10-04 3:41 ` [PATCH v2 2/2] phy: usb: update Broadcom driver table to use designated initializers Sam Edwards
2024-10-04 17:21 ` Florian Fainelli
@ 2024-10-04 23:35 ` Justin Chen
1 sibling, 0 replies; 13+ messages in thread
From: Justin Chen @ 2024-10-04 23:35 UTC (permalink / raw)
To: Sam Edwards, Al Cooper
Cc: Broadcom internal kernel review list, Vinod Koul,
Kishon Vijay Abraham I, Florian Fainelli, linux-kernel, linux-phy
On 10/3/24 8:41 PM, Sam Edwards wrote:
> The Broadcom USB PHY driver contains a lookup table
> (`reg_bits_map_tables`) to resolve register bitmaps unique to certain
> versions of the USB PHY as found in various Broadcom chip families.
> Historically, this table was just kept carefully in sync with the
> "selector" enum every time the latter changed to ensure consistency.
> However, a recent commit [1] introduced two new enumerators but did not
> adjust the array for BCM4908, thus breaking the xHCI controller (and
> boot process) on this platform and revealing the fragility of this
> approach.
>
> Since these arrays are a little sparse (many elements are zero) and the
> position of the array elements is significant only insofar as they agree
> with the enumerators, designated initializers are a better fit than
> positional initializers here. Convert this table accordingly.
>
> [1] 4536fe9640b6 ("phy: usb: suppress OC condition for 7439b2")
>
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>
Reviewed-by: Justin Chen <justin.chen@broadcom.com>
> ---
> drivers/phy/broadcom/phy-brcm-usb-init.c | 435 +++++++++++------------
> 1 file changed, 215 insertions(+), 220 deletions(-)
>
> diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.c b/drivers/phy/broadcom/phy-brcm-usb-init.c
> index 5ebb3a616115..da23078878a9 100644
> --- a/drivers/phy/broadcom/phy-brcm-usb-init.c
> +++ b/drivers/phy/broadcom/phy-brcm-usb-init.c
> @@ -193,256 +193,251 @@ static const u32
> usb_reg_bits_map_table[BRCM_FAMILY_COUNT][USB_CTRL_SELECTOR_COUNT] = {
> /* 3390B0 */
> [BRCM_FAMILY_3390A0] = {
> - USB_CTRL_SETUP_SCB1_EN_MASK,
> - USB_CTRL_SETUP_SCB2_EN_MASK,
> - USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> - USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_MASK,
> - 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> - 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
> - USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_USB_PWRDN_MASK,
> - 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> - 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
> - 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
> - USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> - ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB1_EN_MASK,
> + [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB2_EN_MASK,
> + [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
> + USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> + [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
> + USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_MASK,
> + [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
> + USB_CTRL_USB_PM_USB_PWRDN_MASK,
> + [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
> + USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> + [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> + [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
> },
> /* 4908 */
> [BRCM_FAMILY_4908] = {
> - 0, /* USB_CTRL_SETUP_SCB1_EN_MASK */
> - 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
> - 0, /* USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
> - 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK */
> - 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK */
> - 0, /* USB_CTRL_SETUP_OC3_DISABLE_MASK */
> - 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> - 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
> - USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_USB_PWRDN_MASK,
> - 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
> - 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
> - 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
> - 0, /* USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK */
> - 0, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
> + USB_CTRL_USB_PM_USB_PWRDN_MASK,
> },
> /* 7250b0 */
> [BRCM_FAMILY_7250B0] = {
> - USB_CTRL_SETUP_SCB1_EN_MASK,
> - USB_CTRL_SETUP_SCB2_EN_MASK,
> - USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> - 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
> - USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_MASK,
> - USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
> - 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
> - USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
> - 0, /* USB_CTRL_USB_PM_USB_PWRDN_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
> - 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
> - 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
> - USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
> - ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB1_EN_MASK,
> + [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB2_EN_MASK,
> + [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
> + USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_MASK,
> + [USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_SELECTOR] =
> + USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
> + [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
> + [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
> + [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
> },
> /* 7271a0 */
> [BRCM_FAMILY_7271A0] = {
> - 0, /* USB_CTRL_SETUP_SCB1_EN_MASK */
> - 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
> - USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> - USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_MASK,
> - 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> - USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_USB_PWRDN_MASK,
> - 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> - USB_CTRL_USB_PM_SOFT_RESET_MASK,
> - USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK,
> - USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK,
> - USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> - ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
> + USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> + [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
> + USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_MASK,
> + [USB_CTRL_USB_PM_BDC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
> + USB_CTRL_USB_PM_USB_PWRDN_MASK,
> + [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
> + USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> + [USB_CTRL_USB_PM_SOFT_RESET_SELECTOR] =
> + USB_CTRL_USB_PM_SOFT_RESET_MASK,
> + [USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_SELECTOR] =
> + USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK,
> + [USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_SELECTOR] =
> + USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK,
> + [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> + [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
> },
> /* 7364a0 */
> [BRCM_FAMILY_7364A0] = {
> - USB_CTRL_SETUP_SCB1_EN_MASK,
> - USB_CTRL_SETUP_SCB2_EN_MASK,
> - USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> - 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
> - USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_MASK,
> - USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
> - 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
> - USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
> - 0, /* USB_CTRL_USB_PM_USB_PWRDN_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
> - 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
> - 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
> - USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
> - ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB1_EN_MASK,
> + [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB2_EN_MASK,
> + [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
> + USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_MASK,
> + [USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_SELECTOR] =
> + USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
> + [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
> + [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
> + [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
> },
> /* 7366c0 */
> [BRCM_FAMILY_7366C0] = {
> - USB_CTRL_SETUP_SCB1_EN_MASK,
> - USB_CTRL_SETUP_SCB2_EN_MASK,
> - USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> - 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
> - USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_MASK,
> - 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> - 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
> - USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
> - USB_CTRL_USB_PM_USB_PWRDN_MASK,
> - 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
> - 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
> - 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
> - USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
> - ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB1_EN_MASK,
> + [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB2_EN_MASK,
> + [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
> + USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_MASK,
> + [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_XHC_SOFT_RESETB_VAR_MASK,
> + [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
> + USB_CTRL_USB_PM_USB_PWRDN_MASK,
> + [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_USB20_HC_RESETB_MASK,
> + [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
> },
> /* 74371A0 */
> [BRCM_FAMILY_74371A0] = {
> - USB_CTRL_SETUP_SCB1_EN_MASK,
> - USB_CTRL_SETUP_SCB2_EN_MASK,
> - USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK,
> - 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
> - 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK */
> - 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK */
> - 0, /* USB_CTRL_SETUP_OC3_DISABLE_MASK */
> - USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
> - 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB_PM_USB_PWRDN_MASK */
> - USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK,
> - USB_CTRL_USB30_CTL1_USB3_IOC_MASK,
> - USB_CTRL_USB30_CTL1_USB3_IPP_MASK,
> - 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
> - 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
> - 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
> - 0, /* USB_CTRL_USB_PM_USB20_HC_RESETB_MASK */
> - ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB1_EN_MASK,
> + [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB2_EN_MASK,
> + [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
> + USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK,
> + [USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_SELECTOR] =
> + USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
> + [USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB30_CTL1_USB3_IOC_SELECTOR] =
> + USB_CTRL_USB30_CTL1_USB3_IOC_MASK,
> + [USB_CTRL_USB30_CTL1_USB3_IPP_SELECTOR] =
> + USB_CTRL_USB30_CTL1_USB3_IPP_MASK,
> + [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
> },
> /* 7439B0 */
> [BRCM_FAMILY_7439B0] = {
> - USB_CTRL_SETUP_SCB1_EN_MASK,
> - USB_CTRL_SETUP_SCB2_EN_MASK,
> - USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> - USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_MASK,
> - 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> - USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_USB_PWRDN_MASK,
> - 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> - 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
> - 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
> - USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> - ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB1_EN_MASK,
> + [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB2_EN_MASK,
> + [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
> + USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> + [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
> + USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_MASK,
> + [USB_CTRL_USB_PM_BDC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
> + USB_CTRL_USB_PM_USB_PWRDN_MASK,
> + [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
> + USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> + [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> + [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
> },
> /* 7445d0 */
> [BRCM_FAMILY_7445D0] = {
> - USB_CTRL_SETUP_SCB1_EN_MASK,
> - USB_CTRL_SETUP_SCB2_EN_MASK,
> - USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK,
> - 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
> - USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_MASK,
> - USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
> - 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB_PM_USB_PWRDN_MASK */
> - USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK,
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - 0, /* USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK */
> - 0, /* USB_CTRL_USB_PM_SOFT_RESET_MASK */
> - 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
> - USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> - ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_SCB1_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB1_EN_MASK,
> + [USB_CTRL_SETUP_SCB2_EN_SELECTOR] =
> + USB_CTRL_SETUP_SCB2_EN_MASK,
> + [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
> + USB_CTRL_SETUP_SS_EHCI64BIT_EN_VAR_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_MASK,
> + [USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_SELECTOR] =
> + USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK,
> + [USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> + [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
> },
> /* 7260a0 */
> [BRCM_FAMILY_7260A0] = {
> - 0, /* USB_CTRL_SETUP_SCB1_EN_MASK */
> - 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
> - USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> - USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_MASK,
> - 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> - USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_USB_PWRDN_MASK,
> - 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> - USB_CTRL_USB_PM_SOFT_RESET_MASK,
> - USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK,
> - USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK,
> - USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> - ENDIAN_SETTINGS, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_SS_EHCI64BIT_EN_SELECTOR] =
> + USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK,
> + [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
> + USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_MASK,
> + [USB_CTRL_USB_PM_BDC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
> + USB_CTRL_USB_PM_USB_PWRDN_MASK,
> + [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
> + USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> + [USB_CTRL_USB_PM_SOFT_RESET_SELECTOR] =
> + USB_CTRL_USB_PM_SOFT_RESET_MASK,
> + [USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_SELECTOR] =
> + USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK,
> + [USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_SELECTOR] =
> + USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK,
> + [USB_CTRL_USB_PM_USB20_HC_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_USB20_HC_RESETB_VAR_MASK,
> + [USB_CTRL_SETUP_ENDIAN_SELECTOR] = ENDIAN_SETTINGS,
> },
> /* 7278a0 */
> [BRCM_FAMILY_7278A0] = {
> - 0, /* USB_CTRL_SETUP_SCB1_EN_MASK */
> - 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
> - 0, /*USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK */
> - USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> - USB_CTRL_SETUP_OC3_DISABLE_MASK,
> - 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> - USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> - USB_CTRL_USB_PM_USB_PWRDN_MASK,
> - 0, /* USB_CTRL_USB30_CTL1_XHC_SOFT_RESETB_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IOC_MASK */
> - 0, /* USB_CTRL_USB30_CTL1_USB3_IPP_MASK */
> - USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> - USB_CTRL_USB_PM_SOFT_RESET_MASK,
> - 0, /* USB_CTRL_SETUP_CC_DRD_MODE_ENABLE_MASK */
> - 0, /* USB_CTRL_SETUP_STRAP_CC_DRD_MODE_ENABLE_SEL_MASK */
> - 0, /* USB_CTRL_USB_PM_USB20_HC_RESETB_MASK */
> - 0, /* USB_CTRL_SETUP ENDIAN bits */
> + [USB_CTRL_SETUP_STRAP_IPP_SEL_SELECTOR] =
> + USB_CTRL_SETUP_STRAP_IPP_SEL_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT0_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_PORT1_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK,
> + [USB_CTRL_SETUP_OC3_DISABLE_SELECTOR] =
> + USB_CTRL_SETUP_OC3_DISABLE_MASK,
> + [USB_CTRL_USB_PM_BDC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_XHC_SOFT_RESETB_SELECTOR] =
> + USB_CTRL_USB_PM_XHC_SOFT_RESETB_MASK,
> + [USB_CTRL_USB_PM_USB_PWRDN_SELECTOR] =
> + USB_CTRL_USB_PM_USB_PWRDN_MASK,
> + [USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_SELECTOR] =
> + USB_CTRL_USB_DEVICE_CTL1_PORT_MODE_MASK,
> + [USB_CTRL_USB_PM_SOFT_RESET_SELECTOR] =
> + USB_CTRL_USB_PM_SOFT_RESET_MASK,
> },
> };
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array
2024-10-04 16:14 ` Florian Fainelli
@ 2024-10-04 23:35 ` Sam Edwards
2024-10-04 23:37 ` Florian Fainelli
0 siblings, 1 reply; 13+ messages in thread
From: Sam Edwards @ 2024-10-04 23:35 UTC (permalink / raw)
To: Florian Fainelli
Cc: Justin Chen, Al Cooper, Broadcom internal kernel review list,
Vinod Koul, Kishon Vijay Abraham I, linux-kernel, linux-phy
On Fri, Oct 4, 2024 at 9:14 AM Florian Fainelli
<florian.fainelli@broadcom.com> wrote:
>
> On 10/3/24 20:41, Sam Edwards wrote:
> > The Broadcom USB PHY driver contains a lookup table
> > (`reg_bits_map_tables`) to resolve register bitmaps unique to certain
> > versions of the USB PHY as found in various Broadcom chip families. A
> > recent commit (see 'fixes' tag) introduced two new elements to each chip
> > family in this table -- except for one: BCM4908. This resulted in the
> > xHCI controller not being initialized correctly, causing a panic on
> > boot.
>
> Yes, I think I see what happened here, we took the patch in the "fixes"
> tag from the our downstream tree, and it applied just fine, we will keep
> a closer eye on other entries in the future.
>
> >
> > The next patch will update this table to use designated initializers in
> > order to prevent this from happening again. For now, just add back the
> > missing array elements to resolve the regression.
>
> Out of curiosity, can you check whether building with
> -Wmissing-field-initializers would have caught this?
It appears that the answer is no, at least here on Clang. I also just
tried -Wextra to see if any warning would catch it and didn't see one.
My understanding is that -Wmissing-field-initializers is for struct
fields, and a construct like:
int array[3] = {1, 2};
...does not result in a warning because it's considered perfectly
valid standards-compliant C per C's default initialization rule.
Cheers,
Sam
>
> >
> > Fixes: 4536fe9640b6 ("phy: usb: suppress OC condition for 7439b2")
> > Signed-off-by: Sam Edwards <CFSworks@gmail.com>
>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
>
> > ---
> > drivers/phy/broadcom/phy-brcm-usb-init.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/phy/broadcom/phy-brcm-usb-init.c b/drivers/phy/broadcom/phy-brcm-usb-init.c
> > index 39536b6d96a9..5ebb3a616115 100644
> > --- a/drivers/phy/broadcom/phy-brcm-usb-init.c
> > +++ b/drivers/phy/broadcom/phy-brcm-usb-init.c
> > @@ -220,6 +220,8 @@ usb_reg_bits_map_table[BRCM_FAMILY_COUNT][USB_CTRL_SELECTOR_COUNT] = {
> > 0, /* USB_CTRL_SETUP_SCB2_EN_MASK */
> > 0, /* USB_CTRL_SETUP_SS_EHCI64BIT_EN_MASK */
> > 0, /* USB_CTRL_SETUP_STRAP_IPP_SEL_MASK */
> > + 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT0_MASK */
> > + 0, /* USB_CTRL_SETUP_OC3_DISABLE_PORT1_MASK */
> > 0, /* USB_CTRL_SETUP_OC3_DISABLE_MASK */
> > 0, /* USB_CTRL_PLL_CTL_PLL_IDDQ_PWRDN_MASK */
> > 0, /* USB_CTRL_USB_PM_BDC_SOFT_RESETB_MASK */
>
>
> --
> Florian
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array
2024-10-04 23:35 ` Sam Edwards
@ 2024-10-04 23:37 ` Florian Fainelli
0 siblings, 0 replies; 13+ messages in thread
From: Florian Fainelli @ 2024-10-04 23:37 UTC (permalink / raw)
To: Sam Edwards
Cc: Justin Chen, Al Cooper, Broadcom internal kernel review list,
Vinod Koul, Kishon Vijay Abraham I, linux-kernel, linux-phy
On 10/4/24 16:35, Sam Edwards wrote:
> On Fri, Oct 4, 2024 at 9:14 AM Florian Fainelli
> <florian.fainelli@broadcom.com> wrote:
>>
>> On 10/3/24 20:41, Sam Edwards wrote:
>>> The Broadcom USB PHY driver contains a lookup table
>>> (`reg_bits_map_tables`) to resolve register bitmaps unique to certain
>>> versions of the USB PHY as found in various Broadcom chip families. A
>>> recent commit (see 'fixes' tag) introduced two new elements to each chip
>>> family in this table -- except for one: BCM4908. This resulted in the
>>> xHCI controller not being initialized correctly, causing a panic on
>>> boot.
>>
>> Yes, I think I see what happened here, we took the patch in the "fixes"
>> tag from the our downstream tree, and it applied just fine, we will keep
>> a closer eye on other entries in the future.
>>
>>>
>>> The next patch will update this table to use designated initializers in
>>> order to prevent this from happening again. For now, just add back the
>>> missing array elements to resolve the regression.
>>
>> Out of curiosity, can you check whether building with
>> -Wmissing-field-initializers would have caught this?
>
> It appears that the answer is no, at least here on Clang. I also just
> tried -Wextra to see if any warning would catch it and didn't see one.
> My understanding is that -Wmissing-field-initializers is for struct
> fields, and a construct like:
> int array[3] = {1, 2};
> ...does not result in a warning because it's considered perfectly
> valid standards-compliant C per C's default initialization rule.
I suspected that much, thanks for having checked!
--
Florian
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: (subset) [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes
2024-10-04 3:41 [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes Sam Edwards
2024-10-04 3:41 ` [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array Sam Edwards
2024-10-04 3:41 ` [PATCH v2 2/2] phy: usb: update Broadcom driver table to use designated initializers Sam Edwards
@ 2024-10-07 15:48 ` Vinod Koul
2024-10-07 17:06 ` Florian Fainelli
2 siblings, 1 reply; 13+ messages in thread
From: Vinod Koul @ 2024-10-07 15:48 UTC (permalink / raw)
To: Justin Chen, Al Cooper, Sam Edwards
Cc: Broadcom internal kernel review list, Kishon Vijay Abraham I,
Florian Fainelli, linux-kernel, linux-phy, Sam Edwards
On Thu, 03 Oct 2024 20:41:29 -0700, Sam Edwards wrote:
> This is v2 of my previous patch [1] targeted at resolving a crash-on-boot on
> the BCM4908 family due to a missized table.
>
> Changes v1->v2:
> - Florian requested this change be broken into an immediate bugfix (w/ the
> 'fixes' tag) and a second patch containing the coding change aimed at
> preventing this problem from happening again
>
> [...]
Applied, thanks!
[2/2] phy: usb: update Broadcom driver table to use designated initializers
commit: d3712b35f3c694cb932f87194caafc714109ea08
Best regards,
--
~Vinod
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: (subset) [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes
2024-10-07 15:48 ` (subset) [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes Vinod Koul
@ 2024-10-07 17:06 ` Florian Fainelli
2024-10-07 18:12 ` Vinod Koul
0 siblings, 1 reply; 13+ messages in thread
From: Florian Fainelli @ 2024-10-07 17:06 UTC (permalink / raw)
To: Vinod Koul, Justin Chen, Al Cooper, Sam Edwards
Cc: Broadcom internal kernel review list, Kishon Vijay Abraham I,
linux-kernel, linux-phy
On 10/7/24 08:48, Vinod Koul wrote:
>
> On Thu, 03 Oct 2024 20:41:29 -0700, Sam Edwards wrote:
>> This is v2 of my previous patch [1] targeted at resolving a crash-on-boot on
>> the BCM4908 family due to a missized table.
>>
>> Changes v1->v2:
>> - Florian requested this change be broken into an immediate bugfix (w/ the
>> 'fixes' tag) and a second patch containing the coding change aimed at
>> preventing this problem from happening again
>>
>> [...]
>
> Applied, thanks!
>
> [2/2] phy: usb: update Broadcom driver table to use designated initializers
> commit: d3712b35f3c694cb932f87194caafc714109ea08
I looked at your tree and both patches are applied in the "next" branch
and the first one is also in the "fixes" branch, thanks Vinod!
--
Florian
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: (subset) [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes
2024-10-07 17:06 ` Florian Fainelli
@ 2024-10-07 18:12 ` Vinod Koul
2024-10-07 18:17 ` Florian Fainelli
0 siblings, 1 reply; 13+ messages in thread
From: Vinod Koul @ 2024-10-07 18:12 UTC (permalink / raw)
To: Florian Fainelli
Cc: Justin Chen, Al Cooper, Sam Edwards,
Broadcom internal kernel review list, Kishon Vijay Abraham I,
linux-kernel, linux-phy
On 07-10-24, 10:06, Florian Fainelli wrote:
> On 10/7/24 08:48, Vinod Koul wrote:
> >
> > On Thu, 03 Oct 2024 20:41:29 -0700, Sam Edwards wrote:
> > > This is v2 of my previous patch [1] targeted at resolving a crash-on-boot on
> > > the BCM4908 family due to a missized table.
> > >
> > > Changes v1->v2:
> > > - Florian requested this change be broken into an immediate bugfix (w/ the
> > > 'fixes' tag) and a second patch containing the coding change aimed at
> > > preventing this problem from happening again
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [2/2] phy: usb: update Broadcom driver table to use designated initializers
> > commit: d3712b35f3c694cb932f87194caafc714109ea08
>
> I looked at your tree and both patches are applied in the "next" branch and
> the first one is also in the "fixes" branch, thanks Vinod!
First one should go into fixes, whereas second on next (due to
dependency merged fixes into next)
--
~Vinod
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: (subset) [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes
2024-10-07 18:12 ` Vinod Koul
@ 2024-10-07 18:17 ` Florian Fainelli
0 siblings, 0 replies; 13+ messages in thread
From: Florian Fainelli @ 2024-10-07 18:17 UTC (permalink / raw)
To: Vinod Koul
Cc: Justin Chen, Al Cooper, Sam Edwards,
Broadcom internal kernel review list, Kishon Vijay Abraham I,
linux-kernel, linux-phy
On 10/7/24 11:12, Vinod Koul wrote:
> On 07-10-24, 10:06, Florian Fainelli wrote:
>> On 10/7/24 08:48, Vinod Koul wrote:
>>>
>>> On Thu, 03 Oct 2024 20:41:29 -0700, Sam Edwards wrote:
>>>> This is v2 of my previous patch [1] targeted at resolving a crash-on-boot on
>>>> the BCM4908 family due to a missized table.
>>>>
>>>> Changes v1->v2:
>>>> - Florian requested this change be broken into an immediate bugfix (w/ the
>>>> 'fixes' tag) and a second patch containing the coding change aimed at
>>>> preventing this problem from happening again
>>>>
>>>> [...]
>>>
>>> Applied, thanks!
>>>
>>> [2/2] phy: usb: update Broadcom driver table to use designated initializers
>>> commit: d3712b35f3c694cb932f87194caafc714109ea08
>>
>> I looked at your tree and both patches are applied in the "next" branch and
>> the first one is also in the "fixes" branch, thanks Vinod!
>
> First one should go into fixes, whereas second on next (due to
> dependency merged fixes into next)
Yes that makes sense and is what I would have expected. I got worried
about your git hook responding to this patch series and indicating that
only the second patch had been applied, thus making me look into your
tree to double check this had not been the case.
--
Florian
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-10-07 18:17 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-04 3:41 [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes Sam Edwards
2024-10-04 3:41 ` [PATCH v2 1/2] phy: usb: Fix missing elements in BCM4908 USB init array Sam Edwards
2024-10-04 16:14 ` Florian Fainelli
2024-10-04 23:35 ` Sam Edwards
2024-10-04 23:37 ` Florian Fainelli
2024-10-04 23:34 ` Justin Chen
2024-10-04 3:41 ` [PATCH v2 2/2] phy: usb: update Broadcom driver table to use designated initializers Sam Edwards
2024-10-04 17:21 ` Florian Fainelli
2024-10-04 23:35 ` Justin Chen
2024-10-07 15:48 ` (subset) [PATCH v2 0/2] phy: usb: Broadcom BCM4908 USB init fixes Vinod Koul
2024-10-07 17:06 ` Florian Fainelli
2024-10-07 18:12 ` Vinod Koul
2024-10-07 18:17 ` Florian Fainelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).