netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: netdev <netdev@vger.kernel.org>
Subject: Re: [RFC PATCH net-next 2/2] phy: fixed-phy: Allow DT description of an MDIO bus and PHYs.
Date: Sat, 12 Mar 2016 01:05:28 +0100	[thread overview]
Message-ID: <20160312000528.GD23969@lunn.ch> (raw)
In-Reply-To: <56E35530.6000009@gmail.com>

> > +Child nodes represent PHYs on this mdio bus. Standard properties for
> > +fixed links, 'speed', 'full-duplex', 'pause', 'asym-pause',
> > +'link-gpios', as defined above are used. Additionally a 'reg' property
> > +is required for the address of the PHY on the bus. This should be of
> > +value 0 to 31.
> 
> This is a virtual bus, the only limitation is because we re-use to the
> maximum permission extent the real MDIO bus code, and this is putting a
> SW constraint on something that does not have one here.

True. I was too lazy to audit the phy and mdio code to see if it
actually allows phy_addr >= 32. And it is easy to instantiate another
bus if you need it.
 
> > +
> > +Example
> > +-------
> > +
> > +mdio {
> > +	compatible = "linux,mdio-fixed-phy";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	phy0: phy@0 {
> > +		reg = <0>;
> > +		speed = <1000>;
> > +		pause;
> > +		link-gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
> > +	};
> > +
> > +	phy1: phy@8 {
> > +		reg = <8>;
> > +		speed = <100>;
> > +		full-duplex;
> > +	};
> 
> The 'fixed-link' property, although it suffers from one basic issue
> which is that it is placed on the consumer of it (e.g: an Ethernet MAC
> Device Tree node) while being a two-headed snake (you actually describe
> a data-pipe with a fixed-link) is not quite perfect, but this does not
> seem to look any better.

It is actually just the same. Not worse, not better.

I see the real benefit in the drivers. They get fixed-phy for
free. Cleanup on release just works, which most drivers don't even do
now, so leak a fixed phy every time they are unloaded. DSA has some
messy code, which has had a number of bugs, handling fixed phys. By
adding this extra code, which i hope is reasonably simple to
understand and review, we make the MAC drivers a lot simpler and less
error prone.
 
> One thing that Thomas solved nicely was avoid the need for allocating an
> address on the virtual MDIO bus, but this is coming back here, which
> does not seem needed except for correctness wrt. how real MDIO buses.

Actually Thomas's code is broken. Fixed phy addresses are allocated
incrementally. When a fixed phy is released, its address is not
re-used. So as soon as you have gone through 32 alloc/free cycles, all
further allocs fail. This is however fixable.

	Andrew

  reply	other threads:[~2016-03-12  0:05 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-11 23:08 [RFC PATCH net-next 0/2] DT MDIO bus of fixed phys Andrew Lunn
2016-03-11 23:08 ` [RFC PATCH net-next 1/2] of: of_mdio: Factor out fixed-link parsing Andrew Lunn
2016-03-11 23:08 ` [RFC PATCH net-next 2/2] phy: fixed-phy: Allow DT description of an MDIO bus and PHYs Andrew Lunn
2016-03-11 23:30   ` Florian Fainelli
2016-03-12  0:05     ` Andrew Lunn [this message]
2016-03-12 17:32     ` Andrew Lunn
2016-03-11 23:26 ` [RFC PATCH net-next 0/2] DT MDIO bus of fixed phys Florian Fainelli
2016-03-11 23:36   ` Andrew Lunn
2016-03-11 23:38     ` Florian Fainelli
2016-03-12  0:12       ` Andrew Lunn
2016-03-14 16:51         ` Florian Fainelli
2016-03-14 19:04           ` Andrew Lunn

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=20160312000528.GD23969@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=f.fainelli@gmail.com \
    --cc=netdev@vger.kernel.org \
    /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).