| Message ID | 20210628133031.17765-2-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Commit | f26c9d7fedb6dce4dcd8f0e763adda707dcbeca6 |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | [1/2] net: smc911x: Drop redundant CONFIG_SMC911X_16_BIT Kconfig symbol | expand |
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>
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
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) {
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(-)