netdev - Re: [RFC 3/3] staging: octeon: convert to use phylink

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: <ZDgrGRImf0O0bP4r@lenoch>
Date:   Thu, 13 Apr 2023 18:17:29 +0200
From:   Ladislav Michl <oss-lists@...ops.cz>
To:     Dan Carpenter <error27@...il.com>
Cc:     linux-staging@...ts.linux.dev, netdev@...r.kernel.org,
        linux-mips@...r.kernel.org,
        Chris Packham <chris.packham@...iedtelesis.co.nz>
Subject: Re: [RFC 3/3] staging: octeon: convert to use phylink

On Thu, Apr 13, 2023 at 07:10:47PM +0300, Dan Carpenter wrote:
> On Thu, Apr 13, 2023 at 06:04:47PM +0200, Ladislav Michl wrote:
> > > > diff --git a/drivers/staging/octeon/ethernet-rgmii.c b/drivers/staging/octeon/ethernet-rgmii.c
> > > > index 0c4fac31540a..8c6eb0b87254 100644
> > > > --- a/drivers/staging/octeon/ethernet-rgmii.c
> > > > +++ b/drivers/staging/octeon/ethernet-rgmii.c
> > > > @@ -115,17 +115,8 @@ static void cvm_oct_rgmii_poll(struct net_device *dev)
> > > >  
> > > >  	cvm_oct_check_preamble_errors(dev);
> > > >  
> > > > -	if (likely(!status_change))
> > >                    ^
> > > Negated.
> > 
> > On purpose.
> > 
> > > > -		return;
> > > > -
> > > > -	/* Tell core. */
> > > > -	if (link_info.s.link_up) {
> > > > -		if (!netif_carrier_ok(dev))
> > > > -			netif_carrier_on(dev);
> > > > -	} else if (netif_carrier_ok(dev)) {
> > > > -		netif_carrier_off(dev);
> > > > -	}
> > > > -	cvm_oct_note_carrier(priv, link_info);
> > > > +	if (likely(status_change))
> > > 
> > > Originally a status_change was unlikely but now it is likely.
> > 
> > Yes, but originally it returned after condition and now it executes
> > phylink_mac_change. This is just to emulate current bahaviour.
> > Later mac interrupts should be used to drive link change.
> 
> I don't think you have seen the (minor) issue.  Originally it was
> likely that status_change was NOT set.  But now it is likely that is
> *IS* set.

Ahh, will fix that. Thank you,
	ladis