Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752902AbbGWNbt (ORCPT ); Thu, 23 Jul 2015 09:31:49 -0400 Received: from mail-wi0-f170.google.com ([209.85.212.170]:34689 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751768AbbGWNbk (ORCPT ); Thu, 23 Jul 2015 09:31:40 -0400 Date: Thu, 23 Jul 2015 14:31:33 +0100 From: Lee Jones To: Bjorn Andersson Cc: bjorn@kryo.se, Andy Gross , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Samuel Ortiz , devicetree@vger.kernel.org, Mark Brown , linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org Subject: Re: [PATCH v2 05/11] mfd: devicetree: bindings: Add Qualcomm SMD based RPM DT binding Message-ID: <20150723133133.GB3436@x1> References: <1435355419-23602-1-git-send-email-bjorn.andersson@sonymobile.com> <1435355419-23602-6-git-send-email-bjorn.andersson@sonymobile.com> <20150707121609.GZ3182@x1> <20150713214836.GA15178@usrtlx11787.corpusers.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20150713214836.GA15178@usrtlx11787.corpusers.net> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4881 Lines: 154 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 > > > > > > 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 > > > --- > > > .../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: > > > + 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 -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/