* [PATCH 0/3] net: sun8i-emac: Allwinner D1 Support
@ 2022-07-15 5:20 Samuel Holland
2022-07-15 5:20 ` [PATCH 1/3] net: sun8i-emac: Downgrade printf during probe to debug Samuel Holland
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Samuel Holland @ 2022-07-15 5:20 UTC (permalink / raw)
To: u-boot, Jagan Teki, Andre Przywara
Cc: Samuel Holland, Joe Hershberger, Ramon Fried
D1 is a RISC-V SoC containing an EMAC compatible with the A64 EMAC.
However, there are a couple of issues with the driver preventing it
being built for RISC-V. These are resolved by patches 2-3. Patch 1 is
a general cleanup.
Samuel Holland (3):
net: sun8i-emac: Downgrade printf during probe to debug
net: sun8i-emac: Drop use of arch-specific header
net: sun8i-emac: Use common syscon setup for R40
drivers/net/sun8i_emac.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] net: sun8i-emac: Downgrade printf during probe to debug 2022-07-15 5:20 [PATCH 0/3] net: sun8i-emac: Allwinner D1 Support Samuel Holland @ 2022-07-15 5:20 ` Samuel Holland 2022-07-16 17:00 ` Andre Przywara 2022-07-15 5:20 ` [PATCH 2/3] net: sun8i-emac: Drop use of arch-specific header Samuel Holland 2022-07-15 5:20 ` [PATCH 3/3] net: sun8i-emac: Use common syscon setup for R40 Samuel Holland 2 siblings, 1 reply; 9+ messages in thread From: Samuel Holland @ 2022-07-15 5:20 UTC (permalink / raw) To: u-boot, Jagan Teki, Andre Przywara Cc: Samuel Holland, Joe Hershberger, Ramon Fried This just prints the PHY mode taken from the devicetree. It does not need to be printed during every boot. Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/net/sun8i_emac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 2220f84b6978..a4b3492b7647 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -857,7 +857,7 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1); pdata->phy_interface = dev_read_phy_mode(dev); - printf("phy interface%d\n", pdata->phy_interface); + debug("phy interface %d\n", pdata->phy_interface); if (pdata->phy_interface == PHY_INTERFACE_MODE_NA) return -EINVAL; -- 2.35.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] net: sun8i-emac: Downgrade printf during probe to debug 2022-07-15 5:20 ` [PATCH 1/3] net: sun8i-emac: Downgrade printf during probe to debug Samuel Holland @ 2022-07-16 17:00 ` Andre Przywara 2022-08-06 17:38 ` Ramon Fried 0 siblings, 1 reply; 9+ messages in thread From: Andre Przywara @ 2022-07-16 17:00 UTC (permalink / raw) To: Samuel Holland; +Cc: u-boot, Jagan Teki, Joe Hershberger, Ramon Fried On Fri, 15 Jul 2022 00:20:56 -0500 Samuel Holland <samuel@sholland.org> wrote: > This just prints the PHY mode taken from the devicetree. It does not > need to be printed during every boot. That's true, but I think a more important reason is that it spoils the output, doesn't it? It reads: Net: phy interface9 eth0: ethernet@1c30000 at the moment, but I guess the intention was just: Net: eth0: ethernet@1c30000 So given that, and the fact that "interface9" is not really helpful, that looks alright to me. > Signed-off-by: Samuel Holland <samuel@sholland.org> Reviewed-by: Andre Przywara <andre.przywara@arm.com> Thanks, Andre > --- > > drivers/net/sun8i_emac.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c > index 2220f84b6978..a4b3492b7647 100644 > --- a/drivers/net/sun8i_emac.c > +++ b/drivers/net/sun8i_emac.c > @@ -857,7 +857,7 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) > priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1); > > pdata->phy_interface = dev_read_phy_mode(dev); > - printf("phy interface%d\n", pdata->phy_interface); > + debug("phy interface %d\n", pdata->phy_interface); > if (pdata->phy_interface == PHY_INTERFACE_MODE_NA) > return -EINVAL; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] net: sun8i-emac: Downgrade printf during probe to debug 2022-07-16 17:00 ` Andre Przywara @ 2022-08-06 17:38 ` Ramon Fried 0 siblings, 0 replies; 9+ messages in thread From: Ramon Fried @ 2022-08-06 17:38 UTC (permalink / raw) To: Andre Przywara Cc: Samuel Holland, U-Boot Mailing List, Jagan Teki, Joe Hershberger On Sat, Jul 16, 2022 at 8:02 PM Andre Przywara <andre.przywara@arm.com> wrote: > > On Fri, 15 Jul 2022 00:20:56 -0500 > Samuel Holland <samuel@sholland.org> wrote: > > > This just prints the PHY mode taken from the devicetree. It does not > > need to be printed during every boot. > > That's true, but I think a more important reason is that it spoils the > output, doesn't it? It reads: > Net: phy interface9 > eth0: ethernet@1c30000 > at the moment, but I guess the intention was just: > Net: eth0: ethernet@1c30000 > > So given that, and the fact that "interface9" is not really helpful, > that looks alright to me. > > > Signed-off-by: Samuel Holland <samuel@sholland.org> > > Reviewed-by: Andre Przywara <andre.przywara@arm.com> > > Thanks, > Andre > > --- > > > > drivers/net/sun8i_emac.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c > > index 2220f84b6978..a4b3492b7647 100644 > > --- a/drivers/net/sun8i_emac.c > > +++ b/drivers/net/sun8i_emac.c > > @@ -857,7 +857,7 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) > > priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1); > > > > pdata->phy_interface = dev_read_phy_mode(dev); > > - printf("phy interface%d\n", pdata->phy_interface); > > + debug("phy interface %d\n", pdata->phy_interface); > > if (pdata->phy_interface == PHY_INTERFACE_MODE_NA) > > return -EINVAL; > > > Reviewed-by: Ramon Fried <rfried.dev@gmail.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] net: sun8i-emac: Drop use of arch-specific header 2022-07-15 5:20 [PATCH 0/3] net: sun8i-emac: Allwinner D1 Support Samuel Holland 2022-07-15 5:20 ` [PATCH 1/3] net: sun8i-emac: Downgrade printf during probe to debug Samuel Holland @ 2022-07-15 5:20 ` Samuel Holland 2022-07-18 10:03 ` Andre Przywara 2022-08-06 17:39 ` Ramon Fried 2022-07-15 5:20 ` [PATCH 3/3] net: sun8i-emac: Use common syscon setup for R40 Samuel Holland 2 siblings, 2 replies; 9+ messages in thread From: Samuel Holland @ 2022-07-15 5:20 UTC (permalink / raw) To: u-boot, Jagan Teki, Andre Przywara Cc: Samuel Holland, Joe Hershberger, Ramon Fried This header is not used since commit abdbefba2a4e ("net: sun8i_emac: Use consistent clock bitfield definitions"). Dropping it allows the driver to be architecture-independent. Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/net/sun8i_emac.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index a4b3492b7647..9cca8fa4e0a1 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -16,7 +16,6 @@ #include <asm/global_data.h> #include <asm/gpio.h> #include <asm/io.h> -#include <asm/arch/clock.h> #include <common.h> #include <clk.h> #include <dm.h> -- 2.35.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] net: sun8i-emac: Drop use of arch-specific header 2022-07-15 5:20 ` [PATCH 2/3] net: sun8i-emac: Drop use of arch-specific header Samuel Holland @ 2022-07-18 10:03 ` Andre Przywara 2022-08-06 17:39 ` Ramon Fried 1 sibling, 0 replies; 9+ messages in thread From: Andre Przywara @ 2022-07-18 10:03 UTC (permalink / raw) To: Samuel Holland; +Cc: u-boot, Jagan Teki, Joe Hershberger, Ramon Fried On Fri, 15 Jul 2022 00:20:57 -0500 Samuel Holland <samuel@sholland.org> wrote: Hi, > This header is not used since commit abdbefba2a4e ("net: sun8i_emac: Use > consistent clock bitfield definitions"). Dropping it allows the driver > to be architecture-independent. Yes, we don't need this header anymore (all boards compile fine without it), and indeed it seems to be this commit you mentioned that changed that. > Signed-off-by: Samuel Holland <samuel@sholland.org> Reviewed-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre > drivers/net/sun8i_emac.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c > index a4b3492b7647..9cca8fa4e0a1 100644 > --- a/drivers/net/sun8i_emac.c > +++ b/drivers/net/sun8i_emac.c > @@ -16,7 +16,6 @@ > #include <asm/global_data.h> > #include <asm/gpio.h> > #include <asm/io.h> > -#include <asm/arch/clock.h> > #include <common.h> > #include <clk.h> > #include <dm.h> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] net: sun8i-emac: Drop use of arch-specific header 2022-07-15 5:20 ` [PATCH 2/3] net: sun8i-emac: Drop use of arch-specific header Samuel Holland 2022-07-18 10:03 ` Andre Przywara @ 2022-08-06 17:39 ` Ramon Fried 1 sibling, 0 replies; 9+ messages in thread From: Ramon Fried @ 2022-08-06 17:39 UTC (permalink / raw) To: Samuel Holland Cc: U-Boot Mailing List, Jagan Teki, Andre Przywara, Joe Hershberger On Fri, Jul 15, 2022 at 8:21 AM Samuel Holland <samuel@sholland.org> wrote: > > This header is not used since commit abdbefba2a4e ("net: sun8i_emac: Use > consistent clock bitfield definitions"). Dropping it allows the driver > to be architecture-independent. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > drivers/net/sun8i_emac.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c > index a4b3492b7647..9cca8fa4e0a1 100644 > --- a/drivers/net/sun8i_emac.c > +++ b/drivers/net/sun8i_emac.c > @@ -16,7 +16,6 @@ > #include <asm/global_data.h> > #include <asm/gpio.h> > #include <asm/io.h> > -#include <asm/arch/clock.h> > #include <common.h> > #include <clk.h> > #include <dm.h> > -- > 2.35.1 > Reviewed-by: Ramon Fried <rfried.dev@gmail.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] net: sun8i-emac: Use common syscon setup for R40 2022-07-15 5:20 [PATCH 0/3] net: sun8i-emac: Allwinner D1 Support Samuel Holland 2022-07-15 5:20 ` [PATCH 1/3] net: sun8i-emac: Downgrade printf during probe to debug Samuel Holland 2022-07-15 5:20 ` [PATCH 2/3] net: sun8i-emac: Drop use of arch-specific header Samuel Holland @ 2022-07-15 5:20 ` Samuel Holland 2022-07-15 9:24 ` Jagan Teki 2 siblings, 1 reply; 9+ messages in thread From: Samuel Holland @ 2022-07-15 5:20 UTC (permalink / raw) To: u-boot, Jagan Teki, Andre Przywara Cc: Samuel Holland, Joe Hershberger, Ramon Fried While R40 puts the EMAC syscon register at a different address from other variants, the relevant portion of the register's layout is the same. Factor out the register offset so the same code can be shared by all variants. This matches what the Linux driver does. This change provides two benefits beyond the simplification: - R40 boards now respect the RX delays from the devicetree - This resolves a warning on architectures where readl/writel expect the address to have a pointer type, not phys_addr_t. Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/net/sun8i_emac.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 9cca8fa4e0a1..75ecb58e1e45 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -162,7 +162,7 @@ struct emac_eth_dev { enum emac_variant variant; void *mac_reg; - phys_addr_t sysctl_reg; + void *sysctl_reg; struct phy_device *phydev; struct mii_dev *bus; struct clk tx_clk; @@ -317,18 +317,7 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata, { u32 reg; - if (priv->variant == R40_GMAC) { - /* Select RGMII for R40 */ - reg = readl(priv->sysctl_reg + 0x164); - reg |= SC_ETCS_INT_GMII | - SC_EPIT | - (CONFIG_GMAC_TX_DELAY << SC_ETXDC_OFFSET); - - writel(reg, priv->sysctl_reg + 0x164); - return 0; - } - - reg = readl(priv->sysctl_reg + 0x30); + reg = readl(priv->sysctl_reg); reg = sun8i_emac_set_syscon_ephy(priv, reg); @@ -369,7 +358,7 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata, reg |= ((pdata->rx_delay_ps / 100) << SC_ERXDC_OFFSET) & SC_ERXDC_MASK; - writel(reg, priv->sysctl_reg + 0x30); + writel(reg, priv->sysctl_reg); return 0; } @@ -792,6 +781,7 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) struct sun8i_eth_pdata *sun8i_pdata = dev_get_plat(dev); struct eth_pdata *pdata = &sun8i_pdata->eth_pdata; struct emac_eth_dev *priv = dev_get_priv(dev); + phys_addr_t syscon_base; const fdt32_t *reg; int node = dev_of_offset(dev); int offset = 0; @@ -837,13 +827,18 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) __func__); return -EINVAL; } - priv->sysctl_reg = fdt_translate_address((void *)gd->fdt_blob, - offset, reg); - if (priv->sysctl_reg == FDT_ADDR_T_NONE) { + + syscon_base = fdt_translate_address((void *)gd->fdt_blob, offset, reg); + if (syscon_base == FDT_ADDR_T_NONE) { debug("%s: Cannot find syscon base address\n", __func__); return -EINVAL; } + if (priv->variant == R40_GMAC) + priv->sysctl_reg = (void *)syscon_base + 0x164; + else + priv->sysctl_reg = (void *)syscon_base + 0x30; + pdata->phy_interface = -1; priv->phyaddr = -1; priv->use_internal_phy = false; -- 2.35.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] net: sun8i-emac: Use common syscon setup for R40 2022-07-15 5:20 ` [PATCH 3/3] net: sun8i-emac: Use common syscon setup for R40 Samuel Holland @ 2022-07-15 9:24 ` Jagan Teki 0 siblings, 0 replies; 9+ messages in thread From: Jagan Teki @ 2022-07-15 9:24 UTC (permalink / raw) To: Samuel Holland; +Cc: u-boot, Andre Przywara, Joe Hershberger, Ramon Fried On Fri, Jul 15, 2022 at 10:51 AM Samuel Holland <samuel@sholland.org> wrote: > > While R40 puts the EMAC syscon register at a different address from > other variants, the relevant portion of the register's layout is the > same. Factor out the register offset so the same code can be shared > by all variants. This matches what the Linux driver does. > > This change provides two benefits beyond the simplification: > - R40 boards now respect the RX delays from the devicetree > - This resolves a warning on architectures where readl/writel > expect the address to have a pointer type, not phys_addr_t. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > drivers/net/sun8i_emac.c | 29 ++++++++++++----------------- > 1 file changed, 12 insertions(+), 17 deletions(-) > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c > index 9cca8fa4e0a1..75ecb58e1e45 100644 > --- a/drivers/net/sun8i_emac.c > +++ b/drivers/net/sun8i_emac.c > @@ -162,7 +162,7 @@ struct emac_eth_dev { > > enum emac_variant variant; > void *mac_reg; > - phys_addr_t sysctl_reg; > + void *sysctl_reg; > struct phy_device *phydev; > struct mii_dev *bus; > struct clk tx_clk; > @@ -317,18 +317,7 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata, > { > u32 reg; > > - if (priv->variant == R40_GMAC) { > - /* Select RGMII for R40 */ > - reg = readl(priv->sysctl_reg + 0x164); > - reg |= SC_ETCS_INT_GMII | > - SC_EPIT | > - (CONFIG_GMAC_TX_DELAY << SC_ETXDC_OFFSET); > - > - writel(reg, priv->sysctl_reg + 0x164); > - return 0; > - } > - > - reg = readl(priv->sysctl_reg + 0x30); > + reg = readl(priv->sysctl_reg); > > reg = sun8i_emac_set_syscon_ephy(priv, reg); > > @@ -369,7 +358,7 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata, > reg |= ((pdata->rx_delay_ps / 100) << SC_ERXDC_OFFSET) > & SC_ERXDC_MASK; > > - writel(reg, priv->sysctl_reg + 0x30); > + writel(reg, priv->sysctl_reg); > > return 0; > } > @@ -792,6 +781,7 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) > struct sun8i_eth_pdata *sun8i_pdata = dev_get_plat(dev); > struct eth_pdata *pdata = &sun8i_pdata->eth_pdata; > struct emac_eth_dev *priv = dev_get_priv(dev); > + phys_addr_t syscon_base; > const fdt32_t *reg; > int node = dev_of_offset(dev); > int offset = 0; > @@ -837,13 +827,18 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) > __func__); > return -EINVAL; > } > - priv->sysctl_reg = fdt_translate_address((void *)gd->fdt_blob, > - offset, reg); > - if (priv->sysctl_reg == FDT_ADDR_T_NONE) { > + > + syscon_base = fdt_translate_address((void *)gd->fdt_blob, offset, reg); > + if (syscon_base == FDT_ADDR_T_NONE) { > debug("%s: Cannot find syscon base address\n", __func__); > return -EINVAL; > } > > + if (priv->variant == R40_GMAC) > + priv->sysctl_reg = (void *)syscon_base + 0x164; > + else > + priv->sysctl_reg = (void *)syscon_base + 0x30; > + Better to get syscon fields from driver data as this driver support DM. Jagan. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-08-06 17:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-15 5:20 [PATCH 0/3] net: sun8i-emac: Allwinner D1 Support Samuel Holland 2022-07-15 5:20 ` [PATCH 1/3] net: sun8i-emac: Downgrade printf during probe to debug Samuel Holland 2022-07-16 17:00 ` Andre Przywara 2022-08-06 17:38 ` Ramon Fried 2022-07-15 5:20 ` [PATCH 2/3] net: sun8i-emac: Drop use of arch-specific header Samuel Holland 2022-07-18 10:03 ` Andre Przywara 2022-08-06 17:39 ` Ramon Fried 2022-07-15 5:20 ` [PATCH 3/3] net: sun8i-emac: Use common syscon setup for R40 Samuel Holland 2022-07-15 9:24 ` Jagan Teki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).