Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC
From: Andrew F. Davis <hidden>
Date: 2015-11-10 17:52:30
Also in:
lkml
On 11/10/2015 11:04 AM, Mark Brown wrote:
On Tue, Nov 10, 2015 at 10:47:33AM -0600, Andrew F. Davis wrote:quoted
On 11/10/2015 03:57 AM, Mark Brown wrote:quoted
quoted
Of course this is a negative review of the binding! What on earth did you think my feedback meant? The driver and the binding go together.quoted
The bindings should be driver/platform/OS agnostic, changing the bindings because the Linux regulator subsystem maintainer doesn't like them in regulator drivers is then not correct.quoted
If the binding is accepted then the regulator driver will just have to deal with it, so as I said, why not nack the bindings patch, and explain your objection where DT maintainers might see it.If I'm not going to merge the driver because of issues in the DT code it is vanishingly unlikely that I'm going to merge the regulator bindings either. I would have thought it should be clear that my review comments cover both the manifestation of the bindings in the driver and the bindings themselves.
Kind of an interesting situation, if I didn't have the regulator as a separate
node like you want, then I wouldn't really need a separate regulator binding Doc,
for you to merge, it could all be merged as a single MFD binding.
Anyway, All I'm trying to do here is keep things clean in the DT. We only have
one consistent option:
Match all sub parts by compatible:
tps65912: tps65912@2d {
compatible = "ti,tps65912";
reg = <0x58>;
interrupts ...
regulator {
compatible = "ti,tps65912-regulator";
dcdc1 {
regulator-name = "vdd_core";
regulator-min-microvolt = <912000>;
regulator-max-microvolt = <1144000>;
};
...
};
pwrbutton {
compatible = "ti,palmas-pwrbutton";
interrupt-parent = <&tps65912>;
interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
wakeup-source;
ti,palmas-long-press-seconds = <12>;
};
gpio {
compatible = "ti,palmas-gpio";
gpio-controller;
#gpio-cells = <2>;
};
...
};
Or we end up with some hybrid approach, matching some on node name, others
on compatible when needed. Yes, the above matches Linux device model (still
not sure why that is such a problem?), but it also matches modular functionality
in the device.