From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Subject: Re: [PATCH RFC v2 5/5] spmi: document the PMIC arbiter SPMI bindings Date: Fri, 23 Aug 2013 15:55:56 -0600 Message-ID: <5217DA6C.4060903@wwwdotorg.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: Received: from avon.wwwdotorg.org ([70.85.31.133]:40106 "EHLO avon.wwwdotorg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755799Ab3HWV4B (ORCPT ); Fri, 23 Aug 2013 17:56:01 -0400 In-Reply-To: Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Josh Cartwright Cc: Grant Likely , Rob Herring , Greg Kroah-Hartman , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, Sagar Dharia , Gilad Avidov , Michael Bohan , devicetree@vger.kernel.org On 08/09/2013 02:37 PM, Josh Cartwright wrote: Patch description? > diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt > +Required properties: > +- compatible : should be "qcom,spmi-pmic-arb". > +- reg-names : should be "core", "intr", "cnfg" > +- reg : offset and length of the PMIC Arbiter Core register map. > +- reg : offset and length of the PMIC Arbiter Interrupt controller register map. > +- reg : offset and length of the PMIC Arbiter Configuration register map. This seems like it's defining the "reg" property 3 times each with a different meaning. It'd be better to say something like: reg : register specifier. Must contain 3 entries, in the following order: core registers, interrupt register, configuration registers. > + qcom,spmi@fc4c0000 { ... > + qcom,pm8841@4 { Node names typically don't include a vendor prefix. For the first instance above, I think just "spmi@fc4c0000" or even just "spmi" would be appropriate here; the latter being best in the case where there's only 1 SPMI controller and hence no need to include the unit address for uniqueness. From mboxrd@z Thu Jan 1 00:00:00 1970 From: swarren@wwwdotorg.org (Stephen Warren) Date: Fri, 23 Aug 2013 15:55:56 -0600 Subject: [PATCH RFC v2 5/5] spmi: document the PMIC arbiter SPMI bindings In-Reply-To: References: Message-ID: <5217DA6C.4060903@wwwdotorg.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 08/09/2013 02:37 PM, Josh Cartwright wrote: Patch description? > diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt > +Required properties: > +- compatible : should be "qcom,spmi-pmic-arb". > +- reg-names : should be "core", "intr", "cnfg" > +- reg : offset and length of the PMIC Arbiter Core register map. > +- reg : offset and length of the PMIC Arbiter Interrupt controller register map. > +- reg : offset and length of the PMIC Arbiter Configuration register map. This seems like it's defining the "reg" property 3 times each with a different meaning. It'd be better to say something like: reg : register specifier. Must contain 3 entries, in the following order: core registers, interrupt register, configuration registers. > + qcom,spmi at fc4c0000 { ... > + qcom,pm8841 at 4 { Node names typically don't include a vendor prefix. For the first instance above, I think just "spmi at fc4c0000" or even just "spmi" would be appropriate here; the latter being best in the case where there's only 1 SPMI controller and hence no need to include the unit address for uniqueness.