RE: [PATCH] gianfar: Solve ethernet TX/RX problems for ls1021a

[Johnson CH Chen (éæå)]

Hi Vladimir,

Vladimir Oltean <olteanv@xxxxxxxxx> æ 2020å1æ7æ éä äå11:49åéï
>
> Hi Chen,
>
> On Tue, 7 Jan 2020 at 12:37, Johnson CH Chen (éæå)
> <JohnsonCH.Chen@xxxxxxxx> wrote:
> >
> > Add dma_endian_le to solve ethernet TX/RX problems for freescale 
> > ls1021a. Without this, it will result in rx-busy-errors by ethtool, and transmit queue timeout in ls1021a's platforms.
> >
> > Signed-off-by: Johnson Chen <johnsonch.chen@xxxxxxxx>
> > ---
>
> This patch is not valid. The endianness configuration in 
> eTSECx_DMACTRL is reserved and not applicable.
> What is the value of SCFG_ETSECDMAMCR bits ETSEC_BD and ETSEC_FR_DATA 
> on your board? Typically this is configured by the bootloader.
>

Thanks your suggestion. I use linux-fsl-sdk-v1.7, and find "dma-endian-le" is used in ls1021a.dtsi and gianfar.c/.h. For bootloader, version is U-Boot version is 2015.01-dirty and it seems old and not includes "SCFG_ETSECDMAMCR bits".

It seems solution is included in bootloader, not in device tree for
freescale/NXP: https://lxr.missinglinkelectronics.com/uboot/board/freescale/ls1021aiot/ls1021aiot.c

It means bootloader provides functions are the same as device tree's.
So what's benefit for this desgin? It seems we need to upgrade kernel and bootloader to satisfy our need, not just upgrade kernel only. So many thanks!

> >  drivers/net/ethernet/freescale/gianfar.c | 3 +++  
> > drivers/net/ethernet/freescale/gianfar.h | 4 ++++
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/gianfar.c 
> > b/drivers/net/ethernet/freescale/gianfar.c
> > index 72868a28b621..ab4e45199df9 100644
> > --- a/drivers/net/ethernet/freescale/gianfar.c
> > +++ b/drivers/net/ethernet/freescale/gianfar.c
> > @@ -833,6 +833,7 @@ static int gfar_of_init(struct platform_device 
> > *ofdev, struct net_device **pdev)
> >
> >         /* Find the TBI PHY.  If it's not there, we don't support SGMII */
> >         priv->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
> > +       priv->dma_endian_le = of_property_read_bool(np, 
> > + "fsl,dma-endian-le");
> >
> >         return 0;
> >
> > @@ -1209,6 +1210,8 @@ static void gfar_start(struct gfar_private *priv)
> >         /* Initialize DMACTRL to have WWR and WOP */
> >         tempval = gfar_read(&regs->dmactrl);
> >         tempval |= DMACTRL_INIT_SETTINGS;
> > +       if (priv->dma_endian_le)
> > +               tempval |= DMACTRL_LE;
> >         gfar_write(&regs->dmactrl, tempval);
> >
> >         /* Make sure we aren't stopped */ diff --git 
> > a/drivers/net/ethernet/freescale/gianfar.h 
> > b/drivers/net/ethernet/freescale/gianfar.h
> > index 432c6a818ae5..aae07db5206b 100644
> > --- a/drivers/net/ethernet/freescale/gianfar.h
> > +++ b/drivers/net/ethernet/freescale/gianfar.h
> > @@ -215,6 +215,7 @@ extern const char gfar_driver_version[];
> >  #define DMACTRL_INIT_SETTINGS   0x000000c3
> >  #define DMACTRL_GRS             0x00000010
> >  #define DMACTRL_GTS             0x00000008
> > +#define DMACTRL_LE             0x00008000
> >
> >  #define TSTAT_CLEAR_THALT_ALL  0xFF000000
> >  #define TSTAT_CLEAR_THALT      0x80000000
> > @@ -1140,6 +1141,9 @@ struct gfar_private {
> >                 tx_pause_en:1,
> >                 rx_pause_en:1;
> >
> > +       /* little endian dma buffer and descriptor host interface */
> > +       unsigned int dma_endian_le;
> > +
> >         /* The total tx and rx ring size for the enabled queues */
> >         unsigned int total_tx_ring_size;
> >         unsigned int total_rx_ring_size;
> > --
> > 2.11.0
> >
> > Best regards,
> > Johnson
>
> Regards,
> -Vladimir

Best regards,
Johnson