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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 8 Jan 2020 07:15:49 +0000
From:   Johnson CH Chen (陳昭勳) 
        <JohnsonCH.Chen@...a.com>
To:     Vladimir Oltean <olteanv@...il.com>
CC:     "claudiu.manoil@....com" <claudiu.manoil@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "zero19850401@...il.com" <zero19850401@...il.com>
Subject: RE: [PATCH] gianfar: Solve ethernet TX/RX problems for ls1021a

Hi Vladimir,

Vladimir Oltean <olteanv@...il.com> 於 2020年1月7日 週二 下午11:49寫道：
>
> Hi Chen,
>
> On Tue, 7 Jan 2020 at 12:37, Johnson CH Chen (陳昭勳)
> <JohnsonCH.Chen@...a.com> 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@...a.com>
> > ---
>
> 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