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: <CACWXhKmwjXb_71hmGfKh7NCC3iAhFTB1uEVhY0qq8kz24o3TYg@mail.gmail.com>
Date: Tue, 25 Jul 2023 10:02:50 +0800
From: Feiyang Chen <chris.chenfeiyang@...il.com>
To: Andrew Lunn <andrew@...n.ch>
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

On Fri, Jul 21, 2023 at 5:04 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> > Hi, Andrew,
> >
> > Sorry, I currently don't have an exact timeline for when the OUI will
> > be available. The next hardware version will address these bugs, so
> > we won't be going with this driver.
>
> Not having an OUI breaks the standard. So i was actually thinking you
> should trap the reads to the ID registers in the MDIO bus driver and
> return valid values. Some Marvell Ethernet switch integrated PHYs have
> a valid OUI, and no device part. We trap those and insert valid
> values. So there is some president for doing this. Doing this would
> also allow you to avoid the PHY driver poking around the MAC drivers
> PCI bus.
>
> > > 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().
> > >
> >
> > Well, can I return -EINVAL in the .set_link_ksettings callback?
>
> If the PHY is broken, the PHY should do it. If the MAC is broken, the
> MAC should do it. We have clean separation here, even when the
> hardware is integrated.
>

Hi, Andrew,

I get it. To be honest, I'm not familiar with the hardware, so I asked
the colleague who designed this chip. The real problem is the same as I
described in Patch 10 - the Ethernet controller doesn't work well with
the PHY. If the controller works properly, we can force 1000. Are there
any methods to work around this problem in the MAC driver?

> > Considering that our next hardware version will have the OUI
> > allocated and these bugs fixed, I won't submit this driver in
> > the next patch version. I believe we can just use the generic
> > PHY for now.
>
> The problem with the generic driver is you cannot have workarounds in
> it. You don't want to put PHY workarounds in the MAC driver.
>

Sure, we should not put PHY workarounds in the MAC driver and vice
versa.

This PHY driver was designed to solve two problems at first:
1. We cannot force 1000.
2. We cannot use half duplex.

Now, we are sure that the MAC is broken, not the PHY, so I think we
can solve these problems in the MAC driver.

Thanks,
Feiyang

>     Andrew
>