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

["Bruno Prémont" <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>]

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

Hi Maxime,

On Tue, 04 November 2014 Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> 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?

The backup/rtc battery is completely distinct from main battery.

It's presence is in no way related to that of the main battery,
thus I would not put it into the same node.
As all the information needed is included in the property I see
no reason to move it into a separate 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.

In that case, I better omit it completely.

> > 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.

Thanks,
Bruno

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 648 bytes --]