| Message ID | 20211001150636.7500-8-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0b395f289451b4674c1db8949f0c441d7a2ff4fe |
| 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 | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 1 maintainers not CCed: s.shtylyov@omp.ru |
| 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, 43 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On 10/1/21 6:06 PM, Biju Das wrote: > R-Car AVB-DMAC has 4 Transmit start request queues, whereas > RZ/G2L has only 1 Transmit start request queue. The TCCR bits are called transmit start request (queue 0/1), not transmit start request queue 0/1. I think you've read too much value into them for what is just TX queue 0/1. > Add a tsrq variable to struct ravb_hw_info to handle this > difference. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > RFC->v1: > * Added tsrq variable instead of multi_tsrq feature bit. > --- > drivers/net/ethernet/renesas/ravb.h | 1 + > drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++-- > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h > index 9cd3a15743b4..c586070193ef 100644 > --- a/drivers/net/ethernet/renesas/ravb.h > +++ b/drivers/net/ethernet/renesas/ravb.h > @@ -997,6 +997,7 @@ struct ravb_hw_info { > netdev_features_t net_features; > int stats_len; > size_t max_rx_len; > + u32 tsrq; I'd call it 'tccr_value' instead. > unsigned aligned_tx: 1; > > /* hardware features */ > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index ac141a491ca2..4784832bd93c 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -790,11 +790,13 @@ static void ravb_rcv_snd_enable(struct net_device *ndev) > /* function for waiting dma process finished */ > static int ravb_stop_dma(struct net_device *ndev) > { > + struct ravb_private *priv = netdev_priv(ndev); > + const struct ravb_hw_info *info = priv->info; > int error; > > /* Wait for stopping the hardware TX process */ > - error = ravb_wait(ndev, TCCR, > - TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0); > + error = ravb_wait(ndev, TCCR, info->tsrq, 0); > + > if (error) > return error; > > @@ -2128,6 +2130,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = { > .net_features = NETIF_F_RXCSUM, > .stats_len = ARRAY_SIZE(ravb_gstrings_stats), > .max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1, > + .tsrq = TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, > .internal_delay = 1, > .tx_counters = 1, > .multi_irqs = 1, > @@ -2150,6 +2153,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = { > .net_features = NETIF_F_RXCSUM, > .stats_len = ARRAY_SIZE(ravb_gstrings_stats), > .max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1, > + .tsrq = TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, > .aligned_tx = 1, > .gptp = 1, > .nc_queue = 1, > @@ -2165,6 +2169,7 @@ static const struct ravb_hw_info gbeth_hw_info = { > .dmac_init = ravb_dmac_init_gbeth, > .emac_init = ravb_emac_init_gbeth, > .max_rx_len = GBETH_RX_BUFF_MAX + RAVB_ALIGN - 1, > + .tsrq = TCCR_TSRQ0, > .aligned_tx = 1, > .tx_counters = 1, > }; > [...] MBR, Sergey
On 10/4/21 9:00 PM, Sergey Shtylyov wrote: [...] > The TCCR bits are called transmit start request (queue 0/1), not transmit start request queue 0/1. > I think you've read too much value into them for what is just TX queue 0/1. > >> Add a tsrq variable to struct ravb_hw_info to handle this >> difference. >> >> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> >> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >> --- >> RFC->v1: >> * Added tsrq variable instead of multi_tsrq feature bit. >> --- >> drivers/net/ethernet/renesas/ravb.h | 1 + >> drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++-- >> 2 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h >> index 9cd3a15743b4..c586070193ef 100644 >> --- a/drivers/net/ethernet/renesas/ravb.h >> +++ b/drivers/net/ethernet/renesas/ravb.h >> @@ -997,6 +997,7 @@ struct ravb_hw_info { >> netdev_features_t net_features; >> int stats_len; >> size_t max_rx_len; >> + u32 tsrq; > > I'd call it 'tccr_value' instead. Or even better, 'tccr_mask'... [...] MBR, Sergey
> -----Original Message----- > From: Sergei Shtylyov <sergei.shtylyov@gmail.com> > Sent: 04 October 2021 19:37 > To: Sergey Shtylyov <s.shtylyov@omp.ru>; Biju Das > <biju.das.jz@bp.renesas.com>; David S. Miller <davem@davemloft.net>; Jakub > Kicinski <kuba@kernel.org> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>; Sergey Shtylyov > <s.shtylyov@omprussia.ru>; Adam Ford <aford173@gmail.com>; Andrew Lunn > <andrew@lunn.ch>; Yuusuke Ashizuka <ashiduka@fujitsu.com>; Yoshihiro > Shimoda <yoshihiro.shimoda.uh@renesas.com>; netdev@vger.kernel.org; linux- > renesas-soc@vger.kernel.org; Chris Paterson <Chris.Paterson2@renesas.com>; > Biju Das <biju.das@bp.renesas.com>; Prabhakar Mahadev Lad > <prabhakar.mahadev-lad.rj@bp.renesas.com> > Subject: Re: [PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info > > On 10/4/21 9:00 PM, Sergey Shtylyov wrote: > > [...] > > The TCCR bits are called transmit start request (queue 0/1), not > transmit start request queue 0/1. > > I think you've read too much value into them for what is just TX queue > 0/1. > > > >> Add a tsrq variable to struct ravb_hw_info to handle this difference. > >> > >> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > >> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > >> --- > >> RFC->v1: > >> * Added tsrq variable instead of multi_tsrq feature bit. > >> --- > >> drivers/net/ethernet/renesas/ravb.h | 1 + > >> drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++-- > >> 2 files changed, 8 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/net/ethernet/renesas/ravb.h > >> b/drivers/net/ethernet/renesas/ravb.h > >> index 9cd3a15743b4..c586070193ef 100644 > >> --- a/drivers/net/ethernet/renesas/ravb.h > >> +++ b/drivers/net/ethernet/renesas/ravb.h > >> @@ -997,6 +997,7 @@ struct ravb_hw_info { > >> netdev_features_t net_features; > >> int stats_len; > >> size_t max_rx_len; > >> + u32 tsrq; > > > > I'd call it 'tccr_value' instead. > > Or even better, 'tccr_mask'... We are not masking anything here right. tccr_value will be ok, as it implies real tccr register value. Regards, Biju > > [...] > > MBR, Sergey
On 10/4/21 9:47 PM, Biju Das wrote: [...] >>> The TCCR bits are called transmit start request (queue 0/1), not >> transmit start request queue 0/1. >>> I think you've read too much value into them for what is just TX queue >> 0/1. >>> >>>> Add a tsrq variable to struct ravb_hw_info to handle this difference. >>>> >>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> >>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >>>> --- >>>> RFC->v1: >>>> * Added tsrq variable instead of multi_tsrq feature bit. >>>> --- >>>> drivers/net/ethernet/renesas/ravb.h | 1 + >>>> drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++-- >>>> 2 files changed, 8 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/renesas/ravb.h >>>> b/drivers/net/ethernet/renesas/ravb.h >>>> index 9cd3a15743b4..c586070193ef 100644 >>>> --- a/drivers/net/ethernet/renesas/ravb.h >>>> +++ b/drivers/net/ethernet/renesas/ravb.h >>>> @@ -997,6 +997,7 @@ struct ravb_hw_info { >>>> netdev_features_t net_features; >>>> int stats_len; >>>> size_t max_rx_len; >>>> + u32 tsrq; >>> >>> I'd call it 'tccr_value' instead. >> >> Or even better, 'tccr_mask'... > > We are not masking anything here right. We do -- we pass the TCCR mask to ravb_wait(). [...] > Regards, > Biju MBR, Sergey
> -----Original Message----- > From: Sergey Shtylyov <s.shtylyov@omp.ru> > Sent: 04 October 2021 19:54 > To: Biju Das <biju.das.jz@bp.renesas.com>; Sergei Shtylyov > <sergei.shtylyov@gmail.com>; David S. Miller <davem@davemloft.net>; Jakub > Kicinski <kuba@kernel.org> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>; Sergey Shtylyov > <s.shtylyov@omprussia.ru>; Adam Ford <aford173@gmail.com>; Andrew Lunn > <andrew@lunn.ch>; Yuusuke Ashizuka <ashiduka@fujitsu.com>; Yoshihiro > Shimoda <yoshihiro.shimoda.uh@renesas.com>; netdev@vger.kernel.org; linux- > renesas-soc@vger.kernel.org; Chris Paterson <Chris.Paterson2@renesas.com>; > Biju Das <biju.das@bp.renesas.com>; Prabhakar Mahadev Lad > <prabhakar.mahadev-lad.rj@bp.renesas.com> > Subject: Re: [PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info > > On 10/4/21 9:47 PM, Biju Das wrote: > > [...] > >>> The TCCR bits are called transmit start request (queue 0/1), not > >> transmit start request queue 0/1. > >>> I think you've read too much value into them for what is just TX queue > >> 0/1. > >>> > >>>> Add a tsrq variable to struct ravb_hw_info to handle this difference. > >>>> > >>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > >>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > >>>> --- > >>>> RFC->v1: > >>>> * Added tsrq variable instead of multi_tsrq feature bit. > >>>> --- > >>>> drivers/net/ethernet/renesas/ravb.h | 1 + > >>>> drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++-- > >>>> 2 files changed, 8 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/drivers/net/ethernet/renesas/ravb.h > >>>> b/drivers/net/ethernet/renesas/ravb.h > >>>> index 9cd3a15743b4..c586070193ef 100644 > >>>> --- a/drivers/net/ethernet/renesas/ravb.h > >>>> +++ b/drivers/net/ethernet/renesas/ravb.h > >>>> @@ -997,6 +997,7 @@ struct ravb_hw_info { > >>>> netdev_features_t net_features; > >>>> int stats_len; > >>>> size_t max_rx_len; > >>>> + u32 tsrq; > >>> > >>> I'd call it 'tccr_value' instead. > >> > >> Or even better, 'tccr_mask'... > > > > We are not masking anything here right. > > We do -- we pass the TCCR mask to ravb_wait(). Agreed. will use "tccr_mask" in next RFC version. > > [...] > > > Regards, > > Biju > > MBR, Sergey
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h index 9cd3a15743b4..c586070193ef 100644 --- a/drivers/net/ethernet/renesas/ravb.h +++ b/drivers/net/ethernet/renesas/ravb.h @@ -997,6 +997,7 @@ struct ravb_hw_info { netdev_features_t net_features; int stats_len; size_t max_rx_len; + u32 tsrq; unsigned aligned_tx: 1; /* hardware features */ diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index ac141a491ca2..4784832bd93c 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -790,11 +790,13 @@ static void ravb_rcv_snd_enable(struct net_device *ndev) /* function for waiting dma process finished */ static int ravb_stop_dma(struct net_device *ndev) { + struct ravb_private *priv = netdev_priv(ndev); + const struct ravb_hw_info *info = priv->info; int error; /* Wait for stopping the hardware TX process */ - error = ravb_wait(ndev, TCCR, - TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0); + error = ravb_wait(ndev, TCCR, info->tsrq, 0); + if (error) return error; @@ -2128,6 +2130,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = { .net_features = NETIF_F_RXCSUM, .stats_len = ARRAY_SIZE(ravb_gstrings_stats), .max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1, + .tsrq = TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, .internal_delay = 1, .tx_counters = 1, .multi_irqs = 1, @@ -2150,6 +2153,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = { .net_features = NETIF_F_RXCSUM, .stats_len = ARRAY_SIZE(ravb_gstrings_stats), .max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1, + .tsrq = TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, .aligned_tx = 1, .gptp = 1, .nc_queue = 1, @@ -2165,6 +2169,7 @@ static const struct ravb_hw_info gbeth_hw_info = { .dmac_init = ravb_dmac_init_gbeth, .emac_init = ravb_emac_init_gbeth, .max_rx_len = GBETH_RX_BUFF_MAX + RAVB_ALIGN - 1, + .tsrq = TCCR_TSRQ0, .aligned_tx = 1, .tx_counters = 1, };