netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Heiner Kallweit <hkallweit1@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	David Miller <davem@davemloft.net>, Ray Jui <rjui@broadcom.com>,
	Scott Branden <sbranden@broadcom.com>,
	bcm-kernel-feedback-list@broadcom.com,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH net-next 0/2] net: phy: set 1Gbps as default for driver features
Date: Tue, 18 Dec 2018 19:09:36 +0100	[thread overview]
Message-ID: <01b6ec4c-83da-5d91-e879-602c05bdf01a@gmail.com> (raw)
In-Reply-To: <20181218093952.GB8334@lunn.ch>

On 18.12.2018 10:39, Andrew Lunn wrote:
> On Tue, Dec 18, 2018 at 07:34:50AM +0100, Heiner Kallweit wrote:
>> Whether a PHY is 100Mbps or 1Gbps-capable can be autodetected,
>> therefore it's not needed to define this manually in the driver.
>> genphy_config_init() will remove 1Gbps from phydev->supported if
>> not supported. Having said that PHY drivers for 100Mbps not
>> calling genphy_config_init() still have to set the features field.
>> As most PHY's are 1Gbps-capable let's use this as default.
> 
> Hi Heiner
> 
> I'm not sure i like this. Today most PHYs are 1G. But multi-gige PHYs
> are starting to appear. In 5 years time, i expect most new PHYs will
> be 2.5G and 5G capable, maybe 10G. We then end up with the odd
> situation that 10M, 100M and 2.5G, 5G and 10G all need features, but 1G
> not.
> 

I see the point and thought about this too. My conclusion:
Basic idea is that we shouldn’t force driver authors to specify things
we can autodetect in phylib. And AFAICS supported speeds can be
autodetected based on the information in clause 22 and clause 45
(speed ability) registers. What remains as a challenge is to autodetect
C22 vs. C22-capable C45 vs. C45. So far it seems that C45 PHY’s can be
used only via DT configuration, MDIO bus scan (mdiobus_scan can’t deal
with C45) doesn't work with C45.

Coming back to why 1Gbps as default (at least for now):
Technically I could have used also 10Gbps as default, genphy_config_init()
masks everything above 1Gbps anyway. I think also for 2.5 Gbps and 5Gbps
PHY’s would don’t have to specify the supported speeds. What we need is
an extension to genphy_config_init() which can deal with C22 only so far.
Maybe mv3310_config_init() from the marvell10g driver can be at least
partially reused.

Eventually defining 1Gbps as default isn’t something we would need to roll
back later once PHYs above 1Gbps are the standard. We can deal with this in
phylib w/o affecting existing drivers. Therefore I think the proposed change
is future-proof.

> I would prefer to keep it consistent and always have a features.
> 
>   Andrew
> 
Heiner

  reply	other threads:[~2018-12-18 18:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-18  6:34 [PATCH net-next 0/2] net: phy: set 1Gbps as default for driver features Heiner Kallweit
2018-12-18  6:36 ` [PATCH net-next 1/2] " Heiner Kallweit
2018-12-18  6:39 ` [PATCH net-next 2/2] net: phy: remove feature definition from 1Gbps PHY drivers Heiner Kallweit
2018-12-18  9:39 ` [PATCH net-next 0/2] net: phy: set 1Gbps as default for driver features Andrew Lunn
2018-12-18 18:09   ` Heiner Kallweit [this message]
2018-12-19  5:46     ` David Miller
2018-12-19  9:21       ` Andrew Lunn
2018-12-19 19:30         ` Heiner Kallweit
2018-12-19 19:34           ` 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=01b6ec4c-83da-5d91-e879-602c05bdf01a@gmail.com \
    --to=hkallweit1@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=rjui@broadcom.com \
    --cc=sbranden@broadcom.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).