netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Ronnie.Kunin@microchip.com
Cc: Raju.Lakkaraju@microchip.com, netdev@vger.kernel.org,
	davem@davemloft.net, kuba@kernel.org, andrew@lunn.ch,
	horms@kernel.org, hkallweit1@gmail.com, richardcochran@gmail.com,
	rdunlap@infradead.org, Bryan.Whitehead@microchip.com,
	edumazet@google.com, pabeni@redhat.com,
	linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com
Subject: Re: [PATCH net-next V3 3/4] net: lan743x: Migrate phylib to phylink
Date: Thu, 8 Aug 2024 22:07:28 +0100	[thread overview]
Message-ID: <ZrUzkF8jj50ZgGhk@shell.armlinux.org.uk> (raw)
In-Reply-To: <PH8PR11MB796562D6C8964A6B6A1CC7E595B92@PH8PR11MB7965.namprd11.prod.outlook.com>

On Thu, Aug 08, 2024 at 08:23:38PM +0000, Ronnie.Kunin@microchip.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?

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

  reply	other threads:[~2024-08-08 21:07 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-30 14:06 [PATCH net-next V3 0/4] Add support to PHYLINK for LAN743x/PCI11x1x chips Raju Lakkaraju
2024-07-30 14:06 ` [PATCH net-next V3 1/4] net: lan743x: Create separate PCS power reset function Raju Lakkaraju
2024-07-30 14:06 ` [PATCH net-next V3 2/4] net: lan743x: Create separate Link Speed Duplex state function Raju Lakkaraju
2024-07-30 14:06 ` [PATCH net-next V3 3/4] net: lan743x: Migrate phylib to phylink Raju Lakkaraju
2024-07-30 14:56   ` Russell King (Oracle)
2024-08-01 11:03     ` Raju Lakkaraju
2024-08-01 16:27       ` Russell King (Oracle)
2024-08-01 23:46         ` Ronnie.Kunin
2024-08-02  8:37           ` Russell King (Oracle)
2024-08-08 20:23             ` Ronnie.Kunin
2024-08-08 21:07               ` Russell King (Oracle) [this message]
2024-08-16 17:38                 ` Raju Lakkaraju
2024-08-17 11:42                   ` Russell King (Oracle)
2024-07-30 14:06 ` [PATCH net-next V3 4/4] net: lan743x: Add support to ethtool phylink get and set settings Raju Lakkaraju
2024-07-30 15:11   ` Russell King (Oracle)
2024-08-01 11:05     ` Raju Lakkaraju

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZrUzkF8jj50ZgGhk@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Bryan.Whitehead@microchip.com \
    --cc=Raju.Lakkaraju@microchip.com \
    --cc=Ronnie.Kunin@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rdunlap@infradead.org \
    --cc=richardcochran@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).