| Message ID | 1420558143-26276-3-git-send-email-ezequiel.garcia@imgtec.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, Jan 6, 2015 at 9:29 AM, Ezequiel Garcia <ezequiel.garcia@imgtec.com> wrote: > From: Phani Movva <Phani.Movva@imgtec.com> > > Add the devicetree binding document for Cosmic Circuits 10001 ADC device. > > Signed-off-by: Phani Movva <Phani.Movva@imgtec.com> > Signed-off-by: Naidu Tellapati <Naidu.Tellapati@imgtec.com> > [Ezequiel: minor style cleaning] > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com> > --- > .../devicetree/bindings/iio/adc/cc10001_adc.txt | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt > > diff --git a/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt > new file mode 100644 > index 0000000..b7ba558 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt > @@ -0,0 +1,22 @@ > +* Cosmic Circuits - Analog to Digital Converter (CC-10001-ADC) > + > +Required properties: > + - compatible: Should be "cosmic,10001-adc" > + - reg: Should contain adc registers location and length. > + - clock-names: Should contain "adc". > + - clocks: Should contain a clock specifier for each entry in clock-names > + - vref-supply: The regulator supply ADC reference voltage. > + > +Optional properties: > + - cosmic,adc-reserved-channels: Bitmask of reserved channels, > + i.e. channels that cannot be used by the OS. Seems like this could be pretty common for any ADC as well as having the number of channels, so you can drop the vendor prefix. Do we have any similar properties already in other drivers? I'd reverse the polarity to be enabled channels, then the number of available channels is the top set bit plus 1. You could count channels as is, but then people have to remember to set the high bits on non-existent channels. Absence of the property could still mean you know how many channels and all are enabled. Rob -- 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
Hi Rob, Thanks a lot for the comments. On 01/06/2015 01:30 PM, Rob Herring wrote: > On Tue, Jan 6, 2015 at 9:29 AM, Ezequiel Garcia > <ezequiel.garcia@imgtec.com> wrote: >> From: Phani Movva <Phani.Movva@imgtec.com> >> >> Add the devicetree binding document for Cosmic Circuits 10001 ADC device. >> >> Signed-off-by: Phani Movva <Phani.Movva@imgtec.com> >> Signed-off-by: Naidu Tellapati <Naidu.Tellapati@imgtec.com> >> [Ezequiel: minor style cleaning] >> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com> >> --- >> .../devicetree/bindings/iio/adc/cc10001_adc.txt | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt >> >> diff --git a/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt >> new file mode 100644 >> index 0000000..b7ba558 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt >> @@ -0,0 +1,22 @@ >> +* Cosmic Circuits - Analog to Digital Converter (CC-10001-ADC) >> + >> +Required properties: >> + - compatible: Should be "cosmic,10001-adc" >> + - reg: Should contain adc registers location and length. >> + - clock-names: Should contain "adc". >> + - clocks: Should contain a clock specifier for each entry in clock-names >> + - vref-supply: The regulator supply ADC reference voltage. >> + >> +Optional properties: >> + - cosmic,adc-reserved-channels: Bitmask of reserved channels, >> + i.e. channels that cannot be used by the OS. > > Seems like this could be pretty common for any ADC as well as having > the number of channels, so you can drop the vendor prefix. Do we have > any similar properties already in other drivers? Well, it seems the AT91 ADC has a similar property called adc-channels-used. Not sure if it means the same as here. We are not using it to model a hardware availability but the channels being reserved for another core or OS. See [1]. > I'd reverse the > polarity to be enabled channels, then the number of available channels > is the top set bit plus 1. You could count channels as is, but then > people have to remember to set the high bits on non-existent channels. Hm, isn't this too cumbersome? If we really need to pass the number of channels, shouldn't we have a separate property for that? Note that no driver currently needs it. > Absence of the property could still mean you know how many channels > and all are enabled. > This property was originally proposed to be "called cosmic,adc-available-channels". Andrew suggested to reverse the polarity, and be explicit about absence of property meaning no reserved channels (also in [1]). I'm fine either way. [1] http://www.spinics.net/lists/linux-iio/msg15578.html
On Tue, Jan 6, 2015 at 10:59 AM, Ezequiel Garcia <ezequiel.garcia@imgtec.com> wrote: > Hi Rob, > > Thanks a lot for the comments. > > On 01/06/2015 01:30 PM, Rob Herring wrote: >> On Tue, Jan 6, 2015 at 9:29 AM, Ezequiel Garcia >> <ezequiel.garcia@imgtec.com> wrote: >>> From: Phani Movva <Phani.Movva@imgtec.com> >>> >>> Add the devicetree binding document for Cosmic Circuits 10001 ADC device. >>> >>> Signed-off-by: Phani Movva <Phani.Movva@imgtec.com> >>> Signed-off-by: Naidu Tellapati <Naidu.Tellapati@imgtec.com> >>> [Ezequiel: minor style cleaning] >>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com> >>> --- >>> .../devicetree/bindings/iio/adc/cc10001_adc.txt | 22 ++++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt >>> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt >>> new file mode 100644 >>> index 0000000..b7ba558 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt >>> @@ -0,0 +1,22 @@ >>> +* Cosmic Circuits - Analog to Digital Converter (CC-10001-ADC) >>> + >>> +Required properties: >>> + - compatible: Should be "cosmic,10001-adc" >>> + - reg: Should contain adc registers location and length. >>> + - clock-names: Should contain "adc". >>> + - clocks: Should contain a clock specifier for each entry in clock-names >>> + - vref-supply: The regulator supply ADC reference voltage. >>> + >>> +Optional properties: >>> + - cosmic,adc-reserved-channels: Bitmask of reserved channels, >>> + i.e. channels that cannot be used by the OS. >> >> Seems like this could be pretty common for any ADC as well as having >> the number of channels, so you can drop the vendor prefix. Do we have >> any similar properties already in other drivers? > > Well, it seems the AT91 ADC has a similar property called > adc-channels-used. Not sure if it means the same as here. We are not > using it to model a hardware availability but the channels being > reserved for another core or OS. See [1]. > >> I'd reverse the >> polarity to be enabled channels, then the number of available channels >> is the top set bit plus 1. You could count channels as is, but then >> people have to remember to set the high bits on non-existent channels. > > Hm, isn't this too cumbersome? If we really need to pass the number of > channels, shouldn't we have a separate property for that? Note that no > driver currently needs it. Okay. >> Absence of the property could still mean you know how many channels >> and all are enabled. >> > > This property was originally proposed to be "called > cosmic,adc-available-channels". Andrew suggested to reverse the > polarity, and be explicit about absence of property meaning no reserved > channels (also in [1]). Okay, that's fine. I'd still suggest dropping cosmic. With that: Acked-by: Rob Herring <robh@kernel.org> Rob -- 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
diff --git a/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt new file mode 100644 index 0000000..b7ba558 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/cc10001_adc.txt @@ -0,0 +1,22 @@ +* Cosmic Circuits - Analog to Digital Converter (CC-10001-ADC) + +Required properties: + - compatible: Should be "cosmic,10001-adc" + - reg: Should contain adc registers location and length. + - clock-names: Should contain "adc". + - clocks: Should contain a clock specifier for each entry in clock-names + - vref-supply: The regulator supply ADC reference voltage. + +Optional properties: + - cosmic,adc-reserved-channels: Bitmask of reserved channels, + i.e. channels that cannot be used by the OS. + +Example: +adc: adc@18101600 { + compatible = "cosmic,10001-adc"; + reg = <0x18101600 0x24>; + cosmic,adc-reserved-channels = <0x2>; + clocks = <&adc_clk>; + clock-names = "adc"; + vref-supply = <®_1v8>; +};