netdev - Re: [RFC PATCH 01/10] net: phy: Add driver for Loongson PHY

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: <9568c4ad-e10f-4b76-8766-ec621f788c40@lunn.ch>
Date: Mon, 17 Jul 2023 14:22:27 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Feiyang Chen <chris.chenfeiyang@...il.com>
Cc: Feiyang Chen <chenfeiyang@...ngson.cn>, hkallweit1@...il.com,
	peppe.cavallaro@...com, alexandre.torgue@...s.st.com,
	joabreu@...opsys.com, chenhuacai@...ngson.cn, linux@...linux.org.uk,
	dongbiao@...ngson.cn, loongson-kernel@...ts.loongnix.cn,
	netdev@...r.kernel.org, loongarch@...ts.linux.dev
Subject: Re: [RFC PATCH 01/10] net: phy: Add driver for Loongson PHY

> > > > > +#define PHY_ID_LS7A2000              0x00061ce0
> > > >
> > > > What is Loongson OUI?
> > > >
> > >
> > > Currently, we do not have an OUI for Loongson, but we are in the
> > > process of applying for one.
> >
> > Is the value 0x00061ce0 baked into the silicon? Or can it be changed
> > once you have an OUI?
> >
> 
> Hi, Andrew,
> 
> The value is baked into the silicon.

O.K. Thanks. Do you have an idea how long it will take to get an OUI?

Does the PCI ID uniquely identify the MAC+PHY combination? 

> > > The PHY itself supports half-duplex, but there are issues with the
> > > controller used in the 7A2000 chip. Moreover, the controller only
> > > supports auto-negotiation for gigabit speeds.
> >
> > So you can force 10/100/1000, but auto neg only succeeds for 1G?
> >
> > Are the LP autoneg values available for genphy_read_lpa() to read? If
> > the LP values are available, maybe the PHY driver can resolve the
> > autoneg for 10 an 100.
> >
> 
> I apologize for the confusion caused by my previous description. To
> clarify, the PHY supports auto-negotiation and non-auto-negotiation
> for 10/100, but non-auto-negotiation for 1000 does not work correctly.

So you can force 10/100, but auto-neg 10/100/1000.

So i would suggest .get_features() indicates normal 10/100/1000
operation. Have your .config_aneg function which is used for both
auto-neg and forced configuration check for phydev->autoneg ==
AUTONEG_DISABLE and phydev->speed == SPEED_1000 and return
-EOPNOTSUPP. Otherwise call genphy_config_aneg().

> > So what does genphy_read_abilities() return?
> >
> > Nobody else going to use PHY_LOONGSON_FEATURES, so i would prefer not
> > to add it to the core. If your PHY is designed correctly,
> > genphy_read_abilities() should determine what the PHY can do from its
> > registers. If the registers are wrong, it is better to add a
> > .get_features function.
> >
> 
> genphy_read_abilities() returns 0, and it seems to be working fine.
> The registers are incorrect, so I will use the .get_features function
> to clear some of the half-duplex bits.

You said above that the PHY supports half duplex, the MAC has problems
with half duplex. So it should be the MAC which indicates it does not
unsupported half duplex link modes.

So you need to modify phylink_config.mac_capabilities before
phylink_create() is called. There is already

        /* Half-Duplex can only work with single queue */
	if (priv->plat->tx_queues_to_use > 1)
                priv->phylink_config.mac_capabilities &=
                        ~(MAC_10HD | MAC_100HD | MAC_1000HD);

so you just need to add a quirk for your broken hardware.

	Andrew