netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [RFC net-next 0/2] net: dsa: bcm_sf2: Utilize PHYLINK for all ports
Date: Tue, 26 Jul 2022 09:14:17 -0700	[thread overview]
Message-ID: <174b5e64-a250-e1b2-43b9-474b915ddc22@gmail.com> (raw)
In-Reply-To: <20220726112344.3ar7s7khiacqueus@skbuf>

On 7/26/22 04:23, Vladimir Oltean wrote:
> Hi Florian,
> 
> On Mon, Jul 25, 2022 at 02:49:40PM -0700, Florian Fainelli wrote:
>> Hi all,
>>
>> Although this should work for all devices, since most DTBs on the
>> platforms where bcm_sf2 is use do not populate a 'fixed-link' property
>> for their CPU ports, but rely on the Ethernet controller DT node doing
>> that, we will not be registering a 'fixed-link' instance for CPU ports.
>>
>> This still works because the switch matches the configuration of the
>> Ethernet controller, but on BCM4908 where we want to force 2GBits/sec,
>> that I cannot test, not so sure.
>>
>> So as of now, this series does not produce register for register
>> compatile changes.
> 
> My understanding of this change set is that it stops overriding the link
> status of IMP ports from dsa_switch_ops :: setup (unconditionally) and
> it moves it to phylink_mac_link_up(). But the latter may not be called
> when you lack a fixed-link, and yet the IMP port(s) still work(s).
> 
> This begs the natural question, is overriding the link status ever needed?

It was until we started to unconditionally reset the switch using the "external" reset method as opposed to the "internal" reset method which turned out not to be functional:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eee87e4377a4b86dc2eea0ade162b0dc33f40576

At any rate (no pun intended), 4908 will want a 2GBit/sec IMP port to be set-up and we have no way to do that other than by forcing that setting, either through the bcm_sf2_imp_setup() method or via a hack to the mac_link_up() callback. This is kind of orthogonal in the sense that there is no "official" support for speed 2000 mbits/sec anyway in the emulated SW PHY, PHYLINK or anywhere in between but if we want to fully transition over to PHYLINK to configure all ports, which is absolutely the goal, we will need to find a solution one way or another.

I would prefer if also we sort of "transferred" the 'fixed-link' parameters from the DSA Ethernet controller attached to the CPU port onto the PHYLINK instance of the CPU port in the switch as they ought to be strictly identical otherwise it just won't work. This would ensure that we continue to force the link and it would make me sleep better a night to know that the IMP port is operating strictly the same way it was. My script compares register values before/after for the registers that are static and this was flagged as a difference.
-- 
Florian

  reply	other threads:[~2022-07-26 16:14 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-25 21:49 [RFC net-next 0/2] net: dsa: bcm_sf2: Utilize PHYLINK for all ports Florian Fainelli
2022-07-25 21:49 ` [RFC net-next 1/2] net: dsa: bcm_sf2: Introduce helper for port override offset Florian Fainelli
2022-07-25 21:49 ` [RFC net-next 2/2] net: dsa: bcm_sf2: Have PHYLINK configure CPU/IMP port(s) Florian Fainelli
2022-07-26 11:23 ` [RFC net-next 0/2] net: dsa: bcm_sf2: Utilize PHYLINK for all ports Vladimir Oltean
2022-07-26 16:14   ` Florian Fainelli [this message]
2022-07-27  1:29     ` Vladimir Oltean
2022-07-27  2:17       ` Florian Fainelli
2022-07-27 13:23         ` Vladimir Oltean

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=174b5e64-a250-e1b2-43b9-474b915ddc22@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=vivien.didelot@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).