All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] net: smc911x: Determine bus width at runtime
@ 2021-06-28 13:30 Andre Przywara
  2021-06-28 13:30 ` [PATCH 1/2] net: smc911x: Drop redundant CONFIG_SMC911X_16_BIT Kconfig symbol Andre Przywara
  2021-06-28 13:30 ` [PATCH 2/2] net: smc911x: Determine bus width at runtime Andre Przywara
  0 siblings, 2 replies; 7+ messages in thread
From: Andre Przywara @ 2021-06-28 13:30 UTC (permalink / raw)
  To: Joe Hershberger, Ramon Fried; +Cc: u-boot, Marek Vasut

The SMC911x Ethernet MACs can be integrated using a 16 or 32-bit bus.
The driver needs to know about this choice, which is the reason for us
having a Kconfig symbol for that.

Now this bus width is already described using a devicetree property, and
since the driver is DM compliant and is using the DT now, we should query
this at runtime. We leave the Kconfig choice around, in case the DT is
missing this property.

Patch 1/2 simplifies the current Kconfig choice, to be a single symbol
instead of a two-options choice. This helps with patch 2/2, which queries
the DT to learn about the bus width, falling back to the Kconfig symbol
if the property cannot be found.

Cheers,
Andre

Andre Przywara (2):
  net: smc911x: Drop redundant CONFIG_SMC911X_16_BIT Kconfig symbol
  net: smc911x: Determine bus width at runtime

 drivers/net/Kconfig   | 17 +++++------------
 drivers/net/smc911x.c | 41 ++++++++++++++++++++---------------------
 2 files changed, 25 insertions(+), 33 deletions(-)

-- 
2.17.5


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] net: smc911x: Drop redundant CONFIG_SMC911X_16_BIT Kconfig symbol
  2021-06-28 13:30 [PATCH 0/2] net: smc911x: Determine bus width at runtime Andre Przywara
@ 2021-06-28 13:30 ` Andre Przywara
  2021-06-29 11:18   ` Ramon Fried
  2021-06-28 13:30 ` [PATCH 2/2] net: smc911x: Determine bus width at runtime Andre Przywara
  1 sibling, 1 reply; 7+ messages in thread
From: Andre Przywara @ 2021-06-28 13:30 UTC (permalink / raw)
  To: Joe Hershberger, Ramon Fried; +Cc: u-boot, Marek Vasut

The SMC911x Ethernet driver needs to know which accessor functions it
can use to access the MMIO registers. For that reason we have a Kconfig
choice between 16 and 32-bit bus width.

Since it's only those two options that we (and the Linux kernel)
support, and there does not seem to be any evidence of another bus
width anywhere, limit the Kconfig construct to a simple symbol.

This simplifies the code and allows a later rework to be much easier.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/net/Kconfig   | 17 +++++------------
 drivers/net/smc911x.c | 12 ++----------
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 9fc28b149d0..2a7c8f9a7ff 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -548,21 +548,14 @@ config SMC911X_BASE
 	  of the device (I/O space)
 endif #DM_ETH
 
-choice
-	prompt "SMC911X bus width"
-	default SMC911X_16_BIT
-
 config SMC911X_32_BIT
-	bool "Enable 32-bit interface"
-
-config SMC911X_16_BIT
-	bool "Enable 16-bit interface"
+	bool "Enable SMC911X 32-bit interface"
+	default n
 	help
-	  Define this if data bus is 16 bits. If your processor
-	  automatically converts one 32 bit word to two 16 bit
-	  words you may also try CONFIG_SMC911X_32_BIT.
+	  Define this if data bus is 32 bits. If your processor use a
+	  narrower 16 bit bus or cannot convert one 32 bit word to two 16 bit
+	  words, leave this to "n".
 
-endchoice
 endif #SMC911X
 
 config SUN7I_GMAC
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 7b79831c281..d596d96f4b3 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -48,12 +48,6 @@ static const struct chip_id chip_ids[] =  {
 
 #define DRIVERNAME "smc911x"
 
-#if defined (CONFIG_SMC911X_32_BIT) && \
-	defined (CONFIG_SMC911X_16_BIT)
-#error "SMC911X: Only one of CONFIG_SMC911X_32_BIT and \
-	CONFIG_SMC911X_16_BIT shall be set"
-#endif
-
 #if defined (CONFIG_SMC911X_32_BIT)
 static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
 {
@@ -64,7 +58,7 @@ static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
 {
 	writel(val, priv->iobase + offset);
 }
-#elif defined (CONFIG_SMC911X_16_BIT)
+#else
 static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
 {
 	return (readw(priv->iobase + offset) & 0xffff) |
@@ -75,9 +69,7 @@ static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
 	writew(val & 0xffff, priv->iobase + offset);
 	writew(val >> 16, priv->iobase + offset + 2);
 }
-#else
-#error "SMC911X: undefined bus width"
-#endif /* CONFIG_SMC911X_16_BIT */
+#endif /* CONFIG_SMC911X_32_BIT */
 
 static u32 smc911x_get_mac_csr(struct smc911x_priv *priv, u8 reg)
 {
-- 
2.17.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] net: smc911x: Determine bus width at runtime
  2021-06-28 13:30 [PATCH 0/2] net: smc911x: Determine bus width at runtime Andre Przywara
  2021-06-28 13:30 ` [PATCH 1/2] net: smc911x: Drop redundant CONFIG_SMC911X_16_BIT Kconfig symbol Andre Przywara
@ 2021-06-28 13:30 ` Andre Przywara
  2021-06-29 11:18   ` Ramon Fried
  1 sibling, 1 reply; 7+ messages in thread
From: Andre Przywara @ 2021-06-28 13:30 UTC (permalink / raw)
  To: Joe Hershberger, Ramon Fried; +Cc: u-boot, Marek Vasut

The SMC911x Ethernet MACs can be integrated using a 16 or 32-bit bus.
The driver needs to know about this choice, which is the reason for us
having a Kconfig symbol for that.

Now this bus width is already described using a devicetree property, and
since the driver is DM compliant and is using the DT now, we should query
this at runtime. We leave the Kconfig choice around, in case the DT is
missing this property.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/net/smc911x.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index d596d96f4b3..3afebee4402 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -28,6 +28,7 @@ struct smc911x_priv {
 	phys_addr_t		iobase;
 	const struct chip_id	*chipid;
 	unsigned char		enetaddr[6];
+	bool			use_32_bit_io;
 };
 
 static const struct chip_id chip_ids[] =  {
@@ -48,28 +49,24 @@ static const struct chip_id chip_ids[] =  {
 
 #define DRIVERNAME "smc911x"
 
-#if defined (CONFIG_SMC911X_32_BIT)
 static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
 {
-	return readl(priv->iobase + offset);
-}
+	if (priv->use_32_bit_io)
+		return readl(priv->iobase + offset);
 
-static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
-{
-	writel(val, priv->iobase + offset);
-}
-#else
-static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
-{
 	return (readw(priv->iobase + offset) & 0xffff) |
 	       (readw(priv->iobase + offset + 2) << 16);
 }
+
 static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
 {
-	writew(val & 0xffff, priv->iobase + offset);
-	writew(val >> 16, priv->iobase + offset + 2);
+	if (priv->use_32_bit_io) {
+		writel(val, priv->iobase + offset);
+	} else {
+		writew(val & 0xffff, priv->iobase + offset);
+		writew(val >> 16, priv->iobase + offset + 2);
+	}
 }
-#endif /* CONFIG_SMC911X_32_BIT */
 
 static u32 smc911x_get_mac_csr(struct smc911x_priv *priv, u8 reg)
 {
@@ -493,6 +490,8 @@ int smc911x_initialize(u8 dev_num, int base_addr)
 	priv->iobase = base_addr;
 	priv->dev.iobase = base_addr;
 
+	priv->use_32_bit_io = CONFIG_IS_ENABLED(SMC911X_32_BIT);
+
 	/* Try to detect chip. Will fail if not present. */
 	ret = smc911x_detect_chip(priv);
 	if (ret) {
@@ -603,10 +602,18 @@ static int smc911x_of_to_plat(struct udevice *dev)
 {
 	struct smc911x_priv *priv = dev_get_priv(dev);
 	struct eth_pdata *pdata = dev_get_plat(dev);
+	u32 io_width;
+	int ret;
 
 	pdata->iobase = dev_read_addr(dev);
 	priv->iobase = pdata->iobase;
 
+	ret = dev_read_u32(dev, "reg-io-width", &io_width);
+	if (!ret)
+		priv->use_32_bit_io = (io_width == 4);
+	else
+		priv->use_32_bit_io = CONFIG_IS_ENABLED(SMC911X_32_BIT);
+
 	return 0;
 }
 
-- 
2.17.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] net: smc911x: Drop redundant CONFIG_SMC911X_16_BIT Kconfig symbol
  2021-06-28 13:30 ` [PATCH 1/2] net: smc911x: Drop redundant CONFIG_SMC911X_16_BIT Kconfig symbol Andre Przywara
@ 2021-06-29 11:18   ` Ramon Fried
  2021-06-29 22:14     ` Ramon Fried
  0 siblings, 1 reply; 7+ messages in thread
From: Ramon Fried @ 2021-06-29 11:18 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Joe Hershberger, U-Boot Mailing List, Marek Vasut

On Mon, Jun 28, 2021 at 4:31 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> The SMC911x Ethernet driver needs to know which accessor functions it
> can use to access the MMIO registers. For that reason we have a Kconfig
> choice between 16 and 32-bit bus width.
>
> Since it's only those two options that we (and the Linux kernel)
> support, and there does not seem to be any evidence of another bus
> width anywhere, limit the Kconfig construct to a simple symbol.
>
> This simplifies the code and allows a later rework to be much easier.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/net/Kconfig   | 17 +++++------------
>  drivers/net/smc911x.c | 12 ++----------
>  2 files changed, 7 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 9fc28b149d0..2a7c8f9a7ff 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -548,21 +548,14 @@ config SMC911X_BASE
>           of the device (I/O space)
>  endif #DM_ETH
>
> -choice
> -       prompt "SMC911X bus width"
> -       default SMC911X_16_BIT
> -
>  config SMC911X_32_BIT
> -       bool "Enable 32-bit interface"
> -
> -config SMC911X_16_BIT
> -       bool "Enable 16-bit interface"
> +       bool "Enable SMC911X 32-bit interface"
> +       default n
>         help
> -         Define this if data bus is 16 bits. If your processor
> -         automatically converts one 32 bit word to two 16 bit
> -         words you may also try CONFIG_SMC911X_32_BIT.
> +         Define this if data bus is 32 bits. If your processor use a
> +         narrower 16 bit bus or cannot convert one 32 bit word to two 16 bit
> +         words, leave this to "n".
>
> -endchoice
>  endif #SMC911X
>
>  config SUN7I_GMAC
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index 7b79831c281..d596d96f4b3 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -48,12 +48,6 @@ static const struct chip_id chip_ids[] =  {
>
>  #define DRIVERNAME "smc911x"
>
> -#if defined (CONFIG_SMC911X_32_BIT) && \
> -       defined (CONFIG_SMC911X_16_BIT)
> -#error "SMC911X: Only one of CONFIG_SMC911X_32_BIT and \
> -       CONFIG_SMC911X_16_BIT shall be set"
> -#endif
> -
>  #if defined (CONFIG_SMC911X_32_BIT)
>  static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
>  {
> @@ -64,7 +58,7 @@ static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
>  {
>         writel(val, priv->iobase + offset);
>  }
> -#elif defined (CONFIG_SMC911X_16_BIT)
> +#else
>  static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
>  {
>         return (readw(priv->iobase + offset) & 0xffff) |
> @@ -75,9 +69,7 @@ static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
>         writew(val & 0xffff, priv->iobase + offset);
>         writew(val >> 16, priv->iobase + offset + 2);
>  }
> -#else
> -#error "SMC911X: undefined bus width"
> -#endif /* CONFIG_SMC911X_16_BIT */
> +#endif /* CONFIG_SMC911X_32_BIT */
>
>  static u32 smc911x_get_mac_csr(struct smc911x_priv *priv, u8 reg)
>  {
> --
> 2.17.5
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] net: smc911x: Determine bus width at runtime
  2021-06-28 13:30 ` [PATCH 2/2] net: smc911x: Determine bus width at runtime Andre Przywara
@ 2021-06-29 11:18   ` Ramon Fried
  2021-06-29 22:14     ` Ramon Fried
  0 siblings, 1 reply; 7+ messages in thread
From: Ramon Fried @ 2021-06-29 11:18 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Joe Hershberger, U-Boot Mailing List, Marek Vasut

On Mon, Jun 28, 2021 at 4:31 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> The SMC911x Ethernet MACs can be integrated using a 16 or 32-bit bus.
> The driver needs to know about this choice, which is the reason for us
> having a Kconfig symbol for that.
>
> Now this bus width is already described using a devicetree property, and
> since the driver is DM compliant and is using the DT now, we should query
> this at runtime. We leave the Kconfig choice around, in case the DT is
> missing this property.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/net/smc911x.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index d596d96f4b3..3afebee4402 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -28,6 +28,7 @@ struct smc911x_priv {
>         phys_addr_t             iobase;
>         const struct chip_id    *chipid;
>         unsigned char           enetaddr[6];
> +       bool                    use_32_bit_io;
>  };
>
>  static const struct chip_id chip_ids[] =  {
> @@ -48,28 +49,24 @@ static const struct chip_id chip_ids[] =  {
>
>  #define DRIVERNAME "smc911x"
>
> -#if defined (CONFIG_SMC911X_32_BIT)
>  static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
>  {
> -       return readl(priv->iobase + offset);
> -}
> +       if (priv->use_32_bit_io)
> +               return readl(priv->iobase + offset);
>
> -static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
> -{
> -       writel(val, priv->iobase + offset);
> -}
> -#else
> -static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
> -{
>         return (readw(priv->iobase + offset) & 0xffff) |
>                (readw(priv->iobase + offset + 2) << 16);
>  }
> +
>  static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
>  {
> -       writew(val & 0xffff, priv->iobase + offset);
> -       writew(val >> 16, priv->iobase + offset + 2);
> +       if (priv->use_32_bit_io) {
> +               writel(val, priv->iobase + offset);
> +       } else {
> +               writew(val & 0xffff, priv->iobase + offset);
> +               writew(val >> 16, priv->iobase + offset + 2);
> +       }
>  }
> -#endif /* CONFIG_SMC911X_32_BIT */
>
>  static u32 smc911x_get_mac_csr(struct smc911x_priv *priv, u8 reg)
>  {
> @@ -493,6 +490,8 @@ int smc911x_initialize(u8 dev_num, int base_addr)
>         priv->iobase = base_addr;
>         priv->dev.iobase = base_addr;
>
> +       priv->use_32_bit_io = CONFIG_IS_ENABLED(SMC911X_32_BIT);
> +
>         /* Try to detect chip. Will fail if not present. */
>         ret = smc911x_detect_chip(priv);
>         if (ret) {
> @@ -603,10 +602,18 @@ static int smc911x_of_to_plat(struct udevice *dev)
>  {
>         struct smc911x_priv *priv = dev_get_priv(dev);
>         struct eth_pdata *pdata = dev_get_plat(dev);
> +       u32 io_width;
> +       int ret;
>
>         pdata->iobase = dev_read_addr(dev);
>         priv->iobase = pdata->iobase;
>
> +       ret = dev_read_u32(dev, "reg-io-width", &io_width);
> +       if (!ret)
> +               priv->use_32_bit_io = (io_width == 4);
> +       else
> +               priv->use_32_bit_io = CONFIG_IS_ENABLED(SMC911X_32_BIT);
> +
>         return 0;
>  }
>
> --
> 2.17.5
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] net: smc911x: Drop redundant CONFIG_SMC911X_16_BIT Kconfig symbol
  2021-06-29 11:18   ` Ramon Fried
@ 2021-06-29 22:14     ` Ramon Fried
  0 siblings, 0 replies; 7+ messages in thread
From: Ramon Fried @ 2021-06-29 22:14 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Joe Hershberger, U-Boot Mailing List, Marek Vasut

On Tue, Jun 29, 2021 at 2:18 PM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> On Mon, Jun 28, 2021 at 4:31 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > The SMC911x Ethernet driver needs to know which accessor functions it
> > can use to access the MMIO registers. For that reason we have a Kconfig
> > choice between 16 and 32-bit bus width.
> >
> > Since it's only those two options that we (and the Linux kernel)
> > support, and there does not seem to be any evidence of another bus
> > width anywhere, limit the Kconfig construct to a simple symbol.
> >
> > This simplifies the code and allows a later rework to be much easier.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  drivers/net/Kconfig   | 17 +++++------------
> >  drivers/net/smc911x.c | 12 ++----------
> >  2 files changed, 7 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index 9fc28b149d0..2a7c8f9a7ff 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
> > @@ -548,21 +548,14 @@ config SMC911X_BASE
> >           of the device (I/O space)
> >  endif #DM_ETH
> >
> > -choice
> > -       prompt "SMC911X bus width"
> > -       default SMC911X_16_BIT
> > -
> >  config SMC911X_32_BIT
> > -       bool "Enable 32-bit interface"
> > -
> > -config SMC911X_16_BIT
> > -       bool "Enable 16-bit interface"
> > +       bool "Enable SMC911X 32-bit interface"
> > +       default n
> >         help
> > -         Define this if data bus is 16 bits. If your processor
> > -         automatically converts one 32 bit word to two 16 bit
> > -         words you may also try CONFIG_SMC911X_32_BIT.
> > +         Define this if data bus is 32 bits. If your processor use a
> > +         narrower 16 bit bus or cannot convert one 32 bit word to two 16 bit
> > +         words, leave this to "n".
> >
> > -endchoice
> >  endif #SMC911X
> >
> >  config SUN7I_GMAC
> > diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> > index 7b79831c281..d596d96f4b3 100644
> > --- a/drivers/net/smc911x.c
> > +++ b/drivers/net/smc911x.c
> > @@ -48,12 +48,6 @@ static const struct chip_id chip_ids[] =  {
> >
> >  #define DRIVERNAME "smc911x"
> >
> > -#if defined (CONFIG_SMC911X_32_BIT) && \
> > -       defined (CONFIG_SMC911X_16_BIT)
> > -#error "SMC911X: Only one of CONFIG_SMC911X_32_BIT and \
> > -       CONFIG_SMC911X_16_BIT shall be set"
> > -#endif
> > -
> >  #if defined (CONFIG_SMC911X_32_BIT)
> >  static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
> >  {
> > @@ -64,7 +58,7 @@ static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
> >  {
> >         writel(val, priv->iobase + offset);
> >  }
> > -#elif defined (CONFIG_SMC911X_16_BIT)
> > +#else
> >  static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
> >  {
> >         return (readw(priv->iobase + offset) & 0xffff) |
> > @@ -75,9 +69,7 @@ static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
> >         writew(val & 0xffff, priv->iobase + offset);
> >         writew(val >> 16, priv->iobase + offset + 2);
> >  }
> > -#else
> > -#error "SMC911X: undefined bus width"
> > -#endif /* CONFIG_SMC911X_16_BIT */
> > +#endif /* CONFIG_SMC911X_32_BIT */
> >
> >  static u32 smc911x_get_mac_csr(struct smc911x_priv *priv, u8 reg)
> >  {
> > --
> > 2.17.5
> >
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Applied to u-boot-net/master, Thanks !
Ramon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] net: smc911x: Determine bus width at runtime
  2021-06-29 11:18   ` Ramon Fried
@ 2021-06-29 22:14     ` Ramon Fried
  0 siblings, 0 replies; 7+ messages in thread
From: Ramon Fried @ 2021-06-29 22:14 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Joe Hershberger, U-Boot Mailing List, Marek Vasut

On Tue, Jun 29, 2021 at 2:18 PM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> On Mon, Jun 28, 2021 at 4:31 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > The SMC911x Ethernet MACs can be integrated using a 16 or 32-bit bus.
> > The driver needs to know about this choice, which is the reason for us
> > having a Kconfig symbol for that.
> >
> > Now this bus width is already described using a devicetree property, and
> > since the driver is DM compliant and is using the DT now, we should query
> > this at runtime. We leave the Kconfig choice around, in case the DT is
> > missing this property.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  drivers/net/smc911x.c | 33 ++++++++++++++++++++-------------
> >  1 file changed, 20 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> > index d596d96f4b3..3afebee4402 100644
> > --- a/drivers/net/smc911x.c
> > +++ b/drivers/net/smc911x.c
> > @@ -28,6 +28,7 @@ struct smc911x_priv {
> >         phys_addr_t             iobase;
> >         const struct chip_id    *chipid;
> >         unsigned char           enetaddr[6];
> > +       bool                    use_32_bit_io;
> >  };
> >
> >  static const struct chip_id chip_ids[] =  {
> > @@ -48,28 +49,24 @@ static const struct chip_id chip_ids[] =  {
> >
> >  #define DRIVERNAME "smc911x"
> >
> > -#if defined (CONFIG_SMC911X_32_BIT)
> >  static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
> >  {
> > -       return readl(priv->iobase + offset);
> > -}
> > +       if (priv->use_32_bit_io)
> > +               return readl(priv->iobase + offset);
> >
> > -static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
> > -{
> > -       writel(val, priv->iobase + offset);
> > -}
> > -#else
> > -static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
> > -{
> >         return (readw(priv->iobase + offset) & 0xffff) |
> >                (readw(priv->iobase + offset + 2) << 16);
> >  }
> > +
> >  static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
> >  {
> > -       writew(val & 0xffff, priv->iobase + offset);
> > -       writew(val >> 16, priv->iobase + offset + 2);
> > +       if (priv->use_32_bit_io) {
> > +               writel(val, priv->iobase + offset);
> > +       } else {
> > +               writew(val & 0xffff, priv->iobase + offset);
> > +               writew(val >> 16, priv->iobase + offset + 2);
> > +       }
> >  }
> > -#endif /* CONFIG_SMC911X_32_BIT */
> >
> >  static u32 smc911x_get_mac_csr(struct smc911x_priv *priv, u8 reg)
> >  {
> > @@ -493,6 +490,8 @@ int smc911x_initialize(u8 dev_num, int base_addr)
> >         priv->iobase = base_addr;
> >         priv->dev.iobase = base_addr;
> >
> > +       priv->use_32_bit_io = CONFIG_IS_ENABLED(SMC911X_32_BIT);
> > +
> >         /* Try to detect chip. Will fail if not present. */
> >         ret = smc911x_detect_chip(priv);
> >         if (ret) {
> > @@ -603,10 +602,18 @@ static int smc911x_of_to_plat(struct udevice *dev)
> >  {
> >         struct smc911x_priv *priv = dev_get_priv(dev);
> >         struct eth_pdata *pdata = dev_get_plat(dev);
> > +       u32 io_width;
> > +       int ret;
> >
> >         pdata->iobase = dev_read_addr(dev);
> >         priv->iobase = pdata->iobase;
> >
> > +       ret = dev_read_u32(dev, "reg-io-width", &io_width);
> > +       if (!ret)
> > +               priv->use_32_bit_io = (io_width == 4);
> > +       else
> > +               priv->use_32_bit_io = CONFIG_IS_ENABLED(SMC911X_32_BIT);
> > +
> >         return 0;
> >  }
> >
> > --
> > 2.17.5
> >
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Applied to u-boot-net/master, Thanks !
Ramon

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-06-29 22:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-28 13:30 [PATCH 0/2] net: smc911x: Determine bus width at runtime Andre Przywara
2021-06-28 13:30 ` [PATCH 1/2] net: smc911x: Drop redundant CONFIG_SMC911X_16_BIT Kconfig symbol Andre Przywara
2021-06-29 11:18   ` Ramon Fried
2021-06-29 22:14     ` Ramon Fried
2021-06-28 13:30 ` [PATCH 2/2] net: smc911x: Determine bus width at runtime Andre Przywara
2021-06-29 11:18   ` Ramon Fried
2021-06-29 22:14     ` Ramon Fried

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.