netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Vivien Didelot <vivien.didelot@gmail.com>
Cc: "Baruch Siach" <baruch@tkos.co.il>,
	"Marek Behún" <marek.behun@nic.cz>,
	netdev@vger.kernel.org,
	"Denis Odintsov" <d.odintsov@traviangames.com>
Subject: Re: [PATCH] net: dsa: mv88e6xxx: force cmode write on 6141/6341
Date: Sat, 21 Dec 2019 11:22:17 +0100	[thread overview]
Message-ID: <20191221102217.GA30801@lunn.ch> (raw)
In-Reply-To: <20191220142725.GB2458874@t480s.localdomain>

> Andrew,
> 
> We tend to avoid caching values in the mv88e6xxx driver the more we can and
> query the hardware instead to avoid errors like this. We can consider calling a
> new mv88e6xxx_port_get_cmode() helper when needed (e.g. in higher level callers
> like mv88e6xxx_serdes_power() and mv88e6xxx_serdes_irq_thread_fn()) and pass
> the value down to the routines previously accessing chip->ports[port].cmode,
> as a new argument. I can prepare a patch doing this. What do you think?

Hi Vivien

There is one problem area. The lower ports on the 6390X, port
2-7. They can have two different cmode values, 0x1f for internal PHY,
and 0x09 for 1000BaseX when using an SFP. The switch will swap between
external and internal depending on which gets link first. But in order
for the SFP to get link, the SERDES needs to be powered on. And we
currently decide to power it on, and enable interrupts, via the
cmode. In the normal case, this works, e.g. ports 9 and 10 of 6390,
the fibre port of 6352. The cmode of 6390X is directly writable, we
get the phy-mode from DT and write the correct value. For 6352,
strapping is used, cmode is read only, but has the correct value.

But for the lower ports of 6390X, the hardware cmode defaults to 0x1f
until the fibre gets link, and then it changes to 0x09. It is not
writable. The current generic code does write to the register, which
in this case is pointless since it is read only, and it updates the
cached value, which is R/W. Later, when the port is enabled, we look
at the cached value, and based on that, decide is the SERDES should be
powered up, interrupts enabled. At this point, the cached value is
what we have in DT, but the hardware cmode is probably still 0x1f,
internal PHY. If we were to use the hardware value, we would never
enable the SERDES and so never get link.

This is all a bit hidden in the code. So if you want to remove the
cache, you need to add something in its place. Maybe explicitly keep
the configured value in the port structure, and modify the code using
cmode so that it either looks at the actual cmode, or the configured
cmode, depending on what it is trying to achieve.

ZII devel C is a good test bed for this, port 3 has both a copper PHY
wired to an RJ45 socket, and an SFF.

      Andrew

  reply	other threads:[~2019-12-21 10:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-19  9:48 [PATCH] net: dsa: mv88e6xxx: force cmode write on 6141/6341 Baruch Siach
2019-12-20 19:27 ` Vivien Didelot
2019-12-21 10:22   ` Andrew Lunn [this message]
2019-12-21 21:51   ` Vivien Didelot
2019-12-22  9:43   ` Baruch Siach
2020-01-02 20:45 ` David Miller
2020-01-02 22:36   ` Andrew Lunn
2020-01-02 23:31     ` David Miller

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=20191221102217.GA30801@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=baruch@tkos.co.il \
    --cc=d.odintsov@traviangames.com \
    --cc=marek.behun@nic.cz \
    --cc=netdev@vger.kernel.org \
    --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).