Re: [PATCH] DT: add MDIO node for FMan node

[Scott Wood <scottwood@freescale.com>]

On Thu, 2014-11-13 at 01:11 -0600, Xie Shaohui-B21989 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Thursday, November 13, 2014 2:17 PM
> > To: Xie Shaohui-B21989
> > Cc: Liberman Igal-B31950; linuxppc-dev@lists.ozlabs.org;
> > devicetree@vger.kernel.org; Medve Emilian-EMMEDVE1
> > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> > 
> > On Wed, 2014-11-12 at 07:40 -0600, Xie Shaohui-B21989 wrote:
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Wednesday, November 12, 2014 1:38 AM
> > > > To: Xie Shaohui-B21989
> > > > Cc: Liberman Igal-B31950; linuxppc-dev@lists.ozlabs.org;
> > > > devicetree@vger.kernel.org; Medve Emilian-EMMEDVE1
> > > > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> > > >
> > > > On Tue, 2014-11-11 at 04:32 -0600, Xie Shaohui-B21989 wrote:
> > > > > > -----Original Message-----
> > > > > > From: Wood Scott-B07421
> > > > > > Sent: Tuesday, November 11, 2014 8:23 AM
> > > > > > To: shh.xie@gmail.com
> > > > > > Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org;
> > > > > > Medve Emilian-EMMEDVE1; Xie Shaohui-B21989
> > > > > > Subject: Re: [PATCH] DT: add MDIO node for FMan node
> > > > > >
> > > > > > On Tue, 2014-11-04 at 19:56 +0800, shh.xie@gmail.com wrote:
> > > > > > > From: Shaohui Xie <Shaohui.Xie@freescale.com>
> > > > > > >
> > > > > > > This binding is for FMan MDIO, it covers FMan v2 & FMan v3.
> > > > > > >
> > > > > > > Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> > > > > > > ---
> > > > > > > based on http://patchwork.ozlabs.org/patch/390351/
> > > > > > > for 'next' of
> > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.
> > > > > > > git
> > > > > >
> > > > > > Are there any other FMan pieces that are missing from the above
> > patch?
> > > > > [S.H] I'm adding Igal for this comment.
> > > > >
> > > > > >
> > > > > > > +- bus-frequency
> > > > > > > +		Usage: optional
> > > > > > > +		Value type: <u32>
> > > > > > > +		Definition: Default MDIO bus clock speed.
> > > > > >
> > > > > > Use clocks/clock-names
> > > > > [S.H] The MDIO uses Fman clock and divides it to a proper value
> > > > > which
> > > > is specified by this property.
> > > >
> > > > Use clocks/clock-names to describe that relationship.
> > > >
> > >
> > > [S.H] The MDIO node is sub-node and embedded in Fman node, the
> > > clocks/clock-names is provided by Fman node, should repeat them in
> > > MDIO node? For the default MDIO bus clock speed, maybe "clock-ranges"
> > > should be used?
> > 
> > It's a different clock.  You wouldn't be repeating.  If it's derived from
> > the FMan clock, then maybe you don't need anything here (does the driver
> > know what the divider is, or would that need to be specified in the
> > device tree?), but no more clock-frequency/bus-frequency properties.
> > 
> [S.H] The purpose here is to get a specific clock frequency, driver to use it to calculate the divider.
> Then the Fman clock can be divided to the frequency.

Oh, so this is stating a desired frequency and not something that
already exists?  What determines this frequency?  Is it based on board
design, or just on the MDIO standard, etc?  I'm wondering if the device
tree is the right place for it.

> > > [S.H] since Fman V2 & V3 can be differentiated by compatible, a
> > > boolean property "fsl,fman-internal-mdio" seems better, if defined, it
> > > indicates an internal MDIO. It looks like below:
> > >
> > > fsl,fman-internal-mdio
> > > 		Usage: required for internal MDIO
> > > 		Value type: Boolean
> > > 		Definition: Fman has internal MDIO for internal PCS(Physical
> > > 		Coding Sublayer) PHYs and external MDIO for external PHYs.
> > > 		The settings and programming routines for internal/external
> > > 		MDIO are different. Must be included for internal MDIO.
> > >
> > > How about this?
> > 
> > This looks better, thanks.
> > 
> > This would be set on fman v2 TBI mdio nodes as well, right?
> [S.H] Yes. But it's not used by driver. (I know I should not say this :) )

Saying it is fine as long as you still put it in the device tree. :-)

-Scott