All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stas Sergeev <stsp@list.ru>
To: Florian Fainelli <f.fainelli@gmail.com>, netdev <netdev@vger.kernel.org>
Cc: Linux kernel <linux-kernel@vger.kernel.org>,
	Sebastien Rannou <mxs@sbrk.org>,
	Arnaud Ebalard <arno@natisbad.org>,
	Stas Sergeev <stsp@users.sourceforge.net>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Grant Likely <grant.likely@linaro.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH 2/3] of_mdio: add new DT property 'autoneg' for fixed-link
Date: Fri, 10 Jul 2015 23:08:54 +0300	[thread overview]
Message-ID: <55A02656.7020508@list.ru> (raw)
In-Reply-To: <55A010F9.7030808@gmail.com>

10.07.2015 21:37, Florian Fainelli пишет:
> On 10/07/15 09:43, Stas Sergeev wrote:
>> Currently for fixed-link the MAC driver decides whether to use the
>> link status auto-negotiation or not.
>> Unfortunately the auto-negotiation may not work when expected by
>> the MAC driver. Sebastien Rannou explains:
>> << Yes, I confirm that my HW does not generate an in-band status. AFAIK, it's
>> a PHY that aggregates 4xSGMIIs to 1xQSGMII ; the MAC side of the PHY (with
>> inband status) is connected to the switch through QSGMII, and in this context
>> we are on the media side of the PHY. >>
>> https://lkml.org/lkml/2015/7/10/206
>>
>> This patch introduces the new boolean property 'autoneg' that allows
>> the user to request the auto-negotiation explicitly.
> The implementation looks better, but the name might still be slightly
> controversial. I would go with "use-in-band-status" which is more
> strictly defined than "autoneg" which could mean anything and everything.
>
> What do you think?
I actually think autoneg is a bit better.

- Autonegotiation is a widely used and known term:
https://en.wikipedia.org/wiki/Autonegotiation
And who knows what in-band status is?
And, more importantly, who knows what is it used for?
Who even knows it is used for autonegotiation?

- When we set autoneg for fixed-link, we basically just
say "no MDIO here, but please do autoneg by any other
means, if possible".

- in-band status is an implementation delail, and it is
specific to a particular protocols. If you request the
in-band status for some protocol that doesn't support
it, perhaps you should get -EINVAL, because such a
config makes no sense. With autonegotiation, the rules
are not that strict: it can be "unimplemented", which doesn't
necessary mean nonsense in the config.

- autonegotiation is a wider term, and may be implemented
by some other means than the in-band status (which is
probably impossible for a fixed-link though).

- In the terms that the driver uses, it is autonegotiation, eg
MVNETA_GMAC_AUTONEG_CONFIG. And when you go down
the implementation details, you see MVNETA_GMAC_INBAND_AN_ENABLE,
which is just one AN bit of many.

So I really would prefer to keep things as is.
But if you insist, I can rename, but there will still be no
-EINVAL checks for obviously wrong configs.

  reply	other threads:[~2015-07-10 20:08 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-10 16:38 [PATCH v2 0/2] net: enable inband link state negotiation only when explicitly requested Stas Sergeev
2015-07-10 16:41 ` [PATCH 1/3] fixed_phy: handle link-down case Stas Sergeev
2015-07-10 20:44   ` Florian Fainelli
2015-07-10 21:14     ` Stas Sergeev
2015-07-11  0:15       ` Florian Fainelli
2015-07-11  8:58         ` Stas Sergeev
     [not found] ` <559FF511.5080102-cmBhpYW9OiY@public.gmane.org>
2015-07-10 16:43   ` [PATCH 2/3] of_mdio: add new DT property 'autoneg' for fixed-link Stas Sergeev
2015-07-10 16:43     ` Stas Sergeev
2015-07-10 18:37     ` Florian Fainelli
2015-07-10 20:08       ` Stas Sergeev [this message]
     [not found]         ` <55A02656.7020508-cmBhpYW9OiY@public.gmane.org>
2015-07-10 20:39           ` Florian Fainelli
2015-07-10 20:39             ` Florian Fainelli
     [not found]             ` <55A02D90.8090903-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-07-10 21:02               ` Stas Sergeev
2015-07-10 21:02                 ` Stas Sergeev
     [not found]                 ` <55A032F5.8020801-cmBhpYW9OiY@public.gmane.org>
2015-07-11  0:22                   ` Florian Fainelli
2015-07-11  0:22                     ` Florian Fainelli
2015-07-11  9:15                     ` Stas Sergeev
2015-07-10 16:45 ` [PATCH 3/3] mvneta: use inband status only when explicitly enabled Stas Sergeev
2015-07-10 20:31 ` [PATCH v2 0/2] net: enable inband link state negotiation only when explicitly requested Florian Fainelli
2015-07-10 20:45   ` Stas Sergeev
2015-07-13  9:54 ` Sebastien Rannou
2015-07-13  9:59   ` Stas Sergeev

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=55A02656.7020508@list.ru \
    --to=stsp@list.ru \
    --cc=arno@natisbad.org \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=galak@codeaurora.org \
    --cc=grant.likely@linaro.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mxs@sbrk.org \
    --cc=netdev@vger.kernel.org \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=stsp@users.sourceforge.net \
    /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.