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@microchip.com wrote:

quoted

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/ (local) . 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