netdev - Re: [PATCH net-next v2 2/3] net: phy: at803x: add fiber support

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 12 Jan 2022 20:49:49 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Robert Hancock <robert.hancock@...ian.com>
Cc:     "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>
Subject: Re: [PATCH net-next v2 2/3] net: phy: at803x: add fiber support

On Wed, Jan 12, 2022 at 12:42:00AM +0000, Robert Hancock wrote:
> On Wed, 2022-01-12 at 01:10 +0100, Andrew Lunn wrote:
> > >  #define AT803X_MODE_CFG_MASK			0x0F
> > > -#define AT803X_MODE_CFG_SGMII			0x01
> > > +#define AT803X_MODE_CFG_BASET_RGMII		0x00
> > > +#define AT803X_MODE_CFG_BASET_SGMII		0x01
> > > +#define AT803X_MODE_CFG_BX1000_RGMII_50		0x02
> > > +#define AT803X_MODE_CFG_BX1000_RGMII_75		0x03
> > > +#define AT803X_MODE_CFG_BX1000_CONV_50		0x04
> > > +#define AT803X_MODE_CFG_BX1000_CONV_75		0x05
> > > +#define AT803X_MODE_CFG_FX100_RGMII_50		0x06
> > > +#define AT803X_MODE_CFG_FX100_CONV_50		0x07
> > > +#define AT803X_MODE_CFG_RGMII_AUTO_MDET		0x0B
> > > +#define AT803X_MODE_CFG_FX100_RGMII_75		0x0E
> > > +#define AT803X_MODE_CFG_FX100_CONV_75		0x0F
> > 
> > Hi Robert
> > 
> > What do these _50, and _75 mean?
> 
> 50 or 75 ohm impedance. Can refer to page 82 of the datasheet at 
> https://www.digikey.ca/en/datasheets/qualcomm/qualcommar8031dsatherosrev10aug2011
>  - these names were chosen to match what it uses.

I know they are getting long, but maybe add OHM to the end?

> > >  #define AT803X_PSSR				0x11	/*PHY-
> > > Specific Status Register*/
> > >  #define AT803X_PSSR_MR_AN_COMPLETE		0x0200
> > > @@ -283,6 +295,8 @@ struct at803x_priv {
> > >  	u16 clk_25m_mask;
> > >  	u8 smarteee_lpi_tw_1g;
> > >  	u8 smarteee_lpi_tw_100m;
> > > +	bool is_fiber;
> > 
> > Is maybe is_100basefx a better name? It makes it clearer it represents
> > a link mode?
> 
> This is meant to indicate the chip is set for any fiber mode (100Base-FX or
> 1000Base-X).

O.K, then is_fibre is O.K.

I noticed code removing the link mode 1000BaseX in the case of
is_fibre && !is_100basex. Does 100BaseFX need removing for !is_fibre?

	Andrew