netdev - Re: [PATCH net-next V3 3/4] net: lan743x: Migrate phylib to phylink

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: <Zr+OsygS+YRkRnL6@HYD-DK-UNGSW21.microchip.com>
Date: Fri, 16 Aug 2024 23:08:59 +0530
From: Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
CC: <Ronnie.Kunin@...rochip.com>, <Raju.Lakkaraju@...rochip.com>,
	<netdev@...r.kernel.org>, <davem@...emloft.net>, <kuba@...nel.org>,
	<andrew@...n.ch>, <horms@...nel.org>, <hkallweit1@...il.com>,
	<richardcochran@...il.com>, <rdunlap@...radead.org>,
	<Bryan.Whitehead@...rochip.com>, <edumazet@...gle.com>, <pabeni@...hat.com>,
	<linux-kernel@...r.kernel.org>, <UNGLinuxDriver@...rochip.com>
Subject: Re: [PATCH net-next V3 3/4] net: lan743x: Migrate phylib to phylink

Hi Russell King,

Thank you for quick response.

The 08/08/2024 22:07, Russell King (Oracle) wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Thu, Aug 08, 2024 at 08:23:38PM +0000, Ronnie.Kunin@...rochip.com wrote:
> > We looked into an alternate way to migrate our lan743x driver from phylib to phylink continuing to support our existing hardware out in the field, without using the phylib's fixed-phy approach that you opposed to, but without modifying the phylib framework either.
> > While investigating how to implement it we came across this which Raju borrowed ideas from: https://lore.kernel.org/linux-arm-kernel/YtGPO5SkMZfN8b%2Fs@shell.armlinux.org.uk/ . He is in the process of testing/cleaning it up and expects to submit it early next week.
> 
> That series died a death because it wasn't acceptable to the swnode
> folk. In any case, that's clearly an over-complex solution for what is
> a simple problem here.
> 
> The simplest solution would be for phylink to provide a new function,
> e.g.
> 
> int phylink_set_fixed_link(struct phylink *pl,
>                            const struct phylink_state *state)
> {
>         const struct phy_setting *s;
>         unsigned long *adv;
> 
>         if (pl->cfg_link_an_mode != MLO_AN_PHY || !state ||
>             !test_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state))
>                 return -EINVAL;
> 
>         s = phy_lookup_setting(state->speed, state->duplex,
>                                pl->supported, true);
>         if (!s)
>                 return -EINVAL;
> 
>         adv = pl->link_config.advertising;
>         linkmode_zero(adv);
>         linkmode_set_bit(s->bit, adv);
>         linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, adv);
> 
>         pl->link_config.speed = state->speed;
>         pl->link_config.duplex = state->duplex;
>         pl->link_config.link = 1;
>         pl->link_config.an_complete = 1;
> 
>         pl->cfg_link_an_mode = MLO_AN_FIXED;
>         pl->cur_link_an_mode = pl->cfg_link_an_mode;
> 
>         return 0;
> }
> 
> You can then call this _instead_ of attaching a PHY to switch phylink
> into fixed-link mode with the specified speed and duplex (assuming
> they are supported by the MAC.)
> 
> Isn't this going to be simpler than trying to use swnodes that need
> to be setup before phylink_create() gets called?
> 

Your suggestion seems to be working well for us. I'm currently testing it on
different boards and checking for corner cases.
I plan to submit it for code review next week.

Quick question: Should I submit your suggested code along with our patches, or
will you be submitting it separately?

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

-- 
Thanks,                                                                         
Raju