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: <Y9PrDPPbtIClVtB4@shell.armlinux.org.uk>
Date:   Fri, 27 Jan 2023 15:17:32 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
Cc:     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 net-next v4 4/4] net: ethernet: renesas: rswitch: Add
 phy_power_{on,off}() calling

On Fri, Jan 27, 2023 at 11:26:21PM +0900, Yoshihiro Shimoda wrote:
> Some Ethernet PHYs (like marvell10g) will decide the host interface
> mode by the media-side speed. So, the rswitch driver needs to
> initialize one of the Ethernet SERDES (r8a779f0-eth-serdes) ports
> after linked the Ethernet PHY up. The r8a779f0-eth-serdes driver has
> .init() for initializing all ports and .power_on() for initializing
> each port. So, add phy_power_{on,off} calling for it.

So how does this work?

88x3310 can change it's MAC facing interface according to the speed
negotiated on the media side, or it can use rate adaption mode, but
if it's not a MACSEC device, the MAC must pace its transmission
rate to that of the media side link.

The former requires one to reconfigure the interface mode in
mac_config(), which I don't see happening in this patch set.

The latter requires some kind of configuration in mac_link_up()
which I also don't see happening in this patch set.

So, I doubt this works properly.

Also, I can't see any sign of any working DT configuration for this
switch to even be able to review a use case - all there is in net-next
is the basic description of the rswitch in a .dtsi and no users. It
may be helpful if there was some visibility of its use, and why
phylink is being used in this driver - because right now with phylink's
MAC methods stubbed out in the way they are, and even after this patch
set, I see little point to this driver using phylink.

Moreover, looking at the binding document, you don't even support SFPs
or fixed link, which are really the two reasons to use phylink over
phylib.

Also, phylink only really makes sense if the methods in its _ops
structures actually do something useful, because without that there
can be no dynamic configuration of the system to suit what is
connected.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ