| Message ID | 20210802102654.5996-7-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add Gigabit Ethernet driver support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 2 maintainers not CCed: rikard.falkeborn@gmail.com yangyingliang@huawei.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 42 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Mon, Aug 02, 2021 at 11:26:52AM +0100, Biju Das wrote: > On R-Car the checksum calculation on RX frames is done by the E-MAC > module, whereas on RZ/G2L it is done by the TOE. > > TOE calculates the checksum of received frames from E-MAC and outputs it to > DMAC. TOE also calculates the checksum of transmission frames from DMAC and > outputs it E-MAC. > > Add net_features and net_hw_features to struct ravb_hw_info, to support > subsequent SoCs without any code changes in the ravb_probe function. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On 8/2/21 1:26 PM, Biju Das wrote: > On R-Car the checksum calculation on RX frames is done by the E-MAC > module, whereas on RZ/G2L it is done by the TOE. > > TOE calculates the checksum of received frames from E-MAC and outputs it to > DMAC. TOE also calculates the checksum of transmission frames from DMAC and > outputs it E-MAC. > > Add net_features and net_hw_features to struct ravb_hw_info, to support > subsequent SoCs without any code changes in the ravb_probe function. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> [...] > diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h > index b765b2b7d9e9..3df813b2e253 100644 > --- a/drivers/net/ethernet/renesas/ravb.h > +++ b/drivers/net/ethernet/renesas/ravb.h > @@ -991,6 +991,8 @@ enum ravb_chip_id { > struct ravb_hw_info { > const char (*gstrings_stats)[ETH_GSTRING_LEN]; > size_t gstrings_size; > + netdev_features_t net_hw_features; > + netdev_features_t net_features; Do we really need both of these here? It seems like the 'feartures' mirrors the enabled features? > enum ravb_chip_id chip_id; > int num_gstat_queue; > int num_tx_desc; > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index 7a69668cb512..2ac962b5b8fb 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c [...] > @@ -2077,14 +2081,14 @@ static int ravb_probe(struct platform_device *pdev) > if (!ndev) > return -ENOMEM; > > - ndev->features = NETIF_F_RXCSUM; > - ndev->hw_features = NETIF_F_RXCSUM; > + info = of_device_get_match_data(&pdev->dev); > + > + ndev->features = info->net_features; > + ndev->hw_features = info->net_hw_features; What value you plan to set her for GbEth, NETIF_F_HW_CSUM? [...] MBR, Sergei
Hi Sergei, Thanks for the feedback. > Subject: Re: [PATCH net-next v2 6/8] ravb: Add net_features and > net_hw_features to struct ravb_hw_info > > On 8/2/21 1:26 PM, Biju Das wrote: > > > On R-Car the checksum calculation on RX frames is done by the E-MAC > > module, whereas on RZ/G2L it is done by the TOE. > > > > TOE calculates the checksum of received frames from E-MAC and outputs > > it to DMAC. TOE also calculates the checksum of transmission frames > > from DMAC and outputs it E-MAC. > > > > Add net_features and net_hw_features to struct ravb_hw_info, to > > support subsequent SoCs without any code changes in the ravb_probe > function. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > [...] > > diff --git a/drivers/net/ethernet/renesas/ravb.h > > b/drivers/net/ethernet/renesas/ravb.h > > index b765b2b7d9e9..3df813b2e253 100644 > > --- a/drivers/net/ethernet/renesas/ravb.h > > +++ b/drivers/net/ethernet/renesas/ravb.h > > @@ -991,6 +991,8 @@ enum ravb_chip_id { struct ravb_hw_info { > > const char (*gstrings_stats)[ETH_GSTRING_LEN]; > > size_t gstrings_size; > > + netdev_features_t net_hw_features; > > + netdev_features_t net_features; > > Do we really need both of these here? R-Car has only Rx Checksum on E-Mac, where as Geth supports Rx Check Sum on E-Mac or Rx/Tx CheckSum on TOE. So there is a hw difference. Please let me know what is the best way to handle this? >It seems like the 'feartures' > mirrors the enabled features? Can you please explain this little bit? > > > enum ravb_chip_id chip_id; > > int num_gstat_queue; > > int num_tx_desc; > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c > > b/drivers/net/ethernet/renesas/ravb_main.c > > index 7a69668cb512..2ac962b5b8fb 100644 > > --- a/drivers/net/ethernet/renesas/ravb_main.c > > +++ b/drivers/net/ethernet/renesas/ravb_main.c > [...] > > @@ -2077,14 +2081,14 @@ static int ravb_probe(struct platform_device > *pdev) > > if (!ndev) > > return -ENOMEM; > > > > - ndev->features = NETIF_F_RXCSUM; > > - ndev->hw_features = NETIF_F_RXCSUM; > > + info = of_device_get_match_data(&pdev->dev); > > + > > + ndev->features = info->net_features; > > + ndev->hw_features = info->net_hw_features; > > What value you plan to set her for GbEth, NETIF_F_HW_CSUM? Yes, That is the plan. .net_hw_features = (NETIF_F_HW_CSUM | NETIF_F_RXCSUM), Regards, Biju > > [...] > > MBR, Sergei
Hello! On 8/5/21 10:18 PM, Biju Das wrote: [...] >>> On R-Car the checksum calculation on RX frames is done by the E-MAC >>> module, whereas on RZ/G2L it is done by the TOE. >>> >>> TOE calculates the checksum of received frames from E-MAC and outputs >>> it to DMAC. TOE also calculates the checksum of transmission frames >>> from DMAC and outputs it E-MAC. >>> >>> Add net_features and net_hw_features to struct ravb_hw_info, to >>> support subsequent SoCs without any code changes in the ravb_probe >> function. >>> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >> >> [...] >>> diff --git a/drivers/net/ethernet/renesas/ravb.h >>> b/drivers/net/ethernet/renesas/ravb.h >>> index b765b2b7d9e9..3df813b2e253 100644 >>> --- a/drivers/net/ethernet/renesas/ravb.h >>> +++ b/drivers/net/ethernet/renesas/ravb.h >>> @@ -991,6 +991,8 @@ enum ravb_chip_id { struct ravb_hw_info { >>> const char (*gstrings_stats)[ETH_GSTRING_LEN]; >>> size_t gstrings_size; >>> + netdev_features_t net_hw_features; >>> + netdev_features_t net_features; >> >> Do we really need both of these here? > > R-Car has only Rx Checksum on E-Mac, where as Geth supports Rx Check Sum on E-Mac or Rx/Tx CheckSum on TOE. > So there is a hw difference. Please let me know what is the best way to handle this? I meant that we could go with only one field of the net_features... Alternatively, we could use our own feature bits... >> It seems like the 'feartures' >> mirrors the enabled features? > > Can you please explain this little bit? Looks like I was wrong. :-) [...] > Regards, > Biju [...] MBR, Sergei
On 8/2/21 1:26 PM, Biju Das wrote: > On R-Car the checksum calculation on RX frames is done by the E-MAC > module, whereas on RZ/G2L it is done by the TOE. > > TOE calculates the checksum of received frames from E-MAC and outputs it to > DMAC. TOE also calculates the checksum of transmission frames from DMAC and > outputs it E-MAC. > > Add net_features and net_hw_features to struct ravb_hw_info, to support > subsequent SoCs without any code changes in the ravb_probe function. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com> [...] MBR, Sergei
Hi Sergei, Thanks for the feedback. > Subject: Re: [PATCH net-next v2 6/8] ravb: Add net_features and > net_hw_features to struct ravb_hw_info > > Hello! > > On 8/5/21 10:18 PM, Biju Das wrote: > > [...] > >>> On R-Car the checksum calculation on RX frames is done by the E-MAC > >>> module, whereas on RZ/G2L it is done by the TOE. > >>> > >>> TOE calculates the checksum of received frames from E-MAC and > >>> outputs it to DMAC. TOE also calculates the checksum of transmission > >>> frames from DMAC and outputs it E-MAC. > >>> > >>> Add net_features and net_hw_features to struct ravb_hw_info, to > >>> support subsequent SoCs without any code changes in the ravb_probe > >> function. > >>> > >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > >> > >> [...] > >>> diff --git a/drivers/net/ethernet/renesas/ravb.h > >>> b/drivers/net/ethernet/renesas/ravb.h > >>> index b765b2b7d9e9..3df813b2e253 100644 > >>> --- a/drivers/net/ethernet/renesas/ravb.h > >>> +++ b/drivers/net/ethernet/renesas/ravb.h > >>> @@ -991,6 +991,8 @@ enum ravb_chip_id { struct ravb_hw_info { > >>> const char (*gstrings_stats)[ETH_GSTRING_LEN]; > >>> size_t gstrings_size; > >>> + netdev_features_t net_hw_features; > >>> + netdev_features_t net_features; > >> > >> Do we really need both of these here? > > > > R-Car has only Rx Checksum on E-Mac, where as Geth supports Rx Check Sum > on E-Mac or Rx/Tx CheckSum on TOE. > > So there is a hw difference. Please let me know what is the best way to > handle this? > > I meant that we could go with only one field of the net_features... > Alternatively, we could use our own feature bits... Basically are you suggesting some thing like unsigned hwcsum_tx_rx: 1; Then, if (hwcsum_tx_rx) { ndev->hw_features = NETIF_F_HW_CSUM | NETIF_F_RXCSUM; } else { ndev->features = NETIF_F_RXCSUM; ndev->hw_features = NETIF_F_RXCSUM; } Cheers, Biju
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h index b765b2b7d9e9..3df813b2e253 100644 --- a/drivers/net/ethernet/renesas/ravb.h +++ b/drivers/net/ethernet/renesas/ravb.h @@ -991,6 +991,8 @@ enum ravb_chip_id { struct ravb_hw_info { const char (*gstrings_stats)[ETH_GSTRING_LEN]; size_t gstrings_size; + netdev_features_t net_hw_features; + netdev_features_t net_features; enum ravb_chip_id chip_id; int num_gstat_queue; int num_tx_desc; diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index 7a69668cb512..2ac962b5b8fb 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -1932,6 +1932,8 @@ static int ravb_mdio_release(struct ravb_private *priv) static const struct ravb_hw_info ravb_gen3_hw_info = { .gstrings_stats = ravb_gstrings_stats, .gstrings_size = sizeof(ravb_gstrings_stats), + .net_hw_features = NETIF_F_RXCSUM, + .net_features = NETIF_F_RXCSUM, .chip_id = RCAR_GEN3, .num_gstat_queue = NUM_RX_QUEUE, .num_tx_desc = NUM_TX_DESC_GEN3, @@ -1942,6 +1944,8 @@ static const struct ravb_hw_info ravb_gen3_hw_info = { static const struct ravb_hw_info ravb_gen2_hw_info = { .gstrings_stats = ravb_gstrings_stats, .gstrings_size = sizeof(ravb_gstrings_stats), + .net_hw_features = NETIF_F_RXCSUM, + .net_features = NETIF_F_RXCSUM, .chip_id = RCAR_GEN2, .num_gstat_queue = NUM_RX_QUEUE, .num_tx_desc = NUM_TX_DESC_GEN2, @@ -2077,14 +2081,14 @@ static int ravb_probe(struct platform_device *pdev) if (!ndev) return -ENOMEM; - ndev->features = NETIF_F_RXCSUM; - ndev->hw_features = NETIF_F_RXCSUM; + info = of_device_get_match_data(&pdev->dev); + + ndev->features = info->net_features; + ndev->hw_features = info->net_hw_features; pm_runtime_enable(&pdev->dev); pm_runtime_get_sync(&pdev->dev); - info = of_device_get_match_data(&pdev->dev); - if (info->chip_id == RCAR_GEN3) irq = platform_get_irq_byname(pdev, "ch22"); else