| Message ID | 20240814221818.2612484-4-jitendra.vegiraju@broadcom.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: Add PCI driver support for BCM8958x | expand |
Hi Jitendra On Wed, Aug 14, 2024 at 03:18:16PM -0700, jitendra.vegiraju@broadcom.com wrote: > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > Integrate dw25gmac support into stmmac hardware interface handling. > Added a new entry to the stmmac_hw table in hwif.c. > Define new macros DW25GMAC_CORE_4_00 and DW25GMAC_ID to identify 25GMAC > device. > Since BCM8958x is an early adaptor device, the synopsis_id reported in HW > is 0x32 and device_id is DWXGMAC_ID. Provide override support by defining > synopsys_dev_id member in struct stmmac_priv so that driver specific setup > functions can override the hardware reported values. > > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > --- > drivers/net/ethernet/stmicro/stmmac/common.h | 2 ++ > drivers/net/ethernet/stmicro/stmmac/hwif.c | 25 ++++++++++++++++++-- > drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 + > 3 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h > index 684489156dce..46edbe73a124 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/common.h > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h > @@ -38,9 +38,11 @@ > #define DWXGMAC_CORE_2_10 0x21 > #define DWXGMAC_CORE_2_20 0x22 > #define DWXLGMAC_CORE_2_00 0x20 > +#define DW25GMAC_CORE_4_00 0x40 > > /* Device ID */ > #define DWXGMAC_ID 0x76 > +#define DW25GMAC_ID 0x55 > #define DWXLGMAC_ID 0x27 > > #define STMMAC_CHAN0 0 /* Always supported and default for all chips */ > diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c > index 29367105df54..97e5594ddcda 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c > +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c > @@ -278,6 +278,27 @@ static const struct stmmac_hwif_entry { > .est = &dwmac510_est_ops, > .setup = dwxlgmac2_setup, > .quirks = stmmac_dwxlgmac_quirks, > + }, { > + .gmac = false, > + .gmac4 = false, > + .xgmac = true, > + .min_id = DW25GMAC_CORE_4_00, > + .dev_id = DW25GMAC_ID, > + .regs = { > + .ptp_off = PTP_XGMAC_OFFSET, > + .mmc_off = MMC_XGMAC_OFFSET, > + .est_off = EST_XGMAC_OFFSET, > + }, > + .desc = &dwxgmac210_desc_ops, > + .dma = &dw25gmac400_dma_ops, > + .mac = &dwxgmac210_ops, > + .hwtimestamp = &stmmac_ptp, > + .mode = NULL, > + .tc = &dwmac510_tc_ops, > + .mmc = &dwxgmac_mmc_ops, > + .est = &dwmac510_est_ops, > + .setup = dwxgmac2_setup, > + .quirks = NULL, > }, This can be replaced with just: + }, { + .gmac = false, + .gmac4 = false, + .xgmac = true, + .min_id = DW25GMAC_CORE_4_00, + .dev_id = DWXGMAC_ID, /* Early DW 25GMAC IP-core had XGMAC ID */ + .regs = { + .ptp_off = PTP_XGMAC_OFFSET, + .mmc_off = MMC_XGMAC_OFFSET, + .est_off = EST_XGMAC_OFFSET, + }, + .desc = &dwxgmac210_desc_ops, + .dma = &dw25gmac400_dma_ops, + .mac = &dwxgmac210_ops, + .hwtimestamp = &stmmac_ptp, + .mode = NULL, + .tc = &dwmac510_tc_ops, + .mmc = &dwxgmac_mmc_ops, + .est = &dwmac510_est_ops, + .setup = dw25gmac_setup, + .quirks = NULL, } and you won't need to pre-define the setup() method in the glue driver. Instead you can define a new dw25xgmac_setup() method in the dwxgmac2_core.c as it's done for the DW XGMAC/LXGMAC IP-cores. Note if your device is capable to work with up to 10Gbps speed, then just set the plat_stmmacenet_data::max_speed field to SPEED_10000. Alternatively if you really need to specify the exact MAC capabilities, then you can implement what Russell suggested here sometime ago: https://lore.kernel.org/netdev/Zf3ifH%2FCjyHtmXE3@shell.armlinux.org.uk/ If you also have a DW 25GMAC-based device with 0x55 device ID, then just add another stmmac_hw[] array entry. > }; > > @@ -304,7 +325,7 @@ int stmmac_hwif_init(struct stmmac_priv *priv) > > /* Save ID for later use */ > priv->synopsys_id = id; > - > + priv->synopsys_dev_id = dev_id; > /* Lets assume some safe values first */ > priv->ptpaddr = priv->ioaddr + > (needs_gmac4 ? PTP_GMAC4_OFFSET : PTP_GMAC3_X_OFFSET); > @@ -339,7 +360,7 @@ int stmmac_hwif_init(struct stmmac_priv *priv) > /* Use synopsys_id var because some setups can override this */ > if (priv->synopsys_id < entry->min_id) > continue; > - if (needs_xgmac && (dev_id ^ entry->dev_id)) > + if (needs_xgmac && (priv->synopsys_dev_id ^ entry->dev_id)) > continue; > > /* Only use generic HW helpers if needed */ > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h > index b23b920eedb1..9784bbaf9a51 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h > @@ -282,6 +282,7 @@ struct stmmac_priv { > struct stmmac_counters mmc; > int hw_cap_support; > int synopsys_id; > + int synopsys_dev_id; With the suggestion above implemented you won't need this. -Serge(y) > u32 msg_enable; > int wolopts; > int wol_irq; > -- > 2.34.1 >
Hi Serge(y) Thank you for reviewing the patch. On Fri, Aug 23, 2024 at 6:49 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > Hi Jitendra > > On Wed, Aug 14, 2024 at 03:18:16PM -0700, jitendra.vegiraju@broadcom.com wrote: > > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > > > Integrate dw25gmac support into stmmac hardware interface handling. > > Added a new entry to the stmmac_hw table in hwif.c. > > Define new macros DW25GMAC_CORE_4_00 and DW25GMAC_ID to identify 25GMAC > > device. > > Since BCM8958x is an early adaptor device, the synopsis_id reported in HW > > is 0x32 and device_id is DWXGMAC_ID. Provide override support by defining > > synopsys_dev_id member in struct stmmac_priv so that driver specific setup > > functions can override the hardware reported values. > > > > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > --- > > drivers/net/ethernet/stmicro/stmmac/common.h | 2 ++ > > drivers/net/ethernet/stmicro/stmmac/hwif.c | 25 ++++++++++++++++++-- > > drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 + > > 3 files changed, 26 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h > > index 684489156dce..46edbe73a124 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/common.h > > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h > > @@ -38,9 +38,11 @@ > > #define DWXGMAC_CORE_2_10 0x21 > > #define DWXGMAC_CORE_2_20 0x22 > > #define DWXLGMAC_CORE_2_00 0x20 > > +#define DW25GMAC_CORE_4_00 0x40 > > > > /* Device ID */ > > #define DWXGMAC_ID 0x76 > > +#define DW25GMAC_ID 0x55 > > #define DWXLGMAC_ID 0x27 > > > > #define STMMAC_CHAN0 0 /* Always supported and default for all chips */ > > diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c > > index 29367105df54..97e5594ddcda 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c > > @@ -278,6 +278,27 @@ static const struct stmmac_hwif_entry { > > .est = &dwmac510_est_ops, > > .setup = dwxlgmac2_setup, > > .quirks = stmmac_dwxlgmac_quirks, > > > + }, { > > + .gmac = false, > > + .gmac4 = false, > > + .xgmac = true, > > + .min_id = DW25GMAC_CORE_4_00, > > + .dev_id = DW25GMAC_ID, > > + .regs = { > > + .ptp_off = PTP_XGMAC_OFFSET, > > + .mmc_off = MMC_XGMAC_OFFSET, > > + .est_off = EST_XGMAC_OFFSET, > > + }, > > + .desc = &dwxgmac210_desc_ops, > > + .dma = &dw25gmac400_dma_ops, > > + .mac = &dwxgmac210_ops, > > + .hwtimestamp = &stmmac_ptp, > > + .mode = NULL, > > + .tc = &dwmac510_tc_ops, > > + .mmc = &dwxgmac_mmc_ops, > > + .est = &dwmac510_est_ops, > > + .setup = dwxgmac2_setup, > > + .quirks = NULL, > > }, > > This can be replaced with just: > > + }, { > + .gmac = false, > + .gmac4 = false, > + .xgmac = true, > + .min_id = DW25GMAC_CORE_4_00, > + .dev_id = DWXGMAC_ID, /* Early DW 25GMAC IP-core had XGMAC ID */ > + .regs = { > + .ptp_off = PTP_XGMAC_OFFSET, > + .mmc_off = MMC_XGMAC_OFFSET, > + .est_off = EST_XGMAC_OFFSET, > + }, > + .desc = &dwxgmac210_desc_ops, > + .dma = &dw25gmac400_dma_ops, > + .mac = &dwxgmac210_ops, > + .hwtimestamp = &stmmac_ptp, > + .mode = NULL, > + .tc = &dwmac510_tc_ops, > + .mmc = &dwxgmac_mmc_ops, > + .est = &dwmac510_est_ops, > + .setup = dw25gmac_setup, > + .quirks = NULL, > } > > and you won't need to pre-define the setup() method in the > glue driver. Instead you can define a new dw25xgmac_setup() method in > the dwxgmac2_core.c as it's done for the DW XGMAC/LXGMAC IP-cores. > > Note if your device is capable to work with up to 10Gbps speed, then > just set the plat_stmmacenet_data::max_speed field to SPEED_10000. > Alternatively if you really need to specify the exact MAC > capabilities, then you can implement what Russell suggested here > sometime ago: > https://lore.kernel.org/netdev/Zf3ifH%2FCjyHtmXE3@shell.armlinux.org.uk/ > I like your suggestion to add one stmmac_hw[] array entry (entry_a) for this "early release" DW25GMAC IP and another entry (entry_b) for final DW25MAC IP, in the process eliminate the need for a new member variable in struct stmmac_priv. However, I would like to bring to your attention that this device requires special handling for both synopsys_id and dev_id. This device is reporting 0x32 for synopsys_id and 0x76(XGMAC) for dev_id. The final 25GMAC spec will have 0x40 for synopsys_id and 0x55(25GMAC) for dev_id. So, in order to avoid falsely qualifying other XGMAC devices with synopsis_id >=0x32 as DW25GMAC, I am thinking we will have to overwrite the synopsys_id to 0x40 (DW25GMAC_CORE_4_00) in glue driver using existing glue driver override mechanism. We can implement dw25gmac_setup() in dwxgmac2_core.c for generic 25GMAC case. But, this glue driver will have to rely on its own setup function to override the synopsys_id as DW25GMAC_CORE_4_00. Do you think it looks reasonable? If yes, do you want me to add the generic 25GMAC stmmac_hw[] entry ("entry_b") now or when such a device becomes available for testing? > If you also have a DW 25GMAC-based device with 0x55 device ID, then > just add another stmmac_hw[] array entry. > > > }; > > > > int hw_cap_support; > > int synopsys_id; > > > + int synopsys_dev_id; > > With the suggestion above implemented you won't need this. Got it. Thanks. > > -Serge(y) > > > u32 msg_enable; > > int wolopts; > > int wol_irq; > > -- > > 2.34.1 > >
Hi Jitendra On Mon, Aug 26, 2024 at 11:53:13AM -0700, Jitendra Vegiraju wrote: > Hi Serge(y) > Thank you for reviewing the patch. > > On Fri, Aug 23, 2024 at 6:49 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > > > Hi Jitendra > > > > On Wed, Aug 14, 2024 at 03:18:16PM -0700, jitendra.vegiraju@broadcom.com wrote: > > > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > > > > > Integrate dw25gmac support into stmmac hardware interface handling. > > > Added a new entry to the stmmac_hw table in hwif.c. > > > Define new macros DW25GMAC_CORE_4_00 and DW25GMAC_ID to identify 25GMAC > > > device. > > > Since BCM8958x is an early adaptor device, the synopsis_id reported in HW > > > is 0x32 and device_id is DWXGMAC_ID. Provide override support by defining > > > synopsys_dev_id member in struct stmmac_priv so that driver specific setup > > > functions can override the hardware reported values. > > > > > > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > > --- > > > drivers/net/ethernet/stmicro/stmmac/common.h | 2 ++ > > > drivers/net/ethernet/stmicro/stmmac/hwif.c | 25 ++++++++++++++++++-- > > > drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 + > > > 3 files changed, 26 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h > > > index 684489156dce..46edbe73a124 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/common.h > > > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h > > > @@ -38,9 +38,11 @@ > > > #define DWXGMAC_CORE_2_10 0x21 > > > #define DWXGMAC_CORE_2_20 0x22 > > > #define DWXLGMAC_CORE_2_00 0x20 > > > +#define DW25GMAC_CORE_4_00 0x40 > > > > > > /* Device ID */ > > > #define DWXGMAC_ID 0x76 > > > +#define DW25GMAC_ID 0x55 > > > #define DWXLGMAC_ID 0x27 > > > > > > #define STMMAC_CHAN0 0 /* Always supported and default for all chips */ > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c > > > index 29367105df54..97e5594ddcda 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c > > > @@ -278,6 +278,27 @@ static const struct stmmac_hwif_entry { > > > .est = &dwmac510_est_ops, > > > .setup = dwxlgmac2_setup, > > > .quirks = stmmac_dwxlgmac_quirks, > > > > > + }, { > > > + .gmac = false, > > > + .gmac4 = false, > > > + .xgmac = true, > > > + .min_id = DW25GMAC_CORE_4_00, > > > + .dev_id = DW25GMAC_ID, > > > + .regs = { > > > + .ptp_off = PTP_XGMAC_OFFSET, > > > + .mmc_off = MMC_XGMAC_OFFSET, > > > + .est_off = EST_XGMAC_OFFSET, > > > + }, > > > + .desc = &dwxgmac210_desc_ops, > > > + .dma = &dw25gmac400_dma_ops, > > > + .mac = &dwxgmac210_ops, > > > + .hwtimestamp = &stmmac_ptp, > > > + .mode = NULL, > > > + .tc = &dwmac510_tc_ops, > > > + .mmc = &dwxgmac_mmc_ops, > > > + .est = &dwmac510_est_ops, > > > + .setup = dwxgmac2_setup, > > > + .quirks = NULL, > > > }, > > > > This can be replaced with just: > > > > + }, { > > + .gmac = false, > > + .gmac4 = false, > > + .xgmac = true, > > + .min_id = DW25GMAC_CORE_4_00, > > + .dev_id = DWXGMAC_ID, /* Early DW 25GMAC IP-core had XGMAC ID */ > > + .regs = { > > + .ptp_off = PTP_XGMAC_OFFSET, > > + .mmc_off = MMC_XGMAC_OFFSET, > > + .est_off = EST_XGMAC_OFFSET, > > + }, > > + .desc = &dwxgmac210_desc_ops, > > + .dma = &dw25gmac400_dma_ops, > > + .mac = &dwxgmac210_ops, > > + .hwtimestamp = &stmmac_ptp, > > + .mode = NULL, > > + .tc = &dwmac510_tc_ops, > > + .mmc = &dwxgmac_mmc_ops, > > + .est = &dwmac510_est_ops, > > + .setup = dw25gmac_setup, > > + .quirks = NULL, > > } > > > > and you won't need to pre-define the setup() method in the > > glue driver. Instead you can define a new dw25xgmac_setup() method in > > the dwxgmac2_core.c as it's done for the DW XGMAC/LXGMAC IP-cores. > > > > Note if your device is capable to work with up to 10Gbps speed, then > > just set the plat_stmmacenet_data::max_speed field to SPEED_10000. > > Alternatively if you really need to specify the exact MAC > > capabilities, then you can implement what Russell suggested here > > sometime ago: > > https://lore.kernel.org/netdev/Zf3ifH%2FCjyHtmXE3@shell.armlinux.org.uk/ > > > I like your suggestion to add one stmmac_hw[] array entry (entry_a) for this > "early release" DW25GMAC IP and another entry (entry_b) for final DW25MAC > IP, in the process eliminate the need for a new member variable in struct > stmmac_priv. > > However, I would like to bring to your attention that this device requires > special handling for both synopsys_id and dev_id. > This device is reporting 0x32 for synopsys_id and 0x76(XGMAC) for dev_id. > The final 25GMAC spec will have 0x40 for synopsys_id and 0x55(25GMAC) for > dev_id. For some reason I was thinking that your device had only the device ID pre-defined with the XGMAC value meanwhile the Synopsys ID was 0x40. Indeed you get to override both of these data in the platform-specific setup() method. > > So, in order to avoid falsely qualifying other XGMAC devices with > synopsis_id >=0x32 as DW25GMAC, I am thinking we will have to overwrite the > synopsys_id to 0x40 (DW25GMAC_CORE_4_00) in glue driver using existing > glue driver override mechanism. > > We can implement dw25gmac_setup() in dwxgmac2_core.c for generic 25GMAC > case. But, this glue driver will have to rely on its own setup function > to override the synopsys_id as DW25GMAC_CORE_4_00. > > Do you think it looks reasonable? What I was trying to avoid was the setup() method re-definition just for the sake of the IP-core version override. Because if not for that you could have created and used the generic DW 25GMAC dw25gmac_setup() function. One of the possible solutions I was thinking was to introduce the plat_stmmacenet_data::{snps_id,dev_id} fields and use their values in the stmmac_hwif_init() procedure instead of the data read from the MAC.VERSION CSR. Another solution could be to add the plat_stmmacenet_data::has_25gmac field and fix the generic driver code to using it where it's relevant. Then you won't need to think about what actual Synopsys ID/Device ID since it would mean a whole new IP-core. -Serge(y) > If yes, do you want me to add the generic 25GMAC stmmac_hw[] entry > ("entry_b") now or when > such a device becomes available for testing? > > > If you also have a DW 25GMAC-based device with 0x55 device ID, then > > just add another stmmac_hw[] array entry. > > > > > > }; > > > > > > int hw_cap_support; > > > int synopsys_id; > > > > > + int synopsys_dev_id; > > > > With the suggestion above implemented you won't need this. > > Got it. Thanks. > > > > > -Serge(y) > > > > > u32 msg_enable; > > > int wolopts; > > > int wol_irq; > > > -- > > > 2.34.1 > > >
On Thu, Aug 29, 2024 at 3:52 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > Hi Jitendra > > On Mon, Aug 26, 2024 at 11:53:13AM -0700, Jitendra Vegiraju wrote: > > Hi Serge(y) > > Thank you for reviewing the patch. > > > > On Fri, Aug 23, 2024 at 6:49 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > > > > > Hi Jitendra > > > > > > On Wed, Aug 14, 2024 at 03:18:16PM -0700, jitendra.vegiraju@broadcom.com wrote: > > > > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > > > > > > > Integrate dw25gmac support into stmmac hardware interface handling. > > > > Added a new entry to the stmmac_hw table in hwif.c. > > > > Define new macros DW25GMAC_CORE_4_00 and DW25GMAC_ID to identify 25GMAC > > > > device. > > > > Since BCM8958x is an early adaptor device, the synopsis_id reported in HW > > > > is 0x32 and device_id is DWXGMAC_ID. Provide override support by defining > > > > synopsys_dev_id member in struct stmmac_priv so that driver specific setup > > > > functions can override the hardware reported values. > > > > > > > > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > > > --- > > > > + }, { > > > > + .gmac = false, > > > > + .gmac4 = false, > > > > + .xgmac = true, > > > > + .min_id = DW25GMAC_CORE_4_00, > > > > + .dev_id = DW25GMAC_ID, > > > > + .regs = { > > > > + .ptp_off = PTP_XGMAC_OFFSET, > > > > + .mmc_off = MMC_XGMAC_OFFSET, > > > > + .est_off = EST_XGMAC_OFFSET, > > > > + }, > > > > + .desc = &dwxgmac210_desc_ops, > > > > + .dma = &dw25gmac400_dma_ops, > > > > + .mac = &dwxgmac210_ops, > > > > + .hwtimestamp = &stmmac_ptp, > > > > + .mode = NULL, > > > > + .tc = &dwmac510_tc_ops, > > > > + .mmc = &dwxgmac_mmc_ops, > > > > + .est = &dwmac510_est_ops, > > > > + .setup = dwxgmac2_setup, > > > > + .quirks = NULL, > > > > }, > > > > > > This can be replaced with just: > > > > > > + }, { > > > + .gmac = false, > > > + .gmac4 = false, > > > + .xgmac = true, > > > + .min_id = DW25GMAC_CORE_4_00, > > > + .dev_id = DWXGMAC_ID, /* Early DW 25GMAC IP-core had XGMAC ID */ > > > + .regs = { > > > + .ptp_off = PTP_XGMAC_OFFSET, > > > + .mmc_off = MMC_XGMAC_OFFSET, > > > + .est_off = EST_XGMAC_OFFSET, > > > + }, > > > + .desc = &dwxgmac210_desc_ops, > > > + .dma = &dw25gmac400_dma_ops, > > > + .mac = &dwxgmac210_ops, > > > + .hwtimestamp = &stmmac_ptp, > > > + .mode = NULL, > > > + .tc = &dwmac510_tc_ops, > > > + .mmc = &dwxgmac_mmc_ops, > > > + .est = &dwmac510_est_ops, > > > + .setup = dw25gmac_setup, > > > + .quirks = NULL, > > > } > > > > > > and you won't need to pre-define the setup() method in the > > > glue driver. Instead you can define a new dw25xgmac_setup() method in > > > the dwxgmac2_core.c as it's done for the DW XGMAC/LXGMAC IP-cores. > > > > > > Note if your device is capable to work with up to 10Gbps speed, then > > > just set the plat_stmmacenet_data::max_speed field to SPEED_10000. > > > Alternatively if you really need to specify the exact MAC > > > capabilities, then you can implement what Russell suggested here > > > sometime ago: > > > https://lore.kernel.org/netdev/Zf3ifH%2FCjyHtmXE3@shell.armlinux.org.uk/ > > > > > I like your suggestion to add one stmmac_hw[] array entry (entry_a) for this > > "early release" DW25GMAC IP and another entry (entry_b) for final DW25MAC > > IP, in the process eliminate the need for a new member variable in struct > > stmmac_priv. > > > > > However, I would like to bring to your attention that this device requires > > special handling for both synopsys_id and dev_id. > > This device is reporting 0x32 for synopsys_id and 0x76(XGMAC) for dev_id. > > The final 25GMAC spec will have 0x40 for synopsys_id and 0x55(25GMAC) for > > dev_id. > > For some reason I was thinking that your device had only the device ID > pre-defined with the XGMAC value meanwhile the Synopsys ID was 0x40. > Indeed you get to override both of these data in the platform-specific > setup() method. > > > > > So, in order to avoid falsely qualifying other XGMAC devices with > > synopsis_id >=0x32 as DW25GMAC, I am thinking we will have to overwrite the > > synopsys_id to 0x40 (DW25GMAC_CORE_4_00) in glue driver using existing > > glue driver override mechanism. > > > > We can implement dw25gmac_setup() in dwxgmac2_core.c for generic 25GMAC > > case. But, this glue driver will have to rely on its own setup function > > to override the synopsys_id as DW25GMAC_CORE_4_00. > > > > Do you think it looks reasonable? > > What I was trying to avoid was the setup() method re-definition just > for the sake of the IP-core version override. Because if not for that > you could have created and used the generic DW 25GMAC dw25gmac_setup() > function. > > One of the possible solutions I was thinking was to introduce the > plat_stmmacenet_data::{snps_id,dev_id} fields and use their values in > the stmmac_hwif_init() procedure instead of the data read from the > MAC.VERSION CSR. > Hi Serge(y), Thanks for the suggestions, I will implement this option since the code change is mostly local. We will have to add following check in hwif.c @@ -313,7 +313,10 @@ int stmmac_hwif_init(struct stmmac_priv *priv) u32 id, dev_id = 0; int i, ret; - if (needs_gmac) { + if (priv->plat->snps_id && priv->plat->snps_dev_id) { + id = priv->plat->snps_id; + dev_id = priv->plat->snps_dev_id; + } else if (needs_gmac) { id = stmmac_get_id(priv, GMAC_VERSION); > Another solution could be to add the plat_stmmacenet_data::has_25gmac > field and fix the generic driver code to using it where it's relevant. > Then you won't need to think about what actual Synopsys ID/Device ID > since it would mean a whole new IP-core. > > -Serge(y) >
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 684489156dce..46edbe73a124 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -38,9 +38,11 @@ #define DWXGMAC_CORE_2_10 0x21 #define DWXGMAC_CORE_2_20 0x22 #define DWXLGMAC_CORE_2_00 0x20 +#define DW25GMAC_CORE_4_00 0x40 /* Device ID */ #define DWXGMAC_ID 0x76 +#define DW25GMAC_ID 0x55 #define DWXLGMAC_ID 0x27 #define STMMAC_CHAN0 0 /* Always supported and default for all chips */ diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c index 29367105df54..97e5594ddcda 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c @@ -278,6 +278,27 @@ static const struct stmmac_hwif_entry { .est = &dwmac510_est_ops, .setup = dwxlgmac2_setup, .quirks = stmmac_dwxlgmac_quirks, + }, { + .gmac = false, + .gmac4 = false, + .xgmac = true, + .min_id = DW25GMAC_CORE_4_00, + .dev_id = DW25GMAC_ID, + .regs = { + .ptp_off = PTP_XGMAC_OFFSET, + .mmc_off = MMC_XGMAC_OFFSET, + .est_off = EST_XGMAC_OFFSET, + }, + .desc = &dwxgmac210_desc_ops, + .dma = &dw25gmac400_dma_ops, + .mac = &dwxgmac210_ops, + .hwtimestamp = &stmmac_ptp, + .mode = NULL, + .tc = &dwmac510_tc_ops, + .mmc = &dwxgmac_mmc_ops, + .est = &dwmac510_est_ops, + .setup = dwxgmac2_setup, + .quirks = NULL, }, }; @@ -304,7 +325,7 @@ int stmmac_hwif_init(struct stmmac_priv *priv) /* Save ID for later use */ priv->synopsys_id = id; - + priv->synopsys_dev_id = dev_id; /* Lets assume some safe values first */ priv->ptpaddr = priv->ioaddr + (needs_gmac4 ? PTP_GMAC4_OFFSET : PTP_GMAC3_X_OFFSET); @@ -339,7 +360,7 @@ int stmmac_hwif_init(struct stmmac_priv *priv) /* Use synopsys_id var because some setups can override this */ if (priv->synopsys_id < entry->min_id) continue; - if (needs_xgmac && (dev_id ^ entry->dev_id)) + if (needs_xgmac && (priv->synopsys_dev_id ^ entry->dev_id)) continue; /* Only use generic HW helpers if needed */ diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h index b23b920eedb1..9784bbaf9a51 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h @@ -282,6 +282,7 @@ struct stmmac_priv { struct stmmac_counters mmc; int hw_cap_support; int synopsys_id; + int synopsys_dev_id; u32 msg_enable; int wolopts; int wol_irq;