Re: [RFC Patch 1/4] mfd: AXP20x: Add power supply bindings documentation

[Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2408 bytes --]

Hi Bruno,

On Mon, Nov 03, 2014 at 09:02:44PM +0100, Bruno Prémont wrote: 
> Doing something like this?:
> 
> --- a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
> @@ -126,9 +126,11 @@
>                                 interrupt-controller;
>                                 #interrupt-cells = <1>;
>  
> -                               backup = <3000000 200>;
> -                               battery.resistance = <100>;
> -                               battery.capacity = <2000>;
> +                               x-powers,backup = <3000000 200>;

I don't really remember what was that property used for. Would it make
sense to put it into the battery node?

> +                               battery: battery@0 {
> +                                       x-powers,resistance = <100>;
> +                                       x-powers,capacity = <2000>;
> +                               };
>                         };
>                 };
> 
> What are the rules to define the label after the colon?
> Looking at the existing nodes it's either some address or a number...

It's not called the label, but the node name, and it's defined in the
ePAPR.

It's <device-class>[@<address>]

The address being something to identify the node on a bus, so it can
be a chip select number, a memory address, an i2c address,
etc. there's really no absolute answer here.

I don't think you really need one in this case.

> and then the following in driver code (also adjusting the other
> property names accessed)?:
> 
> @@ -678,11 +677,11 @@ static int axp20x_battery_config(struct platform_device *pdev,
>         if (ret)
>                 return ret;
>  
> -       np = of_node_get(axp20x->dev->of_node);
> +       np = of_find_node_by_name(axp20x->dev->of_node, "battery");
>         if (!np)
>                 return -ENODEV;
>  
> -       ret = of_property_read_u32_array(np, "battery.ocv", ocv, 16);
> +       ret = of_property_read_u32_array(np, "x-powers,ocv", ocv, 16);
>         for (i = 0; ret == 0 && i < ARRAY_SIZE(ocv); i++)
>                 if (ocv[i] > 100) {
>                         dev_warn(&pdev->dev, "OCV[%d] %u > 100\n", i, ocv[i]);

Yep, it looks sensible.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]