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,
},
};
@@ -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;
--
2.34.1Hi 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)
>
© 2016 - 2026 Red Hat, Inc.