public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Yangbo Lu <yangbo.lu@nxp.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel@pengutronix.de
Subject: Re: sja1105q: proper way to solve PHY clk dependecy
Date: Thu, 24 Mar 2022 16:35:19 +0200	[thread overview]
Message-ID: <20220324143519.yvcgt3u2icnbbafy@skbuf> (raw)
In-Reply-To: <20220324134824.GG4519@pengutronix.de>

On Thu, Mar 24, 2022 at 02:48:24PM +0100, Oleksij Rempel wrote:
> > 3. Clock gating the PHY won't make it lose its settings.
> > 
> > I expect that during the time when the sja1105 switch needs to reset,
> > the PHY just sees this as a few hundreds of ms during which there are no
> > clock edges on the crystal input pin. Sure, the PHY won't do anything
> > during that time, but this is quite different from a reset, is it not?
> > So asserting the hardware reset line of the PHY during the momentary
> > loss of clock, which is what you seem to suggest, will actively do more
> > harm than good.
> 
> can i be sure that MDIO access happens in the period where PHY is
> supplied with stable clk

This is a good question. I suppose not, but I never ran into this issue.
You can try to force this by having the PHY library use poll mode for an
RMII PHY (case in which, IIRC, 3 or 4 PHY registers will be read every 2
seconds), then from user space do something like this:

ip link add br0 type bridge && ip link set br0 up
ip link set swp0 master br0 && ip link set swp0 up
while :; do
	ip link set br0 type bridge vlan_filtering 1
	sleep 1
	ip link set br0 type bridge vlan_filtering 0
	sleep 1
done

Every VLAN awareness change triggers a reset in the switch, and this
ends up calling sja1105_static_config_reload().

If you can artificially reproduce PHY access failures, first it's
interesting to analyze their impact, does the PHY state machine
transition to a halted state, or does it ignore the errors and continue
with the next poll cycle? If it continues, it's probably not worth doing
something.

To avoid the problem, you should probably need to iterate using
dsa_switch_for_each_user_port(), and mutex_lock(&dp->slave->phydev->lock)
for each RMII PHY during the reset procedure (similar to the other
things we lock out during the switch reset). The tricky part seems to be
releasing the locks in the reverse order of the acquire...

      reply	other threads:[~2022-03-24 14:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-23  6:03 sja1105q: proper way to solve PHY clk dependecy Oleksij Rempel
2022-03-23  9:52 ` Vladimir Oltean
2022-03-24 13:48   ` Oleksij Rempel
2022-03-24 14:35     ` Vladimir Oltean [this message]

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=20220324143519.yvcgt3u2icnbbafy@skbuf \
    --to=olteanv@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=vivien.didelot@gmail.com \
    --cc=yangbo.lu@nxp.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