netdev - RE: [RFC/PATCH 05/18] ravb: Exclude gPTP feature support for RZ/G2L

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <OS0PR01MB5922FCB43284B938A4E12D1686A39@OS0PR01MB5922.jpnprd01.prod.outlook.com>
Date:   Thu, 23 Sep 2021 19:45:29 +0000
From:   Biju Das <biju.das.jz@...renesas.com>
To:     Sergey Shtylyov <s.shtylyov@....ru>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
CC:     Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>,
        Andrew Lunn <andrew@...n.ch>,
        Sergei Shtylyov <sergei.shtylyov@...il.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Adam Ford <aford173@...il.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>,
        Chris Paterson <Chris.Paterson2@...esas.com>,
        Biju Das <biju.das@...renesas.com>
Subject: RE: [RFC/PATCH 05/18] ravb: Exclude gPTP feature support for RZ/G2L

Hi Sergei,

> Subject: Re: [RFC/PATCH 05/18] ravb: Exclude gPTP feature support for
> RZ/G2L
> 
> On 9/23/21 10:13 PM, Biju Das wrote:
> 
> [...]
> >>> R-Car supports gPTP feature whereas RZ/G2L does not support it.
> >>> This patch excludes gtp feature support for RZ/G2L by enabling
> >>> no_gptp feature bit.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@...renesas.com>
> >>> ---
> >>>  drivers/net/ethernet/renesas/ravb_main.c | 46
> >>> ++++++++++++++----------
> >>>  1 file changed, 28 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> >>> b/drivers/net/ethernet/renesas/ravb_main.c
> >>> index d38fc33a8e93..8663d83507a0 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb_main.c
> >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> >> [...]
> >>> @@ -953,7 +954,7 @@ static irqreturn_t ravb_interrupt(int irq, void
> >> *dev_id)
> >>>  	}
> >>>
> >>>  	/* gPTP interrupt status summary */
> >>> -	if (iss & ISS_CGIS) {
> >>
> >>    Isn't this bit always 0 on RZ/G2L?
> >
> > This CGIM bit(BIT13) which is present on R-Car Gen3 is not present in
> > RZ/G2L. As per the HW manual
> > BIT13 is reserved bit and read is always 0.
> >
> >>
> >>> +	if (!info->no_gptp && (iss & ISS_CGIS)) {
> 
>    Then extending this check doesn't seem necessary?
> 
> >>>  		ravb_ptp_interrupt(ndev);
> >>>  		result = IRQ_HANDLED;
> >>>  	}
> [...]
> >>> @@ -2116,6 +2119,7 @@ static const struct ravb_hw_info rgeth_hw_info =
> {
> >>>  	.emac_init = ravb_rgeth_emac_init,
> >>>  	.aligned_tx = 1,
> >>>  	.tx_counters = 1,
> >>> +	.no_gptp = 1,
> >>
> >>    Mhm, I definitely don't like the way you "extend" the GbEthernet
> >> info structure. All the applicable flags should be set in the last
> >> patch of the series, not amidst of it.
> >
> > According to me, It is clearer with smaller patches like, what we have
> done with previous 2 patch sets for factorisation.
> > Please correct me, if any one have different opinion.
> 
>    I'm afraid you'd get a partly functioning device with the RZ/G2 info
> introduced amidst of the series and then the necessary flags/values added
> to it. This should definitely be avoided.

It is ok, It is understood, After replacing all  the place holders only we get full functionality.
That is the reason place holders added in first patch, so that we can fill each function at later stage
By smaller patcher. Same case for feature bits.

Regards,
Biju