| Message ID | 20230708104523.228257-4-s-vadapalli@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Add SGMII support for MAIN CPSW on TI's J7200 SoC | expand |
On Sat, Jul 08, 2023 at 04:15:20PM +0530, Siddharth Vadapalli wrote: > From: Suman Anna <s-anna@ti.com> > > 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 <s-anna@ti.com> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> > --- > 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.
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 <s-anna@ti.com> >> >> 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 <s-anna@ti.com> >> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> >> --- >> 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. >
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 <s-anna@ti.com> >>> >>> 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 <s-anna@ti.com> >>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> >>> --- >>> 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. > >> >
On Mon, Jul 31, 2023 at 02:06:26PM +0530, Siddharth Vadapalli wrote: > > > 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 <s-anna@ti.com> > >>> > >>> 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 <s-anna@ti.com> > >>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> > >>> --- > >>> 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. That will break SPL, if that's a use case on these platforms. Please check that whatever approach you take doesn't break other platforms nor include code when not required on other platforms.
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 <common.h> #include <malloc.h> #include <asm/cache.h> +#include <asm/gpio.h> #include <asm/io.h> #include <asm/processor.h> #include <clk.h> @@ -23,6 +24,7 @@ #include <power-domain.h> #include <soc.h> #include <linux/bitops.h> +#include <linux/delay.h> #include <linux/soc/ti/ti-udma.h> #include "cpsw_mdio.h" @@ -93,6 +95,8 @@ #define AM65_CPSW_CPPI_PKT_TYPE 0x7 +#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_desc mdio_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(&cpsw_common->mdio_gpio_reset)) { + dm_gpio_set_value(&cpsw_common->mdio_gpio_reset, 1); + udelay(cpsw_common->reset_delay_us); + dm_gpio_set_value(&cpsw_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, + &cpsw_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;