[PATCH 0/6] octeontx cleanup and fixes
This series provides the following for octeontx: - cleanup to the octeontx common header file by moving CONFIG_SUPPORT_RAW_INITRD to defconfig files, - adds generic distro config support - add missing SBSA watchdog to config - fix an issue found with QML connected to a QSGMII PHY - fix an issue with mii probe failing to find mdio busses - fix a crash when scanning for sata devices
These were tested on a Gateworks Newport GW6404 with the CN8030 SoC.
Suneel Garapati (1): drivers: ata: ahci: update max id if it is more than available ports
Tim Harvey (5): arm: octeontx: move CONFIG_SUPPORT_RAW_INITRD to configs arm: octeontx: support generic distro config arm: octeontx: enable WDT_SBSA drivers: net: octeontx: fix QSGMII net: octeontx: smi: fix mii probe
configs/octeontx_81xx_defconfig | 2 ++ configs/octeontx_83xx_defconfig | 1 + drivers/ata/ahci.c | 3 +++ drivers/net/octeontx/bgx.c | 20 +++++++------------- drivers/net/octeontx/smi.c | 2 ++ include/configs/octeontx_common.h | 31 +++++++++++++++++++++++++------ 6 files changed, 40 insertions(+), 19 deletions(-)
Move CONFIG_SUPPORT_RAW_INITRD out of the octeontx_common header and into the defconfig files.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- configs/octeontx_81xx_defconfig | 1 + configs/octeontx_83xx_defconfig | 1 + include/configs/octeontx_common.h | 2 -- 3 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/configs/octeontx_81xx_defconfig b/configs/octeontx_81xx_defconfig index f0585a7488..9881c1089b 100644 --- a/configs/octeontx_81xx_defconfig +++ b/configs/octeontx_81xx_defconfig @@ -17,6 +17,7 @@ CONFIG_DEBUG_UART=y CONFIG_AHCI=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y +CONFIG_SUPPORT_RAW_INITRD=y CONFIG_OF_BOARD_SETUP=y CONFIG_BOOTDELAY=5 CONFIG_USE_BOOTARGS=y diff --git a/configs/octeontx_83xx_defconfig b/configs/octeontx_83xx_defconfig index 86b4bc5190..1fc7f0cb83 100644 --- a/configs/octeontx_83xx_defconfig +++ b/configs/octeontx_83xx_defconfig @@ -15,6 +15,7 @@ CONFIG_DEBUG_UART=y CONFIG_AHCI=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y +CONFIG_SUPPORT_RAW_INITRD=y CONFIG_OF_BOARD_SETUP=y CONFIG_BOOTDELAY=5 CONFIG_USE_BOOTARGS=y diff --git a/include/configs/octeontx_common.h b/include/configs/octeontx_common.h index 810b2bdbd5..55d329f55e 100644 --- a/include/configs/octeontx_common.h +++ b/include/configs/octeontx_common.h @@ -8,8 +8,6 @@ #ifndef __OCTEONTX_COMMON_H__ #define __OCTEONTX_COMMON_H__
-#define CONFIG_SUPPORT_RAW_INITRD - /** Maximum size of image supported for bootm (and bootable FIT images) */ #define CONFIG_SYS_BOOTM_LEN (256 << 20)
On 26.03.21 01:07, Tim Harvey wrote:
Move CONFIG_SUPPORT_RAW_INITRD out of the octeontx_common header and into the defconfig files.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
configs/octeontx_81xx_defconfig | 1 + configs/octeontx_83xx_defconfig | 1 + include/configs/octeontx_common.h | 2 -- 3 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/configs/octeontx_81xx_defconfig b/configs/octeontx_81xx_defconfig index f0585a7488..9881c1089b 100644 --- a/configs/octeontx_81xx_defconfig +++ b/configs/octeontx_81xx_defconfig @@ -17,6 +17,7 @@ CONFIG_DEBUG_UART=y CONFIG_AHCI=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y +CONFIG_SUPPORT_RAW_INITRD=y CONFIG_OF_BOARD_SETUP=y CONFIG_BOOTDELAY=5 CONFIG_USE_BOOTARGS=y diff --git a/configs/octeontx_83xx_defconfig b/configs/octeontx_83xx_defconfig index 86b4bc5190..1fc7f0cb83 100644 --- a/configs/octeontx_83xx_defconfig +++ b/configs/octeontx_83xx_defconfig @@ -15,6 +15,7 @@ CONFIG_DEBUG_UART=y CONFIG_AHCI=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y +CONFIG_SUPPORT_RAW_INITRD=y CONFIG_OF_BOARD_SETUP=y CONFIG_BOOTDELAY=5 CONFIG_USE_BOOTARGS=y diff --git a/include/configs/octeontx_common.h b/include/configs/octeontx_common.h index 810b2bdbd5..55d329f55e 100644 --- a/include/configs/octeontx_common.h +++ b/include/configs/octeontx_common.h @@ -8,8 +8,6 @@ #ifndef __OCTEONTX_COMMON_H__ #define __OCTEONTX_COMMON_H__
-#define CONFIG_SUPPORT_RAW_INITRD
- /** Maximum size of image supported for bootm (and bootable FIT images) */ #define CONFIG_SYS_BOOTM_LEN (256 << 20)
Viele Grüße, Stefan
On Thu, Mar 25, 2021 at 05:07:32PM -0700, Tim Harvey wrote:
Move CONFIG_SUPPORT_RAW_INITRD out of the octeontx_common header and into the defconfig files.
Signed-off-by: Tim Harvey tharvey@gateworks.com Reviewed-by: Stefan Roese sr@denx.de
Applied to u-boot/master, thanks!
Support Generic Distro Default config
Signed-off-by: Tim Harvey tharvey@gateworks.com --- include/configs/octeontx_common.h | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/include/configs/octeontx_common.h b/include/configs/octeontx_common.h index 55d329f55e..434e54442f 100644 --- a/include/configs/octeontx_common.h +++ b/include/configs/octeontx_common.h @@ -8,6 +8,32 @@ #ifndef __OCTEONTX_COMMON_H__ #define __OCTEONTX_COMMON_H__
+#ifdef CONFIG_DISTRO_DEFAULTS +#define BOOT_TARGET_DEVICES(func) \ + func(MMC, mmc, 0) \ + func(MMC, mmc, 1) \ + func(USB, usb, 0) \ + func(SCSI, scsi, 0) + +#include <config_distro_bootcmd.h> +/* Extra environment variables */ +#define CONFIG_EXTRA_ENV_SETTINGS \ + "autoload=0\0" \ + "loadaddr=0x20080000\0" \ + "kernel_addr_r=0x02000000\0" \ + "ramdisk_addr_r=0x03000000\0" \ + "scriptaddr=0x04000000\0" \ + BOOTENV + +#else + +/** Extra environment settings */ +#define CONFIG_EXTRA_ENV_SETTINGS \ + "loadaddr=20080000\0" \ + "autoload=0\0" + +#endif /* ifdef CONFIG_DISTRO_DEFAULTS*/ + /** Maximum size of image supported for bootm (and bootable FIT images) */ #define CONFIG_SYS_BOOTM_LEN (256 << 20)
@@ -50,11 +76,6 @@ # define CONFIG_SF_DEFAULT_CS 0 #endif
-/** Extra environment settings */ -#define CONFIG_EXTRA_ENV_SETTINGS \ - "loadaddr=20080000\0" \ - "autoload=0\0" - /** Environment defines */ #if defined(CONFIG_ENV_IS_IN_MMC) #define CONFIG_SYS_MMC_ENV_DEV 0
On 26.03.21 01:07, Tim Harvey wrote:
Support Generic Distro Default config
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
include/configs/octeontx_common.h | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/include/configs/octeontx_common.h b/include/configs/octeontx_common.h index 55d329f55e..434e54442f 100644 --- a/include/configs/octeontx_common.h +++ b/include/configs/octeontx_common.h @@ -8,6 +8,32 @@ #ifndef __OCTEONTX_COMMON_H__ #define __OCTEONTX_COMMON_H__
+#ifdef CONFIG_DISTRO_DEFAULTS +#define BOOT_TARGET_DEVICES(func) \
- func(MMC, mmc, 0) \
- func(MMC, mmc, 1) \
- func(USB, usb, 0) \
- func(SCSI, scsi, 0)
+#include <config_distro_bootcmd.h> +/* Extra environment variables */ +#define CONFIG_EXTRA_ENV_SETTINGS \
- "autoload=0\0" \
- "loadaddr=0x20080000\0" \
- "kernel_addr_r=0x02000000\0" \
- "ramdisk_addr_r=0x03000000\0" \
- "scriptaddr=0x04000000\0" \
- BOOTENV
+#else
+/** Extra environment settings */ +#define CONFIG_EXTRA_ENV_SETTINGS \
- "loadaddr=20080000\0" \
- "autoload=0\0"
+#endif /* ifdef CONFIG_DISTRO_DEFAULTS*/
- /** Maximum size of image supported for bootm (and bootable FIT images) */ #define CONFIG_SYS_BOOTM_LEN (256 << 20)
@@ -50,11 +76,6 @@ # define CONFIG_SF_DEFAULT_CS 0 #endif
-/** Extra environment settings */ -#define CONFIG_EXTRA_ENV_SETTINGS \
"loadaddr=20080000\0" \"autoload=0\0"- /** Environment defines */ #if defined(CONFIG_ENV_IS_IN_MMC) #define CONFIG_SYS_MMC_ENV_DEV 0
Viele Grüße, Stefan
On Thu, Mar 25, 2021 at 05:07:33PM -0700, Tim Harvey wrote:
Support Generic Distro Default config
Signed-off-by: Tim Harvey tharvey@gateworks.com Reviewed-by: Stefan Roese sr@denx.de
Applied to u-boot/master, thanks!
The OcteonTX uses ARM's SBSA Watchdog device
Signed-off-by: Tim Harvey tharvey@gateworks.com --- configs/octeontx_81xx_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/octeontx_81xx_defconfig b/configs/octeontx_81xx_defconfig index 9881c1089b..bcf03db406 100644 --- a/configs/octeontx_81xx_defconfig +++ b/configs/octeontx_81xx_defconfig @@ -132,4 +132,5 @@ CONFIG_USB_ETHER_ASIX=y CONFIG_USB_ETHER_ASIX88179=y CONFIG_USB_ETHER_RTL8152=y CONFIG_WDT=y +CONFIG_WDT_SBSA=y CONFIG_ERRNO_STR=y
On 26.03.21 01:07, Tim Harvey wrote:
The OcteonTX uses ARM's SBSA Watchdog device
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
configs/octeontx_81xx_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/octeontx_81xx_defconfig b/configs/octeontx_81xx_defconfig index 9881c1089b..bcf03db406 100644 --- a/configs/octeontx_81xx_defconfig +++ b/configs/octeontx_81xx_defconfig @@ -132,4 +132,5 @@ CONFIG_USB_ETHER_ASIX=y CONFIG_USB_ETHER_ASIX88179=y CONFIG_USB_ETHER_RTL8152=y CONFIG_WDT=y +CONFIG_WDT_SBSA=y CONFIG_ERRNO_STR=y
Viele Grüße, Stefan
On Thu, Mar 25, 2021 at 05:07:34PM -0700, Tim Harvey wrote:
The OcteonTX uses ARM's SBSA Watchdog device
Signed-off-by: Tim Harvey tharvey@gateworks.com Reviewed-by: Stefan Roese sr@denx.de
Applied to u-boot/master, thanks!
Revert a change that occured between the Marvell SDK-10.1.1.0 and SDK-10.3.1.1 which broke QSMII phy support.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- drivers/net/octeontx/bgx.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index 2ea54be84d..a5c0c9fe2b 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -36,7 +36,6 @@ struct lmac { int dmac; u8 mac[6]; bool link_up; - bool init_pend; int lmacid; /* ID within BGX */ int phy_addr; /* ID on board */ struct udevice *dev; @@ -849,6 +848,7 @@ static int bgx_lmac_enable(struct bgx *bgx, int8_t lmacid) u64 cfg;
lmac = &bgx->lmac[lmacid]; + lmac->bgx = bgx;
debug("%s: lmac: %p, lmacid = %d\n", __func__, lmac, lmacid);
@@ -895,16 +895,6 @@ int bgx_poll_for_link(int node, int bgx_idx, int lmacid) debug("%s: %d, lmac: %d/%d/%d %p\n", __FILE__, __LINE__, node, bgx_idx, lmacid, lmac); - if (lmac->init_pend) { - ret = bgx_lmac_enable(lmac->bgx, lmacid); - if (ret < 0) { - printf("BGX%d LMAC%d lmac_enable failed\n", bgx_idx, - lmacid); - return ret; - } - lmac->init_pend = 0; - mdelay(100); - } if (lmac->qlm_mode == QLM_MODE_SGMII || lmac->qlm_mode == QLM_MODE_RGMII || lmac->qlm_mode == QLM_MODE_QSGMII) { @@ -1461,6 +1451,7 @@ int octeontx_bgx_remove(struct udevice *dev)
int octeontx_bgx_probe(struct udevice *dev) { + int err; struct bgx *bgx = dev_get_priv(dev); u8 lmac = 0; int qlm[4] = {-1, -1, -1, -1}; @@ -1540,8 +1531,11 @@ skip_qlm_config: struct lmac *tlmac = &bgx->lmac[lmac];
tlmac->dev = dev; - tlmac->init_pend = 1; - tlmac->bgx = bgx; + err = bgx_lmac_enable(bgx, lmac); + if (err) { + printf("BGX%d failed to enable lmac%d\n", + bgx->bgx_id, lmac); + } }
return 0;
On 26.03.21 01:07, Tim Harvey wrote:
Revert a change that occured between the Marvell SDK-10.1.1.0 and SDK-10.3.1.1 which broke QSMII phy support.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Thanks.
Suneel, do you have a comment on this? Is this revert the "best way" to handle this?
Thanks, Stefan
drivers/net/octeontx/bgx.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index 2ea54be84d..a5c0c9fe2b 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -36,7 +36,6 @@ struct lmac { int dmac; u8 mac[6]; bool link_up;
- bool init_pend; int lmacid; /* ID within BGX */ int phy_addr; /* ID on board */ struct udevice *dev;
@@ -849,6 +848,7 @@ static int bgx_lmac_enable(struct bgx *bgx, int8_t lmacid) u64 cfg;
lmac = &bgx->lmac[lmacid];
lmac->bgx = bgx;
debug("%s: lmac: %p, lmacid = %d\n", __func__, lmac, lmacid);
@@ -895,16 +895,6 @@ int bgx_poll_for_link(int node, int bgx_idx, int lmacid) debug("%s: %d, lmac: %d/%d/%d %p\n", __FILE__, __LINE__, node, bgx_idx, lmacid, lmac);
- if (lmac->init_pend) {
ret = bgx_lmac_enable(lmac->bgx, lmacid);if (ret < 0) {printf("BGX%d LMAC%d lmac_enable failed\n", bgx_idx,lmacid);return ret;}lmac->init_pend = 0;mdelay(100);- } if (lmac->qlm_mode == QLM_MODE_SGMII || lmac->qlm_mode == QLM_MODE_RGMII || lmac->qlm_mode == QLM_MODE_QSGMII) {
@@ -1461,6 +1451,7 @@ int octeontx_bgx_remove(struct udevice *dev)
int octeontx_bgx_probe(struct udevice *dev) {
- int err; struct bgx *bgx = dev_get_priv(dev); u8 lmac = 0; int qlm[4] = {-1, -1, -1, -1};
@@ -1540,8 +1531,11 @@ skip_qlm_config: struct lmac *tlmac = &bgx->lmac[lmac];
tlmac->dev = dev;
tlmac->init_pend = 1;tlmac->bgx = bgx;
err = bgx_lmac_enable(bgx, lmac);if (err) {printf("BGX%d failed to enable lmac%d\n",bgx->bgx_id, lmac);}}
return 0;
Viele Grüße, Stefan
This looks like a workaround than the root cause fix. As this patch just moves the bringup of link from probe stage to on-demand.
On Thu, Mar 25, 2021 at 11:46 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
Revert a change that occured between the Marvell SDK-10.1.1.0 and SDK-10.3.1.1 which broke QSMII phy support.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Thanks.
Suneel, do you have a comment on this? Is this revert the "best way" to handle this?
Thanks, Stefan
drivers/net/octeontx/bgx.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index 2ea54be84d..a5c0c9fe2b 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -36,7 +36,6 @@ struct lmac { int dmac; u8 mac[6]; bool link_up;
bool init_pend; int lmacid; /* ID within BGX */ int phy_addr; /* ID on board */ struct udevice *dev;@@ -849,6 +848,7 @@ static int bgx_lmac_enable(struct bgx *bgx, int8_t lmacid) u64 cfg;
lmac = &bgx->lmac[lmacid];
lmac->bgx = bgx; debug("%s: lmac: %p, lmacid = %d\n", __func__, lmac, lmacid);@@ -895,16 +895,6 @@ int bgx_poll_for_link(int node, int bgx_idx, int lmacid) debug("%s: %d, lmac: %d/%d/%d %p\n", __FILE__, __LINE__, node, bgx_idx, lmacid, lmac);
if (lmac->init_pend) {ret = bgx_lmac_enable(lmac->bgx, lmacid);if (ret < 0) {printf("BGX%d LMAC%d lmac_enable failed\n", bgx_idx,lmacid);return ret;}lmac->init_pend = 0;mdelay(100);} if (lmac->qlm_mode == QLM_MODE_SGMII || lmac->qlm_mode == QLM_MODE_RGMII || lmac->qlm_mode == QLM_MODE_QSGMII) {@@ -1461,6 +1451,7 @@ int octeontx_bgx_remove(struct udevice *dev)
int octeontx_bgx_probe(struct udevice *dev) {
int err; struct bgx *bgx = dev_get_priv(dev); u8 lmac = 0; int qlm[4] = {-1, -1, -1, -1};@@ -1540,8 +1531,11 @@ skip_qlm_config: struct lmac *tlmac = &bgx->lmac[lmac];
tlmac->dev = dev;
tlmac->init_pend = 1;tlmac->bgx = bgx;
err = bgx_lmac_enable(bgx, lmac);if (err) {printf("BGX%d failed to enable lmac%d\n",bgx->bgx_id, lmac);} } return 0;Viele Grüße, Stefan
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
On Fri, Mar 26, 2021 at 9:09 AM Suneel Garapati suneelglinux@gmail.com wrote:
This looks like a workaround than the root cause fix. As this patch just moves the bringup of link from probe stage to on-demand.
On Thu, Mar 25, 2021 at 11:46 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
Revert a change that occured between the Marvell SDK-10.1.1.0 and SDK-10.3.1.1 which broke QSMII phy support.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Thanks.
Suneel, do you have a comment on this? Is this revert the "best way" to handle this?
Thanks, Stefan
drivers/net/octeontx/bgx.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index 2ea54be84d..a5c0c9fe2b 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -36,7 +36,6 @@ struct lmac { int dmac; u8 mac[6]; bool link_up;
bool init_pend; int lmacid; /* ID within BGX */ int phy_addr; /* ID on board */ struct udevice *dev;@@ -849,6 +848,7 @@ static int bgx_lmac_enable(struct bgx *bgx, int8_t lmacid) u64 cfg;
lmac = &bgx->lmac[lmacid];
lmac->bgx = bgx; debug("%s: lmac: %p, lmacid = %d\n", __func__, lmac, lmacid);@@ -895,16 +895,6 @@ int bgx_poll_for_link(int node, int bgx_idx, int lmacid) debug("%s: %d, lmac: %d/%d/%d %p\n", __FILE__, __LINE__, node, bgx_idx, lmacid, lmac);
if (lmac->init_pend) {ret = bgx_lmac_enable(lmac->bgx, lmacid);if (ret < 0) {printf("BGX%d LMAC%d lmac_enable failed\n", bgx_idx,lmacid);return ret;}lmac->init_pend = 0;mdelay(100);} if (lmac->qlm_mode == QLM_MODE_SGMII || lmac->qlm_mode == QLM_MODE_RGMII || lmac->qlm_mode == QLM_MODE_QSGMII) {@@ -1461,6 +1451,7 @@ int octeontx_bgx_remove(struct udevice *dev)
int octeontx_bgx_probe(struct udevice *dev) {
int err; struct bgx *bgx = dev_get_priv(dev); u8 lmac = 0; int qlm[4] = {-1, -1, -1, -1};@@ -1540,8 +1531,11 @@ skip_qlm_config: struct lmac *tlmac = &bgx->lmac[lmac];
tlmac->dev = dev;
tlmac->init_pend = 1;tlmac->bgx = bgx;
err = bgx_lmac_enable(bgx, lmac);if (err) {printf("BGX%d failed to enable lmac%d\n",bgx->bgx_id, lmac);} } return 0;
Suneel,
I agree, it is a workaround and I don't quite understand the reason.
Can you look into the code history of the SDK BDK and see what the reason was for introducing this 'pending' state that defers the bgx_lmac_enable call that breaks QSGMII? That specific change was made between SDK-10.1.1.0 and SDK-10.3.1.1.
What boards and PHY's have you tested octeontx BGX with?
I have the following boards to test with that we designed and support: - GW630x: supports both RGMII and SGMII GbE PHY's - GW640x: supports both RGMII and QSGMII GbE PHy's
The only issue I have that is worked around by the above is QSGMII.
Best regards,
Tim
+ Chandra
On Fri, Mar 26, 2021 at 9:38 AM Tim Harvey tharvey@gateworks.com wrote:
On Fri, Mar 26, 2021 at 9:09 AM Suneel Garapati suneelglinux@gmail.com wrote:
This looks like a workaround than the root cause fix. As this patch just moves the bringup of link from probe stage to on-demand.
On Thu, Mar 25, 2021 at 11:46 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
Revert a change that occured between the Marvell SDK-10.1.1.0 and SDK-10.3.1.1 which broke QSMII phy support.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Thanks.
Suneel, do you have a comment on this? Is this revert the "best way" to handle this?
Thanks, Stefan
drivers/net/octeontx/bgx.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index 2ea54be84d..a5c0c9fe2b 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -36,7 +36,6 @@ struct lmac { int dmac; u8 mac[6]; bool link_up;
bool init_pend; int lmacid; /* ID within BGX */ int phy_addr; /* ID on board */ struct udevice *dev;@@ -849,6 +848,7 @@ static int bgx_lmac_enable(struct bgx *bgx, int8_t lmacid) u64 cfg;
lmac = &bgx->lmac[lmacid];
lmac->bgx = bgx; debug("%s: lmac: %p, lmacid = %d\n", __func__, lmac, lmacid);@@ -895,16 +895,6 @@ int bgx_poll_for_link(int node, int bgx_idx, int lmacid) debug("%s: %d, lmac: %d/%d/%d %p\n", __FILE__, __LINE__, node, bgx_idx, lmacid, lmac);
if (lmac->init_pend) {ret = bgx_lmac_enable(lmac->bgx, lmacid);if (ret < 0) {printf("BGX%d LMAC%d lmac_enable failed\n", bgx_idx,lmacid);return ret;}lmac->init_pend = 0;mdelay(100);} if (lmac->qlm_mode == QLM_MODE_SGMII || lmac->qlm_mode == QLM_MODE_RGMII || lmac->qlm_mode == QLM_MODE_QSGMII) {@@ -1461,6 +1451,7 @@ int octeontx_bgx_remove(struct udevice *dev)
int octeontx_bgx_probe(struct udevice *dev) {
int err; struct bgx *bgx = dev_get_priv(dev); u8 lmac = 0; int qlm[4] = {-1, -1, -1, -1};@@ -1540,8 +1531,11 @@ skip_qlm_config: struct lmac *tlmac = &bgx->lmac[lmac];
tlmac->dev = dev;
tlmac->init_pend = 1;tlmac->bgx = bgx;
err = bgx_lmac_enable(bgx, lmac);if (err) {printf("BGX%d failed to enable lmac%d\n",bgx->bgx_id, lmac);} } return 0;Suneel,
I agree, it is a workaround and I don't quite understand the reason.
Can you look into the code history of the SDK BDK and see what the reason was for introducing this 'pending' state that defers the bgx_lmac_enable call that breaks QSGMII? That specific change was made between SDK-10.1.1.0 and SDK-10.3.1.1.
What boards and PHY's have you tested octeontx BGX with?
I have the following boards to test with that we designed and support:
- GW630x: supports both RGMII and SGMII GbE PHY's
- GW640x: supports both RGMII and QSGMII GbE PHy's
The only issue I have that is worked around by the above is QSGMII.
Best regards,
Tim
On Fri, Mar 26, 2021 at 9:39 AM Suneel Garapati suneelglinux@gmail.com wrote:
- Chandra
On Fri, Mar 26, 2021 at 9:38 AM Tim Harvey tharvey@gateworks.com wrote:
On Fri, Mar 26, 2021 at 9:09 AM Suneel Garapati suneelglinux@gmail.com wrote:
This looks like a workaround than the root cause fix. As this patch just moves the bringup of link from probe stage to on-demand.
On Thu, Mar 25, 2021 at 11:46 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
Revert a change that occured between the Marvell SDK-10.1.1.0 and SDK-10.3.1.1 which broke QSMII phy support.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Thanks.
Suneel, do you have a comment on this? Is this revert the "best way" to handle this?
Thanks, Stefan
drivers/net/octeontx/bgx.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index 2ea54be84d..a5c0c9fe2b 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -36,7 +36,6 @@ struct lmac { int dmac; u8 mac[6]; bool link_up;
bool init_pend; int lmacid; /* ID within BGX */ int phy_addr; /* ID on board */ struct udevice *dev;@@ -849,6 +848,7 @@ static int bgx_lmac_enable(struct bgx *bgx, int8_t lmacid) u64 cfg;
lmac = &bgx->lmac[lmacid];
lmac->bgx = bgx; debug("%s: lmac: %p, lmacid = %d\n", __func__, lmac, lmacid);@@ -895,16 +895,6 @@ int bgx_poll_for_link(int node, int bgx_idx, int lmacid) debug("%s: %d, lmac: %d/%d/%d %p\n", __FILE__, __LINE__, node, bgx_idx, lmacid, lmac);
if (lmac->init_pend) {ret = bgx_lmac_enable(lmac->bgx, lmacid);if (ret < 0) {printf("BGX%d LMAC%d lmac_enable failed\n", bgx_idx,lmacid);return ret;}lmac->init_pend = 0;mdelay(100);} if (lmac->qlm_mode == QLM_MODE_SGMII || lmac->qlm_mode == QLM_MODE_RGMII || lmac->qlm_mode == QLM_MODE_QSGMII) {@@ -1461,6 +1451,7 @@ int octeontx_bgx_remove(struct udevice *dev)
int octeontx_bgx_probe(struct udevice *dev) {
int err; struct bgx *bgx = dev_get_priv(dev); u8 lmac = 0; int qlm[4] = {-1, -1, -1, -1};@@ -1540,8 +1531,11 @@ skip_qlm_config: struct lmac *tlmac = &bgx->lmac[lmac];
tlmac->dev = dev;
tlmac->init_pend = 1;tlmac->bgx = bgx;
err = bgx_lmac_enable(bgx, lmac);if (err) {printf("BGX%d failed to enable lmac%d\n",bgx->bgx_id, lmac);} } return 0;Suneel,
I agree, it is a workaround and I don't quite understand the reason.
Can you look into the code history of the SDK BDK and see what the reason was for introducing this 'pending' state that defers the bgx_lmac_enable call that breaks QSGMII? That specific change was made between SDK-10.1.1.0 and SDK-10.3.1.1.
What boards and PHY's have you tested octeontx BGX with?
I have the following boards to test with that we designed and support:
- GW630x: supports both RGMII and SGMII GbE PHY's
- GW640x: supports both RGMII and QSGMII GbE PHy's
The only issue I have that is worked around by the above is QSGMII.
Sunell / Chandrakala,
Any comments here about what PHY's you have tested octeontx BGX with and what the reasoning was for the code change here between Marvell SDK-10.1.1.0 and SDK-10.3.1.1?
Tim
On Thu, Mar 25, 2021 at 05:07:35PM -0700, Tim Harvey wrote:
Revert a change that occured between the Marvell SDK-10.1.1.0 and SDK-10.3.1.1 which broke QSMII phy support.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Applied to u-boot/master, thanks!
From: Suneel Garapati sgarapati@marvell.com
After check for maximum between max id and available ports, also check if available port count is less than max id and update.
In the case of the CN8030 OcteonTX SoC max_id needs to be reduced to the number of ports found otherwise the following occurs on a scan:
GW6404-B> scsi scan scanning bus for devices... Target spinup took 0 ms. AHCI 0001.0300 32 slots 1 ports 6 Gbps 0x1 impl SATA mode flags: 64bit ncq ilck stag pm led clo only pmp fbss pio slum part ccc apst Device 0: (0:0) Vendor: ATA Prod.: SanDisk SD8SFAT0 Rev: Z233 Type: Hard Disk Capacity: 61057.3 MB = 59.6 GB (125045424 x 512) "Synchronous Abort" handler, esr 0x96000006 elr: 000000000052f824 lr : 000000000052fa10 (reloc) elr: 000000007fee9824 lr : 000000007fee9a10 x0 : 0000000000000001 x1 : 0000000000000001 x2 : 000000007bea3528 x3 : 000000007bea3580 x4 : 0000000000000200 x5 : 0000000000000000 x6 : 0000000000000002 x7 : 000000007bea3540 x8 : 00000000fffffff8 x9 : 0000000000000008 x10: 00000000000186a0 x11: 000000000000000d x12: 0000000000000006 x13: 000000000001869f x14: 0000000000000007 x15: 00000000ffffffff x16: 000000007ff439a5 x17: 000000007ff5730c x18: 000000007bea9de0 x19: 000000007ff7a580 x20: 000000007bec79f8 x21: 0000000000000000 x22: 000000007bea3580 x23: 0000000000000000 x24: 0000000000000000 x25: 000000007bec7a00 x26: 00000000ffffffc0 x27: 000000007bec79d0 x28: 000000007beb51c0 x29: 000000007bea3480
Code: 91246800 940130c2 12800000 1400004f (b9402ae0) Resetting CPU ...
Signed-off-by: Suneel Garapati sgarapati@marvell.com --- drivers/ata/ahci.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 2ef21ec508..98b288254b 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1190,6 +1190,9 @@ int ahci_probe_scsi(struct udevice *ahci_dev, ulong base) */ uc_plat->max_id = max_t(unsigned long, uc_priv->n_ports, uc_plat->max_id); + /* If port count is less than max_id, update max_id */ + if (uc_priv->n_ports < uc_plat->max_id) + uc_plat->max_id = uc_priv->n_ports;
return 0; }
On 26.03.21 01:07, Tim Harvey wrote:
From: Suneel Garapati sgarapati@marvell.com
After check for maximum between max id and available ports, also check if available port count is less than max id and update.
In the case of the CN8030 OcteonTX SoC max_id needs to be reduced to the number of ports found otherwise the following occurs on a scan:
GW6404-B> scsi scan scanning bus for devices... Target spinup took 0 ms. AHCI 0001.0300 32 slots 1 ports 6 Gbps 0x1 impl SATA mode flags: 64bit ncq ilck stag pm led clo only pmp fbss pio slum part ccc apst Device 0: (0:0) Vendor: ATA Prod.: SanDisk SD8SFAT0 Rev: Z233 Type: Hard Disk Capacity: 61057.3 MB = 59.6 GB (125045424 x 512) "Synchronous Abort" handler, esr 0x96000006 elr: 000000000052f824 lr : 000000000052fa10 (reloc) elr: 000000007fee9824 lr : 000000007fee9a10 x0 : 0000000000000001 x1 : 0000000000000001 x2 : 000000007bea3528 x3 : 000000007bea3580 x4 : 0000000000000200 x5 : 0000000000000000 x6 : 0000000000000002 x7 : 000000007bea3540 x8 : 00000000fffffff8 x9 : 0000000000000008 x10: 00000000000186a0 x11: 000000000000000d x12: 0000000000000006 x13: 000000000001869f x14: 0000000000000007 x15: 00000000ffffffff x16: 000000007ff439a5 x17: 000000007ff5730c x18: 000000007bea9de0 x19: 000000007ff7a580 x20: 000000007bec79f8 x21: 0000000000000000 x22: 000000007bea3580 x23: 0000000000000000 x24: 0000000000000000 x25: 000000007bec7a00 x26: 00000000ffffffc0 x27: 000000007bec79d0 x28: 000000007beb51c0 x29: 000000007bea3480
Code: 91246800 940130c2 12800000 1400004f (b9402ae0) Resetting CPU ...
Signed-off-by: Suneel Garapati sgarapati@marvell.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/ata/ahci.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 2ef21ec508..98b288254b 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1190,6 +1190,9 @@ int ahci_probe_scsi(struct udevice *ahci_dev, ulong base) */ uc_plat->max_id = max_t(unsigned long, uc_priv->n_ports, uc_plat->max_id);
/* If port count is less than max_id, update max_id */
if (uc_priv->n_ports < uc_plat->max_id)
uc_plat->max_id = uc_priv->n_ports;return 0; }
Viele Grüße, Stefan
On Thu, Mar 25, 2021 at 05:07:36PM -0700, Tim Harvey wrote:
From: Suneel Garapati sgarapati@marvell.com
After check for maximum between max id and available ports, also check if available port count is less than max id and update.
In the case of the CN8030 OcteonTX SoC max_id needs to be reduced to the number of ports found otherwise the following occurs on a scan:
GW6404-B> scsi scan scanning bus for devices... Target spinup took 0 ms. AHCI 0001.0300 32 slots 1 ports 6 Gbps 0x1 impl SATA mode flags: 64bit ncq ilck stag pm led clo only pmp fbss pio slum part ccc apst Device 0: (0:0) Vendor: ATA Prod.: SanDisk SD8SFAT0 Rev: Z233 Type: Hard Disk Capacity: 61057.3 MB = 59.6 GB (125045424 x 512) "Synchronous Abort" handler, esr 0x96000006 elr: 000000000052f824 lr : 000000000052fa10 (reloc) elr: 000000007fee9824 lr : 000000007fee9a10 x0 : 0000000000000001 x1 : 0000000000000001 x2 : 000000007bea3528 x3 : 000000007bea3580 x4 : 0000000000000200 x5 : 0000000000000000 x6 : 0000000000000002 x7 : 000000007bea3540 x8 : 00000000fffffff8 x9 : 0000000000000008 x10: 00000000000186a0 x11: 000000000000000d x12: 0000000000000006 x13: 000000000001869f x14: 0000000000000007 x15: 00000000ffffffff x16: 000000007ff439a5 x17: 000000007ff5730c x18: 000000007bea9de0 x19: 000000007ff7a580 x20: 000000007bec79f8 x21: 0000000000000000 x22: 000000007bea3580 x23: 0000000000000000 x24: 0000000000000000 x25: 000000007bec7a00 x26: 00000000ffffffc0 x27: 000000007bec79d0 x28: 000000007beb51c0 x29: 000000007bea3480
Code: 91246800 940130c2 12800000 1400004f (b9402ae0) Resetting CPU ...
Signed-off-by: Suneel Garapati sgarapati@marvell.com Reviewed-by: Stefan Roese sr@denx.de
Applied to u-boot/master, thanks!
The fdt node offset is apparently not set properly when probed causing no MDIO busses to be found. Fix this by obtaining the offset.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- drivers/net/octeontx/smi.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 91dcd05e4b..27f4423c6a 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -325,6 +325,8 @@ int octeontx_smi_probe(struct udevice *dev) return -1; }
+ node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, + "cavium,thunder-8890-mdio-nexus"); fdt_for_each_subnode(subnode, gd->fdt_blob, node) { ret = fdt_node_check_compatible(gd->fdt_blob, subnode, "cavium,thunder-8890-mdio");
On 26.03.21 01:07, Tim Harvey wrote:
The fdt node offset is apparently not set properly when probed causing no MDIO busses to be found. Fix this by obtaining the offset.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/net/octeontx/smi.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 91dcd05e4b..27f4423c6a 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -325,6 +325,8 @@ int octeontx_smi_probe(struct udevice *dev) return -1; }
- node = fdt_node_offset_by_compatible(gd->fdt_blob, -1,
"cavium,thunder-8890-mdio-nexus");
Viele Grüße, Stefan
On Thu, Mar 25, 2021 at 11:48 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
The fdt node offset is apparently not set properly when probed causing no MDIO busses to be found. Fix this by obtaining the offset.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/net/octeontx/smi.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 91dcd05e4b..27f4423c6a 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -325,6 +325,8 @@ int octeontx_smi_probe(struct udevice *dev) return -1; }
node = fdt_node_offset_by_compatible(gd->fdt_blob, -1,"cavium,thunder-8890-mdio-nexus"); fdt_for_each_subnode(subnode, gd->fdt_blob, node) { ret = fdt_node_check_compatible(gd->fdt_blob, subnode, "cavium,thunder-8890-mdio");
Honestly this is the wrong fix for this issue and I'm hoping someone could educate me. I'm a bit confused at why there are several ways to work with dt (int offsets vs ofnodes which are unions of int offsets and node pointers???).
The above patch was not needed previously so something changed in the ofnode field of struct udevice between v2019.10 and v2021.01.
Simon, could you explain what the proper way to work with dev->ofnode in probe functions is to loop over subnodes?
Best regards,
Tim
Probably switch to Live tree API is needed like below. ofnode_for_each_subnode(subnode, dev_ofnode(dev)) { ret = ofnode_device_is_compatible(subnode, "cavium,thunder-8890-mdio");
Regards, Suneel
On Fri, Mar 26, 2021 at 8:56 AM Tim Harvey tharvey@gateworks.com wrote:
On Thu, Mar 25, 2021 at 11:48 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
The fdt node offset is apparently not set properly when probed causing no MDIO busses to be found. Fix this by obtaining the offset.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/net/octeontx/smi.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 91dcd05e4b..27f4423c6a 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -325,6 +325,8 @@ int octeontx_smi_probe(struct udevice *dev) return -1; }
node = fdt_node_offset_by_compatible(gd->fdt_blob, -1,"cavium,thunder-8890-mdio-nexus"); fdt_for_each_subnode(subnode, gd->fdt_blob, node) { ret = fdt_node_check_compatible(gd->fdt_blob, subnode, "cavium,thunder-8890-mdio");Honestly this is the wrong fix for this issue and I'm hoping someone could educate me. I'm a bit confused at why there are several ways to work with dt (int offsets vs ofnodes which are unions of int offsets and node pointers???).
The above patch was not needed previously so something changed in the ofnode field of struct udevice between v2019.10 and v2021.01.
Simon, could you explain what the proper way to work with dev->ofnode in probe functions is to loop over subnodes?
Best regards,
Tim
Hi Tim,
On 26.03.21 16:55, Tim Harvey wrote:
On Thu, Mar 25, 2021 at 11:48 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
The fdt node offset is apparently not set properly when probed causing no MDIO busses to be found. Fix this by obtaining the offset.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/net/octeontx/smi.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 91dcd05e4b..27f4423c6a 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -325,6 +325,8 @@ int octeontx_smi_probe(struct udevice *dev) return -1; }
node = fdt_node_offset_by_compatible(gd->fdt_blob, -1,"cavium,thunder-8890-mdio-nexus"); fdt_for_each_subnode(subnode, gd->fdt_blob, node) { ret = fdt_node_check_compatible(gd->fdt_blob, subnode, "cavium,thunder-8890-mdio");Honestly this is the wrong fix for this issue and I'm hoping someone could educate me. I'm a bit confused at why there are several ways to work with dt (int offsets vs ofnodes which are unions of int offsets and node pointers???).
The above patch was not needed previously so something changed in the ofnode field of struct udevice between v2019.10 and v2021.01.
Simon, could you explain what the proper way to work with dev->ofnode in probe functions is to loop over subnodes?
This version is in mainline now. Tim, could you please re-visit this and perhaps switch to using live tree API, as suggested by Suneel:
ofnode_for_each_subnode(subnode, dev_ofnode(dev)) { ret = ofnode_device_is_compatible(subnode, "cavium,thunder-8890-mdio");
Thanks, Stefan
On Mon, Apr 26, 2021 at 10:19 PM Stefan Roese sr@denx.de wrote:
Hi Tim,
On 26.03.21 16:55, Tim Harvey wrote:
On Thu, Mar 25, 2021 at 11:48 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
The fdt node offset is apparently not set properly when probed causing no MDIO busses to be found. Fix this by obtaining the offset.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/net/octeontx/smi.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 91dcd05e4b..27f4423c6a 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -325,6 +325,8 @@ int octeontx_smi_probe(struct udevice *dev) return -1; }
node = fdt_node_offset_by_compatible(gd->fdt_blob, -1,"cavium,thunder-8890-mdio-nexus"); fdt_for_each_subnode(subnode, gd->fdt_blob, node) { ret = fdt_node_check_compatible(gd->fdt_blob, subnode, "cavium,thunder-8890-mdio");Honestly this is the wrong fix for this issue and I'm hoping someone could educate me. I'm a bit confused at why there are several ways to work with dt (int offsets vs ofnodes which are unions of int offsets and node pointers???).
The above patch was not needed previously so something changed in the ofnode field of struct udevice between v2019.10 and v2021.01.
Simon, could you explain what the proper way to work with dev->ofnode in probe functions is to loop over subnodes?
This version is in mainline now. Tim, could you please re-visit this and perhaps switch to using live tree API, as suggested by Suneel:
ofnode_for_each_subnode(subnode, dev_ofnode(dev)) { ret = ofnode_device_is_compatible(subnode, "cavium,thunder-8890-mdio");
Stefan,
Yes, I can submit this but I would really like to understand the original issue. Do you or Simon perhaps know why the fdt node offset in dev passed to probe is wrong? It's not null but it does not appear to point to a device-tree (or perhaps I was using the wrong functions on it not fully understanding the current state of this live tree API).
Best regards,
Tim
Hi Tim,
On 28.04.21 17:11, Tim Harvey wrote:
On Mon, Apr 26, 2021 at 10:19 PM Stefan Roese sr@denx.de wrote:
Hi Tim,
On 26.03.21 16:55, Tim Harvey wrote:
On Thu, Mar 25, 2021 at 11:48 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
The fdt node offset is apparently not set properly when probed causing no MDIO busses to be found. Fix this by obtaining the offset.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/net/octeontx/smi.c | 2 ++ 1 file changed, 2 insertions(+)diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 91dcd05e4b..27f4423c6a 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -325,6 +325,8 @@ int octeontx_smi_probe(struct udevice *dev) return -1; }
node = fdt_node_offset_by_compatible(gd->fdt_blob, -1,"cavium,thunder-8890-mdio-nexus"); fdt_for_each_subnode(subnode, gd->fdt_blob, node) { ret = fdt_node_check_compatible(gd->fdt_blob, subnode, "cavium,thunder-8890-mdio");Honestly this is the wrong fix for this issue and I'm hoping someone could educate me. I'm a bit confused at why there are several ways to work with dt (int offsets vs ofnodes which are unions of int offsets and node pointers???).
The above patch was not needed previously so something changed in the ofnode field of struct udevice between v2019.10 and v2021.01.
Simon, could you explain what the proper way to work with dev->ofnode in probe functions is to loop over subnodes?
This version is in mainline now. Tim, could you please re-visit this and perhaps switch to using live tree API, as suggested by Suneel:
ofnode_for_each_subnode(subnode, dev_ofnode(dev)) { ret = ofnode_device_is_compatible(subnode, "cavium,thunder-8890-mdio");Stefan,
Yes, I can submit this but I would really like to understand the original issue. Do you or Simon perhaps know why the fdt node offset in dev passed to probe is wrong? It's not null but it does not appear to point to a device-tree (or perhaps I was using the wrong functions on it not fully understanding the current state of this live tree API).
I don't have an OcteonTX board installed right now, so it's not easy to really verify this. AFAIU, fdt_for_each_subnode() etc is deprecated and the use of e.g. this API seems "more modern":
ofnode subnode;
dev_for_each_subnode(subnode, dev) { ...
Does this work for you?
Thanks, Stefan
On Wed, Apr 28, 2021 at 10:21 PM Stefan Roese sr@denx.de wrote:
Hi Tim,
On 28.04.21 17:11, Tim Harvey wrote:
On Mon, Apr 26, 2021 at 10:19 PM Stefan Roese sr@denx.de wrote:
Hi Tim,
On 26.03.21 16:55, Tim Harvey wrote:
On Thu, Mar 25, 2021 at 11:48 PM Stefan Roese sr@denx.de wrote:
On 26.03.21 01:07, Tim Harvey wrote:
The fdt node offset is apparently not set properly when probed causing no MDIO busses to be found. Fix this by obtaining the offset.
Signed-off-by: Tim Harvey tharvey@gateworks.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/net/octeontx/smi.c | 2 ++ 1 file changed, 2 insertions(+)diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 91dcd05e4b..27f4423c6a 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -325,6 +325,8 @@ int octeontx_smi_probe(struct udevice *dev) return -1; }
node = fdt_node_offset_by_compatible(gd->fdt_blob, -1,"cavium,thunder-8890-mdio-nexus"); fdt_for_each_subnode(subnode, gd->fdt_blob, node) { ret = fdt_node_check_compatible(gd->fdt_blob, subnode, "cavium,thunder-8890-mdio");Honestly this is the wrong fix for this issue and I'm hoping someone could educate me. I'm a bit confused at why there are several ways to work with dt (int offsets vs ofnodes which are unions of int offsets and node pointers???).
The above patch was not needed previously so something changed in the ofnode field of struct udevice between v2019.10 and v2021.01.
Simon, could you explain what the proper way to work with dev->ofnode in probe functions is to loop over subnodes?
This version is in mainline now. Tim, could you please re-visit this and perhaps switch to using live tree API, as suggested by Suneel:
ofnode_for_each_subnode(subnode, dev_ofnode(dev)) { ret = ofnode_device_is_compatible(subnode, "cavium,thunder-8890-mdio");Stefan,
Yes, I can submit this but I would really like to understand the original issue. Do you or Simon perhaps know why the fdt node offset in dev passed to probe is wrong? It's not null but it does not appear to point to a device-tree (or perhaps I was using the wrong functions on it not fully understanding the current state of this live tree API).
I don't have an OcteonTX board installed right now, so it's not easy to really verify this. AFAIU, fdt_for_each_subnode() etc is deprecated and the use of e.g. this API seems "more modern":
ofnode subnode; dev_for_each_subnode(subnode, dev) { ...Does this work for you?
Stefan,
Yes thanks - I didn't read your original suggestion closely enough. I have a patch I will submit.
Thanks,
Tim
On Thu, Mar 25, 2021 at 05:07:37PM -0700, Tim Harvey wrote:
The fdt node offset is apparently not set properly when probed causing no MDIO busses to be found. Fix this by obtaining the offset.
Signed-off-by: Tim Harvey tharvey@gateworks.com Reviewed-by: Stefan Roese sr@denx.de
Applied to u-boot/master, thanks!
Hi Tim,
On 26.03.21 01:07, Tim Harvey wrote:
This series provides the following for octeontx:
- cleanup to the octeontx common header file by moving CONFIG_SUPPORT_RAW_INITRD to defconfig files,
- adds generic distro config support
- add missing SBSA watchdog to config
- fix an issue found with QML connected to a QSGMII PHY
- fix an issue with mii probe failing to find mdio busses
- fix a crash when scanning for sata devices
These were tested on a Gateworks Newport GW6404 with the CN8030 SoC.
Many thanks for working on this. Do you plan to send your GW6404 support to the list for upstreaming as well?
Thanks, Stefan
On Thu, Mar 25, 2021 at 11:49 PM Stefan Roese sr@denx.de wrote:
Hi Tim,
On 26.03.21 01:07, Tim Harvey wrote:
This series provides the following for octeontx:
- cleanup to the octeontx common header file by moving CONFIG_SUPPORT_RAW_INITRD to defconfig files,
- adds generic distro config support
- add missing SBSA watchdog to config
- fix an issue found with QML connected to a QSGMII PHY
- fix an issue with mii probe failing to find mdio busses
- fix a crash when scanning for sata devices
These were tested on a Gateworks Newport GW6404 with the CN8030 SoC.
Many thanks for working on this. Do you plan to send your GW6404 support to the list for upstreaming as well?
Stefan / Aaron / Suneel,
Many thanks to you all for keeping up on the octeontx/tx2 support - I was happy to notice it finally all got merged.
On top of this series the GW6404 works just fine with nothing other than it's own defconfig. The items I change from the octentx_81xx_defconfig are: - env location and redundancy (board specific so this makes sense) - enabling of generic distro config (which honestly I think every development board from vendors should be using) - CONFIG_SYS_TEXT_BASE: octeontx_81xx_defconfig uses 0x2800000 which makes my board crash. I use 0x500000 which is fine
I can't seem to find the physical memory map for the CN8030 to explain the crash with the different text addr... it's likely that the 0x2800000 is suitable for the CN81xx but not the CN80xx due to the difference in memory map. I know the CN80xx has half the L2 cache as the CN81xx so perhaps this is the reason.
What board are you testing U-Boot octentx with? Do you test with CN803x at all? The Marvell v10 SDK doesn't support the CN80xx because the BDK boot stub became so bloated it didn't fit into its L2 cache (which is half the size of the CN81xx).
Is there any support coming from Marvell to support U-Boot SPL as a replacement for the horribly slow and bulky BDK for OcteonTX?
What is used as an SPL for the OcteonTX2? We are working on a CN91xx board design and I'm waiting for the Marvell SDK v11 drop before I dig too deeply into what U-Boot support is there. I know the CN91xx reference board I had access to booted to U-Boot extremely fast so the BDK must not be used.
I do also have some proposed changes to board/Marvell/octeontx/board-fdt.c ft_board_setup to allow booting with a loaded DTB that I'll probably submit sometime soon.
Best regards,
Tim
participants (4)
-
Stefan Roese -
Suneel Garapati -
Tim Harvey -
Tom Rini