| Message ID | 20210818190800.20191-1-biju.das.jz@bp.renesas.com |
|---|---|
| Headers | show |
| Series | Add Gigabit Ethernet driver support | expand |
Hello: This series was applied to netdev/net-next.git (refs/heads/master): On Wed, 18 Aug 2021 20:07:51 +0100 you wrote: > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are > similar to the R-Car Ethernet AVB IP. > > The Gigabit Ethernet IP consists of Ethernet controller (E-MAC), Internal > TCP/IP Offload Engine (TOE) and Dedicated Direct memory access controller > (DMAC). > > [...] Here is the summary with links: - [net-next,v3,1/9] ravb: Use unsigned int for num_tx_desc variable in struct ravb_private https://git.kernel.org/netdev/net-next/c/cb537b241725 - [net-next,v3,2/9] ravb: Add struct ravb_hw_info to driver data https://git.kernel.org/netdev/net-next/c/ebb091461a9e - [net-next,v3,3/9] ravb: Add aligned_tx to struct ravb_hw_info https://git.kernel.org/netdev/net-next/c/68ca3c923213 - [net-next,v3,4/9] ravb: Add max_rx_len to struct ravb_hw_info https://git.kernel.org/netdev/net-next/c/cb01c672c2a7 - [net-next,v3,5/9] ravb: Add stats_len to struct ravb_hw_info https://git.kernel.org/netdev/net-next/c/25154301fc2b - [net-next,v3,6/9] ravb: Add gstrings_stats and gstrings_size to struct ravb_hw_info https://git.kernel.org/netdev/net-next/c/896a818e0e1d - [net-next,v3,7/9] ravb: Add net_features and net_hw_features to struct ravb_hw_info https://git.kernel.org/netdev/net-next/c/8912ed25daf6 - [net-next,v3,8/9] ravb: Add internal delay hw feature to struct ravb_hw_info https://git.kernel.org/netdev/net-next/c/8bc4caa0abaf - [net-next,v3,9/9] ravb: Add tx_counters to struct ravb_hw_info https://git.kernel.org/netdev/net-next/c/0b81d6731167 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On 8/19/21 2:10 PM, patchwork-bot+netdevbpf@kernel.org wrote: [...] > This series was applied to netdev/net-next.git (refs/heads/master): > > On Wed, 18 Aug 2021 20:07:51 +0100 you wrote: >> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are >> similar to the R-Car Ethernet AVB IP. >> >> The Gigabit Ethernet IP consists of Ethernet controller (E-MAC), Internal >> TCP/IP Offload Engine (TOE) and Dedicated Direct memory access controller >> (DMAC). >> >> [...] > > Here is the summary with links: > - [net-next,v3,1/9] ravb: Use unsigned int for num_tx_desc variable in struct ravb_private > https://git.kernel.org/netdev/net-next/c/cb537b241725 > - [net-next,v3,2/9] ravb: Add struct ravb_hw_info to driver data > https://git.kernel.org/netdev/net-next/c/ebb091461a9e > - [net-next,v3,3/9] ravb: Add aligned_tx to struct ravb_hw_info > https://git.kernel.org/netdev/net-next/c/68ca3c923213 > - [net-next,v3,4/9] ravb: Add max_rx_len to struct ravb_hw_info > https://git.kernel.org/netdev/net-next/c/cb01c672c2a7 > - [net-next,v3,5/9] ravb: Add stats_len to struct ravb_hw_info > https://git.kernel.org/netdev/net-next/c/25154301fc2b > - [net-next,v3,6/9] ravb: Add gstrings_stats and gstrings_size to struct ravb_hw_info > https://git.kernel.org/netdev/net-next/c/896a818e0e1d > - [net-next,v3,7/9] ravb: Add net_features and net_hw_features to struct ravb_hw_info > https://git.kernel.org/netdev/net-next/c/8912ed25daf6 > - [net-next,v3,8/9] ravb: Add internal delay hw feature to struct ravb_hw_info > https://git.kernel.org/netdev/net-next/c/8bc4caa0abaf > - [net-next,v3,9/9] ravb: Add tx_counters to struct ravb_hw_info > https://git.kernel.org/netdev/net-next/c/0b81d6731167 > > You are awesome, thank you! Are we in such a haste? I was just going to review these patches today... MBR, Sergey
On 8/18/21 10:07 PM, Biju Das wrote: > The number of TX descriptors per packet is an unsigned value and > the variable for holding this information should be unsigned. > > This patch replaces the data type of num_tx_desc variable in struct > ravb_private from 'int' to 'unsigned int'. > This patch also updates the data type of local variables to unsigned int, > where the local variables are evaluated using num_tx_desc. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] MBR, Sergey
> Are we in such a haste? I was just going to review these patches > today... I guess the thinking is, fixup patches can always be applied after the fact. But i agree, i really liked the 3 day wait time for patches to be merged, it gave a reasonable amount of time for reviews, without slowing down development work. The current 1 day or less does seem counter productive, i expect there are less reviews happening as a result, lower quality code, more bugs... Andrew
On 8/18/21 10:07 PM, Biju Das wrote: > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are > similar to the R-Car Ethernet AVB IP. With a few changes in the driver we > can support both IPs. > > This patch adds the struct ravb_hw_info to hold hw features, driver data > and function pointers to support both the IPs. It also replaces the driver > data chip type with struct ravb_hw_info by moving chip type to it. > > 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> > --- > v2->v3: > * Retained Rb tag from Andrew, since there is no functionality change > apart from just splitting the patch into 2. Also updated the commit > description. > v2: > * Incorporated Andrew and Sergei's review comments for making it smaller patch > and provided detailed description. [...] > static inline u32 ravb_read(struct net_device *ndev, enum ravb_reg reg) > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index 94eb9136752d..b6554e5e13af 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c [...] > @@ -2113,7 +2122,7 @@ static int ravb_probe(struct platform_device *pdev) > } > } > > - priv->chip_id = chip_id; > + priv->chip_id = info->chip_id; Do we still need priv->chip_id? > priv->clk = devm_clk_get(&pdev->dev, NULL); > if (IS_ERR(priv->clk)) { [...] Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] MBR, Sergey
On 8/18/21 10:08 PM, Biju Das wrote: > The register for retrieving TX counters is present only on R-Car Gen3 > and RZ/G2L; it is not present on R-Car Gen2. > > Add the tx_counters hw feature bit to struct ravb_hw_info, to enable this > feature specifically for R-Car Gen3 now and later extend it to RZ/G2L. > > 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> > --- > v2->v3: > * Retained Rb tag from Andrew, since change is just renaming the variable > and comment update. > v2: > * Incorporated Andrew and Sergei's review comments for making it smaller patch > and provided detailed description. [...] Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] MBR, Sergey
Hi Sergei, > Subject: Re: [PATCH net-next v3 2/9] ravb: Add struct ravb_hw_info to > driver data > > On 8/18/21 10:07 PM, Biju Das wrote: > > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC > > are similar to the R-Car Ethernet AVB IP. With a few changes in the > > driver we can support both IPs. > > > > This patch adds the struct ravb_hw_info to hold hw features, driver > > data and function pointers to support both the IPs. It also replaces > > the driver data chip type with struct ravb_hw_info by moving chip type > to it. > > > > 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> > > --- > > v2->v3: > > * Retained Rb tag from Andrew, since there is no functionality change > > apart from just splitting the patch into 2. Also updated the commit > > description. > > v2: > > * Incorporated Andrew and Sergei's review comments for making it > smaller patch > > and provided detailed description. > > [...] > > static inline u32 ravb_read(struct net_device *ndev, enum ravb_reg > > reg) diff --git a/drivers/net/ethernet/renesas/ravb_main.c > > b/drivers/net/ethernet/renesas/ravb_main.c > > index 94eb9136752d..b6554e5e13af 100644 > > --- a/drivers/net/ethernet/renesas/ravb_main.c > > +++ b/drivers/net/ethernet/renesas/ravb_main.c > [...] > > @@ -2113,7 +2122,7 @@ static int ravb_probe(struct platform_device > *pdev) > > } > > } > > > > - priv->chip_id = chip_id; > > + priv->chip_id = info->chip_id; > > Do we still need priv->chip_id? The patch currently merged is preparation patch, subsequent patch will replace all the chip_id in ravb_main with hardware features and driver features. After that both priv->chip_id and info_chipid is not required for ravb_main.c However ptp driver[1] still uses it, by adding a feature bit we can replace that as well. So going forward, there won't be any priv->chip_id or info->chip_id. Does it makes sense? [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/renesas/ravb_ptp.c?h=v5.14-rc6#n200 Regards, Biju > > > priv->clk = devm_clk_get(&pdev->dev, NULL); > > if (IS_ERR(priv->clk)) { > [...] > > Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> > > [...] > > MBR, Sergey
On 8/19/21 8:33 PM, Biju Das wrote: [...] >>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC >>> are similar to the R-Car Ethernet AVB IP. With a few changes in the >>> driver we can support both IPs. >>> >>> This patch adds the struct ravb_hw_info to hold hw features, driver >>> data and function pointers to support both the IPs. It also replaces >>> the driver data chip type with struct ravb_hw_info by moving chip type >> to it. >>> >>> 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> [...] >>> reg) diff --git a/drivers/net/ethernet/renesas/ravb_main.c >>> b/drivers/net/ethernet/renesas/ravb_main.c >>> index 94eb9136752d..b6554e5e13af 100644 >>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c >> [...] >>> @@ -2113,7 +2122,7 @@ static int ravb_probe(struct platform_device >> *pdev) >>> } >>> } >>> >>> - priv->chip_id = chip_id; >>> + priv->chip_id = info->chip_id; >> >> Do we still need priv->chip_id? > > The patch currently merged is preparation patch, subsequent patch will replace > all the chip_id in ravb_main with hardware features and driver features. > After that both priv->chip_id and info_chipid is not required for ravb_main.c > > However ptp driver[1] still uses it, by adding a feature bit we can replace > that as well. So going forward, there won't be any priv->chip_id or info->chip_id. > > Does it makes sense? > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/renesas/ravb_ptp.c?h=v5.14-rc6#n200 OK, seems sane, go ahead. :-) > Regards, > Biju MBR, Sergey
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are similar to the R-Car Ethernet AVB IP. The Gigabit Ethernet IP consists of Ethernet controller (E-MAC), Internal TCP/IP Offload Engine (TOE) and Dedicated Direct memory access controller (DMAC). With a few changes in the driver we can support both IPs. Currently a runtime decision based on the chip type is used to distinguish the HW differences between the SoC families. This patch series is in preparation for supporting the RZ/G2L SoC by replacing driver data chip type with struct ravb_hw_info by moving chip type to it and also adding gstrings_stats, gstrings_size, net_hw_features, net_features, aligned_tx, stats_len, max_rx_len variables to it. This patch also adds the feature bit for {RX, TX} clock internal delays and TX counters HW features found on R-Car Gen3 to struct ravb_hw_info. This patch series is based on net-next. v2->v3: * Removed num_gstat_queue variable from struct ravb_hw_info. * started using unsigned int for num_tx_desc variable in struct ravb_private * split the patch 'Add struct ravb_hw_info to driver data' into two * Renamed skb_sz to max_rx_len and tx_drop_cntrs to tx_counters and also updated the comments. v1->v2: * Replaced driver data chip type with struct ravb_hw_info * Added gstrings_stats, gstrings_size, net_hw_features, net_features, num_gstat_queue, num_tx_desc, stats_len, skb_sz to struct ravb_hw_info * Added internal_delay and tx_drop_cntrs hw feature bit to struct ravb_hw_info RFC->V1 * Incorporated feedback from Andrew, Sergei, Geert and Prabhakar * https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=515525 Biju Das (9): ravb: Use unsigned int for num_tx_desc variable in struct ravb_private ravb: Add struct ravb_hw_info to driver data ravb: Add aligned_tx to struct ravb_hw_info ravb: Add max_rx_len to struct ravb_hw_info ravb: Add stats_len to struct ravb_hw_info ravb: Add gstrings_stats and gstrings_size to struct ravb_hw_info ravb: Add net_features and net_hw_features to struct ravb_hw_info ravb: Add internal delay hw feature to struct ravb_hw_info ravb: Add tx_counters to struct ravb_hw_info drivers/net/ethernet/renesas/ravb.h | 19 +++- drivers/net/ethernet/renesas/ravb_main.c | 112 ++++++++++++++--------- 2 files changed, 89 insertions(+), 42 deletions(-)