Re: [PATCH 1/5] net: eth-uclass: guard against reentrant eth_init()/eth_halt() calls
On Fri, Apr 26, 2024 at 10:02:24AM +0200, Matthias Schiffer wrote: > With netconsole, any log message can result in an eth_init(), possibly > causing an reentrant call into eth_init() if a driver's ops print > anything: > > eth_init() -> driver.start() -> printf() -> netconsole -> eth_init() > eth_halt() -> driver.stop() -> printf() -> netconsole -> eth_init() > > Rather than expecting every single Ethernet driver to handle this case, > prevent the reentrant calls in eth_init() and eth_halt(). > > The issue was noticed on an AM62x board, where a bootm after > simultaneous netconsole and TFTP would result in a crash. Link: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1350550/sk-am62a-lp-am65_cpsw_nuss_port-error-in-u-boot-while-using-netconsole-functionality/ > > Signed-off-by: Matthias Schiffer > --- > net/eth-uclass.c | 40 > 1 file changed, 32 insertions(+), 8 deletions(-) > > diff --git a/net/eth-uclass.c b/net/eth-uclass.c > index 3d0ec91dfa4..193218a328b 100644 > --- a/net/eth-uclass.c > +++ b/net/eth-uclass.c > @@ -48,6 +48,8 @@ struct eth_uclass_priv { > > /* eth_errno - This stores the most recent failure code from DM functions */ > static int eth_errno; > +/* Are we currently in eth_init() or eth_halt()? */ > +static bool in_init_halt; > > /* board-specific Ethernet Interface initializations. */ > __weak int board_interface_eth_init(struct udevice *dev, > @@ -285,11 +287,19 @@ U_BOOT_ENV_CALLBACK(ethaddr, on_ethaddr); > > int eth_init(void) > { > - char *ethact = env_get("ethact"); > - char *ethrotate = env_get("ethrotate"); > struct udevice *current = NULL; > struct udevice *old_current; > int ret = -ENODEV; > + char *ethrotate; > + char *ethact; > + > + if (in_init_halt) > + return -EBUSY; > + > + in_init_halt = true; > + > + ethact = env_get("ethact"); > + ethrotate = env_get("ethrotate"); > > /* >* When 'ethrotate' variable is set to 'no' and 'ethact' variable > @@ -298,8 +308,10 @@ int eth_init(void) > if ((ethrotate != NULL) && (strcmp(ethrotate, "no") == 0)) { > if (ethact) { > current = eth_get_dev_by_name(ethact); > - if (!current) > - return -EINVAL; > + if (!current) { > + ret = -EINVAL; > + goto end; > + } > } > } > > @@ -307,7 +319,8 @@ int eth_init(void) > current = eth_get_dev(); > if (!current) { > log_err("No ethernet found.\n"); > - return -ENODEV; > + ret = -ENODEV; > + goto end; > } > } > > @@ -324,7 +337,8 @@ int eth_init(void) > > priv->state = ETH_STATE_ACTIVE; > priv->running = true; > - return 0; > + ret = 0; > + goto end; > } > } else { > ret = eth_errno; > @@ -344,6 +358,8 @@ int eth_init(void) > current = eth_get_dev(); > } while (old_current != current); > > +end: > + in_init_halt = false; > return ret; > } > > @@ -352,17 +368,25 @@ void eth_halt(void) > struct udevice *current; > struct eth_device_priv *priv; > > + if (in_init_halt) > + return; > + > + in_init_halt = true; > + > current = eth_get_dev(); > if (!current) > - return; > + goto end; > > priv = dev_get_uclass_priv(current); > if (!priv || !priv->running) > - return; > + goto end; > > eth_get_ops(current)->stop(current); > priv->state = ETH_STATE_PASSIVE; > priv->running = false; > + > +end: > + in_init_halt = false; > } > > int eth_is_active(struct udevice *dev) > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > https://www.tq-group.com/ >
Re: [PATCH v2] net: ti: am65-cpsw-nuss: handle missing PHY in am65_cpsw_phy_init() gracefully
On Thu, Mar 28, 2024 at 07:29:10AM +0100, A. Sverdlin wrote: > From: Alexander Sverdlin > > am65_cpsw_ofdata_parse_phy() tries to handle the case when PHY is not > specified in DT gracefully: > > am65_cpsw_nuss_port ethernet@800port@1: can't parse phy-handle port 1 (-2) > > am65_cpsw_mdio_init() in turn is prepared for this, checks > if priv->has_phy == 0 and bails out (leaving cpsw_common->bus == NULL). > > am65_cpsw_phy_init() however is not prepared for this and calls > phy_connect(cpsw_common->bus, ...) unconditionally, which leads to: > > "Synchronous Abort" handler, esr 0x860d, far 0x0 > elr: 808ab000 lr : 8083bde4 (reloc) > > where lr points to the instruction right after bus->read() in get_phy_id(). > > Fixes: 9d0dca1199d1 ("net: ethernet: ti: Introduce am654 gigabit eth switch > subsystem driver") > Signed-off-by: Alexander Sverdlin Reviewed-by: Siddharth Vadapalli Regards, Siddharth.
Re: [PATCH] net: ti: am65-cpsw-nuss: Don't crash in am65_cpsw_phy_init()
On Wed, Mar 27, 2024 at 06:09:40PM +0100, A. Sverdlin wrote: > From: Alexander Sverdlin In the $subject, "Don't crash.." seems to indicate the consequence of not handling the absence of PHYs gracefully within am65_cpsw_phy_init(). So the subject should probably be reworded to emphasize this instead. Something like: net: ti: am65-cpsw-nuss: Update am65_cpsw_phy_init() to handle PHY absence The rest of the patch looks good to me. With the subject reworded, Reviewed-by: Siddharth Vadapalli Regards, Siddharth. > > am65_cpsw_ofdata_parse_phy() tries to handle the case when PHY is not > specified in DT gracefully: > > am65_cpsw_nuss_port ethernet@800port@1: can't parse phy-handle port 1 (-2) > > am65_cpsw_mdio_init() is turn is prepared for this, checks > if priv->has_phy == 0 and bails out (leaving cpsw_common->bus == NULL). > > am65_cpsw_phy_init() however is not prepared for this and calls > phy_connect(cpsw_common->bus, ...) unconditionally, which leads to: > > "Synchronous Abort" handler, esr 0x860d, far 0x0 > elr: 808ab000 lr : 8083bde4 (reloc) > > where lr points to the instruction right after bus->read() in get_phy_id(). > > Fixes: 9d0dca1199d1 ("net: ethernet: ti: Introduce am654 gigabit eth switch > subsystem driver") > Signed-off-by: Alexander Sverdlin > --- > drivers/net/ti/am65-cpsw-nuss.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c > index 6da018c0f9d5..d1e452b6b43c 100644 > --- a/drivers/net/ti/am65-cpsw-nuss.c > +++ b/drivers/net/ti/am65-cpsw-nuss.c > @@ -722,6 +722,9 @@ static int am65_cpsw_phy_init(struct udevice *dev) > u32 supported = PHY_GBIT_FEATURES; > int ret; > > + if (!priv->has_phy || !cpsw_common->bus) > + return 0; > + > phydev = phy_connect(cpsw_common->bus, >priv->phy_addr, >priv->dev, > -- > 2.44.0 >
Re: [PATCH] dma: ti: k3-udma: Fix error handling for setup_resources() in udma_probe()
On 24/02/20 03:24PM, Siddharth Vadapalli wrote: > On 24/02/20 12:51PM, Dan Carpenter wrote: > > On Fri, Feb 16, 2024 at 04:11:05PM +0530, Siddharth Vadapalli wrote: > > > diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c > > > index eea9ec9659..8a6625f034 100644 > > > --- a/drivers/dma/ti/k3-udma.c > > > +++ b/drivers/dma/ti/k3-udma.c > > > @@ -1770,9 +1770,11 @@ static int udma_probe(struct udevice *dev) > > > return PTR_ERR(ud->ringacc); > > > > > > ud->dev = dev; > > > - ud->ch_count = setup_resources(ud); > > > - if (ud->ch_count <= 0) > > > - return ud->ch_count; > > > + ret = setup_resources(ud); > > > + if (ret <= 0) > > > + return ret; > > > > The code was like this originally, but setup_resources() can't actually > > return zero so it would be nicer to say: > > ret = setup_resources(ud); > > if (ret < 0) > > return ret; > > Thank you for reviewing the patch and pointing this out. I will fix it > and post the v2 patch. I have posted the v2 patch at: https://patchwork.ozlabs.org/project/uboot/patch/20240220100451.1053667-1-s-vadapa...@ti.com/ Regards, Siddharth.
[PATCH v2] dma: ti: k3-udma: Fix error handling for setup_resources() in udma_probe()
In udma_probe() the return value of setup_resources() is stored in the u32 "ch_count" member of "struct udma_dev", due to which any negative return value which indicates an error is masked. Fix this by storing the return value of setup_resources() in the already declared integer variable "ret", followed by assigning it to the "ch_count" member of "struct udma_dev" in case of no error. While at it, change the "return ret" at the end of udma_probe() to a "return 0", to explicitly indicate that probe was successful. Fixes: a8837cf43839 ("dma: ti: k3-udma: Query DMA channels allocated from Resource Manager") Signed-off-by: Siddharth Vadapalli --- Hello, This patch is based on commit 3e6f2a94bf Merge tag 'u-boot-imx-master-20240219' of https://gitlab.denx.de/u-boot/custodians/u-boot-imx of the master branch of U-Boot. v1: https://patchwork.ozlabs.org/project/uboot/patch/20240216104105.494602-1-s-vadapa...@ti.com/ Changes since v1: - Rebased patch on latest U-Boot tree. - As pointed out by Dan in reply to the v1 patch, since the return value of setup_resources() cannot be zero, update the check performed on the return value accordingly. Regards, Siddharth. drivers/dma/ti/k3-udma.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index eea9ec9659..ed0a9bf487 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -1770,9 +1770,11 @@ static int udma_probe(struct udevice *dev) return PTR_ERR(ud->ringacc); ud->dev = dev; - ud->ch_count = setup_resources(ud); - if (ud->ch_count <= 0) - return ud->ch_count; + ret = setup_resources(ud); + if (ret < 0) + return ret; + + ud->ch_count = ret; for (i = 0; i < ud->bchan_cnt; i++) { struct udma_bchan *bchan = >bchans[i]; @@ -1831,7 +1833,7 @@ static int udma_probe(struct udevice *dev) uc_priv->supported = DMA_SUPPORTS_MEM_TO_MEM | DMA_SUPPORTS_MEM_TO_DEV; - return ret; + return 0; } static int udma_push_to_ring(struct k3_nav_ring *ring, void *elem) -- 2.34.1
Re: [PATCH] dma: ti: k3-udma: Fix error handling for setup_resources() in udma_probe()
On 24/02/20 12:51PM, Dan Carpenter wrote: > On Fri, Feb 16, 2024 at 04:11:05PM +0530, Siddharth Vadapalli wrote: > > diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c > > index eea9ec9659..8a6625f034 100644 > > --- a/drivers/dma/ti/k3-udma.c > > +++ b/drivers/dma/ti/k3-udma.c > > @@ -1770,9 +1770,11 @@ static int udma_probe(struct udevice *dev) > > return PTR_ERR(ud->ringacc); > > > > ud->dev = dev; > > - ud->ch_count = setup_resources(ud); > > - if (ud->ch_count <= 0) > > - return ud->ch_count; > > + ret = setup_resources(ud); > > + if (ret <= 0) > > + return ret; > > The code was like this originally, but setup_resources() can't actually > return zero so it would be nicer to say: > ret = setup_resources(ud); > if (ret < 0) > return ret; Thank you for reviewing the patch and pointing this out. I will fix it and post the v2 patch. Regards, Siddharth.
[PATCH] dma: ti: k3-udma: Fix error handling for setup_resources() in udma_probe()
In udma_probe() the return value of setup_resources() is stored in the u32 "ch_count" member of "struct udma_dev", due to which any negative return value which indicates an error is masked. Fix this by storing the return value of setup_resources() in the already declared integer variable "ret", followed by assigning it to the "ch_count" member of "struct udma_dev" in case of no error. While at it, change the "return ret" at the end of udma_probe() to a "return 0", to explicitly indicate that probe was successful. Fixes: a8837cf43839 ("dma: ti: k3-udma: Query DMA channels allocated from Resource Manager") Signed-off-by: Siddharth Vadapalli --- Hello, This patch is based on commit 9e00b6993f Merge tag 'u-boot-dfu-20240215' of https://source.denx.de/u-boot/custodians/u-boot-dfu of the master branch of U-Boot. Regards, Siddharth. drivers/dma/ti/k3-udma.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index eea9ec9659..8a6625f034 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -1770,9 +1770,11 @@ static int udma_probe(struct udevice *dev) return PTR_ERR(ud->ringacc); ud->dev = dev; - ud->ch_count = setup_resources(ud); - if (ud->ch_count <= 0) - return ud->ch_count; + ret = setup_resources(ud); + if (ret <= 0) + return ret; + + ud->ch_count = ret; for (i = 0; i < ud->bchan_cnt; i++) { struct udma_bchan *bchan = >bchans[i]; @@ -1831,7 +1833,7 @@ static int udma_probe(struct udevice *dev) uc_priv->supported = DMA_SUPPORTS_MEM_TO_MEM | DMA_SUPPORTS_MEM_TO_DEV; - return ret; + return 0; } static int udma_push_to_ring(struct k3_nav_ring *ring, void *elem) -- 2.34.1
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
On 20/01/24 22:11, Tom Rini wrote: > On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote: >> Hello Tom, >> >> On 12/01/24 18:56, Tom Rini wrote: ... >>> The list of conditionals in common/spl/spl.c::board_init_r() should be >>> updated and probably use SPL_NET as the option to check for. >> >> Thank you for reviewing the patch and pointing this out. I wasn't aware of >> it. I >> assume that you are referring to the following change: >> >> if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || >> - IS_ENABLED(CONFIG_SPL_ATF)) >> + IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET)) >> dram_init_banksize(); >> >> I shall replace the current patch with the above change in the v2 series. >> Since >> this is in the common section, is there a generic reason I could provide in >> the >> commit message rather than the existing commit message which seems to be >> board >> specific? Also, I hope that the above change will not cause regressions for >> other non-TI devices. Please let me know. > > Yes, that's the area, and just note that networking also requires the > DDR to be initialized. > Thank you for confirming and providing your suggestion for the contents of the commit message. -- Regards, Siddharth.
Re: [PATCH 00/10] Add support for Ethernet Boot on SK-AM62
On 12/01/24 18:31, Nishanth Menon wrote: > On 18:17-20240112, Siddharth Vadapalli wrote: >> >> >> On 12/01/24 18:12, Nishanth Menon wrote: >>> On 18:06-20240112, Siddharth Vadapalli wrote: >>>> >>>> >>>> On 12/01/24 18:02, Nishanth Menon wrote: >>>>> On 12:17-20240112, Siddharth Vadapalli wrote: >>>>>> Hello, >>>>>> >>>>>> This series enables Ethernet Boot on SK-AM62 device. >>>>>> Product Link: https://www.ti.com/tool/SK-AM62 >>>>>> User Guide: https://www.ti.com/lit/pdf/spruj40 >>>>>> >>>>>> Ethernet Boot flow is as follows: >>>>>> 1. The BOOT MODE pins are configured for Ethernet Boot. >>>>>> 2. On powering on the device, ROM uses the Ethernet Port corresponding >>>>>> to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot". >>>>>> 3. The TFTP server and DHCP server on the receiver device need to be >>>>>> configured such that VCI string "TI K3 Bootp Boot" maps to the file >>>>>> "tiboot3.bin" and the TFTP server should be capable of transferring >>>>>> it to the device. >>>>>> 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server. >>>>>> 5. The "tiboot3.bin" file is expected to be built using the config: >>>>>> am62x_evm_r5_ethboot_defconfig >>>>>> introduced in this series, which shall enable "tispl.bin" to be fetched >>>>>> over TFTP using "tiboot3.bin". >>>>>> 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its >>>>>> NET_VCI_STRING, thereby implying that the DHCP server and TFTP server >>>>>> need to be configured to transfer "tispl.bin" to the device. >>>>>> 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP >>>>>> server. >>>>>> 8. The "tispl.bin" file is expected to be built using the config: >>>>>> am62x_evm_a53_defconfig >>>>>> which has been updated in this series to enable Ethernet Boot specific >>>>>> configs, allowing "u-boot.img" to be fetched over TFTP using >>>>>> "tispl.bin". >>>>>> 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its >>>>>> NET_VCI_STRING. The DHCP server and TFTP server need to be configured to >>>>>> transfer "u-boot.img" to the device for the aforementioned >>>>>> NET_VCI_STRING. >>>>>> 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and >>>>>> executes it on the device, completing the process of Ethernet Boot on the >>>>>> device. >>>>>> >>>>>> NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode >>>>>> of operation due to which it is expected that the Link Partner also >>>>>> supports the same mode of operation. >>>>>> Additionally, enabling "phy_gmii_sel" node at SPL stage will be >>>>>> necessary and is not added as a part of this series with the aim of >>>>>> adding the "bootph-all" property to its counterpart in Linux device-tree >>>>>> first. >>>>> >>>>> >>>>> NAK - instead of writing all this up in the commit message, why would >>>>> you not spend that time updating the excellent documentation we have? >>>> >>>> I plan to document it after the feature is in. The reason for mentioning >>>> the >>>> content above is for explaining the flow in case anyone wishes to test and >>>> verify it. Wouldn't documenting it make it appear that the feature is >>>> present >>>> when it isn't? >>> >>> So you are saying this series does NOT work! why are you sending patches >>> then? If you are introducing a feature and enabling it, ensure you send >>> documentation along with it allowing people to be able to actually use >>> the feature. >> >> I have mentioned in the "NOTE" above that enabling "phy_gmii_sel" node at SPL >> stage by adding the "bootph-all" property is necessary to verify this series. >> I cannot post that with this series since Linux device-tree needs to have the >> property added first and the merge window is closed now. Once it is in the >> Linux >> device-tree, syncing U-Boot device-tree with Linux device-tree will enable >> Ethernet Boot which is when the feature will work. That is when I was >> planning >> to document it. However, based on your feedback, in the next version for this >> series I will add the documentation as well along with the note that >> "phy_gmii_sel" needs to be enabled at SPL stage for the feature to work. >> > > considered first posting a patch to kernel.org (merge window has > nothing to do with posting and having patches reviewed) and in the > interim, doing that in u-boot.dtsi so that the next sync will drop it > from u-boot.dtsi? > > OR hold the series back till it is merged into kernel.org master? > > Either way, please do not send patches to the list that does not work. > > Since it is now hard to trust your patches without detailed cover letter > analysis, next time you are updating the series post test logs as well > with just the patches applied and no additional changes. Thank you for clarifying regarding the approach to be taken. I shall include the logs when I post the v2 series, in addition to the Documentation update. > -- Regards, Siddharth.
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
Hello Tom, On 12/01/24 18:56, Tom Rini wrote: > On Fri, Jan 12, 2024 at 12:17:50PM +0530, Siddharth Vadapalli wrote: > >> From: Kishon Vijay Abraham I >> >> Call dram_init_banksize() from spl_board_init() otherwise TFTP download >> fails with error "TFTP error: trying to overwrite reserved memory..." >> due to lmb_get_free_size() not able to find unreserved region due >> to lack of DRAM size info. Required to support Ethernet boot on AM62x. >> >> Signed-off-by: Kishon Vijay Abraham I >> Signed-off-by: Siddharth Vadapalli >> --- >> board/ti/am62x/evm.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/board/ti/am62x/evm.c b/board/ti/am62x/evm.c >> index ad93908840..35f291d83a 100644 >> --- a/board/ti/am62x/evm.c >> +++ b/board/ti/am62x/evm.c >> @@ -85,6 +85,9 @@ void spl_board_init(void) >> if (IS_ENABLED(CONFIG_SPL_SPLASH_SCREEN) && IS_ENABLED(CONFIG_SPL_BMP)) >> splash_display(); >> >> +if (IS_ENABLED(CONFIG_SPL_ETH)) >> +/* Init DRAM size for R5/A53 SPL */ >> +dram_init_banksize(); > > The list of conditionals in common/spl/spl.c::board_init_r() should be > updated and probably use SPL_NET as the option to check for. Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I assume that you are referring to the following change: if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || - IS_ENABLED(CONFIG_SPL_ATF)) + IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET)) dram_init_banksize(); I shall replace the current patch with the above change in the v2 series. Since this is in the common section, is there a generic reason I could provide in the commit message rather than the existing commit message which seems to be board specific? Also, I hope that the above change will not cause regressions for other non-TI devices. Please let me know. > -- Regards, Siddharth.
Re: [PATCH 00/10] Add support for Ethernet Boot on SK-AM62
On 12/01/24 18:12, Nishanth Menon wrote: > On 18:06-20240112, Siddharth Vadapalli wrote: >> >> >> On 12/01/24 18:02, Nishanth Menon wrote: >>> On 12:17-20240112, Siddharth Vadapalli wrote: >>>> Hello, >>>> >>>> This series enables Ethernet Boot on SK-AM62 device. >>>> Product Link: https://www.ti.com/tool/SK-AM62 >>>> User Guide: https://www.ti.com/lit/pdf/spruj40 >>>> >>>> Ethernet Boot flow is as follows: >>>> 1. The BOOT MODE pins are configured for Ethernet Boot. >>>> 2. On powering on the device, ROM uses the Ethernet Port corresponding >>>> to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot". >>>> 3. The TFTP server and DHCP server on the receiver device need to be >>>> configured such that VCI string "TI K3 Bootp Boot" maps to the file >>>> "tiboot3.bin" and the TFTP server should be capable of transferring >>>> it to the device. >>>> 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server. >>>> 5. The "tiboot3.bin" file is expected to be built using the config: >>>> am62x_evm_r5_ethboot_defconfig >>>> introduced in this series, which shall enable "tispl.bin" to be fetched >>>> over TFTP using "tiboot3.bin". >>>> 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its >>>> NET_VCI_STRING, thereby implying that the DHCP server and TFTP server >>>> need to be configured to transfer "tispl.bin" to the device. >>>> 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP >>>> server. >>>> 8. The "tispl.bin" file is expected to be built using the config: >>>> am62x_evm_a53_defconfig >>>> which has been updated in this series to enable Ethernet Boot specific >>>> configs, allowing "u-boot.img" to be fetched over TFTP using >>>> "tispl.bin". >>>> 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its >>>> NET_VCI_STRING. The DHCP server and TFTP server need to be configured to >>>> transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING. >>>> 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and >>>> executes it on the device, completing the process of Ethernet Boot on the >>>> device. >>>> >>>> NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode >>>> of operation due to which it is expected that the Link Partner also >>>> supports the same mode of operation. >>>> Additionally, enabling "phy_gmii_sel" node at SPL stage will be >>>> necessary and is not added as a part of this series with the aim of >>>> adding the "bootph-all" property to its counterpart in Linux device-tree >>>> first. >>> >>> >>> NAK - instead of writing all this up in the commit message, why would >>> you not spend that time updating the excellent documentation we have? >> >> I plan to document it after the feature is in. The reason for mentioning the >> content above is for explaining the flow in case anyone wishes to test and >> verify it. Wouldn't documenting it make it appear that the feature is present >> when it isn't? > > So you are saying this series does NOT work! why are you sending patches > then? If you are introducing a feature and enabling it, ensure you send > documentation along with it allowing people to be able to actually use > the feature. I have mentioned in the "NOTE" above that enabling "phy_gmii_sel" node at SPL stage by adding the "bootph-all" property is necessary to verify this series. I cannot post that with this series since Linux device-tree needs to have the property added first and the merge window is closed now. Once it is in the Linux device-tree, syncing U-Boot device-tree with Linux device-tree will enable Ethernet Boot which is when the feature will work. That is when I was planning to document it. However, based on your feedback, in the next version for this series I will add the documentation as well along with the note that "phy_gmii_sel" needs to be enabled at SPL stage for the feature to work. Please let me know if that is acceptable. > -- Regards, Siddharth.
Re: [PATCH 09/10] configs: am62x_evm_a53_defconfig: Enable configs required for Ethboot
On 12/01/24 18:03, Nishanth Menon wrote: > On 12:17-20240112, Siddharth Vadapalli wrote: >> From: Kishon Vijay Abraham I >> >> Enable config options needed to support Ethernet boot on AM62x SK. >> >> Signed-off-by: Kishon Vijay Abraham I >> Signed-off-by: Siddharth Vadapalli >> --- >> configs/am62x_evm_a53_defconfig | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/configs/am62x_evm_a53_defconfig >> b/configs/am62x_evm_a53_defconfig >> index aa96c1b312..5c56b17a3e 100644 >> --- a/configs/am62x_evm_a53_defconfig >> +++ b/configs/am62x_evm_a53_defconfig >> @@ -17,6 +17,7 @@ CONFIG_OF_LIBFDT_OVERLAY=y >> CONFIG_DM_RESET=y >> CONFIG_SPL_MMC=y >> CONFIG_SPL_SERIAL=y >> +CONFIG_SPL_DRIVERS_MISC=y >> CONFIG_SPL_STACK_R_ADDR=0x8200 >> CONFIG_SPL_SIZE_LIMIT=0x4 >> CONFIG_SPL_SIZE_LIMIT_PROVIDE_STACK=0x800 >> @@ -37,13 +38,19 @@ CONFIG_SPL_HAS_BSS_LINKER_SECTION=y >> CONFIG_SPL_BSS_START_ADDR=0x80c8 >> CONFIG_SPL_BSS_MAX_SIZE=0x8 >> CONFIG_SPL_SYS_REPORT_STACK_F_USAGE=y >> +CONFIG_SPL_BOARD_INIT=y >> CONFIG_SPL_SYS_MALLOC_SIMPLE=y >> CONFIG_SPL_STACK_R=y >> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y >> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400 >> +CONFIG_SPL_DMA=y >> +CONFIG_SPL_ENV_SUPPORT=y >> +CONFIG_SPL_ETH=y >> CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img" >> CONFIG_SPL_DM_MAILBOX=y >> CONFIG_SPL_DM_SPI_FLASH=y >> +CONFIG_SPL_NET=y >> +CONFIG_SPL_NET_VCI_STRING="AM62X U-Boot A53 SPL" >> CONFIG_SPL_POWER_DOMAIN=y >> # CONFIG_SPL_SPI_FLASH_TINY is not set >> CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y >> @@ -61,6 +68,7 @@ CONFIG_SPL_DM=y >> CONFIG_SPL_DM_SEQ_ALIAS=y >> CONFIG_REGMAP=y >> CONFIG_SPL_REGMAP=y >> +CONFIG_SPL_SYSCON=y >> CONFIG_SPL_OF_TRANSLATE=y >> CONFIG_CLK=y >> CONFIG_SPL_CLK=y >> -- >> 2.34.1 >> > > NAK again - why cant we use config fragments? Ok. I will use config fragments. > -- Regards, Siddharth.
Re: [PATCH 08/10] configs: am62: Add configs for enabling ETHBOOT in R5SPL
On 12/01/24 18:01, Nishanth Menon wrote: > On 12:17-20240112, Siddharth Vadapalli wrote: >> From: Kishon Vijay Abraham I >> >> Add configs for enabling ETHBOOT in R5SPL. Adding a separate config >> minimizes the risk of going past the R5-SPL size limit for any future >> config additions. >> >> Signed-off-by: Kishon Vijay Abraham I >> Signed-off-by: Andreas Dannenberg >> Signed-off-by: Siddharth Vadapalli >> --- >> configs/am62x_evm_r5_ethboot_defconfig | 110 + > > NAK. use config fragments please. Ok. -- Regards, Siddharth.
Re: [PATCH 00/10] Add support for Ethernet Boot on SK-AM62
On 12/01/24 18:02, Nishanth Menon wrote: > On 12:17-20240112, Siddharth Vadapalli wrote: >> Hello, >> >> This series enables Ethernet Boot on SK-AM62 device. >> Product Link: https://www.ti.com/tool/SK-AM62 >> User Guide: https://www.ti.com/lit/pdf/spruj40 >> >> Ethernet Boot flow is as follows: >> 1. The BOOT MODE pins are configured for Ethernet Boot. >> 2. On powering on the device, ROM uses the Ethernet Port corresponding >> to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot". >> 3. The TFTP server and DHCP server on the receiver device need to be >> configured such that VCI string "TI K3 Bootp Boot" maps to the file >> "tiboot3.bin" and the TFTP server should be capable of transferring >> it to the device. >> 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server. >> 5. The "tiboot3.bin" file is expected to be built using the config: >> am62x_evm_r5_ethboot_defconfig >> introduced in this series, which shall enable "tispl.bin" to be fetched >> over TFTP using "tiboot3.bin". >> 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its >> NET_VCI_STRING, thereby implying that the DHCP server and TFTP server >> need to be configured to transfer "tispl.bin" to the device. >> 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP >> server. >> 8. The "tispl.bin" file is expected to be built using the config: >> am62x_evm_a53_defconfig >> which has been updated in this series to enable Ethernet Boot specific >> configs, allowing "u-boot.img" to be fetched over TFTP using >> "tispl.bin". >> 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its >> NET_VCI_STRING. The DHCP server and TFTP server need to be configured to >> transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING. >> 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and >> executes it on the device, completing the process of Ethernet Boot on the >> device. >> >> NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode >> of operation due to which it is expected that the Link Partner also >> supports the same mode of operation. >> Additionally, enabling "phy_gmii_sel" node at SPL stage will be >> necessary and is not added as a part of this series with the aim of >> adding the "bootph-all" property to its counterpart in Linux device-tree >> first. > > > NAK - instead of writing all this up in the commit message, why would > you not spend that time updating the excellent documentation we have? I plan to document it after the feature is in. The reason for mentioning the content above is for explaining the flow in case anyone wishes to test and verify it. Wouldn't documenting it make it appear that the feature is present when it isn't? ... -- Regards, Siddharth.
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
Hello Nishanth, On 12/01/24 17:56, Nishanth Menon wrote: > On 12:17-20240112, Siddharth Vadapalli wrote: >> From: Kishon Vijay Abraham I >> >> Call dram_init_banksize() from spl_board_init() otherwise TFTP download >> fails with error "TFTP error: trying to overwrite reserved memory..." >> due to lmb_get_free_size() not able to find unreserved region due >> to lack of DRAM size info. Required to support Ethernet boot on AM62x. >> >> Signed-off-by: Kishon Vijay Abraham I >> Signed-off-by: Siddharth Vadapalli >> --- >> board/ti/am62x/evm.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/board/ti/am62x/evm.c b/board/ti/am62x/evm.c >> index ad93908840..35f291d83a 100644 >> --- a/board/ti/am62x/evm.c >> +++ b/board/ti/am62x/evm.c >> @@ -85,6 +85,9 @@ void spl_board_init(void) >> if (IS_ENABLED(CONFIG_SPL_SPLASH_SCREEN) && IS_ENABLED(CONFIG_SPL_BMP)) >> splash_display(); >> >> +if (IS_ENABLED(CONFIG_SPL_ETH)) >> +/* Init DRAM size for R5/A53 SPL */ >> +dram_init_banksize(); >> } >> >> #if defined(CONFIG_K3_AM64_DDRSS) >> -- >> 2.34.1 >> > > Are you sure? tftp seems to work without this patch. > > https://gist.github.com/nmenon/91e3282e00e38ae42e8cf640be2ab888 I noticed the error pointed out in the commit message at the A53 SPL stage when the U-Boot Image is being fetched over TFTP without this patch, so I have verified that this patch is necessary. The logs you have shared verify TFTP at U-Boot, but this patch is for enabling TFTP at A53 SPL for fetching U-Boot image via TFTP. -- Regards, Siddharth.
[PATCH 10/10] arm: dts: k3-am625-r5-sk: Enable DM services for main_pktdma
Enable DM services for main_pktdma during R5 SPL stage. Signed-off-by: Siddharth Vadapalli --- arch/arm/dts/k3-am625-r5-sk.dts | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/arm/dts/k3-am625-r5-sk.dts b/arch/arm/dts/k3-am625-r5-sk.dts index 6b9f40e555..0912b953db 100644 --- a/arch/arm/dts/k3-am625-r5-sk.dts +++ b/arch/arm/dts/k3-am625-r5-sk.dts @@ -83,3 +83,8 @@ reg = <0x00 0x0fc4 0x00 0x100>, <0x00 0x6000 0x00 0x0800>; }; + +_pktdma { + ti,sci = <_tifs>; + bootph-all; +}; -- 2.34.1
[PATCH 09/10] configs: am62x_evm_a53_defconfig: Enable configs required for Ethboot
From: Kishon Vijay Abraham I Enable config options needed to support Ethernet boot on AM62x SK. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli --- configs/am62x_evm_a53_defconfig | 8 1 file changed, 8 insertions(+) diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig index aa96c1b312..5c56b17a3e 100644 --- a/configs/am62x_evm_a53_defconfig +++ b/configs/am62x_evm_a53_defconfig @@ -17,6 +17,7 @@ CONFIG_OF_LIBFDT_OVERLAY=y CONFIG_DM_RESET=y CONFIG_SPL_MMC=y CONFIG_SPL_SERIAL=y +CONFIG_SPL_DRIVERS_MISC=y CONFIG_SPL_STACK_R_ADDR=0x8200 CONFIG_SPL_SIZE_LIMIT=0x4 CONFIG_SPL_SIZE_LIMIT_PROVIDE_STACK=0x800 @@ -37,13 +38,19 @@ CONFIG_SPL_HAS_BSS_LINKER_SECTION=y CONFIG_SPL_BSS_START_ADDR=0x80c8 CONFIG_SPL_BSS_MAX_SIZE=0x8 CONFIG_SPL_SYS_REPORT_STACK_F_USAGE=y +CONFIG_SPL_BOARD_INIT=y CONFIG_SPL_SYS_MALLOC_SIMPLE=y CONFIG_SPL_STACK_R=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400 +CONFIG_SPL_DMA=y +CONFIG_SPL_ENV_SUPPORT=y +CONFIG_SPL_ETH=y CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img" CONFIG_SPL_DM_MAILBOX=y CONFIG_SPL_DM_SPI_FLASH=y +CONFIG_SPL_NET=y +CONFIG_SPL_NET_VCI_STRING="AM62X U-Boot A53 SPL" CONFIG_SPL_POWER_DOMAIN=y # CONFIG_SPL_SPI_FLASH_TINY is not set CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y @@ -61,6 +68,7 @@ CONFIG_SPL_DM=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y +CONFIG_SPL_SYSCON=y CONFIG_SPL_OF_TRANSLATE=y CONFIG_CLK=y CONFIG_SPL_CLK=y -- 2.34.1
[PATCH 08/10] configs: am62: Add configs for enabling ETHBOOT in R5SPL
From: Kishon Vijay Abraham I Add configs for enabling ETHBOOT in R5SPL. Adding a separate config minimizes the risk of going past the R5-SPL size limit for any future config additions. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Andreas Dannenberg Signed-off-by: Siddharth Vadapalli --- configs/am62x_evm_r5_ethboot_defconfig | 110 + 1 file changed, 110 insertions(+) create mode 100644 configs/am62x_evm_r5_ethboot_defconfig diff --git a/configs/am62x_evm_r5_ethboot_defconfig b/configs/am62x_evm_r5_ethboot_defconfig new file mode 100644 index 00..4912f97d7c --- /dev/null +++ b/configs/am62x_evm_r5_ethboot_defconfig @@ -0,0 +1,110 @@ +CONFIG_ARM=y +CONFIG_ARCH_K3=y +CONFIG_SYS_MALLOC_F_LEN=0x9000 +CONFIG_SPL_GPIO=y +CONFIG_SPL_LIBCOMMON_SUPPORT=y +CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_NR_DRAM_BANKS=2 +CONFIG_SOC_K3_AM625=y +CONFIG_TARGET_AM625_R5_EVM=y +CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y +CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x43c3a7f0 +CONFIG_ENV_SIZE=0x2 +CONFIG_DM_GPIO=y +CONFIG_DEFAULT_DEVICE_TREE="k3-am625-r5-sk" +CONFIG_SPL_TEXT_BASE=0x43c0 +CONFIG_DM_RESET=y +CONFIG_SPL_MMC=y +CONFIG_SPL_SERIAL=y +CONFIG_SPL_DRIVERS_MISC=y +CONFIG_SPL_STACK_R_ADDR=0x8200 +CONFIG_SPL_SYS_MALLOC_F_LEN=0x7000 +CONFIG_SPL_SIZE_LIMIT=0x3A7F0 +CONFIG_SPL_SIZE_LIMIT_PROVIDE_STACK=0x3500 +CONFIG_SPL_LIBDISK_SUPPORT=y +CONFIG_SPL_LOAD_FIT=y +CONFIG_SPL_LOAD_FIT_ADDRESS=0x8008 +# CONFIG_DISPLAY_CPUINFO is not set +CONFIG_SPL_SIZE_LIMIT_SUBTRACT_GD=y +CONFIG_SPL_SIZE_LIMIT_SUBTRACT_MALLOC=y +CONFIG_SPL_MAX_SIZE=0x3B000 +CONFIG_SPL_PAD_TO=0x0 +CONFIG_SPL_HAS_BSS_LINKER_SECTION=y +CONFIG_SPL_BSS_START_ADDR=0x43c3b000 +CONFIG_SPL_BSS_MAX_SIZE=0x3000 +CONFIG_SPL_SYS_REPORT_STACK_F_USAGE=y +CONFIG_SPL_BOARD_INIT=y +CONFIG_SPL_SYS_MALLOC_SIMPLE=y +CONFIG_SPL_STACK_R=y +CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x20 +CONFIG_SPL_SEPARATE_BSS=y +CONFIG_SPL_EARLY_BSS=y +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x400 +CONFIG_SPL_DMA=y +CONFIG_SPL_ENV_SUPPORT=y +CONFIG_SPL_ETH=y +CONFIG_SPL_I2C=y +CONFIG_SPL_DM_MAILBOX=y +CONFIG_SPL_NET=y +CONFIG_SPL_NET_VCI_STRING="AM62X U-Boot R5 SPL" +CONFIG_SPL_DM_RESET=y +CONFIG_SPL_POWER_DOMAIN=y +CONFIG_SPL_RAM_SUPPORT=y +CONFIG_SPL_RAM_DEVICE=y +CONFIG_SPL_REMOTEPROC=y +CONFIG_SPL_YMODEM_SUPPORT=y +CONFIG_HUSH_PARSER=y +CONFIG_CMD_ASKENV=y +CONFIG_CMD_GPT=y +CONFIG_CMD_MMC=y +CONFIG_CMD_REMOTEPROC=y +# CONFIG_CMD_SETEXPR is not set +CONFIG_CMD_DHCP=y +CONFIG_CMD_TIME=y +CONFIG_CMD_FAT=y +CONFIG_OF_CONTROL=y +CONFIG_SPL_OF_CONTROL=y +CONFIG_SPL_MULTI_DTB_FIT=y +CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y +CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_SPL_DM=y +CONFIG_SPL_DM_SEQ_ALIAS=y +CONFIG_REGMAP=y +CONFIG_SPL_REGMAP=y +CONFIG_SPL_SYSCON=y +CONFIG_SPL_OF_TRANSLATE=y +CONFIG_CLK=y +CONFIG_SPL_CLK=y +CONFIG_SPL_CLK_CCF=y +CONFIG_SPL_CLK_K3_PLL=y +CONFIG_SPL_CLK_K3=y +CONFIG_DMA_CHANNELS=y +CONFIG_TI_K3_NAVSS_UDMA=y +CONFIG_TI_SCI_PROTOCOL=y +CONFIG_DA8XX_GPIO=y +CONFIG_DM_I2C=y +CONFIG_DM_MAILBOX=y +CONFIG_K3_SEC_PROXY=y +CONFIG_PHY_TI_DP83867=y +CONFIG_TI_AM65_CPSW_NUSS=y +CONFIG_PINCTRL=y +# CONFIG_PINCTRL_GENERIC is not set +CONFIG_SPL_PINCTRL=y +# CONFIG_SPL_PINCTRL_GENERIC is not set +CONFIG_PINCTRL_SINGLE=y +CONFIG_POWER_DOMAIN=y +CONFIG_TI_POWER_DOMAIN=y +CONFIG_K3_SYSTEM_CONTROLLER=y +CONFIG_REMOTEPROC_TI_K3_ARM64=y +CONFIG_RESET_TI_SCI=y +CONFIG_SPECIFY_CONSOLE_INDEX=y +CONFIG_DM_SERIAL=y +CONFIG_SOC_DEVICE=y +CONFIG_SOC_DEVICE_TI_K3=y +CONFIG_SOC_TI=y +CONFIG_TIMER=y +CONFIG_SPL_TIMER=y +CONFIG_OMAP_TIMER=y +CONFIG_LIB_RATIONAL=y +CONFIG_SPL_LIB_RATIONAL=y -- 2.34.1
[PATCH 07/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
From: Kishon Vijay Abraham I In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS driver in board_init_f(). Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli --- arch/arm/mach-k3/am625_init.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c index 6c96e88114..b89dd206e5 100644 --- a/arch/arm/mach-k3/am625_init.c +++ b/arch/arm/mach-k3/am625_init.c @@ -209,6 +209,16 @@ void board_init_f(ulong dummy) if (ret) panic("DRAM init failed: %d\n", ret); } + + if (IS_ENABLED(CONFIG_SPL_ETH) && IS_ENABLED(CONFIG_TI_AM65_CPSW_NUSS) && + spl_boot_device() == BOOT_DEVICE_ETHERNET) { + struct udevice *cpswdev; + + if (uclass_get_device_by_driver(UCLASS_MISC, DM_DRIVER_GET(am65_cpsw_nuss), + )) + printf("Failed to probe am65_cpsw_nuss driver\n"); + } + spl_enable_cache(); } -- 2.34.1
[PATCH 06/10] arm: mach-k3: am62: Update SoC autogenerated data to enable Ethernet Boot
From: Andreas Dannenberg In order to enable Ethernet Boot using CPSW, update the clock data. Signed-off-by: Andreas Dannenberg Signed-off-by: Siddharth Vadapalli --- arch/arm/mach-k3/r5/am62x/clk-data.c | 79 ++-- 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/arch/arm/mach-k3/r5/am62x/clk-data.c b/arch/arm/mach-k3/r5/am62x/clk-data.c index d7bfed0e03..880f05c40b 100644 --- a/arch/arm/mach-k3/r5/am62x/clk-data.c +++ b/arch/arm/mach-k3/r5/am62x/clk-data.c @@ -3,9 +3,9 @@ * AM62X specific clock platform data * * This file is auto generated. Please do not hand edit and report any issues - * to Dave Gerlach . + * to Bryan Brattlof . * - * Copyright (C) 2020-2022 Texas Instruments Incorporated - https://www.ti.com/ + * Copyright (C) 2020-2024 Texas Instruments Incorporated - https://www.ti.com/ */ #include @@ -57,7 +57,7 @@ static const char * const sam62_pll_ctrl_wrap_mcu_0_sysclkout_clk_parents[] = { static const char * const clkout0_ctrl_out0_parents[] = { "hsdiv4_16fft_main_2_hsdivout1_clk", - "hsdiv4_16fft_main_2_hsdivout1_clk10", + "hsdiv4_16fft_main_2_hsdivout1_clk", }; static const char * const clk_32k_rc_sel_out0_parents[] = { @@ -158,46 +158,45 @@ static const struct clk_data clk_list[] = { CLK_FIXED_RATE("fss_ul_main_0_ospi_0_ospi_oclk_clk", 0, 0), CLK_DIV("hsdiv0_16fft_mcu_32khz_gen_0_hsdivout0_clk", "gluelogic_hfosc0_clkout", 0x4508030, 0, 7, 0, 0), CLK_FIXED_RATE("mshsi2c_main_0_porscl", 0, 0), - CLK_PLL("pllfracf_ssmod_16fft_main_0_foutvcop_clk", "gluelogic_hfosc0_clkout", 0x68, 0), - CLK_DIV("pllfracf_ssmod_16fft_main_0_foutpostdiv_clk_subdiv", "pllfracf_ssmod_16fft_main_0_foutvcop_clk", 0x680038, 16, 3, 0, CLK_DIVIDER_ONE_BASED), - CLK_DIV("pllfracf_ssmod_16fft_main_0_foutpostdiv_clk", "pllfracf_ssmod_16fft_main_0_foutpostdiv_clk_subdiv", 0x680038, 24, 3, 0, CLK_DIVIDER_ONE_BASED), - CLK_PLL("pllfracf_ssmod_16fft_main_1_foutvcop_clk", "gluelogic_hfosc0_clkout", 0x681000, 0), - CLK_DIV("pllfracf_ssmod_16fft_main_1_foutpostdiv_clk_subdiv", "pllfracf_ssmod_16fft_main_1_foutvcop_clk", 0x681038, 16, 3, 0, CLK_DIVIDER_ONE_BASED), - CLK_DIV("pllfracf_ssmod_16fft_main_1_foutpostdiv_clk", "pllfracf_ssmod_16fft_main_1_foutpostdiv_clk_subdiv", 0x681038, 24, 3, 0, CLK_DIVIDER_ONE_BASED), - CLK_PLL("pllfracf_ssmod_16fft_main_12_foutvcop_clk", "gluelogic_hfosc0_clkout", 0x68c000, 0), - CLK_PLL("pllfracf_ssmod_16fft_main_15_foutvcop_clk", "gluelogic_hfosc0_clkout", 0x68f000, 0), - CLK_PLL("pllfracf_ssmod_16fft_main_2_foutvcop_clk", "gluelogic_hfosc0_clkout", 0x682000, 0), - CLK_DIV("pllfracf_ssmod_16fft_main_2_foutpostdiv_clk_subdiv", "pllfracf_ssmod_16fft_main_2_foutvcop_clk", 0x682038, 16, 3, 0, CLK_DIVIDER_ONE_BASED), - CLK_DIV("pllfracf_ssmod_16fft_main_2_foutpostdiv_clk", "pllfracf_ssmod_16fft_main_2_foutpostdiv_clk_subdiv", 0x682038, 24, 3, 0, CLK_DIVIDER_ONE_BASED), - CLK_PLL("pllfracf_ssmod_16fft_main_8_foutvcop_clk", "gluelogic_hfosc0_clkout", 0x688000, 0), - CLK_PLL("pllfracf_ssmod_16fft_mcu_0_foutvcop_clk", "gluelogic_hfosc0_clkout", 0x404, 0), - CLK_DIV("postdiv1_16fft_main_1_hsdivout5_clk", "pllfracf_ssmod_16fft_main_1_foutpostdiv_clk", 0x681094, 0, 7, 0, 0), - CLK_DIV("postdiv4_16ff_main_0_hsdivout5_clk", "pllfracf_ssmod_16fft_main_0_foutpostdiv_clk", 0x680094, 0, 7, 0, 0), - CLK_DIV("postdiv4_16ff_main_0_hsdivout6_clk", "pllfracf_ssmod_16fft_main_0_foutpostdiv_clk", 0x680098, 0, 7, 0, 0), - CLK_DIV("postdiv4_16ff_main_0_hsdivout8_clk", "pllfracf_ssmod_16fft_main_0_foutpostdiv_clk", 0x6800a0, 0, 7, 0, 0), - CLK_DIV("postdiv4_16ff_main_2_hsdivout5_clk", "pllfracf_ssmod_16fft_main_2_foutpostdiv_clk", 0x682094, 0, 7, 0, 0), - CLK_DIV("postdiv4_16ff_main_2_hsdivout8_clk", "pllfracf_ssmod_16fft_main_2_foutpostdiv_clk", 0x6820a0, 0, 7, 0, 0), - CLK_DIV("postdiv4_16ff_main_2_hsdivout9_clk", "pllfracf_ssmod_16fft_main_2_foutpostdiv_clk", 0x6820a4, 0, 7, 0, 0), + CLK_PLL("pllfracf2_ssmod_16fft_main_0_foutvcop_clk", "gluelogic_hfosc0_clkout", 0x68, 0), + CLK_DIV("pllfracf2_ssmod_16fft_main_0_foutpostdiv_clk_subdiv", "pllfracf2_ssmod_16fft_main_0_foutvcop_clk", 0x680038, 16, 3, 0, CLK_DIVIDER_ONE_BASED), + CLK_DIV("pllfracf2_ssmod_16fft_main_0_foutpostdiv_clk", "pllfracf2_ssmod_16fft_main_0_foutpostdi
[PATCH 05/10] dma: ti: k3-udma: Add support for native configuration of chan/flow
From: Kishon Vijay Abraham I In absence of Device Manager (DM) services such as at R5 SPL stage, driver will have to natively setup TCHAN/RCHAN/RFLOW cfg registers. Existing UDMA driver performed the above mentioned configuration for UDMA. Add similar configuration for PKTDMA here. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli --- drivers/dma/ti/k3-udma.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index 8a62d63dfe..3bd1e7668b 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -2110,6 +2110,9 @@ static int bcdma_tisci_tx_channel_config(struct udma_chan *uc) if (ret) dev_err(ud->dev, "tchan%d cfg failed %d\n", tchan->id, ret); + if (IS_ENABLED(CONFIG_K3_DM_FW)) + udma_alloc_tchan_raw(uc); + return ret; } @@ -2158,6 +2161,9 @@ static int pktdma_tisci_rx_channel_config(struct udma_chan *uc) dev_err(ud->dev, "flow%d config failed: %d\n", uc->rflow->id, ret); + if (IS_ENABLED(CONFIG_K3_DM_FW)) + udma_alloc_rchan_raw(uc); + return ret; } -- 2.34.1
[PATCH 04/10] soc: ti: k3-navss-ringacc: Fix reset ring API
From: Vignesh Raghavendra Expectation of k3_ringacc_ring_reset_raw() is to reset the ring to requested size and not to 0. Fix this. Signed-off-by: Vignesh Raghavendra Signed-off-by: Siddharth Vadapalli --- drivers/soc/ti/k3-navss-ringacc-u-boot.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/soc/ti/k3-navss-ringacc-u-boot.c b/drivers/soc/ti/k3-navss-ringacc-u-boot.c index f958239c2a..5d650b9de7 100644 --- a/drivers/soc/ti/k3-navss-ringacc-u-boot.c +++ b/drivers/soc/ti/k3-navss-ringacc-u-boot.c @@ -25,9 +25,16 @@ struct k3_nav_ring_cfg_regs { #define KNAV_RINGACC_CFG_RING_SIZE_ELSIZE_MASK GENMASK(26, 24) #define KNAV_RINGACC_CFG_RING_SIZE_ELSIZE_SHIFT(24) +#define KNAV_RINGACC_CFG_RING_SIZE_MASKGENMASK(15, 0) + static void k3_ringacc_ring_reset_raw(struct k3_nav_ring *ring) { - writel(0, >cfg->size); + u32 reg; + + reg = readl(>cfg->size); + reg &= KNAV_RINGACC_CFG_RING_SIZE_MASK; + reg |= ring->size; + writel(reg, >cfg->size); } static void k3_ringacc_ring_reconfig_qmode_raw(struct k3_nav_ring *ring, enum k3_nav_ring_mode mode) -- 2.34.1
[PATCH 03/10] soc: ti: k3-navss-ringacc: Initialize base address of ring cfg registers
From: Kishon Vijay Abraham I Initialize base address of ring config registers required to natively setup ring cfg registers in the absence of Device Manager (DM) services at R5 SPL stage. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli --- drivers/soc/ti/k3-navss-ringacc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/soc/ti/k3-navss-ringacc.c b/drivers/soc/ti/k3-navss-ringacc.c index 7a2fbb0db6..31e9b372ee 100644 --- a/drivers/soc/ti/k3-navss-ringacc.c +++ b/drivers/soc/ti/k3-navss-ringacc.c @@ -1030,8 +1030,8 @@ static int k3_nav_ringacc_init(struct udevice *dev, struct k3_nav_ringacc *ringa struct k3_nav_ringacc *k3_ringacc_dmarings_init(struct udevice *dev, struct k3_ringacc_init_data *data) { + void __iomem *base_rt, *base_cfg; struct k3_nav_ringacc *ringacc; - void __iomem *base_rt; int i; ringacc = devm_kzalloc(dev, sizeof(*ringacc), GFP_KERNEL); @@ -1049,6 +1049,10 @@ struct k3_nav_ringacc *k3_ringacc_dmarings_init(struct udevice *dev, if (!base_rt) return ERR_PTR(-EINVAL); + base_cfg = dev_read_addr_name_ptr(dev, "cfg"); + if (!base_cfg) + return ERR_PTR(-EINVAL); + ringacc->rings = devm_kzalloc(dev, sizeof(*ringacc->rings) * ringacc->num_rings * 2, @@ -1063,6 +1067,7 @@ struct k3_nav_ringacc *k3_ringacc_dmarings_init(struct udevice *dev, for (i = 0; i < ringacc->num_rings; i++) { struct k3_nav_ring *ring = >rings[i]; + ring->cfg = base_cfg + KNAV_RINGACC_CFG_REGS_STEP * i; ring->rt = base_rt + K3_DMARING_RING_RT_REGS_STEP * i; ring->parent = ringacc; ring->ring_id = i; -- 2.34.1
[PATCH 02/10] firmware: ti_sci: Add No-OP for "RX_FL_CFG"
From: Kishon Vijay Abraham I RX_FL_CFG message should not be forwarded to TIFS and should be handled within R5 SPL (when DM services are not available). Add a no-op function to not handle RX_FL_CFG messages. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli --- drivers/firmware/ti_sci.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c index 6e9f93e9a3..a13a2ead08 100644 --- a/drivers/firmware/ti_sci.c +++ b/drivers/firmware/ti_sci.c @@ -2445,6 +2445,12 @@ fail: return ret; } +static int ti_sci_cmd_rm_udmap_rx_flow_cfg_noop(const struct ti_sci_handle *handle, + const struct ti_sci_msg_rm_udmap_flow_cfg *params) +{ + return 0; +} + /** * ti_sci_cmd_set_fwl_region() - Request for configuring a firewall region * @handle:pointer to TI SCI handle @@ -2884,7 +2890,7 @@ static __maybe_unused int ti_sci_dm_probe(struct udevice *dev) udmap_ops = >rm_udmap_ops; udmap_ops->tx_ch_cfg = ti_sci_cmd_rm_udmap_tx_ch_cfg; udmap_ops->rx_ch_cfg = ti_sci_cmd_rm_udmap_rx_ch_cfg; - udmap_ops->rx_flow_cfg = ti_sci_cmd_rm_udmap_rx_flow_cfg; + udmap_ops->rx_flow_cfg = ti_sci_cmd_rm_udmap_rx_flow_cfg_noop; return ret; } -- 2.34.1
[PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
From: Kishon Vijay Abraham I Call dram_init_banksize() from spl_board_init() otherwise TFTP download fails with error "TFTP error: trying to overwrite reserved memory..." due to lmb_get_free_size() not able to find unreserved region due to lack of DRAM size info. Required to support Ethernet boot on AM62x. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli --- board/ti/am62x/evm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/board/ti/am62x/evm.c b/board/ti/am62x/evm.c index ad93908840..35f291d83a 100644 --- a/board/ti/am62x/evm.c +++ b/board/ti/am62x/evm.c @@ -85,6 +85,9 @@ void spl_board_init(void) if (IS_ENABLED(CONFIG_SPL_SPLASH_SCREEN) && IS_ENABLED(CONFIG_SPL_BMP)) splash_display(); + if (IS_ENABLED(CONFIG_SPL_ETH)) + /* Init DRAM size for R5/A53 SPL */ + dram_init_banksize(); } #if defined(CONFIG_K3_AM64_DDRSS) -- 2.34.1
[PATCH 00/10] Add support for Ethernet Boot on SK-AM62
Hello, This series enables Ethernet Boot on SK-AM62 device. Product Link: https://www.ti.com/tool/SK-AM62 User Guide: https://www.ti.com/lit/pdf/spruj40 Ethernet Boot flow is as follows: 1. The BOOT MODE pins are configured for Ethernet Boot. 2. On powering on the device, ROM uses the Ethernet Port corresponding to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot". 3. The TFTP server and DHCP server on the receiver device need to be configured such that VCI string "TI K3 Bootp Boot" maps to the file "tiboot3.bin" and the TFTP server should be capable of transferring it to the device. 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server. 5. The "tiboot3.bin" file is expected to be built using the config: am62x_evm_r5_ethboot_defconfig introduced in this series, which shall enable "tispl.bin" to be fetched over TFTP using "tiboot3.bin". 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its NET_VCI_STRING, thereby implying that the DHCP server and TFTP server need to be configured to transfer "tispl.bin" to the device. 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP server. 8. The "tispl.bin" file is expected to be built using the config: am62x_evm_a53_defconfig which has been updated in this series to enable Ethernet Boot specific configs, allowing "u-boot.img" to be fetched over TFTP using "tispl.bin". 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its NET_VCI_STRING. The DHCP server and TFTP server need to be configured to transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING. 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and executes it on the device, completing the process of Ethernet Boot on the device. NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode of operation due to which it is expected that the Link Partner also supports the same mode of operation. Additionally, enabling "phy_gmii_sel" node at SPL stage will be necessary and is not added as a part of this series with the aim of adding the "bootph-all" property to its counterpart in Linux device-tree first. This series is based on commit: f28a77589e Merge tag 'dm-next-7jan23' of https://source.denx.de/u-boot/custodians/u-boot-dm into next of the next branch of u-boot. Regards, Siddharth. Andreas Dannenberg (1): arm: mach-k3: am62: Update SoC autogenerated data to enable Ethernet Boot Kishon Vijay Abraham I (7): board: ti: am62x: Init DRAM size in R5/A53 SPL firmware: ti_sci: Add No-OP for "RX_FL_CFG" soc: ti: k3-navss-ringacc: Initialize base address of ring cfg registers dma: ti: k3-udma: Add support for native configuration of chan/flow arm: mach-k3: am625_init: Probe AM65 CPSW NUSS configs: am62: Add configs for enabling ETHBOOT in R5SPL configs: am62x_evm_a53_defconfig: Enable configs required for Ethboot Siddharth Vadapalli (1): arm: dts: k3-am625-r5-sk: Enable DM services for main_pktdma Vignesh Raghavendra (1): soc: ti: k3-navss-ringacc: Fix reset ring API arch/arm/dts/k3-am625-r5-sk.dts | 5 ++ arch/arm/mach-k3/am625_init.c| 10 +++ arch/arm/mach-k3/r5/am62x/clk-data.c | 79 board/ti/am62x/evm.c | 3 + configs/am62x_evm_a53_defconfig | 8 ++ configs/am62x_evm_r5_ethboot_defconfig | 110 +++ drivers/dma/ti/k3-udma.c | 6 ++ drivers/firmware/ti_sci.c| 8 +- drivers/soc/ti/k3-navss-ringacc-u-boot.c | 9 +- drivers/soc/ti/k3-navss-ringacc.c| 7 +- 10 files changed, 202 insertions(+), 43 deletions(-) create mode 100644 configs/am62x_evm_r5_ethboot_defconfig -- 2.34.1
[PATCH] include: env: ti: ti_common: Run main_cpsw0_qsgmii_phyinit conditionally
From: Manorit Chawdhry The main_cpsw0_qsgmii_phyinit command is defined only for certain TI SoCs which have the do_main_cpsw0_qsgmii_phyinit variable set. Add a check to ensure that the main_cpsw0_qsgmii_phyinit command is run only for such SoCs. Signed-off-by: Manorit Chawdhry Signed-off-by: Siddharth Vadapalli --- Hello, This patch is based on commit 65eed68772 test/py: Disable error E0611 in two cases for pylint Regards, Siddharth. include/env/ti/ti_common.env | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/env/ti/ti_common.env b/include/env/ti/ti_common.env index f5d84216e3..f0f89a2287 100644 --- a/include/env/ti/ti_common.env +++ b/include/env/ti/ti_common.env @@ -25,7 +25,10 @@ run_fit=run get_fit_config; bootm ${addr_fit}#${name_fit_config}${overlaystring} bootcmd_ti_mmc= run findfdt; run init_${boot}; #if CONFIG_CMD_REMOTEPROC - run main_cpsw0_qsgmii_phyinit; run boot_rprocs; + if test ${do_main_cpsw0_qsgmii_phyinit} -eq 1; + then run main_cpsw0_qsgmii_phyinit; + fi + run boot_rprocs; #endif if test ${boot_fit} -eq 1; then run get_fit_${boot}; run get_fit_overlaystring; run run_fit; -- 2.34.1
Re: [PATCH 1/2] net: ti: am65-cpsw-nuss: Workaround for buggy PHY/Board
On 25/08/23 13:22, Roger Quadros wrote: > Siddharth, > > On 25/08/2023 08:42, Siddharth Vadapalli wrote: >> Roger, >> >> On 25/08/23 00:39, Roger Quadros wrote: >>> >>> >>> On 24/08/2023 21:24, Tom Rini wrote: >>>> On Thu, Aug 24, 2023 at 11:34:29PM +0530, Siddharth Vadapalli wrote: >>>>> Hello Roger, >>>>> >>>>> On 22-08-2023 17:43, Roger Quadros wrote: ... >> > > This doesn't sound like a clean solution. > All hardware specific details (like reset ready-time) should come from the > device tree and not passed around in PHY APIs. > > Why to do such an intrusive change instead of fixing the am65-cpsw-nuss driver > to properly use the PHY uclass driver model and encode the required delays > in the device tree? Encoding delays in device-tree and using them is better. Kindly ignore my suggestion. > -- Regards, Siddharth.
Re: [PATCH 1/2] net: ti: am65-cpsw-nuss: Workaround for buggy PHY/Board
Roger, On 25/08/23 00:39, Roger Quadros wrote: > > > On 24/08/2023 21:24, Tom Rini wrote: >> On Thu, Aug 24, 2023 at 11:34:29PM +0530, Siddharth Vadapalli wrote: >>> Hello Roger, >>> >>> On 22-08-2023 17:43, Roger Quadros wrote: ... > > Even a single "PHY not found" print is not OK. It looks like an > error while it should not. > > The correct solution is to use the MDIO uclass framework and add > some generic handling in the class driver. drivers/net/eth-phy-uclass.c > > We could provide the delay time in the 'reset-deassert-us' property. > Although I'm not sure if this is the correct property for this case > as there is no RESET GPIO on the board. What we really want is delay > from power-on-reset. Which means we might have to introduce a new property > and use time from boot to determine if PHY is ready or not? > > NOTE: PHY ready time is different for hardware reset and power-on-reset. > 50ms vs 150ms Another alternative could be that of implementing the for-loop within phy_connect() along with the delay, by updating the function with a new parameter "tries" which indicates the number of retries before finally printing "PHY not found" in case of an error. Optionally, another parameter "delay" can be added, which indicates the delay within the for-loop. This implementation will require updating all drivers in drivers/net which use phy_connect(), with the "tries" parameter set to 1 for them. The am65-cpsw-nuss driver can set "tries" to 5 as done in the current patch. The idea behind moving the for-loop within phy_connect() is that it will help solve the issue for other drivers as well, if they potentially encounter such buggy PHYs in future boards. > -- Regards, Siddharth.
Re: [PATCH 1/2] net: ti: am65-cpsw-nuss: Workaround for buggy PHY/Board
Hello Roger, On 22-08-2023 17:43, Roger Quadros wrote: > Beagleplay has a buggy Ethernet PHY implementation for the Gigabit > PHY in the sense that it is non responsive over MDIO immediately > after power-up/reset. > > We need to either try multiple times or wait sufficiently long enough > (couple of 10s of ms?) before the PHY begins to respond correctly. > > One theory is that the PHY is configured to operate on MDIO address 0 > which it treats as a special broadcast address. > > Datasheet states: > "PHYAD (config pins) sets the PHY address for the device. > The RTL8211F(I)/RTL8211FD(I) supports PHY addresses from 0x01 to 0x07. > Note 1: An MDIO command with PHY address=0 is a broadcast from the MAC; > each PHY device should respond." > > This issue is not seen with the other PHY (different make) on the board > which is configured for address 0x1. > > As a woraround we try to probe the PHY multiple times instead of giving > up on the first attempt. > > Signed-off-by: Roger Quadros > --- > drivers/net/ti/am65-cpsw-nuss.c | 19 ++- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c > index 51a8167d14..4f8a2f9b93 100644 > --- a/drivers/net/ti/am65-cpsw-nuss.c > +++ b/drivers/net/ti/am65-cpsw-nuss.c > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > > #include "cpsw_mdio.h" > > @@ -678,14 +679,22 @@ static int am65_cpsw_phy_init(struct udevice *dev) > struct am65_cpsw_priv *priv = dev_get_priv(dev); > struct am65_cpsw_common *cpsw_common = priv->cpsw_common; > struct eth_pdata *pdata = dev_get_plat(dev); > - struct phy_device *phydev; > u32 supported = PHY_GBIT_FEATURES; > + struct phy_device *phydev; > + int tries; > int ret; > > - phydev = phy_connect(cpsw_common->bus, > - priv->phy_addr, > - priv->dev, > - pdata->phy_interface); > + /* Some boards (e.g. beagleplay) don't work on first go */ > + for (tries = 1; tries <= 5; tries++) { > + phydev = phy_connect(cpsw_common->bus, > + priv->phy_addr, > + priv->dev, > + pdata->phy_interface); > + if (phydev) > + break; > + > + mdelay(10); > + } After rethinking about the above implementation and the second patch of this series, the second patch could be dropped altogether if the following implementation is acceptable: phydev = phy_connect(cpsw_common->bus, priv->phy_addr, priv->dev, pdata->phy_interface); if (!phydev) { /* Some boards (e.g. beagleplay) don't work on first go */ mdelay(50); phydev = phy_connect(cpsw_common->bus, priv->phy_addr, priv->dev, pdata->phy_interface); } if (!phydev) { dev_err(dev, "phy_connect() failed\n"); ... With this, there would be at most one "PHY not found" print, which should be fine. The mdelay value of 50 could be replaced with a sufficiently large value which guarantees success for Beagleplay. -- Regards, Siddharth.
Re: [PATCH 1/2] net: ti: am65-cpsw-nuss: Workaround for buggy PHY/Board
On 23/08/23 13:22, Roger Quadros wrote: > Hi Siddharth, > > On 23/08/2023 07:35, Siddharth Vadapalli wrote: >> Roger, >> >> On 22/08/23 17:43, Roger Quadros wrote: >>> Beagleplay has a buggy Ethernet PHY implementation for the Gigabit >>> PHY in the sense that it is non responsive over MDIO immediately >>> after power-up/reset. >>> >>> We need to either try multiple times or wait sufficiently long enough >>> (couple of 10s of ms?) before the PHY begins to respond correctly. >> >> Based on the datasheet at: >> https://datasheet.lcsc.com/lcsc/1912111437_Realtek-Semicon-RTL8211F-CG_C187932.pdf >> in the section 7.17. PHY Reset (Hardware Reset), it is stated that >> software has to wait for at least 50 ms before accessing the PHY >> registers. Is this why the for-loop in the code below tries for at most >> 5 times with a delay of 10 ms before the next try? If yes, then isn't it > > Good catch. This might be the reason why PHY is not responding in the first > few attempts. > >> better to move the delay into the realtek PHY driver at >> drivers/net/phy/realtek.c > > We are currently at the MDIO bus driver where we don't even know what PHY > is on the bus. So this delay cannot come at the realtec PHY driver. > It has to come at the MDIO bus driver level. Thank you for clarifying. > >> Shouldn't it be the PHY driver which ensures that any reads/writes to the PHY >> registers are valid? It can ensure this by having a one time 50ms delay for >> the >> very first access to the PHY registers.> ... >> > -- Regards, Siddharth.
Re: [PATCH 2/2] net: phy: Change "PHY not found" message to debug()
On 22/08/23 17:43, Roger Quadros wrote: > Some boards (e.g. Beagleplay) need multiple attempts to detect the PHY > and the multiple "PHY not found" prints are not nice. I tried grepping for calls to "phy_connect" across the drivers present in drivers/net. Most of them simply return -ENODEV on failure. The ones which add prints indicating failure, don't use debug(). For this reason, I believe that the drivers which don't have any prints in case of failure might be relying on the printf() within phy_connect() to indicate failure. Therefore, if the printf() is being changed to debug() in phy_connect(), maybe all the drivers which currently return -ENODEV, should have a print added to them as a part of this patch. > > Change them to debug(). > > Signed-off-by: Roger Quadros > --- > drivers/net/phy/phy.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index ae21acb059..3a524bcd81 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -928,7 +928,7 @@ struct phy_device *phy_connect(struct mii_dev *bus, int > addr, > if (phydev) > phy_connect_dev(phydev, dev, interface); > else > - printf("Could not get PHY for %s: addr %d\n", bus->name, addr); > + debug("Could not get PHY for %s: addr %d\n", bus->name, addr); > return phydev; > } > -- Regards, Siddharth.
Re: [PATCH 1/2] net: ti: am65-cpsw-nuss: Workaround for buggy PHY/Board
Roger, On 22/08/23 17:43, Roger Quadros wrote: > Beagleplay has a buggy Ethernet PHY implementation for the Gigabit > PHY in the sense that it is non responsive over MDIO immediately > after power-up/reset. > > We need to either try multiple times or wait sufficiently long enough > (couple of 10s of ms?) before the PHY begins to respond correctly. Based on the datasheet at: https://datasheet.lcsc.com/lcsc/1912111437_Realtek-Semicon-RTL8211F-CG_C187932.pdf in the section 7.17. PHY Reset (Hardware Reset), it is stated that software has to wait for at least 50 ms before accessing the PHY registers. Is this why the for-loop in the code below tries for at most 5 times with a delay of 10 ms before the next try? If yes, then isn't it better to move the delay into the realtek PHY driver at drivers/net/phy/realtek.c Shouldn't it be the PHY driver which ensures that any reads/writes to the PHY registers are valid? It can ensure this by having a one time 50ms delay for the very first access to the PHY registers. > > One theory is that the PHY is configured to operate on MDIO address 0 > which it treats as a special broadcast address. > > Datasheet states: > "PHYAD (config pins) sets the PHY address for the device. > The RTL8211F(I)/RTL8211FD(I) supports PHY addresses from 0x01 to 0x07. > Note 1: An MDIO command with PHY address=0 is a broadcast from the MAC; > each PHY device should respond." > > This issue is not seen with the other PHY (different make) on the board > which is configured for address 0x1. > > As a woraround we try to probe the PHY multiple times instead of giving > up on the first attempt. > > Signed-off-by: Roger Quadros > --- > drivers/net/ti/am65-cpsw-nuss.c | 19 ++- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c > index 51a8167d14..4f8a2f9b93 100644 > --- a/drivers/net/ti/am65-cpsw-nuss.c > +++ b/drivers/net/ti/am65-cpsw-nuss.c > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > > #include "cpsw_mdio.h" > > @@ -678,14 +679,22 @@ static int am65_cpsw_phy_init(struct udevice *dev) > struct am65_cpsw_priv *priv = dev_get_priv(dev); > struct am65_cpsw_common *cpsw_common = priv->cpsw_common; > struct eth_pdata *pdata = dev_get_plat(dev); > - struct phy_device *phydev; > u32 supported = PHY_GBIT_FEATURES; > + struct phy_device *phydev; > + int tries; > int ret; > > - phydev = phy_connect(cpsw_common->bus, > - priv->phy_addr, > - priv->dev, > - pdata->phy_interface); > + /* Some boards (e.g. beagleplay) don't work on first go */ > + for (tries = 1; tries <= 5; tries++) { > + phydev = phy_connect(cpsw_common->bus, > + priv->phy_addr, > + priv->dev, > + pdata->phy_interface); > + if (phydev) > + break; > + > + mdelay(10); > + } > > if (!phydev) { > dev_err(dev, "phy_connect() failed\n"); -- Regards, Siddharth.
Re: [PATCH] net: Fix the displayed value of bytes transferred
On 14/08/23 10:06, Siddharth Vadapalli wrote: > Hello Tom, > > On 11/08/23 21:45, Tom Rini wrote: >> On Fri, Aug 11, 2023 at 10:49:23AM +0530, Siddharth Vadapalli wrote: >>> Ravi, >>> >>> On 10/08/23 17:00, Ravi Gunasekaran wrote: ... >> >> Uh, maybe I'm just missing something, but I think there's two things. >> First, this should be "%u" for "unsigned decimal". Second, >> doc/develop/printf.rst needs to be fixed since: >> int %d, %x >> unsigned int%d, %x >> >> Should is wrong and should say %u, %x, because, well, that's what would >> be correct, yes? > > Thank you for reviewing the patch. Yes, %u works well and can print u32 > variable > accurately. I tested it for 0x. %d prints -1 for the same. So, %lu > isn't > necessary and %u is sufficient. I will replace %lu with %u and post the v2 > patch. Additionally, I will include a patch in the v2 series to update the > Documentation as pointed out by you. I have posted the v2 series at: https://patchwork.ozlabs.org/project/uboot/list/?series=368661=%2A=both > >> > -- Regards, Siddharth.
[PATCH v2 2/2] doc: printf() codes: Fix format specifier for unsigned int
The format specifier for the "unsigned int" variable is documented as "%d". However, it should be "%u". Thus, fix it. Fixes: f5e9035043fb ("doc: printf() codes") Reported-by: Tom Rini Signed-off-by: Siddharth Vadapalli --- doc/develop/printf.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/develop/printf.rst b/doc/develop/printf.rst index 7b9aea0687..05909a1f22 100644 --- a/doc/develop/printf.rst +++ b/doc/develop/printf.rst @@ -111,7 +111,7 @@ unsigned char %u, %x short %d, %x unsigned short %u, %x int %d, %x -unsigned int%d, %x +unsigned int%u, %x long%ld, %lx unsigned long %lu, %lx long long %lld, %llx -- 2.34.1
[PATCH v2 1/2] net: Fix the displayed value of bytes transferred
In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable "net_boot_file_size" is printed using "%d", resulting in negative values being reported for large file sizes. Fix this by using "%u" to print the decimal value corresponding to the bytes transferred. Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file") Signed-off-by: Siddharth Vadapalli --- net/net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/net.c b/net/net.c index 43abbac7c3..e6f61f0f8f 100644 --- a/net/net.c +++ b/net/net.c @@ -716,7 +716,7 @@ restart: case NETLOOP_SUCCESS: net_cleanup_loop(); if (net_boot_file_size > 0) { - printf("Bytes transferred = %d (%x hex)\n", + printf("Bytes transferred = %u (%x hex)\n", net_boot_file_size, net_boot_file_size); env_set_hex("filesize", net_boot_file_size); env_set_hex("fileaddr", image_load_addr); -- 2.34.1
[PATCH v2 0/2] Fix format specifier for net_boot_file_size
Hello, This series fixes the format specifier for printing the decimal value of the variable "net_boot_file_size", changing it from "%d" to "%u". With the format specifier being "%d", for large file sizes, the value displayed is negative. Using "%u" fixes this. Additionally, as reported by Tom Rini in the mail thread corresponding to the v1 patch of this series, the documentation for the printf format specifiers needs to be fixed for the "unsigned int" variable. Thus, update the documentation as well. Regards, Siddharth. --- v1: https://patchwork.ozlabs.org/project/uboot/patch/20230810091523.3168975-1-s-vadapa...@ti.com/ Changes since v1: - Use "%u" instead of "%lu" to display the decimal value of the u32 variable "net_boot_file_size", as suggested by Tom Rini. - Add a new patch to update the documentation for printf format specifiers, changing the format specifier from "%d" to "%u" for the "unsigned int" variable, as reported by Tom Rini. Siddharth Vadapalli (2): net: Fix the displayed value of bytes transferred doc: printf() codes: Fix format specifier for unsigned int doc/develop/printf.rst | 2 +- net/net.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.34.1
Re: [PATCH] net: Fix the displayed value of bytes transferred
Hello Tom, On 11/08/23 21:45, Tom Rini wrote: > On Fri, Aug 11, 2023 at 10:49:23AM +0530, Siddharth Vadapalli wrote: >> Ravi, >> >> On 10/08/23 17:00, Ravi Gunasekaran wrote: >>> Siddharth, >>> >>> On 8/10/23 2:45 PM, Siddharth Vadapalli wrote: >>>> In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable >>>> "net_boot_file_size" is printed using "%d", resulting in negative values >>>> being reported for large file sizes. Fix this by using "%lu" to print >>>> the decimal value corresponding to the bytes transferred. >>>> >>>> Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file") >>>> Signed-off-by: Siddharth Vadapalli >>>> --- >>>> net/net.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/net/net.c b/net/net.c >>>> index 43abbac7c3..7aaeafc247 100644 >>>> --- a/net/net.c >>>> +++ b/net/net.c >>>> @@ -716,7 +716,7 @@ restart: >>>>case NETLOOP_SUCCESS: >>>>net_cleanup_loop(); >>>>if (net_boot_file_size > 0) { >>>> - printf("Bytes transferred = %d (%x hex)\n", >>>> + printf("Bytes transferred = %lu (%x hex)\n", >>> >>> 'net_boot_file_size' is of type u32. Using "%lu" will throw a warning for >>> this. >>> As per [0], format specifier for 'unsigned int' is "%d, %x'. >>> >>> You could perhaps change the data type of 'net_boot_file_size' to 'ulong' >>> as well. >> >> The issue here isn't the size of the variable itself, but the format >> specifier. >> For large file sizes, the hex value printed for the variable is correct, but >> the >> decimal value is negative. >> >>> >>> [0] - https://u-boot.readthedocs.io/en/latest/develop/printf.html > > Uh, maybe I'm just missing something, but I think there's two things. > First, this should be "%u" for "unsigned decimal". Second, > doc/develop/printf.rst needs to be fixed since: > int %d, %x > unsigned int%d, %x > > Should is wrong and should say %u, %x, because, well, that's what would > be correct, yes? Thank you for reviewing the patch. Yes, %u works well and can print u32 variable accurately. I tested it for 0x. %d prints -1 for the same. So, %lu isn't necessary and %u is sufficient. I will replace %lu with %u and post the v2 patch. Additionally, I will include a patch in the v2 series to update the Documentation as pointed out by you. > -- Regards, Siddharth.
Re: [PATCH] net: Fix the displayed value of bytes transferred
Ravi, On 10/08/23 17:00, Ravi Gunasekaran wrote: > Siddharth, > > On 8/10/23 2:45 PM, Siddharth Vadapalli wrote: >> In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable >> "net_boot_file_size" is printed using "%d", resulting in negative values >> being reported for large file sizes. Fix this by using "%lu" to print >> the decimal value corresponding to the bytes transferred. >> >> Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file") >> Signed-off-by: Siddharth Vadapalli >> --- >> net/net.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/net.c b/net/net.c >> index 43abbac7c3..7aaeafc247 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -716,7 +716,7 @@ restart: >> case NETLOOP_SUCCESS: >> net_cleanup_loop(); >> if (net_boot_file_size > 0) { >> -printf("Bytes transferred = %d (%x hex)\n", >> +printf("Bytes transferred = %lu (%x hex)\n", > > 'net_boot_file_size' is of type u32. Using "%lu" will throw a warning for > this. > As per [0], format specifier for 'unsigned int' is "%d, %x'. > > You could perhaps change the data type of 'net_boot_file_size' to 'ulong' as > well. The issue here isn't the size of the variable itself, but the format specifier. For large file sizes, the hex value printed for the variable is correct, but the decimal value is negative. > > [0] - https://u-boot.readthedocs.io/en/latest/develop/printf.html > >> net_boot_file_size, net_boot_file_size); >> env_set_hex("filesize", net_boot_file_size); >> env_set_hex("fileaddr", image_load_addr); > -- Regards, Siddharth.
[PATCH] net: Fix the displayed value of bytes transferred
In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable "net_boot_file_size" is printed using "%d", resulting in negative values being reported for large file sizes. Fix this by using "%lu" to print the decimal value corresponding to the bytes transferred. Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file") Signed-off-by: Siddharth Vadapalli --- net/net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/net.c b/net/net.c index 43abbac7c3..7aaeafc247 100644 --- a/net/net.c +++ b/net/net.c @@ -716,7 +716,7 @@ restart: case NETLOOP_SUCCESS: net_cleanup_loop(); if (net_boot_file_size > 0) { - printf("Bytes transferred = %d (%x hex)\n", + printf("Bytes transferred = %lu (%x hex)\n", net_boot_file_size, net_boot_file_size); env_set_hex("filesize", net_boot_file_size); env_set_hex("fileaddr", image_load_addr); -- 2.34.1
[PATCH v2 6/6] configs: j7200_evm_a72: Enable configs for SGMII support with MAIN CPSW0
The MAIN CPSW0 instance of CPSW Ethernet Switch on TI's J7200 SoC supports SGMII mode. To enable support for utilizing the SGMII daughtercard with TI's DP83869 PHY, enable the corresponding config. Also, since the SGMII daughtercard is connected to the I2C GPIO Expander 2 connector on the J7200 EVM, powering on the Ethernet PHY and resetting it requires GPIO Hogging capability. Enable it as well. Signed-off-by: Siddharth Vadapalli --- configs/j7200_evm_a72_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configs/j7200_evm_a72_defconfig b/configs/j7200_evm_a72_defconfig index c68d52537e..8dad928493 100644 --- a/configs/j7200_evm_a72_defconfig +++ b/configs/j7200_evm_a72_defconfig @@ -115,6 +115,7 @@ CONFIG_FASTBOOT_FLASH=y CONFIG_FASTBOOT_FLASH_MMC_DEV=0 CONFIG_FASTBOOT_CMD_OEM_FORMAT=y CONFIG_TI_SCI_PROTOCOL=y +CONFIG_GPIO_HOG=y CONFIG_DA8XX_GPIO=y CONFIG_DM_PCA953X=y CONFIG_DM_I2C=y @@ -147,6 +148,7 @@ CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_MTD=y CONFIG_MULTIPLEXER=y CONFIG_MUX_MMIO=y +CONFIG_PHY_TI_DP83869=y CONFIG_PHY_FIXED=y CONFIG_TI_AM65_CPSW_NUSS=y CONFIG_PHY=y -- 2.34.1
[PATCH v2 5/6] phy: ti: j721e-wiz: Add SGMII support in WIZ driver for J721E
Enable full rate divider configuration support for J721E_WIZ_16G for SGMII. Signed-off-by: Siddharth Vadapalli --- drivers/phy/ti/phy-j721e-wiz.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c index cba87f5093..7261339907 100644 --- a/drivers/phy/ti/phy-j721e-wiz.c +++ b/drivers/phy/ti/phy-j721e-wiz.c @@ -590,6 +590,8 @@ static int wiz_phy_fullrt_div(struct wiz *wiz, int lane) if (wiz->lane_phy_type[lane] == PHY_TYPE_PCIE) return regmap_field_write(wiz->p0_fullrt_div[lane], 0x1); break; + + case J721E_WIZ_16G: case J721E_WIZ_10G: if (wiz->lane_phy_type[lane] == PHY_TYPE_SGMII) return regmap_field_write(wiz->p0_fullrt_div[lane], 0x2); -- 2.34.1
[PATCH v2 4/6] phy: ti: phy-j721e-wiz: Add SGMII support in wiz driver for J7200
Select the same mac divider for SGMII too as the one being used for QSGMII. Enable full rate divider configuration support for J721E_WIZ_10G for SGMII. Signed-off-by: Siddharth Vadapalli --- drivers/phy/ti/phy-j721e-wiz.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c index 34314d0bd1..cba87f5093 100644 --- a/drivers/phy/ti/phy-j721e-wiz.c +++ b/drivers/phy/ti/phy-j721e-wiz.c @@ -585,12 +585,18 @@ static int wiz_reset_assert(struct reset_ctl *reset_ctl) static int wiz_phy_fullrt_div(struct wiz *wiz, int lane) { - if (wiz->type != AM64_WIZ_10G) + switch (wiz->type) { + case AM64_WIZ_10G: + if (wiz->lane_phy_type[lane] == PHY_TYPE_PCIE) + return regmap_field_write(wiz->p0_fullrt_div[lane], 0x1); + break; + case J721E_WIZ_10G: + if (wiz->lane_phy_type[lane] == PHY_TYPE_SGMII) + return regmap_field_write(wiz->p0_fullrt_div[lane], 0x2); + break; + default: return 0; - - if (wiz->lane_phy_type[lane] == PHY_TYPE_PCIE) - return regmap_field_write(wiz->p0_fullrt_div[lane], 0x1); - + } return 0; } @@ -706,7 +712,8 @@ static int wiz_p_mac_div_sel(struct wiz *wiz) int i; for (i = 0; i < num_lanes; i++) { - if (wiz->lane_phy_type[i] == PHY_TYPE_QSGMII) { + if (wiz->lane_phy_type[i] == PHY_TYPE_SGMII || + wiz->lane_phy_type[i] == PHY_TYPE_QSGMII) { ret = regmap_field_write(wiz->p_mac_div_sel0[i], 1); if (ret) return ret; -- 2.34.1
[PATCH v2 3/6] net: ti: am65-cpsw-nuss: Add logic to support MDIO reset
From: Suman Anna Enhance the AM65 CPSW NUSS driver to perform a MDIO reset using a GPIO line. Logic is also added to perform a pre and post delay around reset using the optional 'reset-delay-us' and 'reset-post-delay-us' properties. This is similar to the reset being performed in the Linux kernel. The reset is done once when the CPSW MDIO bus is being initialized. Signed-off-by: Suman Anna Signed-off-by: Siddharth Vadapalli --- drivers/net/ti/am65-cpsw-nuss.c | 38 - 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c index 456c3eb5b2..f4e5809380 100644 --- a/drivers/net/ti/am65-cpsw-nuss.c +++ b/drivers/net/ti/am65-cpsw-nuss.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -26,6 +27,7 @@ #include #include #include +#include #include #include "cpsw_mdio.h" @@ -96,6 +98,8 @@ #define AM65_CPSW_CPPI_PKT_TYPE0x7 +#define DEFAULT_GPIO_RESET_DELAY 10 + struct am65_cpsw_port { fdt_addr_t port_base; fdt_addr_t port_sgmii_base; @@ -120,6 +124,10 @@ struct am65_cpsw_common { struct mii_dev *bus; u32 bus_freq; + struct gpio_descmdio_gpio_reset; + u32 reset_delay_us; + u32 reset_post_delay_us; + struct dma dma_tx; struct dma dma_rx; u32 rx_next; @@ -679,6 +687,16 @@ static int am65_cpsw_mdio_init(struct udevice *dev) if (!priv->has_phy || cpsw_common->bus) return 0; + if (IS_ENABLED(CONFIG_DM_GPIO)) { + if (dm_gpio_is_valid(_common->mdio_gpio_reset)) { + dm_gpio_set_value(_common->mdio_gpio_reset, 1); + udelay(cpsw_common->reset_delay_us); + dm_gpio_set_value(_common->mdio_gpio_reset, 0); + if (cpsw_common->reset_post_delay_us > 0) + udelay(cpsw_common->reset_post_delay_us); + } + } + ret = am65_cpsw_mdio_setup(dev); if (ret) return ret; @@ -818,7 +836,7 @@ out: static int am65_cpsw_probe_nuss(struct udevice *dev) { struct am65_cpsw_common *cpsw_common = dev_get_priv(dev); - ofnode ports_np, node; + ofnode ports_np, node, mdio_np; int ret, i; struct udevice *port_dev; @@ -845,6 +863,24 @@ static int am65_cpsw_probe_nuss(struct udevice *dev) AM65_CPSW_CPSW_NU_ALE_BASE; cpsw_common->mdio_base = cpsw_common->ss_base + AM65_CPSW_MDIO_BASE; + if (IS_ENABLED(CONFIG_DM_GPIO)) { + /* get bus level PHY reset GPIO details */ + mdio_np = dev_read_subnode(dev, "mdio"); + if (!ofnode_valid(mdio_np)) { + ret = -ENOENT; + goto out; + } + + cpsw_common->reset_delay_us = ofnode_read_u32_default(mdio_np, "reset-delay-us", + DEFAULT_GPIO_RESET_DELAY); + cpsw_common->reset_post_delay_us = ofnode_read_u32_default(mdio_np, + "reset-post-delay-us", + 0); + ret = gpio_request_by_name_nodev(mdio_np, "reset-gpios", 0, +_common->mdio_gpio_reset, +GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE); + } + ports_np = dev_read_subnode(dev, "ethernet-ports"); if (!ofnode_valid(ports_np)) { ret = -ENOENT; -- 2.34.1
[PATCH v2 2/6] net: ti: am65-cpsw-nuss: Add support for SGMII mode
Add support for configuring the CPSW Ethernet Switch in SGMII mode. Signed-off-by: Siddharth Vadapalli --- drivers/net/ti/am65-cpsw-nuss.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c index 51a8167d14..456c3eb5b2 100644 --- a/drivers/net/ti/am65-cpsw-nuss.c +++ b/drivers/net/ti/am65-cpsw-nuss.c @@ -57,6 +57,12 @@ #define AM65_CPSW_PN_REG_SA_L 0x308 #define AM65_CPSW_PN_REG_SA_H 0x30c +#define AM65_CPSW_SGMII_CONTROL_REG 0x010 +#define AM65_CPSW_SGMII_MR_ADV_ABILITY_REG 0x018 +#define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLEBIT(0) + +#define ADVERTISE_SGMII0x1 + #define AM65_CPSW_ALE_CTL_REG 0x8 #define AM65_CPSW_ALE_CTL_REG_ENABLE BIT(31) #define AM65_CPSW_ALE_CTL_REG_RESET_TBLBIT(30) @@ -92,6 +98,7 @@ struct am65_cpsw_port { fdt_addr_t port_base; + fdt_addr_t port_sgmii_base; fdt_addr_t macsl_base; booldisabled; u32 mac_control; @@ -204,6 +211,8 @@ static int am65_cpsw_update_link(struct am65_cpsw_priv *priv) mac_control |= AM65_CPSW_MACSL_CTL_REG_FULL_DUPLEX; if (phy->speed == 100) mac_control |= AM65_CPSW_MACSL_CTL_REG_IFCTL_A; + if (phy->interface == PHY_INTERFACE_MODE_SGMII) + mac_control |= AM65_CPSW_MACSL_CTL_EXT_EN; } if (mac_control == port->mac_control) @@ -229,6 +238,7 @@ out: #define AM65_GMII_SEL_MODE_MII 0 #define AM65_GMII_SEL_MODE_RMII1 #define AM65_GMII_SEL_MODE_RGMII 2 +#define AM65_GMII_SEL_MODE_SGMII 3 #define AM65_GMII_SEL_RGMII_IDMODE BIT(4) @@ -280,6 +290,10 @@ static int am65_cpsw_gmii_sel_k3(struct am65_cpsw_priv *priv, rgmii_id = true; break; + case PHY_INTERFACE_MODE_SGMII: + mode = AM65_GMII_SEL_MODE_SGMII; + break; + default: dev_warn(dev, "Unsupported PHY mode: %u. Defaulting to MII.\n", @@ -420,6 +434,13 @@ static int am65_cpsw_start(struct udevice *dev) goto err_dis_rx; } + if (priv->phydev->interface == PHY_INTERFACE_MODE_SGMII) { + writel(ADVERTISE_SGMII, + port->port_sgmii_base + AM65_CPSW_SGMII_MR_ADV_ABILITY_REG); + writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE, + port->port_sgmii_base + AM65_CPSW_SGMII_CONTROL_REG); + } + ret = phy_startup(priv->phydev); if (ret) { dev_err(dev, "phy_startup failed\n"); @@ -872,6 +893,8 @@ static int am65_cpsw_probe_nuss(struct udevice *dev) port->port_base = cpsw_common->cpsw_base + AM65_CPSW_CPSW_NU_PORTS_OFFSET + (i * AM65_CPSW_CPSW_NU_PORTS_OFFSET); + port->port_sgmii_base = cpsw_common->ss_base + + (i * AM65_CPSW_SGMII_BASE); port->macsl_base = port->port_base + AM65_CPSW_CPSW_NU_PORT_MACSL_OFFSET; } -- 2.34.1
[PATCH v2 1/6] dma: ti: Update J21E PSIL endpoint information for MAIN CPSW0
From: Suman Anna The PSIL endpoint data for J721E currently covers only the MCU domain CPSW0 instance. Add the data for the MAIN domain CPSW0 as well to allow the MAIN domain Ethernet ports to be usable on any platform using J721E SoC. Additionally, since J721E's PSIL endpoint data is applicable to J7200 SoC as well, the MAIN CPSW0 instance on J7200 will also be usable now. Signed-off-by: Suman Anna [s-vadapa...@ti.com: Update commit message indicating support for J7200] Signed-off-by: Siddharth Vadapalli --- drivers/dma/ti/k3-psil-j721e.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/dma/ti/k3-psil-j721e.c b/drivers/dma/ti/k3-psil-j721e.c index 105ffd946f..8e57e860f2 100644 --- a/drivers/dma/ti/k3-psil-j721e.c +++ b/drivers/dma/ti/k3-psil-j721e.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com + * Copyright (C) 2019-2023 Texas Instruments Incorporated - https://www.ti.com * Author: Peter Ujfalusi */ @@ -21,13 +21,15 @@ /* PSI-L source thread IDs, used for RX (DMA_DEV_TO_MEM) */ static struct psil_ep j721e_src_ep_map[] = { - /* CPSW0 */ + /* MCU_CPSW0 */ PSIL_ETHERNET(0x7000), + /* MAIN_CPSW0 */ + PSIL_ETHERNET(0x4a00), }; /* PSI-L destination thread IDs, used for TX (DMA_MEM_TO_DEV) */ static struct psil_ep j721e_dst_ep_map[] = { - /* CPSW0 */ + /* MCU_CPSW0 */ PSIL_ETHERNET(0xf000), PSIL_ETHERNET(0xf001), PSIL_ETHERNET(0xf002), @@ -36,6 +38,15 @@ static struct psil_ep j721e_dst_ep_map[] = { PSIL_ETHERNET(0xf005), PSIL_ETHERNET(0xf006), PSIL_ETHERNET(0xf007), + /* MAIN_CPSW0 */ + PSIL_ETHERNET(0xca00), + PSIL_ETHERNET(0xca01), + PSIL_ETHERNET(0xca02), + PSIL_ETHERNET(0xca03), + PSIL_ETHERNET(0xca04), + PSIL_ETHERNET(0xca05), + PSIL_ETHERNET(0xca06), + PSIL_ETHERNET(0xca07), }; struct psil_ep_map j721e_ep_map = { -- 2.34.1
[PATCH v2 0/6] Add SGMII support for MAIN CPSW on TI's J7200 SoC
Hello, This series adds support for SGMII mode to the CPSW driver to enable the functionality on TI's J7200 SoC. Supporting SGMII mode also requires changes to the WIZ driver which acts as a wrapper for the SerDes used by the CPSW MAC to transmit data to the Ethernet PHY daughtercard mounted on the I2C GPIO Expander 2 connector on the J7200 EVM. Powering on and resetting the Ethernet PHY requires MDIO support which is added to the CPSW driver. For supporting DMA transactions from the MAIN CPSW instance to the A72 Host on J7200 SoC, the corresponding PSI-L endpoint information is added for the J721E SoC, which is applicable to J7200 SoC as well. The SGMII daughtercard used for testing SGMII mode has TI's DP83869 PHY. Thus, enable the config for DP83869 driver functionality. Also, enable GPIO HOG config. Regards, Siddharth. --- v1: https://patchwork.ozlabs.org/project/uboot/list/?series=362969=%2A=both Changes since v1: - Add checks in the am65-cpsw-nuss.c driver for the DM_GPIO config being enabled: if (IS_ENABLED(CONFIG_DM_GPIO)) to fix build errors for SoCs which don't have DM_GPIO config enabled. - Rebase series on commit 7755b22007 of u-boot master. **NOTE**: This series is based on top of commit: 7755b22007 Merge tag 'x86-pull-20230801' of https://source.denx.de/u-boot/custodians/u-boot-x86 Siddharth Vadapalli (4): net: ti: am65-cpsw-nuss: Add support for SGMII mode phy: ti: phy-j721e-wiz: Add SGMII support in wiz driver for J7200 phy: ti: j721e-wiz: Add SGMII support in WIZ driver for J721E configs: j7200_evm_a72: Enable configs for SGMII support with MAIN CPSW0 Suman Anna (2): dma: ti: Update J21E PSIL endpoint information for MAIN CPSW0 net: ti: am65-cpsw-nuss: Add logic to support MDIO reset configs/j7200_evm_a72_defconfig | 2 ++ drivers/dma/ti/k3-psil-j721e.c | 17 +++-- drivers/net/ti/am65-cpsw-nuss.c | 61 - drivers/phy/ti/phy-j721e-wiz.c | 21 4 files changed, 91 insertions(+), 10 deletions(-) -- 2.34.1
Re: [PATCH 3/6] net: ti: am65-cpsw-nuss: Add logic to support MDIO reset
On 31/07/23 10:13, Siddharth Vadapalli wrote: > Tom, > > On 22/07/23 01:06, Tom Rini wrote: >> On Sat, Jul 08, 2023 at 04:15:20PM +0530, Siddharth Vadapalli wrote: >> >>> From: Suman Anna >>> >>> Enhance the AM65 CPSW NUSS driver to perform a MDIO reset using a GPIO >>> line. Logic is also added to perform a pre and post delay around reset >>> using the optional 'reset-delay-us' and 'reset-post-delay-us' properties. >>> This is similar to the reset being performed in the Linux kernel. The >>> reset is done once when the CPSW MDIO bus is being initialized. >>> >>> Signed-off-by: Suman Anna >>> Signed-off-by: Siddharth Vadapalli >>> --- >>> drivers/net/ti/am65-cpsw-nuss.c | 34 - >>> 1 file changed, 33 insertions(+), 1 deletion(-) >> >> So this breaks building on am62x_evm_a53 because now TI_AM65_CPSW_NUSS >> needs to select DM_GPIO. And the next problem is that we don't enable a >> GPIO driver on that platform as we should I suspect make DA8XX_GPIO >> default y if K3. > > Sorry for the delayed response. I am planning to address this issue by using: > #if CONFIG_IS_ENABLED(DM_GPIO) Instead of CONFIG_IS_ENABLED(DM_GPIO), I will use: if (IS_ENABLED(CONFIG_DM_GPIO)) { in the functions. > in the driver, similar to how it is done for other ethernet drivers such as: > drivers/net/mvneta.c and drivers/net/mvpp2.c. > > Also, the GPIO hog and reset capabilities are only required on those boards > which have the Ethernet PHY present on a daughtercard connected to the board. > > I will post the v2 of this series with the above changes to fix the build > issues. Please let me know if the above approach is acceptable. > >> > -- Regards, Siddharth.
Re: [PATCH 3/6] net: ti: am65-cpsw-nuss: Add logic to support MDIO reset
Tom, On 22/07/23 01:06, Tom Rini wrote: > On Sat, Jul 08, 2023 at 04:15:20PM +0530, Siddharth Vadapalli wrote: > >> From: Suman Anna >> >> Enhance the AM65 CPSW NUSS driver to perform a MDIO reset using a GPIO >> line. Logic is also added to perform a pre and post delay around reset >> using the optional 'reset-delay-us' and 'reset-post-delay-us' properties. >> This is similar to the reset being performed in the Linux kernel. The >> reset is done once when the CPSW MDIO bus is being initialized. >> >> Signed-off-by: Suman Anna >> Signed-off-by: Siddharth Vadapalli >> --- >> drivers/net/ti/am65-cpsw-nuss.c | 34 - >> 1 file changed, 33 insertions(+), 1 deletion(-) > > So this breaks building on am62x_evm_a53 because now TI_AM65_CPSW_NUSS > needs to select DM_GPIO. And the next problem is that we don't enable a > GPIO driver on that platform as we should I suspect make DA8XX_GPIO > default y if K3. Sorry for the delayed response. I am planning to address this issue by using: #if CONFIG_IS_ENABLED(DM_GPIO) in the driver, similar to how it is done for other ethernet drivers such as: drivers/net/mvneta.c and drivers/net/mvpp2.c. Also, the GPIO hog and reset capabilities are only required on those boards which have the Ethernet PHY present on a daughtercard connected to the board. I will post the v2 of this series with the above changes to fix the build issues. Please let me know if the above approach is acceptable. > -- Regards, Siddharth.
Re: [PATCH v4 1/2] net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node
Hello Maxime, Thank you for the patch. On 24/07/23 19:27, Maxime Ripard wrote: > The binding represents the MDIO controller as a child device tree > node of the MAC device tree node. > > The U-Boot driver mostly ignores that child device tree node and just > hardcodes the resources it uses to support both the MAC and MDIO in a > single driver. > > However, some resources like pinctrl muxing states are thus ignored. > This has been a problem with some device trees that will put some > pinctrl states on the MDIO device tree node, like the SK-AM62 Device > Tree does. > > Let's rework the driver a bit to create a dummy MDIO driver that we will > then get during our initialization to force the core to select the right > muxing. > > Signed-off-by: Maxime Ripard LGTM. Reviewed-by: Siddharth Vadapalli > --- > drivers/net/ti/Kconfig | 1 + > drivers/net/ti/am65-cpsw-nuss.c | 60 > + > 2 files changed, 61 insertions(+) > > diff --git a/drivers/net/ti/Kconfig b/drivers/net/ti/Kconfig > index d9f1c019a885..02660e4fbb44 100644 > --- a/drivers/net/ti/Kconfig > +++ b/drivers/net/ti/Kconfig > @@ -41,6 +41,7 @@ endchoice > config TI_AM65_CPSW_NUSS > bool "TI K3 AM65x MCU CPSW Nuss Ethernet controller driver" > depends on ARCH_K3 > + imply DM_MDIO > imply MISC_INIT_R > imply MISC > imply SYSCON > diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c > index ce52106e5238..51a8167d14a9 100644 > --- a/drivers/net/ti/am65-cpsw-nuss.c > +++ b/drivers/net/ti/am65-cpsw-nuss.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -605,14 +606,62 @@ static const struct soc_attr k3_mdio_soc_data[] = { > { /* sentinel */ }, > }; > > +static ofnode am65_cpsw_find_mdio(ofnode parent) > +{ > + ofnode node; > + > + ofnode_for_each_subnode(node, parent) > + if (ofnode_device_is_compatible(node, "ti,cpsw-mdio")) > + return node; > + > + return ofnode_null(); > +} > + > +static int am65_cpsw_mdio_setup(struct udevice *dev) > +{ > + struct am65_cpsw_priv *priv = dev_get_priv(dev); > + struct am65_cpsw_common *cpsw_common = priv->cpsw_common; > + struct udevice *mdio_dev; > + ofnode mdio; > + int ret; > + > + mdio = am65_cpsw_find_mdio(dev_ofnode(cpsw_common->dev)); > + if (!ofnode_valid(mdio)) > + return 0; > + > + /* > + * The MDIO controller is represented in the DT binding by a > + * subnode of the MAC controller. > + * > + * We don't have a DM driver for the MDIO device yet, and thus any > + * pinctrl setting on its node will be ignored. > + * > + * However, we do need to make sure the pins states tied to the > + * MDIO node are configured properly. Fortunately, the core DM > + * does that for use when we get a device, so we can work around > + * that whole issue by just requesting a dummy MDIO driver to > + * probe, and our pins will get muxed. > + */ > + ret = uclass_get_device_by_ofnode(UCLASS_MDIO, mdio, _dev); > + if (ret) > + return ret; > + > + return 0; > +} > + > static int am65_cpsw_mdio_init(struct udevice *dev) > { > struct am65_cpsw_priv *priv = dev_get_priv(dev); > struct am65_cpsw_common *cpsw_common = priv->cpsw_common; > + int ret; > > if (!priv->has_phy || cpsw_common->bus) > return 0; > > + ret = am65_cpsw_mdio_setup(dev); > + if (ret) > + return ret; > + > cpsw_common->bus = cpsw_mdio_init(dev->name, > cpsw_common->mdio_base, > cpsw_common->bus_freq, > @@ -868,3 +917,14 @@ U_BOOT_DRIVER(am65_cpsw_nuss_port) = { > .plat_auto = sizeof(struct eth_pdata), > .flags = DM_FLAG_ALLOC_PRIV_DMA | DM_FLAG_OS_PREPARE, > }; > + > +static const struct udevice_id am65_cpsw_mdio_ids[] = { > + { .compatible = "ti,cpsw-mdio" }, > + { } > +}; > + > +U_BOOT_DRIVER(am65_cpsw_mdio) = { > + .name = "am65_cpsw_mdio", > + .id = UCLASS_MDIO, > + .of_match = am65_cpsw_mdio_ids, > +}; > -- Regards, Siddharth.
Re: [PATCH] arm: mach-k3: *: dev-data: Update to use ARRAY_SIZE
On 14/07/23 01:06, Nishanth Menon wrote: > Instead of hard-coding the count of entries manually, use ARRAY_SIZE > to keep the count updates appropriately. > > Cc: Bryan Brattlof > Suggested-by: Ravi Gunasekaran > Signed-off-by: Nishanth Menon Reviewed-by: Siddharth Vadapalli > --- > > Link to discussion that triggered this: > https://lore.kernel.org/all/20230713185927.3h6vqap4x3h62...@bryanbrattlof.com/ > > arch/arm/mach-k3/am62ax/dev-data.c | 8 > arch/arm/mach-k3/am62x/dev-data.c | 8 > arch/arm/mach-k3/j7200/dev-data.c | 8 > arch/arm/mach-k3/j721e/dev-data.c | 8 > arch/arm/mach-k3/j721s2/dev-data.c | 8 > 5 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/arch/arm/mach-k3/am62ax/dev-data.c > b/arch/arm/mach-k3/am62ax/dev-data.c > index 74739c638524..abf5d8e91aa2 100644 > --- a/arch/arm/mach-k3/am62ax/dev-data.c > +++ b/arch/arm/mach-k3/am62ax/dev-data.c > @@ -66,8 +66,8 @@ const struct ti_k3_pd_platdata am62ax_pd_platdata = { > .pd = soc_pd_list, > .lpsc = soc_lpsc_list, > .devs = soc_dev_list, > - .num_psc = 2, > - .num_pd = 4, > - .num_lpsc = 14, > - .num_devs = 19, > + .num_psc = ARRAY_SIZE(soc_psc_list), > + .num_pd = ARRAY_SIZE(soc_pd_list), > + .num_lpsc = ARRAY_SIZE(soc_lpsc_list), > + .num_devs = ARRAY_SIZE(soc_dev_list), > }; > diff --git a/arch/arm/mach-k3/am62x/dev-data.c > b/arch/arm/mach-k3/am62x/dev-data.c > index 616d0650b9c0..1a6f9e2ca03e 100644 > --- a/arch/arm/mach-k3/am62x/dev-data.c > +++ b/arch/arm/mach-k3/am62x/dev-data.c > @@ -71,8 +71,8 @@ const struct ti_k3_pd_platdata am62x_pd_platdata = { > .pd = soc_pd_list, > .lpsc = soc_lpsc_list, > .devs = soc_dev_list, > - .num_psc = 2, > - .num_pd = 5, > - .num_lpsc = 16, > - .num_devs = 21, > + .num_psc = ARRAY_SIZE(soc_psc_list), > + .num_pd = ARRAY_SIZE(soc_pd_list), > + .num_lpsc = ARRAY_SIZE(soc_lpsc_list), > + .num_devs = ARRAY_SIZE(soc_dev_list), > }; > diff --git a/arch/arm/mach-k3/j7200/dev-data.c > b/arch/arm/mach-k3/j7200/dev-data.c > index c1a4dab6941a..4ddc34210ebb 100644 > --- a/arch/arm/mach-k3/j7200/dev-data.c > +++ b/arch/arm/mach-k3/j7200/dev-data.c > @@ -75,8 +75,8 @@ const struct ti_k3_pd_platdata j7200_pd_platdata = { > .pd = soc_pd_list, > .lpsc = soc_lpsc_list, > .devs = soc_dev_list, > - .num_psc = 2, > - .num_pd = 6, > - .num_lpsc = 17, > - .num_devs = 23, > + .num_psc = ARRAY_SIZE(soc_psc_list), > + .num_pd = ARRAY_SIZE(soc_pd_list), > + .num_lpsc = ARRAY_SIZE(soc_lpsc_list), > + .num_devs = ARRAY_SIZE(soc_dev_list), > }; > diff --git a/arch/arm/mach-k3/j721e/dev-data.c > b/arch/arm/mach-k3/j721e/dev-data.c > index f0afa3552b7f..97f017f8af50 100644 > --- a/arch/arm/mach-k3/j721e/dev-data.c > +++ b/arch/arm/mach-k3/j721e/dev-data.c > @@ -73,8 +73,8 @@ const struct ti_k3_pd_platdata j721e_pd_platdata = { > .pd = soc_pd_list, > .lpsc = soc_lpsc_list, > .devs = soc_dev_list, > - .num_psc = 2, > - .num_pd = 5, > - .num_lpsc = 16, > - .num_devs = 23, > + .num_psc = ARRAY_SIZE(soc_psc_list), > + .num_pd = ARRAY_SIZE(soc_pd_list), > + .num_lpsc = ARRAY_SIZE(soc_lpsc_list), > + .num_devs = ARRAY_SIZE(soc_dev_list), > }; > diff --git a/arch/arm/mach-k3/j721s2/dev-data.c > b/arch/arm/mach-k3/j721s2/dev-data.c > index 35e8b17eb1e7..8c999a3c5a8b 100644 > --- a/arch/arm/mach-k3/j721s2/dev-data.c > +++ b/arch/arm/mach-k3/j721s2/dev-data.c > @@ -79,8 +79,8 @@ const struct ti_k3_pd_platdata j721s2_pd_platdata = { > .pd = soc_pd_list, > .lpsc = soc_lpsc_list, > .devs = soc_dev_list, > - .num_psc = 2, > - .num_pd = 6, > - .num_lpsc = 19, > - .num_devs = 25, > + .num_psc = ARRAY_SIZE(soc_psc_list), > + .num_pd = ARRAY_SIZE(soc_pd_list), > + .num_lpsc = ARRAY_SIZE(soc_lpsc_list), > + .num_devs = ARRAY_SIZE(soc_dev_list), > }; -- Regards, Siddharth.
[PATCH 6/6] configs: j7200_evm_a72: Enable configs for SGMII support with MAIN CPSW0
The MAIN CPSW0 instance of CPSW Ethernet Switch on TI's J7200 SoC supports SGMII mode. To enable support for utilizing the SGMII daughtercard with TI's DP83869 PHY, enable the corresponding config. Also, since the SGMII daughtercard is connected to the I2C GPIO Expander 2 connector on the J7200 EVM, powering on the Ethernet PHY and resetting it requires GPIO Hogging capability. Enable it as well. Signed-off-by: Siddharth Vadapalli --- configs/j7200_evm_a72_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configs/j7200_evm_a72_defconfig b/configs/j7200_evm_a72_defconfig index e40900fffa..e5b5c85ed1 100644 --- a/configs/j7200_evm_a72_defconfig +++ b/configs/j7200_evm_a72_defconfig @@ -122,6 +122,7 @@ CONFIG_FASTBOOT_FLASH=y CONFIG_FASTBOOT_FLASH_MMC_DEV=0 CONFIG_FASTBOOT_CMD_OEM_FORMAT=y CONFIG_TI_SCI_PROTOCOL=y +CONFIG_GPIO_HOG=y CONFIG_DA8XX_GPIO=y CONFIG_DM_PCA953X=y CONFIG_DM_I2C=y @@ -154,6 +155,7 @@ CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_MTD=y CONFIG_MULTIPLEXER=y CONFIG_MUX_MMIO=y +CONFIG_PHY_TI_DP83869=y CONFIG_PHY_FIXED=y CONFIG_TI_AM65_CPSW_NUSS=y CONFIG_PHY=y -- 2.34.1
[PATCH 4/6] phy: ti: phy-j721e-wiz: Add SGMII support in wiz driver for J7200
Select the same mac divider for SGMII too as the one being used for QSGMII. Enable full rate divider configuration support for J721E_WIZ_10G for SGMII. Signed-off-by: Siddharth Vadapalli --- drivers/phy/ti/phy-j721e-wiz.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c index 23397175d3..12421f90b0 100644 --- a/drivers/phy/ti/phy-j721e-wiz.c +++ b/drivers/phy/ti/phy-j721e-wiz.c @@ -575,12 +575,18 @@ static int wiz_reset_assert(struct reset_ctl *reset_ctl) static int wiz_phy_fullrt_div(struct wiz *wiz, int lane) { - if (wiz->type != AM64_WIZ_10G) + switch (wiz->type) { + case AM64_WIZ_10G: + if (wiz->lane_phy_type[lane] == PHY_TYPE_PCIE) + return regmap_field_write(wiz->p0_fullrt_div[lane], 0x1); + break; + case J721E_WIZ_10G: + if (wiz->lane_phy_type[lane] == PHY_TYPE_SGMII) + return regmap_field_write(wiz->p0_fullrt_div[lane], 0x2); + break; + default: return 0; - - if (wiz->lane_phy_type[lane] == PHY_TYPE_PCIE) - return regmap_field_write(wiz->p0_fullrt_div[lane], 0x1); - + } return 0; } @@ -696,7 +702,8 @@ static int wiz_p_mac_div_sel(struct wiz *wiz) int i; for (i = 0; i < num_lanes; i++) { - if (wiz->lane_phy_type[i] == PHY_TYPE_QSGMII) { + if (wiz->lane_phy_type[i] == PHY_TYPE_SGMII || + wiz->lane_phy_type[i] == PHY_TYPE_QSGMII) { ret = regmap_field_write(wiz->p_mac_div_sel0[i], 1); if (ret) return ret; -- 2.34.1
[PATCH 5/6] phy: ti: j721e-wiz: Add SGMII support in WIZ driver for J721E
Enable full rate divider configuration support for J721E_WIZ_16G for SGMII. Signed-off-by: Siddharth Vadapalli --- drivers/phy/ti/phy-j721e-wiz.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c index 12421f90b0..ecb2f684ef 100644 --- a/drivers/phy/ti/phy-j721e-wiz.c +++ b/drivers/phy/ti/phy-j721e-wiz.c @@ -580,6 +580,8 @@ static int wiz_phy_fullrt_div(struct wiz *wiz, int lane) if (wiz->lane_phy_type[lane] == PHY_TYPE_PCIE) return regmap_field_write(wiz->p0_fullrt_div[lane], 0x1); break; + + case J721E_WIZ_16G: case J721E_WIZ_10G: if (wiz->lane_phy_type[lane] == PHY_TYPE_SGMII) return regmap_field_write(wiz->p0_fullrt_div[lane], 0x2); -- 2.34.1
[PATCH 3/6] net: ti: am65-cpsw-nuss: Add logic to support MDIO reset
From: Suman Anna Enhance the AM65 CPSW NUSS driver to perform a MDIO reset using a GPIO line. Logic is also added to perform a pre and post delay around reset using the optional 'reset-delay-us' and 'reset-post-delay-us' properties. This is similar to the reset being performed in the Linux kernel. The reset is done once when the CPSW MDIO bus is being initialized. Signed-off-by: Suman Anna Signed-off-by: Siddharth Vadapalli --- drivers/net/ti/am65-cpsw-nuss.c | 34 - 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c index 403de3ea25..f39d1adc99 100644 --- a/drivers/net/ti/am65-cpsw-nuss.c +++ b/drivers/net/ti/am65-cpsw-nuss.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -23,6 +24,7 @@ #include #include #include +#include #include #include "cpsw_mdio.h" @@ -93,6 +95,8 @@ #define AM65_CPSW_CPPI_PKT_TYPE0x7 +#define DEFAULT_GPIO_RESET_DELAY 10 + struct am65_cpsw_port { fdt_addr_t port_base; fdt_addr_t port_sgmii_base; @@ -119,6 +123,10 @@ struct am65_cpsw_common { struct mii_dev *bus; u32 bus_freq; + struct gpio_descmdio_gpio_reset; + u32 reset_delay_us; + u32 reset_post_delay_us; + struct dma dma_tx; struct dma dma_rx; u32 rx_next; @@ -590,6 +598,14 @@ static int am65_cpsw_mdio_init(struct udevice *dev) if (!priv->has_phy || cpsw_common->bus) return 0; + if (dm_gpio_is_valid(_common->mdio_gpio_reset)) { + dm_gpio_set_value(_common->mdio_gpio_reset, 1); + udelay(cpsw_common->reset_delay_us); + dm_gpio_set_value(_common->mdio_gpio_reset, 0); + if (cpsw_common->reset_post_delay_us > 0) + udelay(cpsw_common->reset_post_delay_us); + } + cpsw_common->bus = cpsw_mdio_init(dev->name, cpsw_common->mdio_base, cpsw_common->bus_freq, @@ -723,7 +739,7 @@ out: static int am65_cpsw_probe_nuss(struct udevice *dev) { struct am65_cpsw_common *cpsw_common = dev_get_priv(dev); - ofnode ports_np, node; + ofnode ports_np, node, mdio_np; int ret, i; struct udevice *port_dev; @@ -752,6 +768,22 @@ static int am65_cpsw_probe_nuss(struct udevice *dev) AM65_CPSW_CPSW_NU_ALE_BASE; cpsw_common->mdio_base = cpsw_common->ss_base + AM65_CPSW_MDIO_BASE; + /* get bus level PHY reset GPIO details */ + mdio_np = dev_read_subnode(dev, "mdio"); + if (!ofnode_valid(mdio_np)) { + ret = -ENOENT; + goto out; + } + + cpsw_common->reset_delay_us = ofnode_read_u32_default(mdio_np, "reset-delay-us", + DEFAULT_GPIO_RESET_DELAY); + cpsw_common->reset_post_delay_us = ofnode_read_u32_default(mdio_np, + "reset-post-delay-us", + 0); + ret = gpio_request_by_name_nodev(mdio_np, "reset-gpios", 0, +_common->mdio_gpio_reset, +GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE); + ports_np = dev_read_subnode(dev, "ethernet-ports"); if (!ofnode_valid(ports_np)) { ret = -ENOENT; -- 2.34.1
[PATCH 1/6] dma: ti: Update J21E PSIL endpoint information for MAIN CPSW0
From: Suman Anna The PSIL endpoint data for J721E currently covers only the MCU domain CPSW0 instance. Add the data for the MAIN domain CPSW0 as well to allow the MAIN domain Ethernet ports to be usable on any platform using J721E SoC. Additionally, since J721E's PSIL endpoint data is applicable to J7200 SoC as well, the MAIN CPSW0 instance on J7200 will also be usable now. Signed-off-by: Suman Anna [s-vadapa...@ti.com: Update commit message indicating support for J7200] Signed-off-by: Siddharth Vadapalli --- drivers/dma/ti/k3-psil-j721e.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/dma/ti/k3-psil-j721e.c b/drivers/dma/ti/k3-psil-j721e.c index 105ffd946f..8e57e860f2 100644 --- a/drivers/dma/ti/k3-psil-j721e.c +++ b/drivers/dma/ti/k3-psil-j721e.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com + * Copyright (C) 2019-2023 Texas Instruments Incorporated - https://www.ti.com * Author: Peter Ujfalusi */ @@ -21,13 +21,15 @@ /* PSI-L source thread IDs, used for RX (DMA_DEV_TO_MEM) */ static struct psil_ep j721e_src_ep_map[] = { - /* CPSW0 */ + /* MCU_CPSW0 */ PSIL_ETHERNET(0x7000), + /* MAIN_CPSW0 */ + PSIL_ETHERNET(0x4a00), }; /* PSI-L destination thread IDs, used for TX (DMA_MEM_TO_DEV) */ static struct psil_ep j721e_dst_ep_map[] = { - /* CPSW0 */ + /* MCU_CPSW0 */ PSIL_ETHERNET(0xf000), PSIL_ETHERNET(0xf001), PSIL_ETHERNET(0xf002), @@ -36,6 +38,15 @@ static struct psil_ep j721e_dst_ep_map[] = { PSIL_ETHERNET(0xf005), PSIL_ETHERNET(0xf006), PSIL_ETHERNET(0xf007), + /* MAIN_CPSW0 */ + PSIL_ETHERNET(0xca00), + PSIL_ETHERNET(0xca01), + PSIL_ETHERNET(0xca02), + PSIL_ETHERNET(0xca03), + PSIL_ETHERNET(0xca04), + PSIL_ETHERNET(0xca05), + PSIL_ETHERNET(0xca06), + PSIL_ETHERNET(0xca07), }; struct psil_ep_map j721e_ep_map = { -- 2.34.1
[PATCH 2/6] net: ti: am65-cpsw-nuss: Add support for SGMII mode
Add support for configuring the CPSW Ethernet Switch in SGMII mode. Signed-off-by: Siddharth Vadapalli --- drivers/net/ti/am65-cpsw-nuss.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c index 523a4c9f91..403de3ea25 100644 --- a/drivers/net/ti/am65-cpsw-nuss.c +++ b/drivers/net/ti/am65-cpsw-nuss.c @@ -54,6 +54,12 @@ #define AM65_CPSW_PN_REG_SA_L 0x308 #define AM65_CPSW_PN_REG_SA_H 0x30c +#define AM65_CPSW_SGMII_CONTROL_REG 0x010 +#define AM65_CPSW_SGMII_MR_ADV_ABILITY_REG 0x018 +#define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLEBIT(0) + +#define ADVERTISE_SGMII0x1 + #define AM65_CPSW_ALE_CTL_REG 0x8 #define AM65_CPSW_ALE_CTL_REG_ENABLE BIT(31) #define AM65_CPSW_ALE_CTL_REG_RESET_TBLBIT(30) @@ -89,6 +95,7 @@ struct am65_cpsw_port { fdt_addr_t port_base; + fdt_addr_t port_sgmii_base; fdt_addr_t macsl_base; booldisabled; u32 mac_control; @@ -203,6 +210,8 @@ static int am65_cpsw_update_link(struct am65_cpsw_priv *priv) mac_control |= AM65_CPSW_MACSL_CTL_REG_FULL_DUPLEX; if (phy->speed == 100) mac_control |= AM65_CPSW_MACSL_CTL_REG_IFCTL_A; + if (phy->interface == PHY_INTERFACE_MODE_SGMII) + mac_control |= AM65_CPSW_MACSL_CTL_EXT_EN; } if (mac_control == port->mac_control) @@ -228,6 +237,7 @@ out: #define AM65_GMII_SEL_MODE_MII 0 #define AM65_GMII_SEL_MODE_RMII1 #define AM65_GMII_SEL_MODE_RGMII 2 +#define AM65_GMII_SEL_MODE_SGMII 3 #define AM65_GMII_SEL_RGMII_IDMODE BIT(4) @@ -260,6 +270,10 @@ static void am65_cpsw_gmii_sel_k3(struct am65_cpsw_priv *priv, rgmii_id = true; break; + case PHY_INTERFACE_MODE_SGMII: + mode = AM65_GMII_SEL_MODE_SGMII; + break; + default: dev_warn(common->dev, "Unsupported PHY mode: %u. Defaulting to MII.\n", @@ -396,6 +410,13 @@ static int am65_cpsw_start(struct udevice *dev) goto err_dis_rx; } + if (priv->phydev->interface == PHY_INTERFACE_MODE_SGMII) { + writel(ADVERTISE_SGMII, + port->port_sgmii_base + AM65_CPSW_SGMII_MR_ADV_ABILITY_REG); + writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE, + port->port_sgmii_base + AM65_CPSW_SGMII_CONTROL_REG); + } + ret = phy_startup(priv->phydev); if (ret) { dev_err(dev, "phy_startup failed\n"); @@ -779,6 +800,8 @@ static int am65_cpsw_probe_nuss(struct udevice *dev) port->port_base = cpsw_common->cpsw_base + AM65_CPSW_CPSW_NU_PORTS_OFFSET + (i * AM65_CPSW_CPSW_NU_PORTS_OFFSET); + port->port_sgmii_base = cpsw_common->ss_base + + AM65_CPSW_SGMII_BASE * (i); port->macsl_base = port->port_base + AM65_CPSW_CPSW_NU_PORT_MACSL_OFFSET; } -- 2.34.1
[PATCH 0/6] Add SGMII support for MAIN CPSW on TI's J7200 SoC
Hello, This series adds support for SGMII mode to the CPSW driver to enable the functionality on TI's J7200 SoC. Supporting SGMII mode also requires changes to the WIZ driver which acts as a wrapper for the SerDes used by the CPSW MAC to transmit data to the Ethernet PHY daughtercard mounted on the I2C GPIO Expander 2 connector on the J7200 EVM. Powering on and resetting the Ethernet PHY requires MDIO support which is added to the CPSW driver. For supporting DMA transactions from the MAIN CPSW instance to the A72 Host on J7200 SoC, the corresponding PSI-L endpoint information is added for the J721E SoC, which is applicable to J7200 SoC as well. The SGMII daughtercard used for testing SGMII mode has TI's DP83869 PHY. Thus, enable the config for DP83869 driver functionality. Also, enable GPIO HOG config. **NOTE**: This series is based on top of commit: 0beb649053 MAINTAINERS: correct at91 tree link and depends on the following patch being merged as well: Link: https://patchwork.ozlabs.org/project/uboot/patch/20230614222853.574427-1-dannenb...@ti.com/ Regards, Siddharth. Siddharth Vadapalli (4): net: ti: am65-cpsw-nuss: Add support for SGMII mode phy: ti: phy-j721e-wiz: Add SGMII support in wiz driver for J7200 phy: ti: j721e-wiz: Add SGMII support in WIZ driver for J721E configs: j7200_evm_a72: Enable configs for SGMII support with MAIN CPSW0 Suman Anna (2): dma: ti: Update J21E PSIL endpoint information for MAIN CPSW0 net: ti: am65-cpsw-nuss: Add logic to support MDIO reset configs/j7200_evm_a72_defconfig | 2 ++ drivers/dma/ti/k3-psil-j721e.c | 17 -- drivers/net/ti/am65-cpsw-nuss.c | 57 - drivers/phy/ti/phy-j721e-wiz.c | 21 4 files changed, 87 insertions(+), 10 deletions(-) -- 2.34.1
Re: [PATCH] net: ti: am65-cpsw-nuss: Use dedicated port mode control registers
Hello Tom, Can this patch please be merged? On 15/06/23 03:58, Andreas Dannenberg wrote: > The different CPSW sub-system Ethernet ports have different PHY mode > control registers. In order to allow the modes to get configured > independently only the register for the port in question must be > accessed, otherwise we would just be re-configuring the mode for port 1, > while leaving all others at their power-on defaults. Fix this issue by > adding a port-number based offset to the mode control base register > address based on the fact that the control registers for the different > ports are spaced exactly 0x4 bytes apart. > > Fixes: 9d0dca1199d1 ("net: ethernet: ti: Introduce am654 gigabit eth switch > subsystem driver") > Signed-off-by: Andreas Dannenberg > Reviewed-by: Siddharth Vadapalli > --- > Driver changes bench-tested on an SK-AM62 EVM by iterating through > different variations of RGMII and RMII modes for CPSW ports 1 and 2 and > checking operation as well as CTRL_ENET1_CTRL and CTRL_ENET2_CTRL > control register contents from the U-Boot command line via 'md.l'. > Testing was done on top of today's 'next' branch. > > drivers/net/ti/am65-cpsw-nuss.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c > index f674b0baa3..523a4c9f91 100644 > --- a/drivers/net/ti/am65-cpsw-nuss.c > +++ b/drivers/net/ti/am65-cpsw-nuss.c > @@ -223,6 +223,8 @@ out: > return phy->link; > } > > +#define AM65_GMII_SEL_PORT_OFFS(x) (0x4 * ((x) - 1)) > + > #define AM65_GMII_SEL_MODE_MII 0 > #define AM65_GMII_SEL_MODE_RMII 1 > #define AM65_GMII_SEL_MODE_RGMII 2 > @@ -233,11 +235,12 @@ static void am65_cpsw_gmii_sel_k3(struct am65_cpsw_priv > *priv, > phy_interface_t phy_mode, int slave) > { > struct am65_cpsw_common *common = priv->cpsw_common; > + fdt_addr_t gmii_sel = common->gmii_sel + AM65_GMII_SEL_PORT_OFFS(slave); > u32 reg; > u32 mode = 0; > bool rgmii_id = false; > > - reg = readl(common->gmii_sel); > + reg = readl(gmii_sel); > > dev_dbg(common->dev, "old gmii_sel: %08x\n", reg); > > @@ -273,9 +276,9 @@ static void am65_cpsw_gmii_sel_k3(struct am65_cpsw_priv > *priv, > reg = mode; > dev_dbg(common->dev, "gmii_sel PHY mode: %u, new gmii_sel: %08x\n", > phy_mode, reg); > - writel(reg, common->gmii_sel); > + writel(reg, gmii_sel); > > - reg = readl(common->gmii_sel); > + reg = readl(gmii_sel); > if (reg != mode) > dev_err(common->dev, > "gmii_sel PHY mode NOT SET!: requested: %08x, gmii_sel: > %08x\n", -- Regards, Siddharth.
Re: [PATCH v2 02/10] arm: dts: introduce j784s4 u-boot dtbs
Hari, On 21/03/23 19:40, Hari Nagalla wrote: > Introduce the base dts files needed for u-boot or to augment the linux > dtbs for use in the u-boot-spl and u-boot binaries. > > Signed-off-by: Apurva Nandan > Signed-off-by: Hari Nagalla > --- > arch/arm/dts/Makefile|2 + > arch/arm/dts/k3-j784s4-ddr-evm-lp4-4266.dtsi | 8757 + > arch/arm/dts/k3-j784s4-ddr.dtsi | 8858 ++ > arch/arm/dts/k3-j784s4-evm-u-boot.dtsi | 135 + > arch/arm/dts/k3-j784s4-r5-evm.dts| 209 + > 5 files changed, 17961 insertions(+) > create mode 100644 arch/arm/dts/k3-j784s4-ddr-evm-lp4-4266.dtsi > create mode 100644 arch/arm/dts/k3-j784s4-ddr.dtsi > create mode 100644 arch/arm/dts/k3-j784s4-evm-u-boot.dtsi > create mode 100644 arch/arm/dts/k3-j784s4-r5-evm.dts > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > index 7a577deb50..7690fd21db 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -1264,6 +1264,8 @@ dtb-$(CONFIG_SOC_K3_J721S2) += > k3-am68-sk-base-board.dtb\ > k3-am68-sk-r5-base-board.dtb\ > k3-j721s2-common-proc-board.dtb\ > k3-j721s2-r5-common-proc-board.dtb [...] > + > +_sdhci1 { > + u-boot,dm-spl; > +}; > diff --git a/arch/arm/dts/k3-j784s4-r5-evm.dts > b/arch/arm/dts/k3-j784s4-r5-evm.dts > new file mode 100644 > index 00..7350a9be34 > --- /dev/null > +++ b/arch/arm/dts/k3-j784s4-r5-evm.dts > @@ -0,0 +1,209 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ > + */ > + > +/dts-v1/; > + > +#include "k3-j784s4.dtsi" > +#include > +#include "k3-j784s4-ddr-evm-lp4-4266.dtsi" > +#include "k3-j784s4-ddr.dtsi" Based on the patch by Nishanth at: https://lore.kernel.org/r/20230414075726.387461-13...@ti.com/ It might be better for "k3-j784s4-r5-evm.dts" to inherit "k3-j784s4-evm.dts" as it will prevent duplication. [...] -- Regards, Siddharth.
Re: [PATCH v2 01/10] arm: dts: introduce j784s4 dtbs from linux kernel
Hello Hari, On 21/03/23 19:40, Hari Nagalla wrote: > Introduce the basic j784s4 SoC dtbs from the linux kernel along with > the new j784s4 specific pinmux definitions that we will use to generate > the dtbs for the u-boot-spl and u-boot binaries. > > Signed-off-by: Apurva Nandan > Signed-off-by: Hari Nagalla > --- > arch/arm/dts/k3-j784s4-evm.dts | 196 + > arch/arm/dts/k3-j784s4-main.dtsi | 1007 > arch/arm/dts/k3-j784s4-mcu-wakeup.dtsi | 311 > arch/arm/dts/k3-j784s4.dtsi| 287 +++ > include/dt-bindings/pinctrl/k3.h |3 + > 5 files changed, 1804 insertions(+) > create mode 100644 arch/arm/dts/k3-j784s4-evm.dts > create mode 100644 arch/arm/dts/k3-j784s4-main.dtsi > create mode 100644 arch/arm/dts/k3-j784s4-mcu-wakeup.dtsi > create mode 100644 arch/arm/dts/k3-j784s4.dtsi > > diff --git a/arch/arm/dts/k3-j784s4-evm.dts b/arch/arm/dts/k3-j784s4-evm.dts > new file mode 100644 > index 00..8cd4a7ecc1 > --- /dev/null > +++ b/arch/arm/dts/k3-j784s4-evm.dts > @@ -0,0 +1,196 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ > + * [...] > + > +_gpio0 { > + status = "okay"; > +}; > diff --git a/arch/arm/dts/k3-j784s4-main.dtsi > b/arch/arm/dts/k3-j784s4-main.dtsi > new file mode 100644 > index 00..7edf324ac1 > --- /dev/null > +++ b/arch/arm/dts/k3-j784s4-main.dtsi > @@ -0,0 +1,1007 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for J784S4 SoC Family Main Domain peripherals > + * > + * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ > + */ > + > +_main { > + msmc_ram: sram@7000 { > + compatible = "mmio-sram"; > + reg = <0x00 0x7000 0x00 0x80>; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0x00 0x00 0x7000 0x80>; > + > + atf-sram@0 { > + reg = <0x00 0x2>; > + }; > + > + tifs-sram@1f { > + reg = <0x1f 0x1>; > + }; > + > + l3cache-sram@20 { [...] > + > + main_navss: bus@3000 { > + compatible = "simple-bus"; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges = <0x00 0x3000 0x00 0x3000 0x00 0x0c40>; The property: ti,sci-dev-id = <280>; is missing here. Please refer: https://lore.kernel.org/r/20230314152611.140969-2-j-choudh...@ti.com/ > + dma-coherent; > + dma-ranges; > + > + main_navss_intr: interrupt-controller@310e { > + compatible = "ti,sci-intr"; > + reg = <0x00 0x310e 0x00 0x4000>; > + ti,intr-trigger-type = <4>; > + interrupt-controller; > + interrupt-parent = <>; > + #interrupt-cells = <1>; > + ti,sci = <>; > + ti,sci-dev-id = <283>; > + ti,interrupt-ranges = <0 64 64>, > + <64 448 64>, > + <128 672 64>; > + }; > + [...] > + interrupt-names = "int0", "int1"; > + bosch,mram-cfg = <0x00 128 64 64 64 64 32 32>; > + status = "disabled"; > + }; > +}; > diff --git a/arch/arm/dts/k3-j784s4-mcu-wakeup.dtsi > b/arch/arm/dts/k3-j784s4-mcu-wakeup.dtsi > new file mode 100644 > index 00..93952af618 > --- /dev/null > +++ b/arch/arm/dts/k3-j784s4-mcu-wakeup.dtsi > @@ -0,0 +1,311 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for J784S4 SoC Family MCU/WAKEUP Domain peripherals > + * > + * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ > + */ > + > +_mcu_wakeup { > + sms: system-controller@44083000 { > + compatible = "ti,k2g-sci"; > + ti,host-id = <12>; > + > + mbox-names = "rx", "tx"; > + > + mboxes = <_proxy_main 11>, > + <_proxy_main 13>; > + > + reg-names = "debug_messages"; > + reg = <0x00 0x44083000 0x00 0x1000>; > + > + k3_pds: power-controller { [...] > + interrupt-names = "int0", "int1"; > + bosch,mram-cfg = <0x00 128 64 64 64 64 32 32>; > + status = "disabled"; > + }; > + > + mcu_navss: bus@2838{ > + compatible = "simple-bus"; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges = <0x00 0x2838 0x00 0x2838 0x00 0x0388>; The property ti,sci-dev-id = <323>; is missing here. Please refer: https://lore.kernel.org/r/20230314152611.140969-2-j-choudh...@ti.com/ > + dma-coherent; > + dma-ranges; > + > + mcu_ringacc:
Re: [PATCH] configs: am62: Add configs for enabling ETHBOOT in R5SPL
Hello, Kindly ignore this patch. On 11-04-2023 15:03, Siddharth Vadapalli wrote: > From: Kishon Vijay Abraham I > > Add configs for enabling ETHBOOT in R5SPL. Adding a separate config > minimizes the risk of going past the R5-SPL size limit for any future > config additions. > > Signed-off-by: Kishon Vijay Abraham I > Signed-off-by: Andreas Dannenberg > Signed-off-by: Siddharth Vadapalli > --- > configs/am62x_evm_r5_ethboot_defconfig | 118 + > 1 file changed, 118 insertions(+) > create mode 100644 configs/am62x_evm_r5_ethboot_defconfig > > diff --git a/configs/am62x_evm_r5_ethboot_defconfig > b/configs/am62x_evm_r5_ethboot_defconfig > new file mode 100644 > index 00..d9f642cad3 > --- /dev/null > +++ b/configs/am62x_evm_r5_ethboot_defconfig > @@ -0,0 +1,118 @@ > +CONFIG_ARM=y > +CONFIG_ARCH_K3=y > +CONFIG_SPL_GPIO_SUPPORT=y > +CONFIG_SPL_LIBCOMMON_SUPPORT=y > +CONFIG_SPL_LIBGENERIC_SUPPORT=y > +CONFIG_SYS_MALLOC_F_LEN=0x9000 > +CONFIG_SOC_K3_AM625=y > +CONFIG_TARGET_AM625_R5_EVM=y > +CONFIG_ENV_SIZE=0x2 > +CONFIG_ENV_OFFSET=0x68 > +CONFIG_DM_GPIO=y > +CONFIG_SPL_TEXT_BASE=0x43c0 > +CONFIG_SPL_MMC_SUPPORT=y > +CONFIG_SPL_SERIAL_SUPPORT=y > +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y > +CONFIG_SPL_STACK_R_ADDR=0x8200 > +CONFIG_SPL_SIZE_LIMIT=0x4 > +CONFIG_SPL_SIZE_LIMIT_PROVIDE_STACK=0x4000 > +CONFIG_SPL_FS_FAT=y > +CONFIG_SPL_LIBDISK_SUPPORT=y > +CONFIG_DEFAULT_DEVICE_TREE="k3-am625-r5-sk" > +CONFIG_SPL_LOAD_FIT=y > +CONFIG_SPL_LOAD_FIT_ADDRESS=0x8008 > +CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y > +# CONFIG_DISPLAY_CPUINFO is not set > +CONFIG_SPL_SIZE_LIMIT_SUBTRACT_GD=y > +CONFIG_SPL_SIZE_LIMIT_SUBTRACT_MALLOC=y > +CONFIG_SPL_SYS_REPORT_STACK_F_USAGE=y > +CONFIG_SPL_BOARD_INIT=y > +CONFIG_SPL_SYS_MALLOC_SIMPLE=y > +CONFIG_SPL_STACK_R=y > +CONFIG_SPL_SEPARATE_BSS=y > +CONFIG_SPL_EARLY_BSS=y > +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y > +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x400 > +CONFIG_SPL_DMA=y > +CONFIG_SPL_ENV_SUPPORT=y > +CONFIG_SPL_ETH_SUPPORT=y > +CONFIG_SPL_I2C_SUPPORT=y > +CONFIG_SPL_DM_MAILBOX=y > +CONFIG_SPL_NET_SUPPORT=y > +CONFIG_SPL_NET_VCI_STRING="AM62X U-Boot R5 SPL" > +CONFIG_SPL_DM_RESET=y > +CONFIG_SPL_POWER_DOMAIN=y > +CONFIG_SPL_RAM_SUPPORT=y > +CONFIG_SPL_RAM_DEVICE=y > +CONFIG_SPL_REMOTEPROC=y > +CONFIG_HUSH_PARSER=y > +CONFIG_CMD_ASKENV=y > +CONFIG_CMD_DFU=y > +CONFIG_CMD_GPT=y > +CONFIG_CMD_MMC=y > +CONFIG_CMD_REMOTEPROC=y > +# CONFIG_CMD_SETEXPR is not set > +CONFIG_CMD_DHCP=y > +CONFIG_CMD_TIME=y > +CONFIG_CMD_FAT=y > +CONFIG_OF_CONTROL=y > +CONFIG_SPL_OF_CONTROL=y > +CONFIG_SPL_MULTI_DTB_FIT=y > +CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y > +CONFIG_ENV_IS_IN_MMC=y > +CONFIG_SYS_RELOC_GD_ENV_ADDR=y > +CONFIG_SYS_MMC_ENV_PART=1 > +CONFIG_DM=y > +CONFIG_SPL_DM=y > +CONFIG_SPL_DM_SEQ_ALIAS=y > +CONFIG_REGMAP=y > +CONFIG_SPL_REGMAP=y > +CONFIG_SPL_OF_TRANSLATE=y > +CONFIG_CLK=y > +CONFIG_SPL_CLK=y > +CONFIG_SPL_CLK_CCF=y > +CONFIG_SPL_CLK_K3=y > +CONFIG_SPL_CLK_K3_PLL=y > +CONFIG_DMA_CHANNELS=y > +CONFIG_TI_K3_NAVSS_UDMA=y > +CONFIG_TI_SCI_PROTOCOL=y > +CONFIG_DA8XX_GPIO=y > +CONFIG_DM_I2C=y > +CONFIG_SYS_I2C_OMAP24XX=y > +CONFIG_DM_MAILBOX=y > +CONFIG_K3_SEC_PROXY=y > +CONFIG_DM_MMC=y > +CONFIG_SPL_MMC_HS200_SUPPORT=y > +CONFIG_MMC_SDHCI=y > +CONFIG_MMC_SDHCI_ADMA=y > +CONFIG_SPL_MMC_SDHCI_ADMA=y > +CONFIG_MMC_SDHCI_AM654=y > +CONFIG_DM_SPI_FLASH=y > +CONFIG_SPI_FLASH_SPANSION=y > +CONFIG_SPI_FLASH_STMICRO=y > +CONFIG_DM_ETH=y > +CONFIG_TI_AM65_CPSW_NUSS=y > +CONFIG_PINCTRL=y > +# CONFIG_PINCTRL_GENERIC is not set > +CONFIG_SPL_PINCTRL=y > +# CONFIG_SPL_PINCTRL_GENERIC is not set > +CONFIG_PINCTRL_SINGLE=y > +CONFIG_POWER_DOMAIN=y > +CONFIG_TI_POWER_DOMAIN=y > +CONFIG_K3_SYSTEM_CONTROLLER=y > +CONFIG_REMOTEPROC_TI_K3_ARM64=y > +CONFIG_DM_RESET=y > +CONFIG_RESET_TI_SCI=y > +CONFIG_SPECIFY_CONSOLE_INDEX=y > +CONFIG_DM_SERIAL=y > +CONFIG_SOC_DEVICE=y > +CONFIG_SOC_DEVICE_TI_K3=y > +CONFIG_SOC_TI=y > +CONFIG_SPI=y > +CONFIG_DM_SPI=y > +CONFIG_CADENCE_QSPI=y > +CONFIG_TIMER=y > +CONFIG_SPL_TIMER=y > +CONFIG_OMAP_TIMER=y > +CONFIG_LIB_RATIONAL=y > +CONFIG_SPL_LIB_RATIONAL=y -- Regards, Siddharth.
[PATCH] configs: j7200_evm_a72: Enhance bootcmd to configure ethernet PHY
From: Kishon Vijay Abraham I Update the default BOOTCOMMAND to provide an automatic and easier way to configure ethernet PHY before loading the firmware. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli --- configs/j7200_evm_a72_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/j7200_evm_a72_defconfig b/configs/j7200_evm_a72_defconfig index 9b6512bacb..8a32c40543 100644 --- a/configs/j7200_evm_a72_defconfig +++ b/configs/j7200_evm_a72_defconfig @@ -32,7 +32,7 @@ CONFIG_SPL_LOAD_FIT=y CONFIG_SPL_LOAD_FIT_ADDRESS=0x8100 CONFIG_OF_BOARD_SETUP=y CONFIG_DISTRO_DEFAULTS=y -CONFIG_BOOTCOMMAND="run findfdt; run envboot; run init_${boot}; run boot_rprocs; run get_kern_${boot}; run get_fdt_${boot}; run get_overlay_${boot}; run run_kern" +CONFIG_BOOTCOMMAND="run findfdt; run envboot; run init_${boot}; run main_cpsw0_qsgmii_phyinit; run boot_rprocs; run get_kern_${boot}; run get_fdt_${boot}; run get_overlay_${boot}; run run_kern" CONFIG_LOGLEVEL=7 CONFIG_SPL_MAX_SIZE=0xc CONFIG_SPL_HAS_BSS_LINKER_SECTION=y -- 2.25.1
[PATCH] configs: am62: Add configs for enabling ETHBOOT in R5SPL
From: Kishon Vijay Abraham I Add configs for enabling ETHBOOT in R5SPL. Adding a separate config minimizes the risk of going past the R5-SPL size limit for any future config additions. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Andreas Dannenberg Signed-off-by: Siddharth Vadapalli --- configs/am62x_evm_r5_ethboot_defconfig | 118 + 1 file changed, 118 insertions(+) create mode 100644 configs/am62x_evm_r5_ethboot_defconfig diff --git a/configs/am62x_evm_r5_ethboot_defconfig b/configs/am62x_evm_r5_ethboot_defconfig new file mode 100644 index 00..d9f642cad3 --- /dev/null +++ b/configs/am62x_evm_r5_ethboot_defconfig @@ -0,0 +1,118 @@ +CONFIG_ARM=y +CONFIG_ARCH_K3=y +CONFIG_SPL_GPIO_SUPPORT=y +CONFIG_SPL_LIBCOMMON_SUPPORT=y +CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_SYS_MALLOC_F_LEN=0x9000 +CONFIG_SOC_K3_AM625=y +CONFIG_TARGET_AM625_R5_EVM=y +CONFIG_ENV_SIZE=0x2 +CONFIG_ENV_OFFSET=0x68 +CONFIG_DM_GPIO=y +CONFIG_SPL_TEXT_BASE=0x43c0 +CONFIG_SPL_MMC_SUPPORT=y +CONFIG_SPL_SERIAL_SUPPORT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_SPL_STACK_R_ADDR=0x8200 +CONFIG_SPL_SIZE_LIMIT=0x4 +CONFIG_SPL_SIZE_LIMIT_PROVIDE_STACK=0x4000 +CONFIG_SPL_FS_FAT=y +CONFIG_SPL_LIBDISK_SUPPORT=y +CONFIG_DEFAULT_DEVICE_TREE="k3-am625-r5-sk" +CONFIG_SPL_LOAD_FIT=y +CONFIG_SPL_LOAD_FIT_ADDRESS=0x8008 +CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y +# CONFIG_DISPLAY_CPUINFO is not set +CONFIG_SPL_SIZE_LIMIT_SUBTRACT_GD=y +CONFIG_SPL_SIZE_LIMIT_SUBTRACT_MALLOC=y +CONFIG_SPL_SYS_REPORT_STACK_F_USAGE=y +CONFIG_SPL_BOARD_INIT=y +CONFIG_SPL_SYS_MALLOC_SIMPLE=y +CONFIG_SPL_STACK_R=y +CONFIG_SPL_SEPARATE_BSS=y +CONFIG_SPL_EARLY_BSS=y +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x400 +CONFIG_SPL_DMA=y +CONFIG_SPL_ENV_SUPPORT=y +CONFIG_SPL_ETH_SUPPORT=y +CONFIG_SPL_I2C_SUPPORT=y +CONFIG_SPL_DM_MAILBOX=y +CONFIG_SPL_NET_SUPPORT=y +CONFIG_SPL_NET_VCI_STRING="AM62X U-Boot R5 SPL" +CONFIG_SPL_DM_RESET=y +CONFIG_SPL_POWER_DOMAIN=y +CONFIG_SPL_RAM_SUPPORT=y +CONFIG_SPL_RAM_DEVICE=y +CONFIG_SPL_REMOTEPROC=y +CONFIG_HUSH_PARSER=y +CONFIG_CMD_ASKENV=y +CONFIG_CMD_DFU=y +CONFIG_CMD_GPT=y +CONFIG_CMD_MMC=y +CONFIG_CMD_REMOTEPROC=y +# CONFIG_CMD_SETEXPR is not set +CONFIG_CMD_DHCP=y +CONFIG_CMD_TIME=y +CONFIG_CMD_FAT=y +CONFIG_OF_CONTROL=y +CONFIG_SPL_OF_CONTROL=y +CONFIG_SPL_MULTI_DTB_FIT=y +CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y +CONFIG_ENV_IS_IN_MMC=y +CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_SYS_MMC_ENV_PART=1 +CONFIG_DM=y +CONFIG_SPL_DM=y +CONFIG_SPL_DM_SEQ_ALIAS=y +CONFIG_REGMAP=y +CONFIG_SPL_REGMAP=y +CONFIG_SPL_OF_TRANSLATE=y +CONFIG_CLK=y +CONFIG_SPL_CLK=y +CONFIG_SPL_CLK_CCF=y +CONFIG_SPL_CLK_K3=y +CONFIG_SPL_CLK_K3_PLL=y +CONFIG_DMA_CHANNELS=y +CONFIG_TI_K3_NAVSS_UDMA=y +CONFIG_TI_SCI_PROTOCOL=y +CONFIG_DA8XX_GPIO=y +CONFIG_DM_I2C=y +CONFIG_SYS_I2C_OMAP24XX=y +CONFIG_DM_MAILBOX=y +CONFIG_K3_SEC_PROXY=y +CONFIG_DM_MMC=y +CONFIG_SPL_MMC_HS200_SUPPORT=y +CONFIG_MMC_SDHCI=y +CONFIG_MMC_SDHCI_ADMA=y +CONFIG_SPL_MMC_SDHCI_ADMA=y +CONFIG_MMC_SDHCI_AM654=y +CONFIG_DM_SPI_FLASH=y +CONFIG_SPI_FLASH_SPANSION=y +CONFIG_SPI_FLASH_STMICRO=y +CONFIG_DM_ETH=y +CONFIG_TI_AM65_CPSW_NUSS=y +CONFIG_PINCTRL=y +# CONFIG_PINCTRL_GENERIC is not set +CONFIG_SPL_PINCTRL=y +# CONFIG_SPL_PINCTRL_GENERIC is not set +CONFIG_PINCTRL_SINGLE=y +CONFIG_POWER_DOMAIN=y +CONFIG_TI_POWER_DOMAIN=y +CONFIG_K3_SYSTEM_CONTROLLER=y +CONFIG_REMOTEPROC_TI_K3_ARM64=y +CONFIG_DM_RESET=y +CONFIG_RESET_TI_SCI=y +CONFIG_SPECIFY_CONSOLE_INDEX=y +CONFIG_DM_SERIAL=y +CONFIG_SOC_DEVICE=y +CONFIG_SOC_DEVICE_TI_K3=y +CONFIG_SOC_TI=y +CONFIG_SPI=y +CONFIG_DM_SPI=y +CONFIG_CADENCE_QSPI=y +CONFIG_TIMER=y +CONFIG_SPL_TIMER=y +CONFIG_OMAP_TIMER=y +CONFIG_LIB_RATIONAL=y +CONFIG_SPL_LIB_RATIONAL=y -- 2.25.1
Re: [PATCH] board: ti: j721s2: Add support to detect daughtercards
Hello Tom, On 10/04/23 19:00, Tom Rini wrote: > On Mon, Apr 10, 2023 at 11:40:15AM +0530, Siddharth Vadapalli wrote: > >> From: Kishon Vijay Abraham I >> >> Add support to detect daughtercards (GESI Ethernet card) in-order >> to set the MAC address of the main CPSW2G interface. >> >> Signed-off-by: Kishon Vijay Abraham I >> Signed-off-by: Siddharth Vadapalli >> --- >> board/ti/j721s2/evm.c | 130 ++ >> 1 file changed, 130 insertions(+) > > Do we (a) use this interface in U-Boot? If not (b) can Linux not read Yes, the MAIN CPSW2G interface can be used in U-Boot. > the MAC directly? > -- Regards, Siddharth.
[PATCH] board: ti: j721s2: Add support to detect daughtercards
From: Kishon Vijay Abraham I Add support to detect daughtercards (GESI Ethernet card) in-order to set the MAC address of the main CPSW2G interface. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli --- board/ti/j721s2/evm.c | 130 ++ 1 file changed, 130 insertions(+) diff --git a/board/ti/j721s2/evm.c b/board/ti/j721s2/evm.c index c86715fa21..b920c441a9 100644 --- a/board/ti/j721s2/evm.c +++ b/board/ti/j721s2/evm.c @@ -171,6 +171,135 @@ static void setup_serial(void) snprintf(serial_string, sizeof(serial_string), "%016lx", board_serial); env_set("serial#", serial_string); } + +/* + * Declaration of daughtercards to probe. Note that when adding more + * cards they should be grouped by the 'i2c_addr' field to allow for a + * more efficient probing process. + */ +static const struct { + u8 i2c_addr;/* I2C address of card EEPROM */ + char *card_name;/* EEPROM-programmed card name */ + char *dtbo_name;/* Device tree overlay to apply */ + u8 eth_offset; /* ethXaddr MAC address index offset */ +} ext_cards[] = { + { + 0x52, + "J7X-GESI-EXP", + "k3-j721s2-gesi-exp-board.dtbo", + 1, /* Start populating from eth1addr */ + }, +}; + +#define DAUGHTER_CARD_NO_OF_MAC_ADDR 5 +static bool daughter_card_detect_flags[ARRAY_SIZE(ext_cards)]; + +static int probe_daughtercards(void) +{ + char mac_addr[DAUGHTER_CARD_NO_OF_MAC_ADDR][TI_EEPROM_HDR_ETH_ALEN]; + bool eeprom_read_success; + struct ti_am6_eeprom ep; + u8 previous_i2c_addr; + u8 mac_addr_cnt; + int i; + int ret; + + /* Mark previous I2C address variable as not populated */ + previous_i2c_addr = 0xff; + + /* No EEPROM data was read yet */ + eeprom_read_success = false; + + /* Iterate through list of daughtercards */ + for (i = 0; i < ARRAY_SIZE(ext_cards); i++) { + /* Obtain card-specific I2C address */ + u8 i2c_addr = ext_cards[i].i2c_addr; + + /* Read card EEPROM if not already read previously */ + if (i2c_addr != previous_i2c_addr) { + /* Store I2C address so we can avoid reading twice */ + previous_i2c_addr = i2c_addr; + + /* Get and parse the daughter card EEPROM record */ + ret = ti_i2c_eeprom_am6_get(CONFIG_EEPROM_BUS_ADDRESS, + i2c_addr, + , + (char **)mac_addr, + DAUGHTER_CARD_NO_OF_MAC_ADDR, + _addr_cnt); + if (ret) { + debug("%s: No daughtercard EEPROM at 0x%02x found %d\n", + __func__, i2c_addr, ret); + eeprom_read_success = false; + /* Skip to the next daughtercard to probe */ + continue; + } + + /* EEPROM read successful, okay to further process. */ + eeprom_read_success = true; + } + + /* Only continue processing if EEPROM data was read */ + if (!eeprom_read_success) + continue; + + /* Only process the parsed data if we found a match */ + if (strncmp(ep.name, ext_cards[i].card_name, sizeof(ep.name))) + continue; + + printf("Detected: %s rev %s\n", ep.name, ep.version); + daughter_card_detect_flags[i] = true; + + if (!IS_ENABLED(CONFIG_SPL_BUILD)) { + int j; + /* +* Populate any MAC addresses from daughtercard into the U-Boot +* environment, starting with a card-specific offset so we can +* have multiple ext_cards contribute to the MAC pool in a well- +* defined manner. +*/ + for (j = 0; j < mac_addr_cnt; j++) { + if (!is_valid_ethaddr((u8 *)mac_addr[j])) + continue; + + eth_env_set_enetaddr_by_index("eth", ext_cards[i].eth_offset + j, + (uchar *)mac_addr[j]); + } + } + } + + if (!IS_ENABLED(CONFIG_SPL_BUILD)) { + char name_overlays[1024] = { 0 }; + + for (i = 0; i < ARRAY_SI
Re: [PATCH] cpsw_mdio.c: Use correct reg in cpsw_mdio_get_alive
On 09/02/23 14:56, Ulf Samuelsson wrote: > > Den 2023-02-09 kl. 08:29, skrev Siddharth Vadapalli: >> Hello Ulf, >> >> On 07/02/23 13:55, Ulf Samuelsson wrote: >>> cpsw_mdio_get_alive reads the wrong register. >>> See page 2316 in SPRUH73Q AM335x TRM >>> >>> Signed-off-by: Ulf Samuelsson >>> Cc: Joe Hershberger >>> Cc: Ramon Fried >>> --- >>> drivers/net/ti/cpsw_mdio.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/net/ti/cpsw_mdio.c b/drivers/net/ti/cpsw_mdio.c >>> index a5ba73b739..ac791faa81 100644 >>> --- a/drivers/net/ti/cpsw_mdio.c >>> +++ b/drivers/net/ti/cpsw_mdio.c >>> @@ -51,7 +51,7 @@ struct cpsw_mdio_regs { >>> #define USERACCESS_PHY_REG_SHIFT (21) >>> #define USERACCESS_PHY_ADDR_SHIFT (16) >>> #define USERACCESS_DATA GENMASK(15, 0) >>> - } user[0]; >>> + } user[2]; >>> }; >>> #define CPSW_MDIO_DIV_DEF 0xff >>> @@ -366,8 +366,8 @@ u32 cpsw_mdio_get_alive(struct mii_dev *bus) >>> struct cpsw_mdio *mdio = bus->priv; >>> u32 val; >>> - val = readl(>regs->control); >>> - return val & GENMASK(15, 0); >>> + val = readl(>regs->alive); >>> + return val & GENMASK(7, 0); >> >> Referring to the TRM for AM335x at [0], on page 2401, the MDIOALIVE register >> reports the presence of Ethernet PHYs up to ADDR 31. Also, the cpsw driver: >> drivers/net/ti/cpsw.c which uses the cpsw_mdio_get_alive() function, expects >> the >> return value to be a 16 bit value. Therefore, I believe that the GENMASK >> should >> retain the previous value of "GENMASK(15, 0)", while only the register being >> read needs to be changed from "control" to "alive". Please let me know if I >> misunderstood something regarding your implementation. >> > We had an unusual H/W where the PHY (actually a switch) was connected to the > second Ethernet port. All boards I have seen have the PHY connected to the > first > port or have both ports connected. > > IIRC correctly (this patch is > 2 years old), > we had bad data in the bits (15:8) and this is why only the 8 bits. > Only bits [1:0] are used anyway. Thank you for clarifying. Reviewed-by: Siddharth Vadapalli Regards, Siddharth.
Re: [PATCH] cpsw_mdio.c: Use correct reg in cpsw_mdio_get_alive
Hello Ulf, On 07/02/23 13:55, Ulf Samuelsson wrote: > cpsw_mdio_get_alive reads the wrong register. > See page 2316 in SPRUH73Q AM335x TRM > > Signed-off-by: Ulf Samuelsson > Cc: Joe Hershberger > Cc: Ramon Fried > --- > drivers/net/ti/cpsw_mdio.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ti/cpsw_mdio.c b/drivers/net/ti/cpsw_mdio.c > index a5ba73b739..ac791faa81 100644 > --- a/drivers/net/ti/cpsw_mdio.c > +++ b/drivers/net/ti/cpsw_mdio.c > @@ -51,7 +51,7 @@ struct cpsw_mdio_regs { > #define USERACCESS_PHY_REG_SHIFT (21) > #define USERACCESS_PHY_ADDR_SHIFT(16) > #define USERACCESS_DATA GENMASK(15, 0) > - } user[0]; > + } user[2]; > }; > > #define CPSW_MDIO_DIV_DEF0xff > @@ -366,8 +366,8 @@ u32 cpsw_mdio_get_alive(struct mii_dev *bus) > struct cpsw_mdio *mdio = bus->priv; > u32 val; > > - val = readl(>regs->control); > - return val & GENMASK(15, 0); > + val = readl(>regs->alive); > + return val & GENMASK(7, 0); Referring to the TRM for AM335x at [0], on page 2401, the MDIOALIVE register reports the presence of Ethernet PHYs up to ADDR 31. Also, the cpsw driver: drivers/net/ti/cpsw.c which uses the cpsw_mdio_get_alive() function, expects the return value to be a 16 bit value. Therefore, I believe that the GENMASK should retain the previous value of "GENMASK(15, 0)", while only the register being read needs to be changed from "control" to "alive". Please let me know if I misunderstood something regarding your implementation. [0] https://www.ti.com/lit/ug/spruh73q/spruh73q.pdf Regards, Siddharth. > } > > struct mii_dev *cpsw_mdio_init(const char *name, phys_addr_t mdio_base,