2013-08-16 22:30:07

by Josh Cartwright

[permalink] [raw]
Subject: Re: [PATCH RFC 3/3] spmi: document the PMIC arbiter SPMI bindings

Hey Kumar-

Thanks for the review.

On Fri, Aug 16, 2013 at 01:53:27PM -0500, Kumar Gala wrote:
> On Aug 9, 2013, at 3:37 PM, Josh Cartwright wrote:
>
> > Signed-off-by: Josh Cartwright <[email protected]>
> > ---
> > .../devicetree/bindings/spmi/spmi-pmic-arb.txt | 26 ++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/spmi/spmi-pmic-arb.txt
>
> As this is a qcom specific binding, I think the file name should be msm-spmi-pmic-arb.txt or something like that.

Agreed. It might be nice to use a vendor prefix in the name too. How's qcom,msm-pmic-arb.txt sound?

> > diff --git a/Documentation/devicetree/bindings/spmi/spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/spmi-pmic-arb.txt
> > new file mode 100644
> > index 0000000..1c14bf4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/spmi/spmi-pmic-arb.txt
> > @@ -0,0 +1,26 @@
> > +Qualcomm SPMI Controller (PMIC Arbiter)
>
> We should get in the habit of trying to have at least one sentence about what the device is or does.

Certainly.

> > +
> > +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.
> > +- #address-cells : must be set to 1
> > +- #size-cells : must be set to 0
>
> Hmm, you aren't describe anything about child nodes.

Indeed. I'll put together a generic SPMI binding document as you had
suggested before and reference it in this binding.

Josh

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation


2013-08-16 22:16:10

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH RFC 3/3] spmi: document the PMIC arbiter SPMI bindings


On Aug 16, 2013, at 2:25 PM, Josh Cartwright wrote:

> Hey Kumar-
>
> Thanks for the review.
>
> On Fri, Aug 16, 2013 at 01:53:27PM -0500, Kumar Gala wrote:
>> On Aug 9, 2013, at 3:37 PM, Josh Cartwright wrote:
>>
>>> Signed-off-by: Josh Cartwright <[email protected]>
>>> ---
>>> .../devicetree/bindings/spmi/spmi-pmic-arb.txt | 26 ++++++++++++++++++++++
>>> 1 file changed, 26 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/spmi/spmi-pmic-arb.txt
>>
>> As this is a qcom specific binding, I think the file name should be msm-spmi-pmic-arb.txt or something like that.
>
> Agreed. It might be nice to use a vendor prefix in the name too. How's qcom,msm-pmic-arb.txt sound?

Sounds good, as I see we have other examples of having a comma in file names for bindings.

- k

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2013-08-17 00:33:14

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH RFC 3/3] spmi: document the PMIC arbiter SPMI bindings

On 08/16/2013 01:48 PM, Kumar Gala wrote:
>
> On Aug 16, 2013, at 2:25 PM, Josh Cartwright wrote:
>
>> Hey Kumar-
>>
>> Thanks for the review.
>>
>> On Fri, Aug 16, 2013 at 01:53:27PM -0500, Kumar Gala wrote:
>>> On Aug 9, 2013, at 3:37 PM, Josh Cartwright wrote:
>>>
>>>> Signed-off-by: Josh Cartwright <[email protected]>
>>>> ---
>>>> .../devicetree/bindings/spmi/spmi-pmic-arb.txt | 26 ++++++++++++++++++++++
>>>> 1 file changed, 26 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/spmi/spmi-pmic-arb.txt
>>>
>>> As this is a qcom specific binding, I think the file name should be msm-spmi-pmic-arb.txt or something like that.
>>
>> Agreed. It might be nice to use a vendor prefix in the name too. How's qcom,msm-pmic-arb.txt sound?
>
> Sounds good, as I see we have other examples of having a comma in file names for bindings.

The rule I've applied for bindings I created is use the complete
compatible value for the file name. Where multiple are supported (e.g.
SoC versions), just use the first one. Then, the filename is pretty
unambiguous, both from a point of view of `ls` and from choosing a name.