diff mbox series

[V3,2/6] net: stmmac: stop each tx channel independently

Message ID 20210126115854.2530-3-qiangqing.zhang@nxp.com
State Superseded
Headers show
Series None | expand

Commit Message

Joakim Zhang Jan. 26, 2021, 11:58 a.m. UTC 
If clear GMAC_CONFIG_TE bit, it would stop all tx channels, but users
may only want to stop secific tx channel.

Fixes: 48863ce5940f ("stmmac: add DMA support for GMAC 4.xx")
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Willem de Bruijn Jan. 26, 2021, 11:09 p.m. UTC | #1 
On Tue, Jan 26, 2021 at 7:03 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>

> If clear GMAC_CONFIG_TE bit, it would stop all tx channels, but users

> may only want to stop secific tx channel.


secific -> specific

>

> Fixes: 48863ce5940f ("stmmac: add DMA support for GMAC 4.xx")

> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>

> ---

>  drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 4 ----

>  1 file changed, 4 deletions(-)

>

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> index 0b4ee2dbb691..71e50751ef2d 100644

> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> @@ -53,10 +53,6 @@ void dwmac4_dma_stop_tx(void __iomem *ioaddr, u32 chan)

>

>         value &= ~DMA_CONTROL_ST;

>         writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));

> -

> -       value = readl(ioaddr + GMAC_CONFIG);

> -       value &= ~GMAC_CONFIG_TE;

> -       writel(value, ioaddr + GMAC_CONFIG);


Is it safe to partially unwind the actions of dwmac4_dma_start_tx

And would the same reasoning apply to dwmac4_(dma_start|stop)_rx?
Joakim Zhang Jan. 27, 2021, 1:44 a.m. UTC | #2 
> -----Original Message-----

> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>

> Sent: 2021年1月27日 7:10

> To: Joakim Zhang <qiangqing.zhang@nxp.com>

> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue

> <alexandre.torgue@st.com>; Jose Abreu <joabreu@synopsys.com>; David

> Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Network

> Development <netdev@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>;

> Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>

> Subject: Re: [PATCH V3 2/6] net: stmmac: stop each tx channel independently

> 

> On Tue, Jan 26, 2021 at 7:03 AM Joakim Zhang <qiangqing.zhang@nxp.com>

> wrote:

> >

> > If clear GMAC_CONFIG_TE bit, it would stop all tx channels, but users

> > may only want to stop secific tx channel.

> 

> secific -> specific


Thanks. Will correct it.

> >

> > Fixes: 48863ce5940f ("stmmac: add DMA support for GMAC 4.xx")

> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>

> > ---

> >  drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 4 ----

> >  1 file changed, 4 deletions(-)

> >

> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> > b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> > index 0b4ee2dbb691..71e50751ef2d 100644

> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> > @@ -53,10 +53,6 @@ void dwmac4_dma_stop_tx(void __iomem *ioaddr,

> u32

> > chan)

> >

> >         value &= ~DMA_CONTROL_ST;

> >         writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));

> > -

> > -       value = readl(ioaddr + GMAC_CONFIG);

> > -       value &= ~GMAC_CONFIG_TE;

> > -       writel(value, ioaddr + GMAC_CONFIG);

> 

> Is it safe to partially unwind the actions of dwmac4_dma_start_tx

> 

> And would the same reasoning apply to dwmac4_(dma_start|stop)_rx?


Sorry, I am not quite understand what you means.

What this patch did is to align to dwmac4_(dma_start|stop)_rx.

dwmac4_dma_start_rx: assert DMA_CONTROL_SR bit for each channel, and assert GMAC_CONFIG_RE bit which targets all channels.
dwmac4_dma_stop_rx: only need clear DMA_CONTROL_SR bit for each channel.

After this patch applied:
dwmac4_dma_start_tx: assert DMA_CONTROL_ST bit for each channel, and assert GMAC_CONFIG_TE bit which targets all channels.
dwmac4_dma_stop_tx: only need clear DMA_CONTROL_ST bit for each channel.

You can refer to drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c, this is the correct sequence. 

Thanks.

Best Regards,
Joakim Zhang
Willem de Bruijn Jan. 27, 2021, 2:16 a.m. UTC | #3 
On Tue, Jan 26, 2021 at 8:44 PM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>

>

> > -----Original Message-----

> > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>

> > Sent: 2021年1月27日 7:10

> > To: Joakim Zhang <qiangqing.zhang@nxp.com>

> > Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue

> > <alexandre.torgue@st.com>; Jose Abreu <joabreu@synopsys.com>; David

> > Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Network

> > Development <netdev@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>;

> > Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>

> > Subject: Re: [PATCH V3 2/6] net: stmmac: stop each tx channel independently

> >

> > On Tue, Jan 26, 2021 at 7:03 AM Joakim Zhang <qiangqing.zhang@nxp.com>

> > wrote:

> > >

> > > If clear GMAC_CONFIG_TE bit, it would stop all tx channels, but users

> > > may only want to stop secific tx channel.

> >

> > secific -> specific

>

> Thanks. Will correct it.

>

> > >

> > > Fixes: 48863ce5940f ("stmmac: add DMA support for GMAC 4.xx")

> > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>

> > > ---

> > >  drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 4 ----

> > >  1 file changed, 4 deletions(-)

> > >

> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> > > b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> > > index 0b4ee2dbb691..71e50751ef2d 100644

> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c

> > > @@ -53,10 +53,6 @@ void dwmac4_dma_stop_tx(void __iomem *ioaddr,

> > u32

> > > chan)

> > >

> > >         value &= ~DMA_CONTROL_ST;

> > >         writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));

> > > -

> > > -       value = readl(ioaddr + GMAC_CONFIG);

> > > -       value &= ~GMAC_CONFIG_TE;

> > > -       writel(value, ioaddr + GMAC_CONFIG);

> >

> > Is it safe to partially unwind the actions of dwmac4_dma_start_tx

> >

> > And would the same reasoning apply to dwmac4_(dma_start|stop)_rx?

>

> Sorry, I am not quite understand what you means.

>

> What this patch did is to align to dwmac4_(dma_start|stop)_rx.

>

> dwmac4_dma_start_rx: assert DMA_CONTROL_SR bit for each channel, and assert GMAC_CONFIG_RE bit which targets all channels.

> dwmac4_dma_stop_rx: only need clear DMA_CONTROL_SR bit for each channel.

>

> After this patch applied:

> dwmac4_dma_start_tx: assert DMA_CONTROL_ST bit for each channel, and assert GMAC_CONFIG_TE bit which targets all channels.

> dwmac4_dma_stop_tx: only need clear DMA_CONTROL_ST bit for each channel.


Oh indeed. Sorry, I should have seen that it exactly brings the tx
logic into agreement with rx. Thanks.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
index 0b4ee2dbb691..71e50751ef2d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
@@ -53,10 +53,6 @@  void dwmac4_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 
 	value &= ~DMA_CONTROL_ST;
 	writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
-
-	value = readl(ioaddr + GMAC_CONFIG);
-	value &= ~GMAC_CONFIG_TE;
-	writel(value, ioaddr + GMAC_CONFIG);
 }
 
 void dwmac4_dma_start_rx(void __iomem *ioaddr, u32 chan)