netdev - Re: [PATCH net-next v11 2/6] net: stmmac: Add multi-channel support

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]

Message-ID: <juwqvnv22ky5avg72prgi2ocx7qy4kqldet4t4qfooerj3p6nn@lrnlkioxxevy>
Date: Fri, 19 Apr 2024 12:17:04 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Yanteng Si <siyanteng@...ngson.cn>
Cc: andrew@...n.ch, hkallweit1@...il.com, peppe.cavallaro@...com, 
	alexandre.torgue@...s.st.com, joabreu@...opsys.com, Jose.Abreu@...opsys.com, 
	chenhuacai@...nel.org, linux@...linux.org.uk, guyinggang@...ngson.cn, 
	netdev@...r.kernel.org, chris.chenfeiyang@...il.com, siyanteng01@...il.com
Subject: Re: [PATCH net-next v11 2/6] net: stmmac: Add multi-channel support

On Fri, Apr 19, 2024 at 05:02:17PM +0800, Yanteng Si wrote:
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> > > index daf79cdbd3ec..f161ec9ac490 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> > > @@ -70,15 +70,17 @@ static void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
> > >   	writel(value, ioaddr + DMA_AXI_BUS_MODE);
> > >   }
> > > -static void dwmac1000_dma_init(void __iomem *ioaddr,
> > > -			       struct stmmac_dma_cfg *dma_cfg, int atds)
> > > +static void dwmac1000_dma_init_channel(struct stmmac_priv *priv,
> > > +				       void __iomem *ioaddr,
> > > +				       struct stmmac_dma_cfg *dma_cfg, u32 chan)
> > please create a pre-requisite/preparation patch with the atds argument
> > movement to the stmmac_dma_cfg structure as I suggested in v8:
> > https://lore.kernel.org/netdev/yzs6eqx2swdhaegxxcbijhtb5tkhkvvyvso2perkessv5swq47@ywmea5xswsug/
> > That will make this patch looking simpler and providing a single
> > coherent change.
> OK.

> > >   	/* Clear the interrupt by writing a logic 1 to the CSR5[15-0] */
> > > -	writel((intr_status & 0x1ffff), ioaddr + DMA_STATUS);
> > > +	writel((intr_status & 0x7ffff), ioaddr + DMA_CHAN_STATUS(chan));
> > I'll ask once again:
> > 
> > "Isn't the mask change going to be implemented in the framework of the
> > Loongson-specific DMA-interrupt handler in some of the further
> > patches?"
> > 
> The future is not going to change.

Not sure I've completely got what you meant. You are adding the
Loongson-specific DMA IRQ handler in Patch 6/6:
https://lore.kernel.org/netdev/cover.1712917541.git.siyanteng@loongson.cn/T/#m439c1d8957cd6997beb0374484a8d0efbeac0182

The change in the patch 2/6 concerns the _generic_ DW MAC DMA IRQ
handler. You can't change the mask here without justification.
Moreover the generic DW MAC doesn't have the status flags behind the
mask you set. That's why earlier we find out a solution with creating
the Loongson-specific DMA IRQ-handler. You have it implemented in the
patch 6/6.

So my question was mostly rhetorical. You should have dropped the mask
change in this patch ever since the Loongson-specific DMA
IRQ-handler was added to your series.

-Serge(y) 

> 
> 
> Thanks,
> 
> Yanteng
> 
> > 
>