diff mbox series

[U-Boot,v10,2/4] net: macb: Fix check for little-endian system in gmac_configure_dma()

Message ID 20190722073732.2616-3-anup.patel@wdc.com
State Superseded
Delegated to: Joe Hershberger
Headers show
Series Update SiFive Unleashed Drivers | expand

Commit Message

Anup Patel July 22, 2019, 7:38 a.m. UTC 
Instead of depending on CONFIG_SYS_LITTLE_ENDIAN, we check at runtime
whether underlying system is little-endian or big-endian. This way
we are not dependent on any U-Boot specific OR compiler specific macro
to check system endianness.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
---
 drivers/net/macb.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Joe Hershberger July 23, 2019, 3:21 a.m. UTC | #1 
On Mon, Jul 22, 2019 at 2:54 AM Anup Patel <Anup.Patel@wdc.com> wrote:
>
> Instead of depending on CONFIG_SYS_LITTLE_ENDIAN, we check at runtime
> whether underlying system is little-endian or big-endian. This way
> we are not dependent on any U-Boot specific OR compiler specific macro
> to check system endianness.
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
> ---
>  drivers/net/macb.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 3db5597373..27a6c6db2b 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -84,6 +84,8 @@ struct macb_dma_desc {
>  struct macb_device {
>         void                    *regs;
>
> +       bool                    is_big_endian;
> +
>         const struct macb_config *config;
>
>         unsigned int            rx_tail;
> @@ -743,11 +745,10 @@ static void gmac_configure_dma(struct macb_device *macb)
>         dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
>         dmacfg &= ~GEM_BIT(ENDIA_PKT);
>
> -#ifdef CONFIG_SYS_LITTLE_ENDIAN
> -               dmacfg &= ~GEM_BIT(ENDIA_DESC);
> -#else
> +       if (macb->is_big_endian)
>                 dmacfg |= GEM_BIT(ENDIA_DESC); /* CPU in big endian */
> -#endif
> +       else
> +               dmacfg &= ~GEM_BIT(ENDIA_DESC);
>
>         dmacfg &= ~GEM_BIT(ADDR64);
>         gem_writel(macb, DMACFG, dmacfg);
> @@ -1221,6 +1222,9 @@ static int macb_eth_probe(struct udevice *dev)
>
>         macb->regs = (void *)pdata->iobase;
>
> +       macb->is_big_endian = (cpu_to_be32(0x12345678) == 0x12345678) ?
> +                               true : false;

You don't need the "? true : false". That's a no-op. Just assign the
result of the comparison.

> +
>         macb->config = (struct macb_config *)dev_get_driver_data(dev);
>         if (!macb->config)
>                 macb->config = &default_gem_config;
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Anup Patel July 23, 2019, 4:07 a.m. UTC | #2 
On Tue, Jul 23, 2019 at 8:55 AM Joe Hershberger <joe.hershberger@ni.com> wrote:
>
> On Mon, Jul 22, 2019 at 2:54 AM Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> > Instead of depending on CONFIG_SYS_LITTLE_ENDIAN, we check at runtime
> > whether underlying system is little-endian or big-endian. This way
> > we are not dependent on any U-Boot specific OR compiler specific macro
> > to check system endianness.
> >
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
> > ---
> >  drivers/net/macb.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> > index 3db5597373..27a6c6db2b 100644
> > --- a/drivers/net/macb.c
> > +++ b/drivers/net/macb.c
> > @@ -84,6 +84,8 @@ struct macb_dma_desc {
> >  struct macb_device {
> >         void                    *regs;
> >
> > +       bool                    is_big_endian;
> > +
> >         const struct macb_config *config;
> >
> >         unsigned int            rx_tail;
> > @@ -743,11 +745,10 @@ static void gmac_configure_dma(struct macb_device *macb)
> >         dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
> >         dmacfg &= ~GEM_BIT(ENDIA_PKT);
> >
> > -#ifdef CONFIG_SYS_LITTLE_ENDIAN
> > -               dmacfg &= ~GEM_BIT(ENDIA_DESC);
> > -#else
> > +       if (macb->is_big_endian)
> >                 dmacfg |= GEM_BIT(ENDIA_DESC); /* CPU in big endian */
> > -#endif
> > +       else
> > +               dmacfg &= ~GEM_BIT(ENDIA_DESC);
> >
> >         dmacfg &= ~GEM_BIT(ADDR64);
> >         gem_writel(macb, DMACFG, dmacfg);
> > @@ -1221,6 +1222,9 @@ static int macb_eth_probe(struct udevice *dev)
> >
> >         macb->regs = (void *)pdata->iobase;
> >
> > +       macb->is_big_endian = (cpu_to_be32(0x12345678) == 0x12345678) ?
> > +                               true : false;
>
> You don't need the "? true : false". That's a no-op. Just assign the
> result of the comparison.

Sure, I will update.

>
> > +
> >         macb->config = (struct macb_config *)dev_get_driver_data(dev);
> >         if (!macb->config)
> >                 macb->config = &default_gem_config;
> > --
> > 2.17.1
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot

Thanks,
Anup
diff mbox series

Patch

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 3db5597373..27a6c6db2b 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -84,6 +84,8 @@  struct macb_dma_desc {
 struct macb_device {
 	void			*regs;
 
+	bool			is_big_endian;
+
 	const struct macb_config *config;
 
 	unsigned int		rx_tail;
@@ -743,11 +745,10 @@  static void gmac_configure_dma(struct macb_device *macb)
 	dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
 	dmacfg &= ~GEM_BIT(ENDIA_PKT);
 
-#ifdef CONFIG_SYS_LITTLE_ENDIAN
-		dmacfg &= ~GEM_BIT(ENDIA_DESC);
-#else
+	if (macb->is_big_endian)
 		dmacfg |= GEM_BIT(ENDIA_DESC); /* CPU in big endian */
-#endif
+	else
+		dmacfg &= ~GEM_BIT(ENDIA_DESC);
 
 	dmacfg &= ~GEM_BIT(ADDR64);
 	gem_writel(macb, DMACFG, dmacfg);
@@ -1221,6 +1222,9 @@  static int macb_eth_probe(struct udevice *dev)
 
 	macb->regs = (void *)pdata->iobase;
 
+	macb->is_big_endian = (cpu_to_be32(0x12345678) == 0x12345678) ?
+				true : false;
+
 	macb->config = (struct macb_config *)dev_get_driver_data(dev);
 	if (!macb->config)
 		macb->config = &default_gem_config;