| Message ID | 1435355419-23602-6-git-send-email-bjorn.andersson@sonymobile.com |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
FAO Mark and DT chaps, > From: Bjorn Andersson <bjorn.andersson@sonymobile.com> > > Add binding documentation for the Qualcomm Resource Power Manager (RPM) > using shared memory (Qualcomm SMD) as transport mechanism. This is found > in 8974 and newer based devices. > > The binding currently describes the rpm itself and the regulator > subnodes. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > --- > .../devicetree/bindings/mfd/qcom-rpm-smd.txt | 117 +++++++++++++++++++++ > include/dt-bindings/mfd/qcom-smd-rpm.h | 28 +++++ > 2 files changed, 145 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt > create mode 100644 include/dt-bindings/mfd/qcom-smd-rpm.h > > diff --git a/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt > new file mode 100644 > index 0000000..e27f5c4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt > @@ -0,0 +1,117 @@ > +Qualcomm Resource Power Manager (RPM) over SMD > + > +This driver is used to interface with the Resource Power Manager (RPM) found in > +various Qualcomm platforms. The RPM allows each component in the system to vote > +for state of the system resources, such as clocks, regulators and bus > +frequencies. > + > +- compatible: > + Usage: required > + Value type: <string> > + Definition: must be one of: > + "qcom,rpm-msm8974" > + > +- qcom,smd-channels: > + Usage: required > + Value type: <stringlist> > + Definition: Shared Memory channel used for communication with the RPM This is going to require a DT Ack. Also, I don't see it being used anywhere. > += SUBDEVICES > + > +The RPM exposes resources to its subnodes. The below bindings specify the set > +of valid subnodes that can operate on these resources. > + > +== Regulators > + > +Regulator nodes are identified by their compatible: > + > +- compatible: > + Usage: required > + Value type: <string> > + Definition: must be one of: > + "qcom,rpm-pm8841-regulators" > + "qcom,rpm-pm8941-regulators" > + > +- vdd_s1-supply: > +- vdd_s2-supply: > +- vdd_s3-supply: > +- vdd_s4-supply: > +- vdd_s5-supply: > +- vdd_s6-supply: > +- vdd_s7-supply: > +- vdd_s8-supply: > + Usage: optional (pm8841 only) > + Value type: <phandle> > + Definition: reference to regulator supplying the input pin, as > + described in the data sheet > + > +- vdd_s1-supply: > +- vdd_s2-supply: > +- vdd_s3-supply: > +- vdd_l1_l3-supply: > +- vdd_l2_lvs1_2_3-supply: > +- vdd_l4_l11-supply: > +- vdd_l5_l7-supply: > +- vdd_l6_l12_l14_l15-supply: > +- vdd_l8_l16_l18_l19-supply: > +- vdd_l9_l10_l17_l22-supply: > +- vdd_l13_l20_l23_l24-supply: > +- vdd_l21-supply: > +- vin_5vs-supply: > + Usage: optional (pm8941 only) > + Value type: <phandle> > + Definition: reference to regulator supplying the input pin, as > + described in the data sheet > + > +The regulator node houses sub-nodes for each regulator within the device. Each > +sub-node is identified using the node's name, with valid values listed for each > +of the pmics below. > + > +pm8841: > + s1, s2, s3, s4, s5, s6, s7, s8 > + > +pm8941: > + s1, s2, s3, s4, l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, > + l14, l15, l16, l17, l18, l19, l20, l21, l22, l23, l24, lvs1, lvs2, > + lvs3, 5vs1, 5vs2 > + > +The content of each sub-node is defined by the standard binding for regulators - > +see regulator.txt. s-regulator.txt-../regulator/regulator.txt- > + > += EXAMPLE > + > + smd { > + compatible = "qcom,smd"; Is an SMD (Shared Memory Device?) real hardware? > + rpm { > + interrupts = <0 168 1>; > + qcom,ipc = <&apcs 8 0>; > + qcom,smd-edge = <15>; The child node won't probe without a compatible string. Shouldn't "qcom,rpm-msm8974" be in here instead? > + rpm_requests { This node appears to be undocumented. Does it represent real h/w? > + compatible = "qcom,rpm-msm8974"; > + qcom,smd-channels = "rpm_requests"; > + > + pm8941-regulators { > + compatible = "qcom,rpm-pm8941-regulators"; > + vdd_l13_l20_l23_l24-supply = <&pm8941_boost>; I'd like Mark to glance at this. > + pm8941_s3: s3 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; Aren't these fixed regulators? > + }; > + > + pm8941_boost: s4 { > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + }; > + > + pm8941_l20: l20 { > + regulator-min-microvolt = <2950000>; > + regulator-max-microvolt = <2950000>; > + }; > + }; > + }; > + }; > + }; > + > diff --git a/include/dt-bindings/mfd/qcom-smd-rpm.h b/include/dt-bindings/mfd/qcom-smd-rpm.h > new file mode 100644 > index 0000000..890ca52 > --- /dev/null > +++ b/include/dt-bindings/mfd/qcom-smd-rpm.h > @@ -0,0 +1,28 @@ > +/* > + * This header provides constants for the Qualcomm RPM bindings. > + */ > + > +#ifndef _DT_BINDINGS_MFD_QCOM_SMD_RPM_H > +#define _DT_BINDINGS_MFD_QCOM_SMD_RPM_H > + > +/* > + * Constants used for addressing resources in the RPM. > + */ > +#define QCOM_SMD_RPM_BUS_CLK 0x316b6c63 > +#define QCOM_SMD_RPM_BUS_MASTER 0x73616d62 > +#define QCOM_SMD_RPM_BUS_SLAVE 0x766c7362 > +#define QCOM_SMD_RPM_CLK_BUF_A 0x616B6C63 > +#define QCOM_SMD_RPM_LDOA 0x616f646c > +#define QCOM_SMD_RPM_LDOB 0x626F646C > +#define QCOM_SMD_RPM_MEM_CLK 0x326b6c63 > +#define QCOM_SMD_RPM_MISC_CLK 0x306b6c63 > +#define QCOM_SMD_RPM_NCPA 0x6170636E > +#define QCOM_SMD_RPM_NCPB 0x6270636E > +#define QCOM_SMD_RPM_OCMEM_PWR 0x706d636f > +#define QCOM_SMD_RPM_QPIC_CLK 0x63697071 > +#define QCOM_SMD_RPM_SMPA 0x61706d73 > +#define QCOM_SMD_RPM_SMPB 0x62706d73 > +#define QCOM_SMD_RPM_SPDM 0x63707362 > +#define QCOM_SMD_RPM_VSA 0x00617376 > + > +#endif
On Tue 07 Jul 05:16 PDT 2015, Lee Jones wrote: > FAO Mark and DT chaps, > > > From: Bjorn Andersson <bjorn.andersson@sonymobile.com> > > > > Add binding documentation for the Qualcomm Resource Power Manager (RPM) > > using shared memory (Qualcomm SMD) as transport mechanism. This is found > > in 8974 and newer based devices. > > > > The binding currently describes the rpm itself and the regulator > > subnodes. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > > --- > > .../devicetree/bindings/mfd/qcom-rpm-smd.txt | 117 +++++++++++++++++++++ > > include/dt-bindings/mfd/qcom-smd-rpm.h | 28 +++++ > > 2 files changed, 145 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt > > create mode 100644 include/dt-bindings/mfd/qcom-smd-rpm.h > > > > diff --git a/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt [..] > > +- qcom,smd-channels: > > + Usage: required > > + Value type: <stringlist> > > + Definition: Shared Memory channel used for communication with the RPM > > This is going to require a DT Ack. > > Also, I don't see it being used anywhere. > It's a common property of all smd devices, defining the smd channel this driver should bind to. > > += SUBDEVICES > > + > > +The RPM exposes resources to its subnodes. The below bindings specify the set > > +of valid subnodes that can operate on these resources. > > + > > +== Regulators > > + [..] > > +The content of each sub-node is defined by the standard binding for regulators - > > +see regulator.txt. > > s-regulator.txt-../regulator/regulator.txt- > Right. > > + > > += EXAMPLE > > + > > + smd { > > + compatible = "qcom,smd"; > > Is an SMD (Shared Memory Device?) real hardware? > SMD is a mechanism for using shared memory for point-to-point communication channels with remote processors in all Qualcomm platforms. So it's not hardware, it's the control mechanism for communicating with real hardware. > > + rpm { > > + interrupts = <0 168 1>; > > + qcom,ipc = <&apcs 8 0>; > > + qcom,smd-edge = <15>; > > The child node won't probe without a compatible string. Shouldn't > "qcom,rpm-msm8974" be in here instead? > These sub-nodes represents a logical grouping of the various channels that exist to this remote processor. For the rpm there is only the "rpm_requests" channel - used for sending regulator & clock requests. > > + rpm_requests { > > This node appears to be undocumented. > This is the actual rpm device node, the smd & rpm nodes above are included for completeness of the example. They should perhaps be dropped to make this clearer. > Does it represent real h/w? > The other end of this smd channel is a micro controller that handles regulator and clock requests for the platform - so this is hardware. This is equivalent to the qcom_rpm driver, but instead of a hardware like register window this uses the same packet based messaging mechanism that's used for other remote peripherals in the Qualcomm platform. > > + compatible = "qcom,rpm-msm8974"; > > + qcom,smd-channels = "rpm_requests"; > > + > > + pm8941-regulators { > > + compatible = "qcom,rpm-pm8941-regulators"; > > + vdd_l13_l20_l23_l24-supply = <&pm8941_boost>; > > I'd like Mark to glance at this. > Right. > > + pm8941_s3: s3 { > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > Aren't these fixed regulators? > In this system configuration most of the regulators have fixed values, but the regulators (hw) are not fixed. > > + }; > > + > > + pm8941_boost: s4 { > > + regulator-min-microvolt = <5000000>; > > + regulator-max-microvolt = <5000000>; > > + }; > > + > > + pm8941_l20: l20 { > > + regulator-min-microvolt = <2950000>; > > + regulator-max-microvolt = <2950000>; > > + }; > > + }; > > + }; > > + }; > > + }; > > + Thanks, Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 13 Jul 2015, Bjorn Andersson wrote: > On Tue 07 Jul 05:16 PDT 2015, Lee Jones wrote: > > > FAO Mark and DT chaps, > > > > > From: Bjorn Andersson <bjorn.andersson@sonymobile.com> > > > > > > Add binding documentation for the Qualcomm Resource Power Manager (RPM) > > > using shared memory (Qualcomm SMD) as transport mechanism. This is found > > > in 8974 and newer based devices. > > > > > > The binding currently describes the rpm itself and the regulator > > > subnodes. > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > > > --- > > > .../devicetree/bindings/mfd/qcom-rpm-smd.txt | 117 +++++++++++++++++++++ > > > include/dt-bindings/mfd/qcom-smd-rpm.h | 28 +++++ > > > 2 files changed, 145 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt > > > create mode 100644 include/dt-bindings/mfd/qcom-smd-rpm.h > > > > > > diff --git a/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt > > [..] > > > > +- qcom,smd-channels: > > > + Usage: required > > > + Value type: <stringlist> > > > + Definition: Shared Memory channel used for communication with the RPM > > > > This is going to require a DT Ack. > > > > Also, I don't see it being used anywhere. > > It's a common property of all smd devices, defining the smd channel this > driver should bind to. Well it's not in the kernel and I can't find the patch that uses it, so my points still stand. > > > += EXAMPLE > > > + > > > + smd { > > > + compatible = "qcom,smd"; > > > > Is an SMD (Shared Memory Device?) real hardware? > > > > SMD is a mechanism for using shared memory for point-to-point > communication channels with remote processors in all Qualcomm platforms. > > So it's not hardware, it's the control mechanism for communicating with > real hardware. Then you can't have a node for it. Virtual nodes which do not represent real h/w are not allowed in Device Tree. > > > + rpm { > > > + interrupts = <0 168 1>; > > > + qcom,ipc = <&apcs 8 0>; > > > + qcom,smd-edge = <15>; > > > > The child node won't probe without a compatible string. Shouldn't > > "qcom,rpm-msm8974" be in here instead? > > > > These sub-nodes represents a logical grouping of the various channels > that exist to this remote processor. For the rpm there is only the > "rpm_requests" channel - used for sending regulator & clock requests. Again, if it's not real h/w and don't have a proper driver, there should be no reason for this node to exist. > > > + rpm_requests { > > > > This node appears to be undocumented. > > This is the actual rpm device node, the smd & rpm nodes above are > included for completeness of the example. > > They should perhaps be dropped to make this clearer. > > > Does it represent real h/w? > > > > The other end of this smd channel is a micro controller that handles > regulator and clock requests for the platform - so this is hardware. > > This is equivalent to the qcom_rpm driver, but instead of a hardware > like register window this uses the same packet based messaging mechanism > that's used for other remote peripherals in the Qualcomm platform. This needs a good review by the DT guys. > > > + compatible = "qcom,rpm-msm8974"; > > > + qcom,smd-channels = "rpm_requests"; > > > + > > > + pm8941-regulators { > > > + compatible = "qcom,rpm-pm8941-regulators"; > > > + vdd_l13_l20_l23_l24-supply = <&pm8941_boost>; > > > > I'd like Mark to glance at this. > > > > Right. > > > > + pm8941_s3: s3 { > > > + regulator-min-microvolt = <1800000>; > > > + regulator-max-microvolt = <1800000>; > > > > Aren't these fixed regulators? > > > > In this system configuration most of the regulators have fixed values, > but the regulators (hw) are not fixed. I'm not sure that's how it works. I believe 'max' and 'min' should describe the upper and lower constraints of the regulator. The actual value it runs it is selected elsewhere. We still need Mark to look at this. > > > + }; > > > + > > > + pm8941_boost: s4 { > > > + regulator-min-microvolt = <5000000>; > > > + regulator-max-microvolt = <5000000>; > > > + }; > > > + > > > + pm8941_l20: l20 { > > > + regulator-min-microvolt = <2950000>; > > > + regulator-max-microvolt = <2950000>; > > > + }; > > > + }; > > > + }; > > > + }; > > > + }; > > > + > > Thanks, > Bjorn
On Thu 23 Jul 06:31 PDT 2015, Lee Jones wrote: > On Mon, 13 Jul 2015, Bjorn Andersson wrote: > > > On Tue 07 Jul 05:16 PDT 2015, Lee Jones wrote: > > > > > FAO Mark and DT chaps, > > > > > > > From: Bjorn Andersson <bjorn.andersson@sonymobile.com> > > > > > > > > Add binding documentation for the Qualcomm Resource Power Manager (RPM) > > > > using shared memory (Qualcomm SMD) as transport mechanism. This is found > > > > in 8974 and newer based devices. > > > > > > > > The binding currently describes the rpm itself and the regulator > > > > subnodes. > > > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > > > > --- > > > > .../devicetree/bindings/mfd/qcom-rpm-smd.txt | 117 +++++++++++++++++++++ > > > > include/dt-bindings/mfd/qcom-smd-rpm.h | 28 +++++ > > > > 2 files changed, 145 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt > > > > create mode 100644 include/dt-bindings/mfd/qcom-smd-rpm.h > > > > > > > > diff --git a/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt > > > > [..] > > > > > > +- qcom,smd-channels: > > > > + Usage: required > > > > + Value type: <stringlist> > > > > + Definition: Shared Memory channel used for communication with the RPM > > > > > > This is going to require a DT Ack. > > > > > > Also, I don't see it being used anywhere. > > > > It's a common property of all smd devices, defining the smd channel this > > driver should bind to. > > Well it's not in the kernel and I can't find the patch that uses it, > so my points still stand. > Patch 3 in this series defines the binding and patch 4 is an implementation of this binding. > > > > += EXAMPLE > > > > + > > > > + smd { > > > > + compatible = "qcom,smd"; > > > > > > Is an SMD (Shared Memory Device?) real hardware? > > > > > > > SMD is a mechanism for using shared memory for point-to-point > > communication channels with remote processors in all Qualcomm platforms. > > > > So it's not hardware, it's the control mechanism for communicating with > > real hardware. > > Then you can't have a node for it. Virtual nodes which do not > represent real h/w are not allowed in Device Tree. > It represent the structure of a Qualcomm platform, but there's not a 1:1 mapping between this node and a discrete component. And without the information it carries there's no way for us to reach e.g. the RPM - which is a discrete physical component. But I understand that this discussion should be held on patch 3 and with the DT maintainers. > > > > + rpm { > > > > + interrupts = <0 168 1>; > > > > + qcom,ipc = <&apcs 8 0>; > > > > + qcom,smd-edge = <15>; > > > > > > The child node won't probe without a compatible string. Shouldn't > > > "qcom,rpm-msm8974" be in here instead? > > > > > > > These sub-nodes represents a logical grouping of the various channels > > that exist to this remote processor. For the rpm there is only the > > "rpm_requests" channel - used for sending regulator & clock requests. > > Again, if it's not real h/w and don't have a proper driver, there > should be no reason for this node to exist. > We need to get hold of the interrupts and that regmap to be able to communicate with the RPM. If this information is not in DT there will be no communication - further we can not move it into the RPM node as with all other remote processors (WiFi, DSP etc) these resources are shared between a number of drivers. > > > > + rpm_requests { > > > > > > This node appears to be undocumented. > > > > This is the actual rpm device node, the smd & rpm nodes above are > > included for completeness of the example. > > > > They should perhaps be dropped to make this clearer. > > > > > Does it represent real h/w? > > > > > > > The other end of this smd channel is a micro controller that handles > > regulator and clock requests for the platform - so this is hardware. > > > > This is equivalent to the qcom_rpm driver, but instead of a hardware > > like register window this uses the same packet based messaging mechanism > > that's used for other remote peripherals in the Qualcomm platform. > > This needs a good review by the DT guys. > Sure > > > > + compatible = "qcom,rpm-msm8974"; > > > > + qcom,smd-channels = "rpm_requests"; > > > > + > > > > + pm8941-regulators { > > > > + compatible = "qcom,rpm-pm8941-regulators"; > > > > + vdd_l13_l20_l23_l24-supply = <&pm8941_boost>; > > > > > > I'd like Mark to glance at this. > > > > > > > Right. > > > > > > + pm8941_s3: s3 { > > > > + regulator-min-microvolt = <1800000>; > > > > + regulator-max-microvolt = <1800000>; > > > > > > Aren't these fixed regulators? > > > > > > > In this system configuration most of the regulators have fixed values, > > but the regulators (hw) are not fixed. > > I'm not sure that's how it works. I believe 'max' and 'min' should > describe the upper and lower constraints of the regulator. The actual > value it runs it is selected elsewhere. > The specified range of the regulator is 1.75-1.85V and this is handled by the implementation, however the board designers have stated that it is only allowed to be configured to 1.8V. So DT is used to narrow the capabilities of the individual component to something that's suitable for this particular system. > We still need Mark to look at this. > Mark, would you mind giving us a statement on the regulator subnode of this binding? Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 23, 2015 at 09:41:28AM -0700, Bjorn Andersson wrote: > > We still need Mark to look at this. > Mark, would you mind giving us a statement on the regulator subnode of > this binding? I have no idea what's going on here, sorry. I've not been reading this thread.
On Thu, 23 Jul 2015, Mark Brown wrote: > On Thu, Jul 23, 2015 at 09:41:28AM -0700, Bjorn Andersson wrote: > > > > We still need Mark to look at this. > > > Mark, would you mind giving us a statement on the regulator subnode of > > this binding? > > I have no idea what's going on here, sorry. I've not been reading this > thread. All of the information you need is in the email you replied to.
On Fri, Jul 24, 2015 at 10:58:47AM +0100, Lee Jones wrote: > On Thu, 23 Jul 2015, Mark Brown wrote: > > On Thu, Jul 23, 2015 at 09:41:28AM -0700, Bjorn Andersson wrote: > > > > We still need Mark to look at this. > > > Mark, would you mind giving us a statement on the regulator subnode of > > > this binding? > > I have no idea what's going on here, sorry. I've not been reading this > > thread. > All of the information you need is in the email you replied to. The mail appears to have an edited down section of what looks like an example rather than the actual binding. I haven't seen the binding so I can't really comment on it.
On Fri, Jul 24, 2015 at 11:24:34AM +0100, Mark Brown wrote: > On Fri, Jul 24, 2015 at 10:58:47AM +0100, Lee Jones wrote: > > On Thu, 23 Jul 2015, Mark Brown wrote: > > > I have no idea what's going on here, sorry. I've not been reading this > > > thread. > > All of the information you need is in the email you replied to. > The mail appears to have an edited down section of what looks like an > example rather than the actual binding. I haven't seen the binding so I > can't really comment on it. I managed to find the binding but I'm afraid I'm still not sure what the concern here is - can someone please be specific about the question(s) you're looking for an answer on?
On Fri, 24 Jul 2015, Mark Brown wrote: > On Fri, Jul 24, 2015 at 11:24:34AM +0100, Mark Brown wrote: > > On Fri, Jul 24, 2015 at 10:58:47AM +0100, Lee Jones wrote: > > > On Thu, 23 Jul 2015, Mark Brown wrote: > > > > > I have no idea what's going on here, sorry. I've not been reading this > > > > thread. > > > > All of the information you need is in the email you replied to. > > > The mail appears to have an edited down section of what looks like an > > example rather than the actual binding. I haven't seen the binding so I > > can't really comment on it. > > I managed to find the binding but I'm afraid I'm still not sure what the > concern here is - can someone please be specific about the question(s) > you're looking for an answer on? [...] >From here: > > > > > + pm8941-regulators { > > > > > + compatible = "qcom,rpm-pm8941-regulators"; > > > > > + vdd_l13_l20_l23_l24-supply = <&pm8941_boost>; > > > > > > > > I'd like Mark to glance at this. Mark: Is this new property okay? > > > > > + pm8941_s3: s3 { > > > > > + regulator-min-microvolt = <1800000>; > > > > > + regulator-max-microvolt = <1800000>; > > > > > > > > Aren't these fixed regulators? > > > > > > In this system configuration most of the regulators have fixed values, > > > but the regulators (hw) are not fixed. > > > > I'm not sure that's how it works. I believe 'max' and 'min' should > > describe the upper and lower constraints of the regulator. The actual > > value it runs it is selected elsewhere. > > The specified range of the regulator is 1.75-1.85V and this is handled > by the implementation, however the board designers have stated that it > is only allowed to be configured to 1.8V. > > So DT is used to narrow the capabilities of the individual component to > something that's suitable for this particular system. > > > We still need Mark to look at this. Is it okay for the regulator-{min,max}-microvolt to be artificially restricted to the required value, despite knowing that the regulator is capable of supply {more,less} voltage?
On Mon, Jul 27, 2015 at 08:29:18AM +0100, Lee Jones wrote: > On Fri, 24 Jul 2015, Mark Brown wrote: > > On Fri, Jul 24, 2015 at 11:24:34AM +0100, Mark Brown wrote: > From here: > > > > > > + pm8941-regulators { > > > > > > + compatible = "qcom,rpm-pm8941-regulators"; > > > > > > + vdd_l13_l20_l23_l24-supply = <&pm8941_boost>; > > > > > I'd like Mark to glance at this. > Mark: Is this new property okay? As far as I can see that looks like a standard supply property, assuming the supply is actually called that why would it be an issue? > > The specified range of the regulator is 1.75-1.85V and this is handled > > by the implementation, however the board designers have stated that it > > is only allowed to be configured to 1.8V. > > So DT is used to narrow the capabilities of the individual component to > > something that's suitable for this particular system. > > > We still need Mark to look at this. > Is it okay for the regulator-{min,max}-microvolt to be artificially > restricted to the required value, despite knowing that the regulator > is capable of supply {more,less} voltage? Yes, that's the entire purpose of those properties - to set the limits the board designers have which will typically be more restrictive than those that the regulator itself is capable of imposing.
On Mon, 27 Jul 2015, Mark Brown wrote: > On Mon, Jul 27, 2015 at 08:29:18AM +0100, Lee Jones wrote: > > On Fri, 24 Jul 2015, Mark Brown wrote: > > > On Fri, Jul 24, 2015 at 11:24:34AM +0100, Mark Brown wrote: > > > From here: > > > > > > > > + pm8941-regulators { > > > > > > > + compatible = "qcom,rpm-pm8941-regulators"; > > > > > > > + vdd_l13_l20_l23_l24-supply = <&pm8941_boost>; > > > > > > > I'd like Mark to glance at this. > > > Mark: Is this new property okay? > > As far as I can see that looks like a standard supply property, assuming > the supply is actually called that why would it be an issue? > > > > The specified range of the regulator is 1.75-1.85V and this is handled > > > by the implementation, however the board designers have stated that it > > > is only allowed to be configured to 1.8V. > > > > So DT is used to narrow the capabilities of the individual component to > > > something that's suitable for this particular system. > > > > > We still need Mark to look at this. > > > Is it okay for the regulator-{min,max}-microvolt to be artificially > > restricted to the required value, despite knowing that the regulator > > is capable of supply {more,less} voltage? > > Yes, that's the entire purpose of those properties - to set the limits > the board designers have which will typically be more restrictive than > those that the regulator itself is capable of imposing. All fine then. Please re-submit with the changes we discussed.
diff --git a/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt new file mode 100644 index 0000000..e27f5c4 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt @@ -0,0 +1,117 @@ +Qualcomm Resource Power Manager (RPM) over SMD + +This driver is used to interface with the Resource Power Manager (RPM) found in +various Qualcomm platforms. The RPM allows each component in the system to vote +for state of the system resources, such as clocks, regulators and bus +frequencies. + +- compatible: + Usage: required + Value type: <string> + Definition: must be one of: + "qcom,rpm-msm8974" + +- qcom,smd-channels: + Usage: required + Value type: <stringlist> + Definition: Shared Memory channel used for communication with the RPM + += SUBDEVICES + +The RPM exposes resources to its subnodes. The below bindings specify the set +of valid subnodes that can operate on these resources. + +== Regulators + +Regulator nodes are identified by their compatible: + +- compatible: + Usage: required + Value type: <string> + Definition: must be one of: + "qcom,rpm-pm8841-regulators" + "qcom,rpm-pm8941-regulators" + +- vdd_s1-supply: +- vdd_s2-supply: +- vdd_s3-supply: +- vdd_s4-supply: +- vdd_s5-supply: +- vdd_s6-supply: +- vdd_s7-supply: +- vdd_s8-supply: + Usage: optional (pm8841 only) + Value type: <phandle> + Definition: reference to regulator supplying the input pin, as + described in the data sheet + +- vdd_s1-supply: +- vdd_s2-supply: +- vdd_s3-supply: +- vdd_l1_l3-supply: +- vdd_l2_lvs1_2_3-supply: +- vdd_l4_l11-supply: +- vdd_l5_l7-supply: +- vdd_l6_l12_l14_l15-supply: +- vdd_l8_l16_l18_l19-supply: +- vdd_l9_l10_l17_l22-supply: +- vdd_l13_l20_l23_l24-supply: +- vdd_l21-supply: +- vin_5vs-supply: + Usage: optional (pm8941 only) + Value type: <phandle> + Definition: reference to regulator supplying the input pin, as + described in the data sheet + +The regulator node houses sub-nodes for each regulator within the device. Each +sub-node is identified using the node's name, with valid values listed for each +of the pmics below. + +pm8841: + s1, s2, s3, s4, s5, s6, s7, s8 + +pm8941: + s1, s2, s3, s4, l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, + l14, l15, l16, l17, l18, l19, l20, l21, l22, l23, l24, lvs1, lvs2, + lvs3, 5vs1, 5vs2 + +The content of each sub-node is defined by the standard binding for regulators - +see regulator.txt. + += EXAMPLE + + smd { + compatible = "qcom,smd"; + + rpm { + interrupts = <0 168 1>; + qcom,ipc = <&apcs 8 0>; + qcom,smd-edge = <15>; + + rpm_requests { + compatible = "qcom,rpm-msm8974"; + qcom,smd-channels = "rpm_requests"; + + pm8941-regulators { + compatible = "qcom,rpm-pm8941-regulators"; + vdd_l13_l20_l23_l24-supply = <&pm8941_boost>; + + pm8941_s3: s3 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + pm8941_boost: s4 { + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + }; + + pm8941_l20: l20 { + regulator-min-microvolt = <2950000>; + regulator-max-microvolt = <2950000>; + }; + }; + }; + }; + }; + diff --git a/include/dt-bindings/mfd/qcom-smd-rpm.h b/include/dt-bindings/mfd/qcom-smd-rpm.h new file mode 100644 index 0000000..890ca52 --- /dev/null +++ b/include/dt-bindings/mfd/qcom-smd-rpm.h @@ -0,0 +1,28 @@ +/* + * This header provides constants for the Qualcomm RPM bindings. + */ + +#ifndef _DT_BINDINGS_MFD_QCOM_SMD_RPM_H +#define _DT_BINDINGS_MFD_QCOM_SMD_RPM_H + +/* + * Constants used for addressing resources in the RPM. + */ +#define QCOM_SMD_RPM_BUS_CLK 0x316b6c63 +#define QCOM_SMD_RPM_BUS_MASTER 0x73616d62 +#define QCOM_SMD_RPM_BUS_SLAVE 0x766c7362 +#define QCOM_SMD_RPM_CLK_BUF_A 0x616B6C63 +#define QCOM_SMD_RPM_LDOA 0x616f646c +#define QCOM_SMD_RPM_LDOB 0x626F646C +#define QCOM_SMD_RPM_MEM_CLK 0x326b6c63 +#define QCOM_SMD_RPM_MISC_CLK 0x306b6c63 +#define QCOM_SMD_RPM_NCPA 0x6170636E +#define QCOM_SMD_RPM_NCPB 0x6270636E +#define QCOM_SMD_RPM_OCMEM_PWR 0x706d636f +#define QCOM_SMD_RPM_QPIC_CLK 0x63697071 +#define QCOM_SMD_RPM_SMPA 0x61706d73 +#define QCOM_SMD_RPM_SMPB 0x62706d73 +#define QCOM_SMD_RPM_SPDM 0x63707362 +#define QCOM_SMD_RPM_VSA 0x00617376 + +#endif