| Message ID | 20240904054815.1341712-2-jitendra.vegiraju@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: stmmac: Add PCI driver support for BCM8958x | expand |
Hi Jitendra On Tue, Sep 03, 2024 at 10:48:11PM -0700, jitendra.vegiraju@broadcom.com wrote: > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > Add hdma configuration support in include/linux/stmmac.h file. > The hdma configuration includes mapping of virtual DMAs to physical DMAs. > Define a new data structure stmmac_hdma_cfg to provide the mapping. > > Introduce new plat_stmmacenet_data::snps_id,snps_dev_id to allow glue > drivers to specify synopsys ID and device id respectively. > These values take precedence over reading from HW register. This facility > provides a mechanism to use setup function from stmmac core module and yet > override MAC.VERSION CSR if the glue driver chooses to do so. > > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > --- > include/linux/stmmac.h | 48 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h > index 338991c08f00..eb8136680a7b 100644 > --- a/include/linux/stmmac.h > +++ b/include/linux/stmmac.h > @@ -89,6 +89,51 @@ struct stmmac_mdio_bus_data { > bool needs_reset; > }; > > +/* DW25GMAC Hyper-DMA Overview > + * Hyper-DMA allows support for large number of Virtual DMA(VDMA) > + * channels using a smaller set of physical DMA channels(PDMA). > + * This is supported by the mapping of VDMAs to Traffic Class(TC) > + * and PDMA to TC in each traffic direction as shown below. > + * > + * VDMAs Traffic Class PDMA > + * +--------+ +------+ +-----------+ > + * |VDMA0 |--------->| TC0 |-------->|PDMA0/TXQ0 | > + *TX +--------+ |----->+------+ +-----------+ > + *Host=> +--------+ | +------+ +-----------+ => MAC > + *SW |VDMA1 |---+ | TC1 | +--->|PDMA1/TXQ1 | > + * +--------+ +------+ | +-----------+ > + * +--------+ +------+----+ +-----------+ > + * |VDMA2 |--------->| TC2 |-------->|PDMA2/TXQ1 | > + * +--------+ +------+ +-----------+ > + * . . . > + * +--------+ +------+ +-----------+ > + * |VDMAn-1 |--------->| TCx-1|-------->|PDMAm/TXQm | > + * +--------+ +------+ +-----------+ > + * > + * +------+ +------+ +------+ > + * |PDMA0 |--------->| TC0 |-------->|VDMA0 | > + * +------+ |----->+------+ +------+ > + *MAC => +------+ | +------+ +------+ > + *RXQs |PDMA1 |---+ | TC1 | +--->|VDMA1 | => Host > + * +------+ +------+ | +------+ > + * . . . > + */ > + > +/* Hyper-DMA mapping configuration > + * Traffic Class associated with each VDMA/PDMA mapping > + * is stored in corresponding array entry. > + */ > +struct stmmac_hdma_cfg { > + u32 tx_vdmas; /* TX VDMA count */ > + u32 rx_vdmas; /* RX VDMA count */ > + u32 tx_pdmas; /* TX PDMA count */ > + u32 rx_pdmas; /* RX PDMA count */ > + u8 *tvdma_tc; /* Tx VDMA to TC mapping array */ > + u8 *rvdma_tc; /* Rx VDMA to TC mapping array */ > + u8 *tpdma_tc; /* Tx PDMA to TC mapping array */ > + u8 *rpdma_tc; /* Rx PDMA to TC mapping array */ > +}; > + > struct stmmac_dma_cfg { > int pbl; > int txpbl; > @@ -101,6 +146,7 @@ struct stmmac_dma_cfg { > bool multi_msi_en; > bool dche; > bool atds; > + struct stmmac_hdma_cfg *hdma_cfg; Based on what you are implementing the _static_ VDMA-TC-PDMA channels mapping I really don't see a value of adding all of these data here. The whole implementation gets to be needlessly overcomplicated. Moreover AFAICS there are some channels left misconfigured in the Patch 2 code. Please see my comments there for more details. > }; > > #define AXI_BLEN 7 > @@ -303,5 +349,7 @@ struct plat_stmmacenet_data { > int msi_tx_base_vec; > const struct dwmac4_addrs *dwmac4_addrs; > unsigned int flags; > + u32 snps_id; > + u32 snps_dev_id; Please move these fields to the head of the structure as the kind of crucial ones, and convert snps_dev_id to just dev_id. snps_id field name was selected based on the VERSION.SNPSVER field name (see SNPS prefix). Following that logic the VERSION.DEVID field should be converted to the dev_id name. -Serge(y) > }; > #endif > -- > 2.34.1 >
Hi Serge, Thank you for taking the time to review the patches. On Tue, Sep 10, 2024 at 11:37 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > Hi Jitendra > > On Tue, Sep 03, 2024 at 10:48:11PM -0700, jitendra.vegiraju@broadcom.com wrote: > > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > > > Add hdma configuration support in include/linux/stmmac.h file. > > The hdma configuration includes mapping of virtual DMAs to physical DMAs. > > Define a new data structure stmmac_hdma_cfg to provide the mapping. > > > > Introduce new plat_stmmacenet_data::snps_id,snps_dev_id to allow glue > > drivers to specify synopsys ID and device id respectively. > > These values take precedence over reading from HW register. This facility > > provides a mechanism to use setup function from stmmac core module and yet > > override MAC.VERSION CSR if the glue driver chooses to do so. > > > > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > --- > > include/linux/stmmac.h | 48 ++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 48 insertions(+) > > > > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h > > index 338991c08f00..eb8136680a7b 100644 > > --- a/include/linux/stmmac.h > > +++ b/include/linux/stmmac.h > > @@ -89,6 +89,51 @@ struct stmmac_mdio_bus_data { > > bool needs_reset; > > }; > > > > +/* DW25GMAC Hyper-DMA Overview > > + * Hyper-DMA allows support for large number of Virtual DMA(VDMA) > > + * channels using a smaller set of physical DMA channels(PDMA). > > + * This is supported by the mapping of VDMAs to Traffic Class(TC) > > + * and PDMA to TC in each traffic direction as shown below. > > + * > > + * VDMAs Traffic Class PDMA > > + * +--------+ +------+ +-----------+ > > + * |VDMA0 |--------->| TC0 |-------->|PDMA0/TXQ0 | > > + *TX +--------+ |----->+------+ +-----------+ > > + *Host=> +--------+ | +------+ +-----------+ => MAC > > + *SW |VDMA1 |---+ | TC1 | +--->|PDMA1/TXQ1 | > > + * +--------+ +------+ | +-----------+ > > + * +--------+ +------+----+ +-----------+ > > + * |VDMA2 |--------->| TC2 |-------->|PDMA2/TXQ1 | > > + * +--------+ +------+ +-----------+ > > + * . . . > > + * +--------+ +------+ +-----------+ > > + * |VDMAn-1 |--------->| TCx-1|-------->|PDMAm/TXQm | > > + * +--------+ +------+ +-----------+ > > + * > > + * +------+ +------+ +------+ > > + * |PDMA0 |--------->| TC0 |-------->|VDMA0 | > > + * +------+ |----->+------+ +------+ > > + *MAC => +------+ | +------+ +------+ > > + *RXQs |PDMA1 |---+ | TC1 | +--->|VDMA1 | => Host > > + * +------+ +------+ | +------+ > > + * . . . > > + */ > > + > > > +/* Hyper-DMA mapping configuration > > + * Traffic Class associated with each VDMA/PDMA mapping > > + * is stored in corresponding array entry. > > + */ > > +struct stmmac_hdma_cfg { > > + u32 tx_vdmas; /* TX VDMA count */ > > + u32 rx_vdmas; /* RX VDMA count */ > > + u32 tx_pdmas; /* TX PDMA count */ > > + u32 rx_pdmas; /* RX PDMA count */ > > + u8 *tvdma_tc; /* Tx VDMA to TC mapping array */ > > + u8 *rvdma_tc; /* Rx VDMA to TC mapping array */ > > + u8 *tpdma_tc; /* Tx PDMA to TC mapping array */ > > + u8 *rpdma_tc; /* Rx PDMA to TC mapping array */ > > +}; > > + > > struct stmmac_dma_cfg { > > int pbl; > > int txpbl; > > @@ -101,6 +146,7 @@ struct stmmac_dma_cfg { > > bool multi_msi_en; > > bool dche; > > bool atds; > > + struct stmmac_hdma_cfg *hdma_cfg; > > Based on what you are implementing the _static_ VDMA-TC-PDMA channels > mapping I really don't see a value of adding all of these data here. > The whole implementation gets to be needlessly overcomplicated. > Moreover AFAICS there are some channels left misconfigured in the > Patch 2 code. Please see my comments there for more details. > I agree, with _static_ VDMA-TC-PDMA channels, maintaining the mapping data appears complicated. The real need comes when adding virtualization (SRIOV) capabilities. I am analyzing your comments in patch2 and will respond after re-evaluation. > > }; > > > > #define AXI_BLEN 7 > > @@ -303,5 +349,7 @@ struct plat_stmmacenet_data { > > int msi_tx_base_vec; > > const struct dwmac4_addrs *dwmac4_addrs; > > unsigned int flags; > > > + u32 snps_id; > > + u32 snps_dev_id; > > Please move these fields to the head of the structure as the kind of > crucial ones, and convert snps_dev_id to just dev_id. > > snps_id field name was selected based on the VERSION.SNPSVER field > name (see SNPS prefix). Following that logic the VERSION.DEVID field > should be converted to the dev_id name. > Thanks for explaining the thinking behind the field naming. That makes sense. I was thinking of it as a prefix for synopsys fields. Will make the change. > -Serge(y) > > > }; > > #endif > > -- > > 2.34.1 > >
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index 338991c08f00..eb8136680a7b 100644 --- a/include/linux/stmmac.h +++ b/include/linux/stmmac.h @@ -89,6 +89,51 @@ struct stmmac_mdio_bus_data { bool needs_reset; }; +/* DW25GMAC Hyper-DMA Overview + * Hyper-DMA allows support for large number of Virtual DMA(VDMA) + * channels using a smaller set of physical DMA channels(PDMA). + * This is supported by the mapping of VDMAs to Traffic Class(TC) + * and PDMA to TC in each traffic direction as shown below. + * + * VDMAs Traffic Class PDMA + * +--------+ +------+ +-----------+ + * |VDMA0 |--------->| TC0 |-------->|PDMA0/TXQ0 | + *TX +--------+ |----->+------+ +-----------+ + *Host=> +--------+ | +------+ +-----------+ => MAC + *SW |VDMA1 |---+ | TC1 | +--->|PDMA1/TXQ1 | + * +--------+ +------+ | +-----------+ + * +--------+ +------+----+ +-----------+ + * |VDMA2 |--------->| TC2 |-------->|PDMA2/TXQ1 | + * +--------+ +------+ +-----------+ + * . . . + * +--------+ +------+ +-----------+ + * |VDMAn-1 |--------->| TCx-1|-------->|PDMAm/TXQm | + * +--------+ +------+ +-----------+ + * + * +------+ +------+ +------+ + * |PDMA0 |--------->| TC0 |-------->|VDMA0 | + * +------+ |----->+------+ +------+ + *MAC => +------+ | +------+ +------+ + *RXQs |PDMA1 |---+ | TC1 | +--->|VDMA1 | => Host + * +------+ +------+ | +------+ + * . . . + */ + +/* Hyper-DMA mapping configuration + * Traffic Class associated with each VDMA/PDMA mapping + * is stored in corresponding array entry. + */ +struct stmmac_hdma_cfg { + u32 tx_vdmas; /* TX VDMA count */ + u32 rx_vdmas; /* RX VDMA count */ + u32 tx_pdmas; /* TX PDMA count */ + u32 rx_pdmas; /* RX PDMA count */ + u8 *tvdma_tc; /* Tx VDMA to TC mapping array */ + u8 *rvdma_tc; /* Rx VDMA to TC mapping array */ + u8 *tpdma_tc; /* Tx PDMA to TC mapping array */ + u8 *rpdma_tc; /* Rx PDMA to TC mapping array */ +}; + struct stmmac_dma_cfg { int pbl; int txpbl; @@ -101,6 +146,7 @@ struct stmmac_dma_cfg { bool multi_msi_en; bool dche; bool atds; + struct stmmac_hdma_cfg *hdma_cfg; }; #define AXI_BLEN 7 @@ -303,5 +349,7 @@ struct plat_stmmacenet_data { int msi_tx_base_vec; const struct dwmac4_addrs *dwmac4_addrs; unsigned int flags; + u32 snps_id; + u32 snps_dev_id; }; #endif