netdev - Re: [PATCH RFC 2/3] net: phy: marvell10g: Add host interface speed configuration

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: <Y0/mbzaUItB1BOzg@shell.armlinux.org.uk>
Date:   Wed, 19 Oct 2022 12:58:39 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
Cc:     Marek Behún <kabel@...nel.org>, andrew@...n.ch,
        hkallweit1@...il.com, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH RFC 2/3] net: phy: marvell10g: Add host interface speed
 configuration

On Wed, Oct 19, 2022 at 12:48:39PM +0200, Marek Behún wrote:
> On Wed, 19 Oct 2022 17:50:51 +0900
> Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com> wrote:
> 
> > Add support for selecting host speed mode. For now, only support
> > 1000M bps.
> > 
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> > ---
> >  drivers/net/phy/marvell10g.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> > index 383a9c9f36e5..daf3242c6078 100644
> > --- a/drivers/net/phy/marvell10g.c
> > +++ b/drivers/net/phy/marvell10g.c
> > @@ -101,6 +101,10 @@ enum {
> >  	MV_AN_21X0_SERDES_CTRL2_AUTO_INIT_DIS	= BIT(13),
> >  	MV_AN_21X0_SERDES_CTRL2_RUN_INIT	= BIT(15),
> >  
> > +	MV_MOD_CONF		= 0xf000,
> > +	MV_MOD_CONF_SPEED_MASK	= 0x00c0,
> > +	MV_MOD_CONF_SPEED_1000	= BIT(7),
> > +
> 
> Where did you get these values from? My documentation says:
>   Mode Configuration
>   Device 31, Register 0xF000
>   Bits
>   7:6   Reserved  R/W  0x3  This must always be 11.

The closest is from the 88x3310 documentation that indicates these are
the default speed, which are used when the media side is down. There
is a specific sequence to update these.

However, as we seem to be talking about the 2110 here, that should be
reflected in these definitions.

Finally, using BIT() for definitions of a field which can be one of
four possible values is not acceptable. BIT() is for single bits
not for a multi-bit field which can take any possible value but just
the value we're representing there just happens to have a single bit
set.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!