[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <zqzkanogzdaqvjjobnoidhl4tqeho5uqldsa2r46ttbxo3y4dt@tdouti2x652a>
Date: Wed, 20 Mar 2024 14:24:23 +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@...ngson.cn, linux@...linux.org.uk, guyinggang@...ngson.cn,
netdev@...r.kernel.org, chris.chenfeiyang@...il.com
Subject: Re: [PATCH net-next v8 11/11] net: stmmac: dwmac-loongson: Disable
coe for some Loongson GNET
Hi Yanteng
On Wed, Mar 13, 2024 at 06:19:47PM +0800, Yanteng Si wrote:
>
> 在 2024/3/13 17:52, Yanteng Si 写道:
> >
> > 在 2024/2/6 06:18, Serge Semin 写道:
> > > On Tue, Jan 30, 2024 at 04:49:16PM +0800, Yanteng Si wrote:
> > > > Some chips of Loongson GNET does not support coe, so disable them.
> > > s/coe/Tx COE
> > OK.
> > >
> > > > Set dma_cap->tx_coe to 0 and overwrite get_hw_feature.
> > > >
> > > > Signed-off-by: Yanteng Si <siyanteng@...ngson.cn>
> > > > Signed-off-by: Feiyang Chen <chenfeiyang@...ngson.cn>
> > > > Signed-off-by: Yinggang Gu <guyinggang@...ngson.cn>
> > > > ---
> > > > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 46
> > > > +++++++++++++++++++
> > > > 1 file changed, 46 insertions(+)
> > > >
> > > > diff --git
> > > > a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > index b78a73ea748b..8018d7d5f31b 100644
> > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > > > @@ -196,6 +196,51 @@ static int dwlgmac_dma_interrupt(struct
> > > > stmmac_priv *priv, void __iomem *ioaddr,
> > > > return ret;
> > > > }
> > > > +static int dwlgmac_get_hw_feature(void __iomem *ioaddr,
> > > Please use GNET-specific prefix.
> > OK. loongson_gnet_get_hw_feature()
> > >
> > > > + struct dma_features *dma_cap)
> > > > +{
> > > > + u32 hw_cap = readl(ioaddr + DMA_HW_FEATURE);
> > > > +
> > > > + if (!hw_cap) {
> > > > + /* 0x00000000 is the value read on old hardware that does not
> > > > + * implement this register
> > > > + */
> > > > + return -EOPNOTSUPP;
> > > > + }
> > > This doesn't seems like possible. All your devices have the
> > > HW-features register. If so please drop.
> > OK, drop it.
> > >
> > > > +
> > > > + dma_cap->mbps_10_100 = (hw_cap & DMA_HW_FEAT_MIISEL);
> > > > + dma_cap->mbps_1000 = (hw_cap & DMA_HW_FEAT_GMIISEL) >> 1;
> > > > + dma_cap->half_duplex = (hw_cap & DMA_HW_FEAT_HDSEL) >> 2;
> > > > + dma_cap->hash_filter = (hw_cap & DMA_HW_FEAT_HASHSEL) >> 4;
> > > > + dma_cap->multi_addr = (hw_cap & DMA_HW_FEAT_ADDMAC) >> 5;
> > > > + dma_cap->pcs = (hw_cap & DMA_HW_FEAT_PCSSEL) >> 6;
> > > > + dma_cap->sma_mdio = (hw_cap & DMA_HW_FEAT_SMASEL) >> 8;
> > > > + dma_cap->pmt_remote_wake_up = (hw_cap & DMA_HW_FEAT_RWKSEL) >> 9;
> > > > + dma_cap->pmt_magic_frame = (hw_cap & DMA_HW_FEAT_MGKSEL) >> 10;
> > > > + /* MMC */
> > > > + dma_cap->rmon = (hw_cap & DMA_HW_FEAT_MMCSEL) >> 11;
> > > > + /* IEEE 1588-2002 */
> > > > + dma_cap->time_stamp =
> > > > + (hw_cap & DMA_HW_FEAT_TSVER1SEL) >> 12;
> > > > + /* IEEE 1588-2008 */
> > > > + dma_cap->atime_stamp = (hw_cap & DMA_HW_FEAT_TSVER2SEL) >> 13;
> > > > + /* 802.3az - Energy-Efficient Ethernet (EEE) */
> > > > + dma_cap->eee = (hw_cap & DMA_HW_FEAT_EEESEL) >> 14;
> > > > + dma_cap->av = (hw_cap & DMA_HW_FEAT_AVSEL) >> 15;
> > > > + /* TX and RX csum */
> > > > + dma_cap->tx_coe = 0;
> > > > + dma_cap->rx_coe_type1 = (hw_cap & DMA_HW_FEAT_RXTYP1COE) >> 17;
> > > > + dma_cap->rx_coe_type2 = (hw_cap & DMA_HW_FEAT_RXTYP2COE) >> 18;
> > > > + dma_cap->rxfifo_over_2048 = (hw_cap &
> > > > DMA_HW_FEAT_RXFIFOSIZE) >> 19;
> > > > + /* TX and RX number of channels */
> > > > + dma_cap->number_rx_channel = (hw_cap & DMA_HW_FEAT_RXCHCNT) >> 20;
> > > > + dma_cap->number_tx_channel = (hw_cap & DMA_HW_FEAT_TXCHCNT) >> 22;
> > > > + /* Alternate (enhanced) DESC mode */
> > > > + dma_cap->enh_desc = (hw_cap & DMA_HW_FEAT_ENHDESSEL) >> 24;
> > > I am not sure whether you need to parse the capability register at all
> > > seeing this is a GNET-specific method. For that device all the
> > > capabilities are already known and can be just initialized in this
> > > method.
> > -dma_cap->tx_coe = (hw_cap & DMA_HW_FEAT_TXCOESEL) >> 16;
> >
> > +dma_cap->tx_coe = 0;
> >
> > I'm a little confused. Actually, I only modified this line, which is
> > used to fix the checksum.
> >
> > 2k2000 of Loongson GNET does not support coe.
>
> Specifically, it is to ensure the normal operation of multiple channels, as
> other channels except for channel 0 cannot perform checksum.
Originally I thought that Tx-COE was fully broken, but seeing it works
for channel 0 changes the situation. While we kept discussing your
series a useful patch was merged into the driver:
https://lore.kernel.org/netdev/20230916063312.7011-3-rohan.g.thomas@intel.com/
The stmmac_txq_cfg::coe_unsupported flag can be used to disable Tx COE
for the particular channels. You can just set the coe_unsupported flag
for the channels greater than zero and the skb_checksum_help() helper
will be utilized for them to calculate the packets control sum. This
will be the most optimal solution since channel zero will still be
serviced by the Tx Checksum Offload Engine and you won't need the
stmmac_dma_ops::get_hw_feature() callback redefinition.
-Serge(y)
>
> Thanks,
>
> Yanteng
>
>
> >
> >
> > Thanks,
> > Yanteng
> >
> > >
> > > -Serge(y)
> > >
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > struct stmmac_pci_info {
> > > > int (*setup)(struct pci_dev *pdev, struct
> > > > plat_stmmacenet_data *plat);
> > > > int (*config)(struct pci_dev *pdev, struct
> > > > plat_stmmacenet_data *plat,
> > > > @@ -542,6 +587,7 @@ static int loongson_dwmac_probe(struct
> > > > pci_dev *pdev,
> > > > ld->dwlgmac_dma_ops = dwmac1000_dma_ops;
> > > > ld->dwlgmac_dma_ops.init_chan = dwlgmac_dma_init_channel;
> > > > ld->dwlgmac_dma_ops.dma_interrupt = dwlgmac_dma_interrupt;
> > > > + ld->dwlgmac_dma_ops.get_hw_feature = dwlgmac_get_hw_feature;
> > > > plat->setup = loongson_setup;
> > > > ret = loongson_dwmac_config_multi_msi(pdev, plat,
> > > > &res, np, 8);
> > > > --
> > > > 2.31.4
> > > >
>
Powered by blists - more mailing lists