| Message ID | 20210722141351.13668-13-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add Gigabit Ethernet driver support | expand |
Hello! On 7/22/21 5:13 PM, Biju Das wrote: > The R-Car AVB module has Magic packet detection, multiple irq's and > timestamp enable features which is not present on RZ/G2L Gigabit ^ are > Ethernet module. Factorise emac and dmac initialization function to > support the later SoC. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > drivers/net/ethernet/renesas/ravb.h | 2 + > drivers/net/ethernet/renesas/ravb_main.c | 58 ++++++++++++++++-------- > 2 files changed, 40 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h > index d82bfa6e57c1..4d5910dcda86 100644 > --- a/drivers/net/ethernet/renesas/ravb.h > +++ b/drivers/net/ethernet/renesas/ravb.h > @@ -992,6 +992,8 @@ struct ravb_ops { > void (*ring_free)(struct net_device *ndev, int q); > void (*ring_format)(struct net_device *ndev, int q); > bool (*alloc_rx_desc)(struct net_device *ndev, int q); > + void (*emac_init)(struct net_device *ndev); > + void (*dmac_init)(struct net_device *ndev); > }; > > struct ravb_drv_data { > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index 3d0f6598b936..e200114376e4 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device *ndev, int q) > } > > /* E-MAC init function */ > -static void ravb_emac_init(struct net_device *ndev) > +static void ravb_emac_init_ex(struct net_device *ndev) > { > /* Receive frame limit set register */ > ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR); > @@ -480,30 +480,19 @@ static void ravb_emac_init(struct net_device *ndev) > ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, ECSIPR); > } > > -/* Device init function for Ethernet AVB */ Grr, this comment seems oudated... > -static int ravb_dmac_init(struct net_device *ndev) > +static void ravb_emac_init(struct net_device *ndev) > { > struct ravb_private *priv = netdev_priv(ndev); > const struct ravb_drv_data *info = priv->info; > - int error; > > - /* Set CONFIG mode */ > - error = ravb_config(ndev); > - if (error) > - return error; > - > - error = ravb_ring_init(ndev, RAVB_BE); > - if (error) > - return error; > - error = ravb_ring_init(ndev, RAVB_NC); > - if (error) { > - ravb_ring_free(ndev, RAVB_BE); > - return error; > - } > + info->ravb_ops->emac_init(ndev); > +} The whole ravb_emac_init() now consists only of a single method call? Why do we need it at all? > > - /* Descriptor format */ > - ravb_ring_format(ndev, RAVB_BE); > - ravb_ring_format(ndev, RAVB_NC); > +/* Device init function for Ethernet AVB */ s/Device/DMAC/. Or this comment shouldn't have been moved. > +static void ravb_dmac_init_ex(struct net_device *ndev) Please no _ex suffixes -- reminds me of Windoze too much. :-) > +{ > + struct ravb_private *priv = netdev_priv(ndev); > + const struct ravb_drv_data *info = priv->info; > > /* Set AVB RX */ > ravb_write(ndev, > @@ -530,6 +519,33 @@ static int ravb_dmac_init(struct net_device *ndev) > ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2); > /* Frame transmitted, timestamp FIFO updated */ > ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC); > +} > + > +static int ravb_dmac_init(struct net_device *ndev) > +{ > + struct ravb_private *priv = netdev_priv(ndev); > + const struct ravb_drv_data *info = priv->info; > + int error; > + > + /* Set CONFIG mode */ > + error = ravb_config(ndev); > + if (error) > + return error; > + > + error = ravb_ring_init(ndev, RAVB_BE); > + if (error) > + return error; > + error = ravb_ring_init(ndev, RAVB_NC); > + if (error) { > + ravb_ring_free(ndev, RAVB_BE); > + return error; > + } > + > + /* Descriptor format */ > + ravb_ring_format(ndev, RAVB_BE); > + ravb_ring_format(ndev, RAVB_NC); > + > + info->ravb_ops->dmac_init(ndev); > > /* Setting the control will start the AVB-DMAC process. */ > ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION); > @@ -2018,6 +2034,8 @@ static const struct ravb_ops ravb_gen3_ops = { > .ring_free = ravb_ring_free_rx, > .ring_format = ravb_ring_format_rx, > .alloc_rx_desc = ravb_alloc_rx_desc, > + .emac_init = ravb_emac_init_ex, > + .dmac_init = ravb_dmac_init_ex, Hmm, why not also gen2?! > }; [...] MBR, Sergei
Hi Sergei, Thanks for the feedback. > <prabhakar.mahadev-lad.rj@bp.renesas.com> > Subject: Re: [PATCH net-next 12/18] ravb: Factorise {emac,dmac} init > function > > Hello! > > On 7/22/21 5:13 PM, Biju Das wrote: > > > The R-Car AVB module has Magic packet detection, multiple irq's and > > timestamp enable features which is not present on RZ/G2L Gigabit > ^ are OK. Will fix this in next patch set. > > > Ethernet module. Factorise emac and dmac initialization function to > > support the later SoC. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > drivers/net/ethernet/renesas/ravb.h | 2 + > > drivers/net/ethernet/renesas/ravb_main.c | 58 > > ++++++++++++++++-------- > > 2 files changed, 40 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/net/ethernet/renesas/ravb.h > > b/drivers/net/ethernet/renesas/ravb.h > > index d82bfa6e57c1..4d5910dcda86 100644 > > --- a/drivers/net/ethernet/renesas/ravb.h > > +++ b/drivers/net/ethernet/renesas/ravb.h > > @@ -992,6 +992,8 @@ struct ravb_ops { > > void (*ring_free)(struct net_device *ndev, int q); > > void (*ring_format)(struct net_device *ndev, int q); > > bool (*alloc_rx_desc)(struct net_device *ndev, int q); > > + void (*emac_init)(struct net_device *ndev); > > + void (*dmac_init)(struct net_device *ndev); > > }; > > > > struct ravb_drv_data { > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c > > b/drivers/net/ethernet/renesas/ravb_main.c > > index 3d0f6598b936..e200114376e4 100644 > > --- a/drivers/net/ethernet/renesas/ravb_main.c > > +++ b/drivers/net/ethernet/renesas/ravb_main.c > > @@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device *ndev, > > int q) } > > > > /* E-MAC init function */ > > -static void ravb_emac_init(struct net_device *ndev) > > +static void ravb_emac_init_ex(struct net_device *ndev) > > { > > /* Receive frame limit set register */ > > ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, > > RFLR); @@ -480,30 +480,19 @@ static void ravb_emac_init(struct > net_device *ndev) > > ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, > > ECSIPR); } > > > > -/* Device init function for Ethernet AVB */ > > Grr, this comment seems oudated... OK. > > > -static int ravb_dmac_init(struct net_device *ndev) > > +static void ravb_emac_init(struct net_device *ndev) > > { > > struct ravb_private *priv = netdev_priv(ndev); > > const struct ravb_drv_data *info = priv->info; > > - int error; > > > > - /* Set CONFIG mode */ > > - error = ravb_config(ndev); > > - if (error) > > - return error; > > - > > - error = ravb_ring_init(ndev, RAVB_BE); > > - if (error) > > - return error; > > - error = ravb_ring_init(ndev, RAVB_NC); > > - if (error) { > > - ravb_ring_free(ndev, RAVB_BE); > > - return error; > > - } > > + info->ravb_ops->emac_init(ndev); > > +} > > The whole ravb_emac_init() now consists only of a single method call? > Why do we need it at all? OK will assign info->emac_init with ravb_emac_init, so GbEthernet just need to fill emac_init function. I will remove the function "ravb_emac_init_ex". > > > > > - /* Descriptor format */ > > - ravb_ring_format(ndev, RAVB_BE); > > - ravb_ring_format(ndev, RAVB_NC); > > +/* Device init function for Ethernet AVB */ > > s/Device/DMAC/. Or this comment shouldn't have been moved. OK. > > > +static void ravb_dmac_init_ex(struct net_device *ndev) > > Please no _ex suffixes -- reminds me of Windoze too much. :-) OK. Will change it to ravb_device_init Regards, Biju > > > +{ > > + struct ravb_private *priv = netdev_priv(ndev); > > + const struct ravb_drv_data *info = priv->info; > > > > /* Set AVB RX */ > > ravb_write(ndev, > > @@ -530,6 +519,33 @@ static int ravb_dmac_init(struct net_device *ndev) > > ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2); > > /* Frame transmitted, timestamp FIFO updated */ > > ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC); > > +} > > + > > +static int ravb_dmac_init(struct net_device *ndev) { > > + struct ravb_private *priv = netdev_priv(ndev); > > + const struct ravb_drv_data *info = priv->info; > > + int error; > > + > > + /* Set CONFIG mode */ > > + error = ravb_config(ndev); > > + if (error) > > + return error; > > + > > + error = ravb_ring_init(ndev, RAVB_BE); > > + if (error) > > + return error; > > + error = ravb_ring_init(ndev, RAVB_NC); > > + if (error) { > > + ravb_ring_free(ndev, RAVB_BE); > > + return error; > > + } > > + > > + /* Descriptor format */ > > + ravb_ring_format(ndev, RAVB_BE); > > + ravb_ring_format(ndev, RAVB_NC); > > + > > + info->ravb_ops->dmac_init(ndev); > > > > /* Setting the control will start the AVB-DMAC process. */ > > ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION); @@ -2018,6 > > +2034,8 @@ static const struct ravb_ops ravb_gen3_ops = { > > .ring_free = ravb_ring_free_rx, > > .ring_format = ravb_ring_format_rx, > > .alloc_rx_desc = ravb_alloc_rx_desc, > > + .emac_init = ravb_emac_init_ex, > > + .dmac_init = ravb_dmac_init_ex, > > Hmm, why not also gen2?!
On 8/20/21 6:42 PM, Biju Das wrote: [...] >>> The R-Car AVB module has Magic packet detection, multiple irq's and >>> timestamp enable features which is not present on RZ/G2L Gigabit >> ^ are > > OK. Will fix this in next patch set. > >> >>> Ethernet module. Factorise emac and dmac initialization function to >>> support the later SoC. >>> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >>> --- >>> drivers/net/ethernet/renesas/ravb.h | 2 + >>> drivers/net/ethernet/renesas/ravb_main.c | 58 >>> ++++++++++++++++-------- >>> 2 files changed, 40 insertions(+), 20 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h >>> b/drivers/net/ethernet/renesas/ravb.h >>> index d82bfa6e57c1..4d5910dcda86 100644 >>> --- a/drivers/net/ethernet/renesas/ravb.h >>> +++ b/drivers/net/ethernet/renesas/ravb.h >>> @@ -992,6 +992,8 @@ struct ravb_ops { >>> void (*ring_free)(struct net_device *ndev, int q); >>> void (*ring_format)(struct net_device *ndev, int q); >>> bool (*alloc_rx_desc)(struct net_device *ndev, int q); >>> + void (*emac_init)(struct net_device *ndev); >>> + void (*dmac_init)(struct net_device *ndev); >>> }; >>> >>> struct ravb_drv_data { >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c >>> b/drivers/net/ethernet/renesas/ravb_main.c >>> index 3d0f6598b936..e200114376e4 100644 >>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c >>> @@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device *ndev, >>> int q) } >>> >>> /* E-MAC init function */ >>> -static void ravb_emac_init(struct net_device *ndev) >>> +static void ravb_emac_init_ex(struct net_device *ndev) >>> { >>> /* Receive frame limit set register */ >>> ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, >>> RFLR); @@ -480,30 +480,19 @@ static void ravb_emac_init(struct >> net_device *ndev) >>> ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, >>> ECSIPR); } >>> >>> -/* Device init function for Ethernet AVB */ >> >> Grr, this comment seems oudated... > > OK. Just don't move the comment. :-) >>> -static int ravb_dmac_init(struct net_device *ndev) >>> +static void ravb_emac_init(struct net_device *ndev) >>> { >>> struct ravb_private *priv = netdev_priv(ndev); >>> const struct ravb_drv_data *info = priv->info; >>> - int error; >>> >>> - /* Set CONFIG mode */ >>> - error = ravb_config(ndev); >>> - if (error) >>> - return error; >>> - >>> - error = ravb_ring_init(ndev, RAVB_BE); >>> - if (error) >>> - return error; >>> - error = ravb_ring_init(ndev, RAVB_NC); >>> - if (error) { >>> - ravb_ring_free(ndev, RAVB_BE); >>> - return error; >>> - } >>> + info->ravb_ops->emac_init(ndev); >>> +} >> >> The whole ravb_emac_init() now consists only of a single method call? >> Why do we need it at all? > > OK will assign info->emac_init with ravb_emac_init, so GbEthernet just need to > fill emac_init function. I will remove the function "ravb_emac_init_ex". Will the EMAC init methods differ so much as to we should provide 2 separate implementations? [...] >>> +static void ravb_dmac_init_ex(struct net_device *ndev) >> >> Please no _ex suffixes -- reminds me of Windoze too much. :-) > > OK. Will change it to ravb_device_init Ugh! Why not leave it named ravb_dmac_init()? > Regards, > Biju > >> >>> +{ >>> + struct ravb_private *priv = netdev_priv(ndev); >>> + const struct ravb_drv_data *info = priv->info; >>> >>> /* Set AVB RX */ >>> ravb_write(ndev, >>> @@ -530,6 +519,33 @@ static int ravb_dmac_init(struct net_device *ndev) >>> ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2); >>> /* Frame transmitted, timestamp FIFO updated */ >>> ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC); >>> +} >>> + >>> +static int ravb_dmac_init(struct net_device *ndev) { >>> + struct ravb_private *priv = netdev_priv(ndev); >>> + const struct ravb_drv_data *info = priv->info; >>> + int error; >>> + >>> + /* Set CONFIG mode */ >>> + error = ravb_config(ndev); >>> + if (error) >>> + return error; >>> + >>> + error = ravb_ring_init(ndev, RAVB_BE); >>> + if (error) >>> + return error; >>> + error = ravb_ring_init(ndev, RAVB_NC); >>> + if (error) { >>> + ravb_ring_free(ndev, RAVB_BE); >>> + return error; >>> + } >>> + >>> + /* Descriptor format */ >>> + ravb_ring_format(ndev, RAVB_BE); >>> + ravb_ring_format(ndev, RAVB_NC); >>> + >>> + info->ravb_ops->dmac_init(ndev); >>> >>> /* Setting the control will start the AVB-DMAC process. */ >>> ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION); @@ -2018,6 >>> +2034,8 @@ static const struct ravb_ops ravb_gen3_ops = { >>> .ring_free = ravb_ring_free_rx, >>> .ring_format = ravb_ring_format_rx, >>> .alloc_rx_desc = ravb_alloc_rx_desc, >>> + .emac_init = ravb_emac_init_ex, >>> + .dmac_init = ravb_dmac_init_ex, >> >> Hmm, why not also gen2?! The question remained unreplied?... :-/ MBR, Sergey
Hi Sergei, > Subject: Re: [PATCH net-next 12/18] ravb: Factorise {emac,dmac} init > function > > On 8/20/21 6:42 PM, Biju Das wrote: > > [...] > >>> The R-Car AVB module has Magic packet detection, multiple irq's and > >>> timestamp enable features which is not present on RZ/G2L Gigabit > >> ^ are > > > > OK. Will fix this in next patch set. > > > >> > >>> Ethernet module. Factorise emac and dmac initialization function to > >>> support the later SoC. > >>> > >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > >>> --- > >>> drivers/net/ethernet/renesas/ravb.h | 2 + > >>> drivers/net/ethernet/renesas/ravb_main.c | 58 > >>> ++++++++++++++++-------- > >>> 2 files changed, 40 insertions(+), 20 deletions(-) > >>> > >>> diff --git a/drivers/net/ethernet/renesas/ravb.h > >>> b/drivers/net/ethernet/renesas/ravb.h > >>> index d82bfa6e57c1..4d5910dcda86 100644 > >>> --- a/drivers/net/ethernet/renesas/ravb.h > >>> +++ b/drivers/net/ethernet/renesas/ravb.h > >>> @@ -992,6 +992,8 @@ struct ravb_ops { > >>> void (*ring_free)(struct net_device *ndev, int q); > >>> void (*ring_format)(struct net_device *ndev, int q); > >>> bool (*alloc_rx_desc)(struct net_device *ndev, int q); > >>> + void (*emac_init)(struct net_device *ndev); > >>> + void (*dmac_init)(struct net_device *ndev); > >>> }; > >>> > >>> struct ravb_drv_data { > >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c > >>> b/drivers/net/ethernet/renesas/ravb_main.c > >>> index 3d0f6598b936..e200114376e4 100644 > >>> --- a/drivers/net/ethernet/renesas/ravb_main.c > >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c > >>> @@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device > >>> *ndev, int q) } > >>> > >>> /* E-MAC init function */ > >>> -static void ravb_emac_init(struct net_device *ndev) > >>> +static void ravb_emac_init_ex(struct net_device *ndev) > >>> { > >>> /* Receive frame limit set register */ > >>> ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, > >>> RFLR); @@ -480,30 +480,19 @@ static void ravb_emac_init(struct > >> net_device *ndev) > >>> ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, > >>> ECSIPR); } > >>> > >>> -/* Device init function for Ethernet AVB */ > >> > >> Grr, this comment seems oudated... > > > > OK. > > Just don't move the comment. :-) > > >>> -static int ravb_dmac_init(struct net_device *ndev) > >>> +static void ravb_emac_init(struct net_device *ndev) > >>> { > >>> struct ravb_private *priv = netdev_priv(ndev); > >>> const struct ravb_drv_data *info = priv->info; > >>> - int error; > >>> > >>> - /* Set CONFIG mode */ > >>> - error = ravb_config(ndev); > >>> - if (error) > >>> - return error; > >>> - > >>> - error = ravb_ring_init(ndev, RAVB_BE); > >>> - if (error) > >>> - return error; > >>> - error = ravb_ring_init(ndev, RAVB_NC); > >>> - if (error) { > >>> - ravb_ring_free(ndev, RAVB_BE); > >>> - return error; > >>> - } > >>> + info->ravb_ops->emac_init(ndev); > >>> +} > >> > >> The whole ravb_emac_init() now consists only of a single method > call? > >> Why do we need it at all? > > > > OK will assign info->emac_init with ravb_emac_init, so GbEthernet just > > need to fill emac_init function. I will remove the function > "ravb_emac_init_ex". > > Will the EMAC init methods differ so much as to we should provide 2 > separate implementations? > > [...] > >>> +static void ravb_dmac_init_ex(struct net_device *ndev) > >> > >> Please no _ex suffixes -- reminds me of Windoze too much. :-) > > > > OK. Will change it to ravb_device_init > > Ugh! Why not leave it named ravb_dmac_init()? Please see [1] below and also planning to send another 10 small incremental patches, So that it is clear to every one. > > > Regards, > > Biju > > > >> > >>> +{ > >>> + struct ravb_private *priv = netdev_priv(ndev); > >>> + const struct ravb_drv_data *info = priv->info; > >>> > >>> /* Set AVB RX */ > >>> ravb_write(ndev, > >>> @@ -530,6 +519,33 @@ static int ravb_dmac_init(struct net_device > *ndev) > >>> ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2); > >>> /* Frame transmitted, timestamp FIFO updated */ > >>> ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC); > >>> +} > >>> + > >>> +static int ravb_dmac_init(struct net_device *ndev) { > >>> + struct ravb_private *priv = netdev_priv(ndev); > >>> + const struct ravb_drv_data *info = priv->info; > >>> + int error; > >>> + > >>> + /* Set CONFIG mode */ > >>> + error = ravb_config(ndev); > >>> + if (error) > >>> + return error; > >>> + > >>> + error = ravb_ring_init(ndev, RAVB_BE); > >>> + if (error) > >>> + return error; > >>> + error = ravb_ring_init(ndev, RAVB_NC); > >>> + if (error) { > >>> + ravb_ring_free(ndev, RAVB_BE); > >>> + return error; > >>> + } > >>> + > >>> + /* Descriptor format */ > >>> + ravb_ring_format(ndev, RAVB_BE); > >>> + ravb_ring_format(ndev, RAVB_NC); > >>> + > >>> + info->ravb_ops->dmac_init(ndev); > >>> > >>> /* Setting the control will start the AVB-DMAC process. */ > >>> ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION); @@ -2018,6 > >>> +2034,8 @@ static const struct ravb_ops ravb_gen3_ops = { > >>> .ring_free = ravb_ring_free_rx, > >>> .ring_format = ravb_ring_format_rx, > >>> .alloc_rx_desc = ravb_alloc_rx_desc, > >>> + .emac_init = ravb_emac_init_ex, > >>> + .dmac_init = ravb_dmac_init_ex, > >> > >> Hmm, why not also gen2?! > > The question remained unreplied?... :-/ ravb_ops for gen3 and gen2 same. But RZ/G2L have different function pointers[1]. As agreed on next patchset, we are avoiding indirection and it will be hw_ifo. In that case there will be 1 entry in gen3 and 1 entry in gen2. Hope this clears Your doubts. [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210722141351.13668-18-biju.das.jz@bp.renesas.com/ Cheers, Biju
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h index d82bfa6e57c1..4d5910dcda86 100644 --- a/drivers/net/ethernet/renesas/ravb.h +++ b/drivers/net/ethernet/renesas/ravb.h @@ -992,6 +992,8 @@ struct ravb_ops { void (*ring_free)(struct net_device *ndev, int q); void (*ring_format)(struct net_device *ndev, int q); bool (*alloc_rx_desc)(struct net_device *ndev, int q); + void (*emac_init)(struct net_device *ndev); + void (*dmac_init)(struct net_device *ndev); }; struct ravb_drv_data { diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index 3d0f6598b936..e200114376e4 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device *ndev, int q) } /* E-MAC init function */ -static void ravb_emac_init(struct net_device *ndev) +static void ravb_emac_init_ex(struct net_device *ndev) { /* Receive frame limit set register */ ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR); @@ -480,30 +480,19 @@ static void ravb_emac_init(struct net_device *ndev) ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, ECSIPR); } -/* Device init function for Ethernet AVB */ -static int ravb_dmac_init(struct net_device *ndev) +static void ravb_emac_init(struct net_device *ndev) { struct ravb_private *priv = netdev_priv(ndev); const struct ravb_drv_data *info = priv->info; - int error; - /* Set CONFIG mode */ - error = ravb_config(ndev); - if (error) - return error; - - error = ravb_ring_init(ndev, RAVB_BE); - if (error) - return error; - error = ravb_ring_init(ndev, RAVB_NC); - if (error) { - ravb_ring_free(ndev, RAVB_BE); - return error; - } + info->ravb_ops->emac_init(ndev); +} - /* Descriptor format */ - ravb_ring_format(ndev, RAVB_BE); - ravb_ring_format(ndev, RAVB_NC); +/* Device init function for Ethernet AVB */ +static void ravb_dmac_init_ex(struct net_device *ndev) +{ + struct ravb_private *priv = netdev_priv(ndev); + const struct ravb_drv_data *info = priv->info; /* Set AVB RX */ ravb_write(ndev, @@ -530,6 +519,33 @@ static int ravb_dmac_init(struct net_device *ndev) ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2); /* Frame transmitted, timestamp FIFO updated */ ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC); +} + +static int ravb_dmac_init(struct net_device *ndev) +{ + struct ravb_private *priv = netdev_priv(ndev); + const struct ravb_drv_data *info = priv->info; + int error; + + /* Set CONFIG mode */ + error = ravb_config(ndev); + if (error) + return error; + + error = ravb_ring_init(ndev, RAVB_BE); + if (error) + return error; + error = ravb_ring_init(ndev, RAVB_NC); + if (error) { + ravb_ring_free(ndev, RAVB_BE); + return error; + } + + /* Descriptor format */ + ravb_ring_format(ndev, RAVB_BE); + ravb_ring_format(ndev, RAVB_NC); + + info->ravb_ops->dmac_init(ndev); /* Setting the control will start the AVB-DMAC process. */ ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION); @@ -2018,6 +2034,8 @@ static const struct ravb_ops ravb_gen3_ops = { .ring_free = ravb_ring_free_rx, .ring_format = ravb_ring_format_rx, .alloc_rx_desc = ravb_alloc_rx_desc, + .emac_init = ravb_emac_init_ex, + .dmac_init = ravb_dmac_init_ex, }; static const struct ravb_drv_data ravb_gen3_data = {