diff mbox series

[RFC/PATCH,12/18] ravb: Add timestamp to struct ravb_hw_info

Message ID 20210923140813.13541-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

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count fail Series longer than 15 patches
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 success CCed 10 of 10 maintainers
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 warning WARNING: line length of 84 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Biju Das Sept. 23, 2021, 2:08 p.m. UTC
R-Car AVB-DMAC supports timestamp feature.
Add a timestamp hw feature bit to struct ravb_hw_info
to add this feature only for R-Car.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  2 +
 drivers/net/ethernet/renesas/ravb_main.c | 68 +++++++++++++++---------
 2 files changed, 45 insertions(+), 25 deletions(-)

Comments

Sergey Shtylyov Sept. 25, 2021, 8:52 p.m. UTC | #1
On 9/23/21 5:08 PM, Biju Das wrote:

> R-Car AVB-DMAC supports timestamp feature.
> Add a timestamp hw feature bit to struct ravb_hw_info
> to add this feature only for R-Car.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  2 +
>  drivers/net/ethernet/renesas/ravb_main.c | 68 +++++++++++++++---------
>  2 files changed, 45 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index ab4909244276..2505de5d4a28 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -1034,6 +1034,7 @@ struct ravb_hw_info {
>  	unsigned mii_rgmii_selection:1;	/* E-MAC supports mii/rgmii selection */
>  	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
>  	unsigned rx_2k_buffers:1;	/* AVB-DMAC has Max 2K buf size on RX */
> +	unsigned timestamp:1;		/* AVB-DMAC has timestamp */

   Isn't this a matter of the gPTP support as well, i.e. no separate flag needed?

[...]
> @@ -1089,6 +1090,7 @@ struct ravb_private {
>  	unsigned int num_tx_desc;	/* TX descriptors per packet */
>  
>  	int duplex;
> +	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];

   Strange place to declare this...

>  
>  	const struct ravb_hw_info *info;
>  	struct reset_control *rstc;
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 9c0d35f4b221..2c375002ebcb 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -949,11 +949,14 @@ static bool ravb_queue_interrupt(struct net_device *ndev, int q)
>  
>  static bool ravb_timestamp_interrupt(struct net_device *ndev)
>  {
> +	struct ravb_private *priv = netdev_priv(ndev);
> +	const struct ravb_hw_info *info = priv->info;
>  	u32 tis = ravb_read(ndev, TIS);
>  
>  	if (tis & TIS_TFUF) {
>  		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
> -		ravb_get_tx_tstamp(ndev);
> +		if (info->timestamp)
> +			ravb_get_tx_tstamp(ndev);

   Shouldn't we just disable TIS.TFUF permanently instead for the non-gPTP case?

[...]

MBR, Sergey
Biju Das Sept. 26, 2021, 6:34 a.m. UTC | #2
HI Sergei,

Thanks for the feedback.

> -----Original Message-----
> Subject: Re: [RFC/PATCH 12/18] ravb: Add timestamp to struct ravb_hw_info
> 
> On 9/23/21 5:08 PM, Biju Das wrote:
> 
> > R-Car AVB-DMAC supports timestamp feature.
> > Add a timestamp hw feature bit to struct ravb_hw_info to add this
> > feature only for R-Car.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |  2 +
> >  drivers/net/ethernet/renesas/ravb_main.c | 68
> > +++++++++++++++---------
> >  2 files changed, 45 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index ab4909244276..2505de5d4a28 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -1034,6 +1034,7 @@ struct ravb_hw_info {
> >  	unsigned mii_rgmii_selection:1;	/* E-MAC supports mii/rgmii
> selection */
> >  	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
> >  	unsigned rx_2k_buffers:1;	/* AVB-DMAC has Max 2K buf size on RX
> */
> > +	unsigned timestamp:1;		/* AVB-DMAC has timestamp */
> 
>    Isn't this a matter of the gPTP support as well, i.e. no separate flag
> needed?

Agreed. Previously it is suggested to use timestamp. I will change it to as part of gPTP support cases.

> 
> [...]
> > @@ -1089,6 +1090,7 @@ struct ravb_private {
> >  	unsigned int num_tx_desc;	/* TX descriptors per packet */
> >
> >  	int duplex;
> > +	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];
> 
>    Strange place to declare this...

Agreed. This has to be on later patch. Will move it.

> 
> >
> >  	const struct ravb_hw_info *info;
> >  	struct reset_control *rstc;
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 9c0d35f4b221..2c375002ebcb 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -949,11 +949,14 @@ static bool ravb_queue_interrupt(struct
> > net_device *ndev, int q)
> >
> >  static bool ravb_timestamp_interrupt(struct net_device *ndev)  {
> > +	struct ravb_private *priv = netdev_priv(ndev);
> > +	const struct ravb_hw_info *info = priv->info;
> >  	u32 tis = ravb_read(ndev, TIS);
> >
> >  	if (tis & TIS_TFUF) {
> >  		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
> > -		ravb_get_tx_tstamp(ndev);
> > +		if (info->timestamp)
> > +			ravb_get_tx_tstamp(ndev);
> 
>    Shouldn't we just disable TIS.TFUF permanently instead for the non-gPTP
> case?

Good catch. As ravb_dmac_init_rgeth(will be renamed to "ravb_dmac_init_gbeth") is not enabling this interrupt as it is not documented in RZ/G2L hardware manual.
So this function never gets called for non-gPTP case.

I will remove this check.

Regards,
Biju


> 
> [...]
> 
> MBR, Sergey
Biju Das Sept. 26, 2021, 4:52 p.m. UTC | #3
Hi Sergei,

> Subject: RE: [RFC/PATCH 12/18] ravb: Add timestamp to struct ravb_hw_info
> 
> HI Sergei,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > Subject: Re: [RFC/PATCH 12/18] ravb: Add timestamp to struct
> > ravb_hw_info
> >
> > On 9/23/21 5:08 PM, Biju Das wrote:
> >
> > > R-Car AVB-DMAC supports timestamp feature.
> > > Add a timestamp hw feature bit to struct ravb_hw_info to add this
> > > feature only for R-Car.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > >  drivers/net/ethernet/renesas/ravb.h      |  2 +
> > >  drivers/net/ethernet/renesas/ravb_main.c | 68
> > > +++++++++++++++---------
> > >  2 files changed, 45 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > > b/drivers/net/ethernet/renesas/ravb.h
> > > index ab4909244276..2505de5d4a28 100644
> > > --- a/drivers/net/ethernet/renesas/ravb.h
> > > +++ b/drivers/net/ethernet/renesas/ravb.h
> > > @@ -1034,6 +1034,7 @@ struct ravb_hw_info {
> > >  	unsigned mii_rgmii_selection:1;	/* E-MAC supports mii/rgmii
> > selection */
> > >  	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
> > >  	unsigned rx_2k_buffers:1;	/* AVB-DMAC has Max 2K buf size on RX
> > */
> > > +	unsigned timestamp:1;		/* AVB-DMAC has timestamp */
> >
> >    Isn't this a matter of the gPTP support as well, i.e. no separate
> > flag needed?
> 
> Agreed. Previously it is suggested to use timestamp. I will change it to
> as part of gPTP support cases.
> 
> >
> > [...]
> > > @@ -1089,6 +1090,7 @@ struct ravb_private {
> > >  	unsigned int num_tx_desc;	/* TX descriptors per packet */
> > >
> > >  	int duplex;
> > > +	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];
> >
> >    Strange place to declare this...
> 
> Agreed. This has to be on later patch. Will move it.
> 
> >
> > >
> > >  	const struct ravb_hw_info *info;
> > >  	struct reset_control *rstc;
> > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > > b/drivers/net/ethernet/renesas/ravb_main.c
> > > index 9c0d35f4b221..2c375002ebcb 100644
> > > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > > @@ -949,11 +949,14 @@ static bool ravb_queue_interrupt(struct
> > > net_device *ndev, int q)
> > >
> > >  static bool ravb_timestamp_interrupt(struct net_device *ndev)  {
> > > +	struct ravb_private *priv = netdev_priv(ndev);
> > > +	const struct ravb_hw_info *info = priv->info;
> > >  	u32 tis = ravb_read(ndev, TIS);
> > >
> > >  	if (tis & TIS_TFUF) {
> > >  		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
> > > -		ravb_get_tx_tstamp(ndev);
> > > +		if (info->timestamp)
> > > +			ravb_get_tx_tstamp(ndev);
> >
> >    Shouldn't we just disable TIS.TFUF permanently instead for the
> > non-gPTP case?
> 
> Good catch. As ravb_dmac_init_rgeth(will be renamed to
> "ravb_dmac_init_gbeth") is not enabling this interrupt as it is not
> documented in RZ/G2L hardware manual.
> So this function never gets called for non-gPTP case.
> 
> I will remove this check.


As discussed, I will drop this patch in the next version and contents of this patch will be added to gPTP support cases.

Regards,
Biju
Sergey Shtylyov Sept. 26, 2021, 8:45 p.m. UTC | #4
On 9/26/21 9:34 AM, Biju Das wrote:

>> -----Original Message-----
>> Subject: Re: [RFC/PATCH 12/18] ravb: Add timestamp to struct ravb_hw_info
>>
>> On 9/23/21 5:08 PM, Biju Das wrote:
>>
>>> R-Car AVB-DMAC supports timestamp feature.
>>> Add a timestamp hw feature bit to struct ravb_hw_info to add this
>>> feature only for R-Car.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> ---
>>>  drivers/net/ethernet/renesas/ravb.h      |  2 +
>>>  drivers/net/ethernet/renesas/ravb_main.c | 68
>>> +++++++++++++++---------
>>>  2 files changed, 45 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>> b/drivers/net/ethernet/renesas/ravb.h
>>> index ab4909244276..2505de5d4a28 100644
>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>> @@ -1034,6 +1034,7 @@ struct ravb_hw_info {
>>>  	unsigned mii_rgmii_selection:1;	/* E-MAC supports mii/rgmii
>> selection */
>>>  	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
>>>  	unsigned rx_2k_buffers:1;	/* AVB-DMAC has Max 2K buf size on RX
>> */
>>> +	unsigned timestamp:1;		/* AVB-DMAC has timestamp */
>>
>>    Isn't this a matter of the gPTP support as well, i.e. no separate flag
>> needed?
> 
> Agreed. Previously it is suggested to use timestamp. I will change it to as part of gPTP support cases.

   TIA. :-)

>>
>> [...]
>>> @@ -1089,6 +1090,7 @@ struct ravb_private {
>>>  	unsigned int num_tx_desc;	/* TX descriptors per packet */
>>>
>>>  	int duplex;
>>> +	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];
>>
>>    Strange place to declare this...
> 
> Agreed. This has to be on later patch. Will move it.

   I only meant that these fields should go together with rx_ring[]. Apparently
we have a case of wrong patch ordering here (as this patch needs this field declared)...

>>>  	const struct ravb_hw_info *info;
>>>  	struct reset_control *rstc;
>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>>> b/drivers/net/ethernet/renesas/ravb_main.c
>>> index 9c0d35f4b221..2c375002ebcb 100644
>>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>>> @@ -949,11 +949,14 @@ static bool ravb_queue_interrupt(struct
>>> net_device *ndev, int q)
>>>
>>>  static bool ravb_timestamp_interrupt(struct net_device *ndev)  {
>>> +	struct ravb_private *priv = netdev_priv(ndev);
>>> +	const struct ravb_hw_info *info = priv->info;
>>>  	u32 tis = ravb_read(ndev, TIS);
>>>
>>>  	if (tis & TIS_TFUF) {
>>>  		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
>>> -		ravb_get_tx_tstamp(ndev);
>>> +		if (info->timestamp)
>>> +			ravb_get_tx_tstamp(ndev);
>>
>>    Shouldn't we just disable TIS.TFUF permanently instead for the non-gPTP
>> case?
> 
> Good catch. As ravb_dmac_init_rgeth(will be renamed to "ravb_dmac_init_gbeth") is not enabling this interrupt as it is not documented in RZ/G2L hardware manual.
> So this function never gets called for non-gPTP case.
> 
> I will remove this check.

   TIA!

> Regards,
> Biju

MBR, Sergey
Sergey Shtylyov Sept. 26, 2021, 8:48 p.m. UTC | #5
On 9/26/21 11:45 PM, Sergey Shtylyov wrote:

>>> -----Original Message-----
>>> Subject: Re: [RFC/PATCH 12/18] ravb: Add timestamp to struct ravb_hw_info
>>>
>>> On 9/23/21 5:08 PM, Biju Das wrote:
>>>
>>>> R-Car AVB-DMAC supports timestamp feature.
>>>> Add a timestamp hw feature bit to struct ravb_hw_info to add this
>>>> feature only for R-Car.
>>>>
>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>> ---
>>>>  drivers/net/ethernet/renesas/ravb.h      |  2 +
>>>>  drivers/net/ethernet/renesas/ravb_main.c | 68
>>>> +++++++++++++++---------
>>>>  2 files changed, 45 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>>> b/drivers/net/ethernet/renesas/ravb.h
>>>> index ab4909244276..2505de5d4a28 100644
>>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>>> @@ -1034,6 +1034,7 @@ struct ravb_hw_info {
>>>>  	unsigned mii_rgmii_selection:1;	/* E-MAC supports mii/rgmii
>>> selection */
>>>>  	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
>>>>  	unsigned rx_2k_buffers:1;	/* AVB-DMAC has Max 2K buf size on RX
>>> */
>>>> +	unsigned timestamp:1;		/* AVB-DMAC has timestamp */
>>>
>>>    Isn't this a matter of the gPTP support as well, i.e. no separate flag
>>> needed?
>>
>> Agreed. Previously it is suggested to use timestamp. I will change it to as part of gPTP support cases.
> 
>    TIA. :-)
> 
>>>
>>> [...]
>>>> @@ -1089,6 +1090,7 @@ struct ravb_private {
>>>>  	unsigned int num_tx_desc;	/* TX descriptors per packet */
>>>>
>>>>  	int duplex;
>>>> +	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];
>>>
>>>    Strange place to declare this...
>>
>> Agreed. This has to be on later patch. Will move it.
> 
>    I only meant that these fields should go together with rx_ring[]. Apparently

   Sorry, this field.

> we have a case of wrong patch ordering here (as this patch needs this field declared)...
[...]

>> Regards,
>> Biju

MBR, Sergey
Biju Das Sept. 27, 2021, 6:04 a.m. UTC | #6
Hi Sergei,

> Subject: Re: [RFC/PATCH 12/18] ravb: Add timestamp to struct ravb_hw_info
> 
> On 9/26/21 9:34 AM, Biju Das wrote:
> 
> >> -----Original Message-----
> >> Subject: Re: [RFC/PATCH 12/18] ravb: Add timestamp to struct
> >> ravb_hw_info
> >>
> >> On 9/23/21 5:08 PM, Biju Das wrote:
> >>
> >>> R-Car AVB-DMAC supports timestamp feature.
> >>> Add a timestamp hw feature bit to struct ravb_hw_info to add this
> >>> feature only for R-Car.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> ---
> >>>  drivers/net/ethernet/renesas/ravb.h      |  2 +
> >>>  drivers/net/ethernet/renesas/ravb_main.c | 68
> >>> +++++++++++++++---------
> >>>  2 files changed, 45 insertions(+), 25 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>> b/drivers/net/ethernet/renesas/ravb.h
> >>> index ab4909244276..2505de5d4a28 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>> @@ -1034,6 +1034,7 @@ struct ravb_hw_info {
> >>>  	unsigned mii_rgmii_selection:1;	/* E-MAC supports mii/rgmii
> >> selection */
> >>>  	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
> >>>  	unsigned rx_2k_buffers:1;	/* AVB-DMAC has Max 2K buf size on RX
> >> */
> >>> +	unsigned timestamp:1;		/* AVB-DMAC has timestamp */
> >>
> >>    Isn't this a matter of the gPTP support as well, i.e. no separate
> >> flag needed?
> >
> > Agreed. Previously it is suggested to use timestamp. I will change it to
> as part of gPTP support cases.
> 
>    TIA. :-)
> 
> >>
> >> [...]
> >>> @@ -1089,6 +1090,7 @@ struct ravb_private {
> >>>  	unsigned int num_tx_desc;	/* TX descriptors per packet */
> >>>
> >>>  	int duplex;
> >>> +	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];
> >>
> >>    Strange place to declare this...
> >
> > Agreed. This has to be on later patch. Will move it.
> 
>    I only meant that these fields should go together with rx_ring[].
> Apparently we have a case of wrong patch ordering here (as this patch
> needs this field declared)...

Exactly that is the case, it is moved to later patch (https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210923140813.13541-14-biju.das.jz@bp.renesas.com/) which is the first user of
This variable.

Regards,
Biju
> 
> >>>  	const struct ravb_hw_info *info;
> >>>  	struct reset_control *rstc;
> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> >>> b/drivers/net/ethernet/renesas/ravb_main.c
> >>> index 9c0d35f4b221..2c375002ebcb 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb_main.c
> >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> >>> @@ -949,11 +949,14 @@ static bool ravb_queue_interrupt(struct
> >>> net_device *ndev, int q)
> >>>
> >>>  static bool ravb_timestamp_interrupt(struct net_device *ndev)  {
> >>> +	struct ravb_private *priv = netdev_priv(ndev);
> >>> +	const struct ravb_hw_info *info = priv->info;
> >>>  	u32 tis = ravb_read(ndev, TIS);
> >>>
> >>>  	if (tis & TIS_TFUF) {
> >>>  		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
> >>> -		ravb_get_tx_tstamp(ndev);
> >>> +		if (info->timestamp)
> >>> +			ravb_get_tx_tstamp(ndev);
> >>
> >>    Shouldn't we just disable TIS.TFUF permanently instead for the
> >> non-gPTP case?
> >
> > Good catch. As ravb_dmac_init_rgeth(will be renamed to
> "ravb_dmac_init_gbeth") is not enabling this interrupt as it is not
> documented in RZ/G2L hardware manual.
> > So this function never gets called for non-gPTP case.
> >
> > I will remove this check.
> 
>    TIA!
> 
> > Regards,
> > Biju
> 
> MBR, Sergey
Biju Das Sept. 27, 2021, 6:10 a.m. UTC | #7
Hi Sergei,

> Subject: Re: [RFC/PATCH 12/18] ravb: Add timestamp to struct ravb_hw_info
> 
> On 9/26/21 11:45 PM, Sergey Shtylyov wrote:
> 
> >>> -----Original Message-----
> >>> Subject: Re: [RFC/PATCH 12/18] ravb: Add timestamp to struct
> >>> ravb_hw_info
> >>>
> >>> On 9/23/21 5:08 PM, Biju Das wrote:
> >>>
> >>>> R-Car AVB-DMAC supports timestamp feature.
> >>>> Add a timestamp hw feature bit to struct ravb_hw_info to add this
> >>>> feature only for R-Car.
> >>>>
> >>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>>> ---
> >>>>  drivers/net/ethernet/renesas/ravb.h      |  2 +
> >>>>  drivers/net/ethernet/renesas/ravb_main.c | 68
> >>>> +++++++++++++++---------
> >>>>  2 files changed, 45 insertions(+), 25 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>>> b/drivers/net/ethernet/renesas/ravb.h
> >>>> index ab4909244276..2505de5d4a28 100644
> >>>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>>> @@ -1034,6 +1034,7 @@ struct ravb_hw_info {
> >>>>  	unsigned mii_rgmii_selection:1;	/* E-MAC supports
> mii/rgmii
> >>> selection */
> >>>>  	unsigned half_duplex:1;		/* E-MAC supports half duplex
> mode */
> >>>>  	unsigned rx_2k_buffers:1;	/* AVB-DMAC has Max 2K buf size
> on RX
> >>> */
> >>>> +	unsigned timestamp:1;		/* AVB-DMAC has timestamp */
> >>>
> >>>    Isn't this a matter of the gPTP support as well, i.e. no separate
> >>> flag needed?
> >>
> >> Agreed. Previously it is suggested to use timestamp. I will change it
> to as part of gPTP support cases.
> >
> >    TIA. :-)
> >
> >>>
> >>> [...]
> >>>> @@ -1089,6 +1090,7 @@ struct ravb_private {
> >>>>  	unsigned int num_tx_desc;	/* TX descriptors per packet */
> >>>>
> >>>>  	int duplex;
> >>>> +	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];
> >>>
> >>>    Strange place to declare this...
> >>
> >> Agreed. This has to be on later patch. Will move it.
> >
> >    I only meant that these fields should go together with rx_ring[].
> > Apparently
> 
>    Sorry, this field.

It is a mistake from my side which ended up this variable in this patch.

Function pointers are introduced to avoid more checks and only rx functions pf RZ/G2L are using
this variable. 

It will be clear you to once you finish reviewing the remaining patches(patch 13 - patch 18).

Regards,
Biju

Regards,
Biju

> 
> > we have a case of wrong patch ordering here (as this patch needs this
> field declared)...
> [...]
> 
> >> Regards,
> >> Biju
> 
> MBR, Sergey
diff mbox series

Patch

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index ab4909244276..2505de5d4a28 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1034,6 +1034,7 @@  struct ravb_hw_info {
 	unsigned mii_rgmii_selection:1;	/* E-MAC supports mii/rgmii selection */
 	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
 	unsigned rx_2k_buffers:1;	/* AVB-DMAC has Max 2K buf size on RX */
+	unsigned timestamp:1;		/* AVB-DMAC has timestamp */
 };
 
 struct ravb_private {
@@ -1089,6 +1090,7 @@  struct ravb_private {
 	unsigned int num_tx_desc;	/* TX descriptors per packet */
 
 	int duplex;
+	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];
 
 	const struct ravb_hw_info *info;
 	struct reset_control *rstc;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 9c0d35f4b221..2c375002ebcb 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -949,11 +949,14 @@  static bool ravb_queue_interrupt(struct net_device *ndev, int q)
 
 static bool ravb_timestamp_interrupt(struct net_device *ndev)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	u32 tis = ravb_read(ndev, TIS);
 
 	if (tis & TIS_TFUF) {
 		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
-		ravb_get_tx_tstamp(ndev);
+		if (info->timestamp)
+			ravb_get_tx_tstamp(ndev);
 		return true;
 	}
 	return false;
@@ -1071,16 +1074,24 @@  static int ravb_poll(struct napi_struct *napi, int budget)
 	struct net_device *ndev = napi->dev;
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_hw_info *info = priv->info;
+	struct ravb_rx_desc *desc;
 	unsigned long flags;
 	int q = napi - priv->napi;
 	int mask = BIT(q);
 	int quota = budget;
+	unsigned int entry;
 
+	if (!info->timestamp) {
+		entry = priv->cur_rx[q] % priv->num_rx_ring[q];
+		desc = &priv->rgeth_rx_ring[q][entry];
+	}
 	/* Processing RX Descriptor Ring */
 	/* Clear RX interrupt */
 	ravb_write(ndev, ~(mask | RIS0_RESERVED), RIS0);
-	if (ravb_rx(ndev, &quota, q))
-		goto out;
+	if (info->timestamp || desc->die_dt != DT_FEMPTY) {
+		if (ravb_rx(ndev, &quota, q))
+			goto out;
+	}
 
 	/* Processing TX Descriptor Ring */
 	spin_lock_irqsave(&priv->lock, flags);
@@ -1689,6 +1700,7 @@  static void ravb_tx_timeout_work(struct work_struct *work)
 static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	unsigned int num_tx_desc = priv->num_tx_desc;
 	u16 q = skb_get_queue_mapping(skb);
 	struct ravb_tstamp_skb *ts_skb;
@@ -1765,28 +1777,30 @@  static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	desc->dptr = cpu_to_le32(dma_addr);
 
 	/* TX timestamp required */
-	if (q == RAVB_NC) {
-		ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC);
-		if (!ts_skb) {
-			if (num_tx_desc > 1) {
-				desc--;
-				dma_unmap_single(ndev->dev.parent, dma_addr,
-						 len, DMA_TO_DEVICE);
+	if (info->timestamp) {
+		if (q == RAVB_NC) {
+			ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC);
+			if (!ts_skb) {
+				if (num_tx_desc > 1) {
+					desc--;
+					dma_unmap_single(ndev->dev.parent, dma_addr,
+							 len, DMA_TO_DEVICE);
+				}
+				goto unmap;
 			}
-			goto unmap;
+			ts_skb->skb = skb_get(skb);
+			ts_skb->tag = priv->ts_skb_tag++;
+			priv->ts_skb_tag &= 0x3ff;
+			list_add_tail(&ts_skb->list, &priv->ts_skb_list);
+
+			/* TAG and timestamp required flag */
+			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+			desc->tagh_tsr = (ts_skb->tag >> 4) | TX_TSR;
+			desc->ds_tagl |= cpu_to_le16(ts_skb->tag << 12);
 		}
-		ts_skb->skb = skb_get(skb);
-		ts_skb->tag = priv->ts_skb_tag++;
-		priv->ts_skb_tag &= 0x3ff;
-		list_add_tail(&ts_skb->list, &priv->ts_skb_list);
 
-		/* TAG and timestamp required flag */
-		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
-		desc->tagh_tsr = (ts_skb->tag >> 4) | TX_TSR;
-		desc->ds_tagl |= cpu_to_le16(ts_skb->tag << 12);
+		skb_tx_timestamp(skb);
 	}
-
-	skb_tx_timestamp(skb);
 	/* Descriptor type must be set after all the above writes */
 	dma_wmb();
 	if (num_tx_desc > 1) {
@@ -1897,10 +1911,12 @@  static int ravb_close(struct net_device *ndev)
 			   "device will be stopped after h/w processes are done.\n");
 
 	/* Clear the timestamp list */
-	list_for_each_entry_safe(ts_skb, ts_skb2, &priv->ts_skb_list, list) {
-		list_del(&ts_skb->list);
-		kfree_skb(ts_skb->skb);
-		kfree(ts_skb);
+	if (info->timestamp) {
+		list_for_each_entry_safe(ts_skb, ts_skb2, &priv->ts_skb_list, list) {
+			list_del(&ts_skb->list);
+			kfree_skb(ts_skb->skb);
+			kfree(ts_skb);
+		}
 	}
 
 	/* PHY disconnect */
@@ -2165,6 +2181,7 @@  static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.multi_tsrq = 1,
 	.magic_pkt = 1,
 	.rx_2k_buffers = 1,
+	.timestamp = 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
@@ -2186,6 +2203,7 @@  static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.multi_tsrq = 1,
 	.magic_pkt = 1,
 	.rx_2k_buffers = 1,
+	.timestamp = 1,
 };
 
 static const struct ravb_hw_info rgeth_hw_info = {