Thread (42 messages) 42 messages, 5 authors, 2015-11-16

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help