Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Thu, 24 Mar 2022 16:35:19 +0200
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
Message-ID: <20220324143519.yvcgt3u2icnbbafy@skbuf>
References: <20220323060331.GA4519@pengutronix.de>
 <20220323095240.y4xnp6ivz57obyvv@skbuf>
 <20220324134824.GG4519@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220324134824.GG4519@pengutronix.de>
Precedence: bulk

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...