All of lore.kernel.org
 help / color / mirror / Atom feed
From: Biju Das <biju.das.jz@bp.renesas.com>
To: Biju Das <biju.das.jz@bp.renesas.com>,
	Sergey Shtylyov <s.shtylyov@omp.ru>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Sergei Shtylyov <sergei.shtylyov@gmail.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Adam Ford <aford173@gmail.com>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>,
	Chris Paterson <Chris.Paterson2@renesas.com>,
	Biju Das <biju.das@bp.renesas.com>
Subject: RE: [RFC/PATCH 06/18] ravb: Add multi_tsrq to struct ravb_hw_info
Date: Sun, 26 Sep 2021 13:54:56 +0000	[thread overview]
Message-ID: <OS0PR01MB592237144A680FEC1E7C013586A69@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <OS0PR01MB5922135B7F17FDE3F4C6091A86A49@OS0PR01MB5922.jpnprd01.prod.outlook.com>

Hi Sergei,

> Subject: RE: [RFC/PATCH 06/18] ravb: Add multi_tsrq to struct ravb_hw_info
> 
> Hi Sergei,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [RFC/PATCH 06/18] ravb: Add multi_tsrq to struct
> > ravb_hw_info
> >
> > On 9/23/21 5:08 PM, Biju Das wrote:
> >
> > > R-Car AVB-DMAC has 4 Transmit start Request queues, whereas RZ/G2L
> > > has only 1 Transmit start Request queue(Best Effort)
> > >
> > > Add a multi_tsrq hw feature bit to struct ravb_hw_info to enable
> > > this only for R-Car. This will allow us to add single TSRQ support
> > > for RZ/G2L.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > >  drivers/net/ethernet/renesas/ravb.h      |  1 +
> > >  drivers/net/ethernet/renesas/ravb_main.c | 12 ++++++++++--
> > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > > b/drivers/net/ethernet/renesas/ravb.h
> > > index bb92469d770e..c043ee555be4 100644
> > > --- a/drivers/net/ethernet/renesas/ravb.h
> > > +++ b/drivers/net/ethernet/renesas/ravb.h
> > > @@ -1006,6 +1006,7 @@ struct ravb_hw_info {
> > >  	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple
> > irqs */
> > >  	unsigned no_gptp:1;		/* AVB-DMAC does not support gPTP
> > feature */
> > >  	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in
> > config mode */
> > > +	unsigned multi_tsrq:1;		/* AVB-DMAC has MULTI TSRQ */
> >
> >    Maybe 'single_tx_q' instead?
> 
> Since it is called transmit start request queue, it is better to be named
> as single_tsrq to match with hardware manual and I will update the comment
> with "GbEthernet DMAC has single TSRQ"
> Please let me know are you ok with it. Other wise I would like to use
> existing name.

On the next revision as you proposed for [1],
I will use a u32 tsrq, instead of bit, there by we can avoid a check.

https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210923140813.13541-12-biju.das.jz@bp.renesas.com/
> 
> >
> > >  };
> > >
> > >  struct ravb_private {
> > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > > b/drivers/net/ethernet/renesas/ravb_main.c
> > > index 8663d83507a0..d37d73f6d984 100644
> > > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > > @@ -776,11 +776,17 @@ 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);
> > > +	if (info->multi_tsrq)
> > > +		error = ravb_wait(ndev, TCCR,
> > > +				  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 |
> > TCCR_TSRQ3, 0);
> > > +	else
> > > +		error = ravb_wait(ndev, TCCR, TCCR_TSRQ0, 0);
> >
> >    Aren't the TSRQ1/2/3 bits reserved on RZ/G2L? If so, this new flag
> > adds a little value, I think... unless you plan to use this flag
> > further in the series?
> 
> It will be confusing for RZ/G2L users. HW manual does not describes
> TSRQ1/2/3 and we are writing undocumented registers which is reserved.
> 
> Tomorrow it can happen that this reserved bits(90% it will not happen)
> will be used for describing something else.
> 
> It is unsafe to use reserved bits. Are you agreeing with this?

As per the above discussion, we can replace the above check as you proposed for [1]

error = ravb_wait(ndev, TCCR, info->tsrq, 0);

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210923140813.13541-12-biju.das.jz@bp.renesas.com/

regards,
Biju

  reply	other threads:[~2021-09-26 13:55 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-23 14:07 [RFC/PATCH 00/18] Add Gigabit Ethernet driver support Biju Das
2021-09-23 14:07 ` [RFC/PATCH 01/18] ravb: Rename "ravb_set_features_rx_csum" function to "ravb_set_features_rcar" Biju Das
2021-09-27 19:54   ` Sergey Shtylyov
2021-09-23 14:07 ` [RFC/PATCH 02/18] ravb: Rename the variables "no_ptp_cfg_active" and "ptp_cfg_active" Biju Das
2021-09-23 16:07   ` Sergey Shtylyov
2021-09-23 16:35     ` Biju Das
2021-09-23 17:57       ` Sergey Shtylyov
2021-09-23 18:20         ` Biju Das
2021-09-26 13:34           ` Biju Das
2021-09-23 14:07 ` [RFC/PATCH 03/18] ravb: Initialize GbEthernet dmac Biju Das
2021-09-23 17:41   ` Sergey Shtylyov
2021-09-23 18:42     ` Biju Das
2021-09-23 19:07   ` Sergey Shtylyov
2021-09-23 19:22     ` Biju Das
2021-09-23 19:29       ` Biju Das
2021-09-26 13:38         ` Biju Das
2021-09-23 14:07 ` [RFC/PATCH 04/18] ravb: Enable aligned_tx and tx_counters for RZ/G2L Biju Das
2021-09-23 18:05   ` Sergey Shtylyov
2021-09-23 18:10     ` Sergey Shtylyov
2021-09-23 18:13     ` Biju Das
2021-09-26 13:40       ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 05/18] ravb: Exclude gPTP feature support " Biju Das
2021-09-23 19:00   ` Sergey Shtylyov
2021-09-23 19:13     ` Biju Das
2021-09-23 19:41       ` Sergey Shtylyov
2021-09-23 19:45         ` Biju Das
2021-09-26 13:48           ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 06/18] ravb: Add multi_tsrq to struct ravb_hw_info Biju Das
2021-09-23 20:19   ` Sergey Shtylyov
2021-09-24  6:19     ` Biju Das
2021-09-26 13:54       ` Biju Das [this message]
2021-09-23 14:08 ` [RFC/PATCH 07/18] ravb: Add magic_pkt " Biju Das
2021-09-23 20:42   ` Sergey Shtylyov
2021-09-24  6:24     ` Biju Das
2021-09-26 13:56       ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 08/18] ravb: Add mii_rgmii_selection " Biju Das
2021-09-24 19:49   ` Sergey Shtylyov
2021-09-25  6:23     ` Biju Das
2021-09-26 15:49       ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 09/18] ravb: Add half_duplex " Biju Das
2021-09-24 20:07   ` Sergey Shtylyov
2021-09-25  6:37     ` Biju Das
2021-09-26 15:51     ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 10/18] ravb: Initialize GbEthernet E-MAC Biju Das
2021-09-24 20:44   ` Sergey Shtylyov
2021-09-25  6:38     ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 11/18] ravb: Add rx_2k_buffers to struct ravb_hw_info Biju Das
2021-09-24 19:35   ` Sergey Shtylyov
2021-09-26 15:48     ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 12/18] ravb: Add timestamp " Biju Das
2021-09-25 20:52   ` Sergey Shtylyov
2021-09-26  6:34     ` Biju Das
2021-09-26 16:52       ` Biju Das
2021-09-26 20:45       ` Sergey Shtylyov
2021-09-26 20:48         ` Sergei Shtylyov
2021-09-27  6:10           ` Biju Das
2021-09-27  6:04         ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 13/18] ravb: Add rx_ring_free function support for GbEthernet Biju Das
2021-09-27 19:28   ` Sergey Shtylyov
2021-09-28  9:24     ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 14/18] ravb: Add rx_ring_format function " Biju Das
2021-09-27 20:32   ` Sergey Shtylyov
2021-09-29  7:49     ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 15/18] ravb: Add rx_alloc helper " Biju Das
2021-09-23 14:08 ` [RFC/PATCH 16/18] ravb: Add Packet receive function for Gigabit Ethernet Biju Das
2021-10-01 17:27   ` Sergey Shtylyov
2021-10-04 14:56     ` Biju Das
2021-09-23 14:08 ` [RFC/PATCH 17/18] ravb: Add carrier_counters to struct ravb_hw_info Biju Das
2021-09-28 20:50   ` Sergey Shtylyov
2021-09-23 14:08 ` [RFC/PATCH 18/18] ravb: Add set_feature support for RZ/G2L Biju Das
2021-09-30 20:39   ` Sergey Shtylyov
2021-10-01  6:53     ` Biju Das
2021-10-01  9:13       ` Sergey Shtylyov
2021-10-01  9:26         ` Biju Das
2021-10-01  8:22     ` Biju Das
2021-09-23 15:11 ` [RFC/PATCH 00/18] Add Gigabit Ethernet driver support Sergey Shtylyov
2021-09-23 15:13   ` Sergey Shtylyov
2021-09-23 15:37   ` Jakub Kicinski
2021-09-23 17:43     ` Sergey Shtylyov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=OS0PR01MB592237144A680FEC1E7C013586A69@OS0PR01MB5922.jpnprd01.prod.outlook.com \
    --to=biju.das.jz@bp.renesas.com \
    --cc=Chris.Paterson2@renesas.com \
    --cc=aford173@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=biju.das@bp.renesas.com \
    --cc=davem@davemloft.net \
    --cc=geert+renesas@glider.be \
    --cc=kuba@kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=s.shtylyov@omp.ru \
    --cc=sergei.shtylyov@gmail.com \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.