[<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
Powered by blists - more mailing lists