[linux-sunxi] Re: Including empty regulator nodes in axp209.dtsi is a BAD idea

On Wed, Jan 14, 2015 at 6:18 PM, Lars Doelle [off-list ref] wrote:

On Tuesday, January 13, 2015 17:46:06 Maxime Ripard wrote:

quoted

On Tue, Jan 13, 2015 at 10:39:01AM +0100, Hans de Goede wrote:

quoted

Hi ChenYu, Maxime,

During the review of a few dts files for new boards Maxime asked me to use
axp209.dtsi to avoid the standard axp209 "boilerplate" present in most
boards using the axp209 pmic.

But axp209.dtsi includes empty regulator nodes, e.g. :

                reg_dcdc3: dcdc3 {
                        regulator-name = "dcdc3";
                };

This is a BAD idea, the presence of these empty nodes causes the
axp20x-regulator driver to actually register regulators for them,
and then on late_init the regulator subsys turns them off, since
they have absolutely no constraints set (nor users registered)
and the regulator subsys assumes that when devicetree is used their
is always a compete set of constraints and that thus turning them
off is safe.

So when I switched to using axp209.dtsi for the bananapro.dts,
and booted the bananapro this is the last message I got from the
kernel while booting:

[    2.314014] dcdc3: disabling

And away went our DRAM power-supply, oops.

So for dcdc2 (CPU) and dcdc3 (DRAM), the boilerplate
should contain reasonable constraints (eg the operating range
from the datasheet)

Indeed.

It is very unclear to me, what to with this dtsi as it is.

Wrt. axp209, I have a specification like in sun7i-a20-olinuxino-lime2.dts.
If I include this dtsi, what does it help or how is it supposed to be of any
use?

In particular, the following section

|                       axp209: pmic at 34 {
|                               compatible = "x-powers,axp209";
|                               reg = <0x34>;
|                               interrupt-parent = <&nmi_intc>;
|                               interrupts = <0 IRQ_TYPE_LEVEL_LOW>;

|                               interrupt-controller;
|                               #interrupt-cells = <1>;

|                               acin-supply = <&reg_axp_ipsout>;
|                               vin2-supply = <&reg_axp_ipsout>;
|                               vin3-supply = <&reg_axp_ipsout>;
|                               ldo24in-supply = <&reg_axp_ipsout>;
|                               ldo3in-supply = <&reg_axp_ipsout>;

We should not be using reg_axp_ipsout, as it is an unregulated supply,
and can be left out.

would be needed anyway as it is not in sun7i-a20.dtsi, followed by the
regulators, to switch them on. Now a dtsi would make sense, if it shortens
the device specific information.

Should not the section above together with the full regulators be in axp209.dtsi
and only the regulators be mentioned in the boards dts, to switch them on? Isn't
this stuff that should be included in sun7i-a20.dtsi anyway, somehow?

It is entirely possible to make a board without using the PMIC, as Olimex
has demonstrated. sun7i-a20 only describes the common SoC bits.

ChenYu