> -----Original Message----- > From: Wood Scott-B07421 > Sent: Wednesday, November 12, 2014 1:38 AM > To: Xie Shaohui-B21989 > Cc: Liberman Igal-B31950; linuxppc-dev@xxxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; 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@xxxxxxxxx > > > Cc: linuxppc-dev@xxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; 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@xxxxxxxxx wrote: > > > > From: Shaohui Xie <Shaohui.Xie@xxxxxxxxxxxxx> > > > > > > > > This binding is for FMan MDIO, it covers FMan v2 & FMan v3. > > > > > > > > Signed-off-by: Shaohui Xie <Shaohui.Xie@xxxxxxxxxxxxx> > > > > --- > > > > 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? > > > > > > > +- interrupts > > > > + Usage: optional > > > > + Value type: <prop-encoded-array> > > > > + Definition: MDIO controller event interrupts. > > > > > > One interrupt or multiple? > > [S.H] One for 1 Gb/s, one for 10 Gb/s. > > Then the binding needs to say that there are two (except for "fsl,fman- > mdio", right?) and which is which. > [S.H] OK. > > > > > > > + > > > > +- type > > > > + Usage: required for FMan v3 > > > > + Value type: <stringlist> > > > > + Definition: A standard property. > > > > > > What standard is "type" defined in? > > > > [S.H] It's to differentiate between the internal and external MDIO, > > I'm not quite sure about naming it, or what could be better way to > > differentiate the MDIOs? > > I'm not questioning the need to convey this information. I'm saying it's > not "A standard property". I'd also like to see a better name. > [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? Thanks! Shaohui ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
