RE: [v2] powerpc/fsl: Update fman dt binding with pcs-phy and tbi-phy

[Igal Liberman]

Hi Rob,

> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: Wednesday, December 30, 2015 6:28 PM
> To: Igal Liberman <igal.liber...@freescale.com>
> Cc: devicet...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Scott Wood
> <scottw...@freescale.com>; Madalin-Cristian Bucur
> <madalin.bu...@freescale.com>; shaohui@freescale.com
> Subject: Re: [v2] powerpc/fsl: Update fman dt binding with pcs-phy and tbi-
> phy
> 
> On Thu, Dec 24, 2015 at 03:42:11AM +0200, igal.liber...@freescale.com
> wrote:
> > From: Igal Liberman <igal.liber...@freescale.com>
> >
> > The FMan contains internal PHY devices used for SGMII connections to
> > external PHYs. When these PHYs are in use a reference is needed for
> > both the external PHY and the internal one. For the external PHY
> > phy-handle provides the reference. For the internal PHY a new handle
> > is required.
> > In dTSEC, the internal PHY is a TBI (Ten Bit Interface) PHY, the
> > handle used will be tbi-handle.
> > In mEMAC, the internal PHY is a PCS (Physical Coding Sublayer) PHY,
> > the handle used will be pcsphy-handle.
> 
> This is fairly commom for 10G eth I think. Can't you use the common PHY
> binding here in the case without internal MDIO bus? Just because you use it
> that doesn't mean you have to use the generic phy subsystem in the kernel.
> 

mEMAC and dTSEC always have internal MDIO bus.
I was requested to use generic PHY API for internal PHY configuration by 
netdev, this part was accepted. 

> Perhaps phy-handle should be deprecated in favor of doing something like
> this if you need a phandle to both:
> 
> phys = <>, <>;
> 

I think that pcsphy-handle and tbi-handle represent the hardware in a good way.
This is the actual name of the hardware.

> > diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> > b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> > index 1fc5328..55c2c03 100644
> > --- a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> > +++ b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> > @@ -315,6 +315,16 @@ PROPERTIES
> > Value type: 
> > Definition: A phandle for 1EEE1588 timer.
> >
> > +- pcsphy-handle
> > +   Usage required for "fsl,fman-memac" MACs
> > +   Value type: 
> > +   Definition: A phandle for pcsphy.
> > +
> > +- tbi-handle
> > +   Usage required for "fsl,fman-dtsec" MACs
> > +   Value type: 
> > +   Definition: A phandle for tbiphy.
> > +
> >  EXAMPLE
> >
> >  fman1_tx28: port@a8000 {
> > @@ -340,6 +350,7 @@ ethernet@e {
> > reg = <0xe 0x1000>;
> > fsl,fman-ports = <_rx8 _tx28>;
> > ptp-timer = <>;
> > +   tbi-handle = <>;
> 
> What does the tbi0 node contain? It should be present in the example.
> 

There is an example in under MDIO section:

mdio@e3120 {
compatible = "fsl,fman-mdio";
reg = <0xe3120 0xee0>;
fsl,fman-internal-mdio;

tbi1: tbi-phy@8 {
reg = <0x8>;
device_type = "tbi-phy";
};
};

I used different indexes to make sure that it's aligned to the other examples 
in the binding document.

> Rob

Thank you for your feedback, 
Igal
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [v2] powerpc/fsl: Update fman dt binding with pcs-phy and tbi-phy

[Rob Herring]

On Thu, Dec 24, 2015 at 03:42:11AM +0200, igal.liber...@freescale.com wrote:
> From: Igal Liberman 
> 
> The FMan contains internal PHY devices used for SGMII connections
> to external PHYs. When these PHYs are in use a reference is needed
> for both the external PHY and the internal one. For the external
> PHY phy-handle provides the reference. For the internal PHY a new
> handle is required.
> In dTSEC, the internal PHY is a TBI (Ten Bit Interface) PHY,
> the handle used will be tbi-handle.
> In mEMAC, the internal PHY is a PCS (Physical Coding Sublayer) PHY,
> the handle used will be pcsphy-handle.

This is fairly commom for 10G eth I think. Can't you use the common PHY 
binding here in the case without internal MDIO bus? Just because you use 
it that doesn't mean you have to use the generic phy subsystem in the 
kernel. 

Perhaps phy-handle should be deprecated in favor of doing something like 
this if you need a phandle to both:

phys = <>, <>;

> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt 
> b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> index 1fc5328..55c2c03 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> +++ b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> @@ -315,6 +315,16 @@ PROPERTIES
>   Value type: 
>   Definition: A phandle for 1EEE1588 timer.
>  
> +- pcsphy-handle
> + Usage required for "fsl,fman-memac" MACs
> + Value type: 
> + Definition: A phandle for pcsphy.
> +
> +- tbi-handle
> + Usage required for "fsl,fman-dtsec" MACs
> + Value type: 
> + Definition: A phandle for tbiphy.
> +
>  EXAMPLE
>  
>  fman1_tx28: port@a8000 {
> @@ -340,6 +350,7 @@ ethernet@e {
>   reg = <0xe 0x1000>;
>   fsl,fman-ports = <_rx8 _tx28>;
>   ptp-timer = <>;
> + tbi-handle = <>;

What does the tbi0 node contain? It should be present in the example.

Rob
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev