Re: [PATCH 5/7] fsl_pmc: update device bindings

[Scott Wood <scottwood@freescale.com>]

On 11/04/2011 07:36 AM, Zhao Chenhui wrote:
> From: Li Yang <leoli@freescale.com>
> 
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  .../devicetree/bindings/powerpc/fsl/pmc.txt        |   63 +++++++++++--------
>  1 files changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt b/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt
> index 07256b7..d84b4f8 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt
> +++ b/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt
> @@ -9,22 +9,27 @@ Properties:
>  
>    "fsl,mpc8548-pmc" should be listed for any chip whose PMC is
>    compatible.  "fsl,mpc8536-pmc" should also be listed for any chip
> -  whose PMC is compatible, and implies deep-sleep capability.
> +  whose PMC is compatible, and implies deep-sleep capability and
> +  wake on user defined packet(wakeup on ARP).

Why does the PMC care?  This is an ethernet controller feature, the PMC
is just keeping the wakeup-relevant parts of the ethernet controller
alive (whatever they happen to be).

Do we have any chips that have ethernet controller support for wake on
user-defined packet, but a sleep mode that doesn't let it be used?

BTW, please remove fsl,mpc8536-pmc from the p1023rds device tree -- it
was wrong before (no deep sleep, though it does appear to have jog
mode...), and is even more wrong with this provision (it has a different
ethernet controller).

> +  "fsl,p1022-pmc" should be listed for any chip whose PMC is
> +  compatible, and implies lossless Ethernet capability during sleep.
>  
>    "fsl,mpc8641d-pmc" should be listed for any chip whose PMC is
>    compatible; all statements below that apply to "fsl,mpc8548-pmc" also
>    apply to "fsl,mpc8641d-pmc".
>  
>    Compatibility does not include bit assignments in SCCR/PMCDR/DEVDISR; these
> -  bit assignments are indicated via the sleep specifier in each device's
> -  sleep property.
> +  bit assignments are indicated via the clock nodes.  Device which has a
> +  controllable clock source should have a "clk-handle" property pointing
> +  to the clock node.

Do we have any code to use this?

Normally that shouldn't matter, but we already an unused binding for
this. :-)

Please provide rationale for doing it this way.  Ideally it should
probably use whatever http://devicetree.org/ClockBindings ends up being.

>  - reg: For devices compatible with "fsl,mpc8349-pmc", the first resource
>    is the PMC block, and the second resource is the Clock Configuration
>    block.
>  
> -  For devices compatible with "fsl,mpc8548-pmc", the first resource
> -  is a 32-byte block beginning with DEVDISR.
> +  For devices compatible with "fsl,mpc8548-pmc", the second resource
> +  is a 32-byte block beginning with DEVDISR if supported.

Huh?

-Scott