On 29/05/14 19:38, Bjorn Andersson wrote:
On Thu, May 29, 2014 at 9:19 AM, Srinivas Kandagatla
<srinivas.kandagatla@xxxxxxxxxx> wrote:
+- reg:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: two entries specifying the RPM's message ram and ipc
register
+
+- reg-names:
+ Usage: required
+ Value type: <string-array>
+ Definition: must contain the following, in order:
+ "msg_ram"
+ "ipc"
+1 for kumar's comment.
cant enforce the order here. should fix it in the driver.
Yes I can, this is as decided by the devicetree maintainers. The order
of e.g. reg and interrupts must be defined.
Does not make sense. Unless Am missing something obvious.
Having reg-names/interrupt-names would give driver flexibility to get
the resources by name, as long as the order of reg and reg-names match.
So the order of reg is really not really necessary. Unless the driver is
coded to access it via index.
Hardly 1/2 bindings documents enforce this.
+= SUBDEVICES
+
+The RPM exposes resources to its subnodes. The below bindings specify the
set
+of valid subnodes that can operate on these resources.
Why should these devices be on sub nodes?
Any reason not to implement it like this,
rpm: rpm@108000 {
compatible = "qcom,rpm-msm8960";
reg = <0x108000 0x1000 0x2011008 0x4>;
interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
interrupt-names = "ack", "err", "wakeup";
};
pm8921_s1: pm8921-s1 {
compatible = "qcom,rpm-pm8921-smps";
regulator-min-microvolt = <1225000>;
regulator-max-microvolt = <1225000>;
regulator-always-on;
qcom,rpm = <&rpm QCOM_RPM_PM8921_S1>;
qcom,switch-mode-frequency = <3200000>;
qcom,hpm-threshold = <100000>;
};
This would simplify the driver code too and handle the interface neatly then
depending on device hierarchy.
rpm would be a interface library to the clients. Makes the drivers more
independent, and re-usable if we do this way.
The subnodes doesn't describe separate pieces of hardware but rather
pieces of the rpm, that's why they should live inside the rpm. There
will not be any re-use of these drivers outside having a rpm as
parent.
I do have some patches for family b, where I'm moving things around a
little bit in the implementation to be able to re-use child-devices
where that makes sense (clock implementation is the same for the two).
But that is implementation specific and does not affect the dt.
Good point, Am more of thinking of some other SOCs might have similar pmic.
Implementation wise, having the individual subnodes as children in the
device model makes a lot of sense, as the children should be probed
when the rpm appears and when the rpm goes away it should bring down
all subnodes. If there was any power management it would be the same
thing.
Thats great, you have already thought about it.
So I think this makes for a cleaner implementation; all I need to do
is to call of_platform_populate at the end of the probe and in remove
I need to tell the children that they should go away. I do not need to
support any phandle based lookups and separate life cycle management.
Am ok with either approaches.
[...
+- qcom,force-mode-none:
+ Usage: optional (default if no other qcom,force-mode is specified)
+ Value type: <empty>
+ Defintion: indicates that the regulator should not be forced to
any
+ particular mode
+
+- qcom,force-mode-lpm:
+ Usage: optional
+ Value type: <empty>
+ Definition: indicates that the regulator should be forced to
operate in
+ low-power-mode
+
+- qcom,force-mode-auto:
+ Usage: optional (only available for 8960/8064)
+ Value type: <empty>
+ Definition: indicates that the regulator should be automatically
pick
+ operating mode
+
+- qcom,force-mode-hpm:
+ Usage: optional (only available for 8960/8064)
+ Value type: <empty>
+ Definition: indicates that the regulator should be forced to
operate in
+ high-power-mode
+
+- qcom,force-mode-bypass: (only for 8960/8064)
+ Usage: optional (only available for 8960/8064)
+ Value type: <empty>
+ Definition: indicates that the regulator should be forced to
operate in
+ bypass mode
+
...]
Probably qcom,force-mode:
Usage: optional.
Value type: <string>
Definition: must be one of:
"none"
"lpm"
"auto"
"hpm"
"bypass"
Makes it much simpler, as they seems to be mutually exclusive. simillar
comments apply to other bindings too.
Please see my answer to Kumar.
Ok. I don’t have a strong feeling on any of those 3 approaches.
Thanks,
srini
Thanks for the comments!
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
