| Message ID | 20141103210244.1425e0c7@neptune.home |
|---|---|
| State | New |
| Headers | show |
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
Hi Maxime, On Tue, 04 November 2014 Maxime Ripard <maxime.ripard@free-electrons.com> 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
Hi Ezaul, On Tue, 04 November 2014 Ezaul Zillmer <ezaulzillmer@gmail.com> wrote: > Cubieboard2 + Kernel 3.18-rc3 > > [ 15.955655] axp20x-regulator axp20x-regulator: regulators node not found > [ 15.962580] LDO1: 1300 mV > [ 15.965732] LDO2: at 3000 mV > [ 15.969120] LDO3: at 2275 mV > [ 15.972314] LDO4: at 2800 mV > [ 15.975700] LDO5: at 2800 mV > [ 15.979075] DCDC2: at 1400 mV > [ 15.982555] DCDC3: at 1250 mV > [ 15.985706] axp20x 0-0034: AXP20X driver loaded > > What was missing for axp20x work of cubieboard2 dt? > as poderiar do to run this drive? Not sure what patches, it at all you have on top of 3.18-rc3. As far as I know 3.18 should not have any AXP driver that's not already present in 3.17. The base AXP driver only includes power-off support and bits for regulators while the DT part of regulators is missing (at least for cubietruck). My patches, ideally combined with Carlo's patches from June or so I mentioned in introduction mail would include power supply and input driver for power button. The regulator DT entries are in some other patch (I think Maxime did have them in some of his trees - probably defining them causes too much trouble because all uses are not yet ready to explicitly use them). On DT side, on top of my patches, for cubieboard2 you might need to add the backup battery property if it has such a battery - I don't think the CB2 has a main battery connector but I might be wrong (I only have a cubietruck). Except for that a hwmon driver would be useful to cover some of the last bits of the AXP like temprature sensor and main voltage sensor. With that about all the features should be covered. Bruno
On Tue, Nov 04, 2014 at 10:08:27PM +0100, Bruno Prémont wrote: > On Tue, 04 November 2014 Maxime Ripard <maxime.ripard@free-electrons.com> 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. Oh. So it is to model some button battery that might be here to keep the RTC state while the board is unpowered? How is that related to the PMIC? Maybe modeling as a fixed regulator would be more accurate. Maxime
> Op 5 nov. 2014, om 15:48 heeft Maxime Ripard <maxime.ripard@free-electrons.com> het volgende geschreven: > > On Tue, Nov 04, 2014 at 10:08:27PM +0100, Bruno Prémont wrote: >> On Tue, 04 November 2014 Maxime Ripard <maxime.ripard@free-electrons.com> 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. > > Oh. So it is to model some button battery that might be here to keep > the RTC state while the board is unpowered? > > How is that related to the PMIC? Some PMICs can recharge it and detect it it's empty or not.
--- 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>; + battery: battery@0 { + x-powers,resistance = <100>; + x-powers,capacity = <2000>; + }; }; };