From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Helmut Grohne <helmut.grohne@intenta.de>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Palmer Dabbelt <palmer@dabbelt.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH] net: macb: reject unsupported rgmii delays
Date: Wed, 17 Jun 2020 13:08:09 +0100 [thread overview]
Message-ID: <20200617120809.GS1551@shell.armlinux.org.uk> (raw)
In-Reply-To: <20200617115201.GA30172@laureti-dev>
On Wed, Jun 17, 2020 at 01:52:01PM +0200, Helmut Grohne wrote:
> On Wed, Jun 17, 2020 at 01:40:25PM +0200, Russell King - ARM Linux admin wrote:
> > > For a fixed-link, the validation function is never called. Therefore, it
> > > cannot reject PHY_INTERFACE_MODE_RGMII. It works in practice.
> >
> > Hmm, I'm not so sure, but then I don't know exactly what code you're
> > using. Looking at mainline, even for a fixed link, you call
> > phylink_create(). phylink_create() will spot the fixed link, and
> > parse the description, calling the validation function. If that
> > fails, it will generate a warning at that point:
> >
> > "fixed link %s duplex %dMbps not recognised"
> >
> > It doesn't cause an operational failure, but it means that you end up
> > with a zero supported mask, which is likely not expected.
> >
> > This is not an expected situation, so I'll modify your claim to "it
> > works but issues a warning" which still means that it's not correct.
>
> I do see that warning. I agree with your correction of my claim. Thank
> you for your attention to detail.
>
> So we have two good reasons for not rejecting delay configuration in the
> validation function now.
>
> The remaining open question seems to be whether configuring a delay on a
> MAC to MAC connection should cause a failure or a only warning. Do you
> have an opinion on that?
>
> All in-tree bindings of the driver seem to use rmii when they specify a
> phy-mode.
This brings up a problem in itself - the phy interface mode is
currently defined in terms of a MAC-to-PHY setup, not a MAC-to-MAC
setup.
With a fixed link, we could be in either a MAC-to-PHY or MAC-to-MAC
setup; we just don't know. However, we don't have is access to the
PHY (if it exists) in the fixed link case to configure it for the
delay.
In the MAC-to-MAC RGMII setup, where neither MAC can insert the
necessary delay, the only way to have a RGMII conformant link is to
have the PCB traces induce the necessary delay. That errs towards
PHY_INTERFACE_MODE_RGMII for this case.
However, considering the MAC-to-PHY RGMII fixed link case, where the
PHY may not be accessible, and may be configured with the necessary
delay, should that case also use PHY_INTERFACE_MODE_RGMII - clearly
that would be as wrong as using PHY_INTERFACE_MODE_RGMII_ID would
be for the MAC-to-MAC RGMII with PCB-delays case.
So, I think a MAC driver should not care about the specific RGMII
mode being asked for in any case, and just accept them all.
I also think that some of this ought to be put in the documentation
as guidance for new implementations.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2020-06-17 12:08 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-16 7:49 [PATCH] net: macb: reject unsupported rgmii delays Helmut Grohne
2020-06-17 9:24 ` Nicolas Ferre
2020-06-17 9:57 ` Vladimir Oltean
2020-06-17 11:15 ` Helmut Grohne
2020-06-17 10:55 ` Russell King - ARM Linux admin
2020-06-17 11:21 ` Helmut Grohne
2020-06-17 11:40 ` Russell King - ARM Linux admin
2020-06-17 11:52 ` Helmut Grohne
2020-06-17 12:08 ` Russell King - ARM Linux admin [this message]
2020-06-18 8:14 ` Helmut Grohne
2020-06-18 8:45 ` Russell King - ARM Linux admin
2020-06-18 9:05 ` Helmut Grohne
2020-06-18 10:01 ` Russell King - ARM Linux admin
2020-06-18 18:26 ` Florian Fainelli
2020-06-18 18:06 ` Florian Fainelli
2020-06-18 18:49 ` Russell King - ARM Linux admin
2020-06-18 19:02 ` Florian Fainelli
2020-06-18 19:10 ` Russell King - ARM Linux admin
2020-06-17 11:32 ` Vladimir Oltean
2020-06-17 11:34 ` Russell King - ARM Linux admin
2020-06-17 11:38 ` Vladimir Oltean
2020-06-17 11:57 ` Russell King - ARM Linux admin
2020-06-18 18:25 ` Florian Fainelli
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=20200617120809.GS1551@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=davem@davemloft.net \
--cc=helmut.grohne@intenta.de \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nicolas.ferre@microchip.com \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.