From: Marius Cristea <[email protected]>
Adding support for Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
Delta-Sigma ADCs with an SPI interface. This driver covers the following part
numbers:
- MCP3561, MCP3562, MCP3564, MCP3561R, MCP3562R, MCP3564R,
- MCP3461, MCP3462, MCP3464, MCP3461R, MCP3462R and MCP3464R.
Differences related to previous patch:
v2:
- fix review comments:
- change the device tree bindings
- change the ADC channel creation (starting from DT)
- use defines, masks and FIELD_PREP() instead of hardcoded values
- mode the PGA from Hardware Gain to scale
- add a current output channel from burnout current
- fix coding style issues
- use self explanatory naming to drop the comment
- renumbered the versioning (start with v1 instead of v0)
v1:
- first version comitted to review
Marius Cristea (2):
dt-bindings: iio: adc: adding MCP3564 ADC
iio: adc: adding support for MCP3564 ADC
.../ABI/testing/sysfs-bus-iio-adc-mcp3564 | 53 +
.../bindings/iio/adc/microchip,mcp3564.yaml | 197 +++
MAINTAINERS | 7 +
drivers/iio/adc/Kconfig | 13 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3564.c | 1485 +++++++++++++++++
6 files changed, 1756 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-mcp3564
create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,mcp3564.yaml
create mode 100644 drivers/iio/adc/mcp3564.c
base-commit: 9e66fb52449538406cea43e9f3889c391350e76e
--
2.34.1
From: Marius Cristea <[email protected]>
This is the device tree schema for iio driver for
Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
Delta-Sigma ADCs with an SPI interface (Microchip's
MCP3461, MCP3462, MCP3464, MCP3461R, MCP3462R,
MCP3464R, MCP3561, MCP3562, MCP3564, MCP3561R,
MCP3562R and MCP3564R analog to digital converters).
Signed-off-by: Marius Cristea <[email protected]>
---
.../bindings/iio/adc/microchip,mcp3564.yaml | 197 ++++++++++++++++++
1 file changed, 197 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,mcp3564.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,mcp3564.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,mcp3564.yaml
new file mode 100644
index 000000000000..297b77eb1cb0
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/microchip,mcp3564.yaml
@@ -0,0 +1,197 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/microchip,mcp3564.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MCP346X and MCP356X ADC Family
+
+maintainers:
+ - Marius Cristea <[email protected]>
+
+description: |
+ Bindings for the Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
+ Delta-Sigma ADCs with an SPI interface. Datasheet can be found here:
+ Datasheet for MCP3561, MCP3562, MCP3564 can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP3561-2-4-Family-Data-Sheet-DS20006181C.pdf
+ Datasheet for MCP3561R, MCP3562R, MCP3564R can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/APID/ProductDocuments/DataSheets/MCP3561_2_4R-Data-Sheet-DS200006391C.pdf
+ Datasheet for MCP3461, MCP3462, MCP3464 can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/APID/ProductDocuments/DataSheets/MCP3461-2-4-Two-Four-Eight-Channel-153.6-ksps-Low-Noise-16-Bit-Delta-Sigma-ADC-Data-Sheet-20006180D.pdf
+ Datasheet for MCP3461R, MCP3462R, MCP3464R can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/APID/ProductDocuments/DataSheets/MCP3461-2-4R-Family-Data-Sheet-DS20006404C.pdf
+
+properties:
+ compatible:
+ enum:
+ - microchip,mcp3461
+ - microchip,mcp3462
+ - microchip,mcp3464
+ - microchip,mcp3461r
+ - microchip,mcp3462r
+ - microchip,mcp3464r
+ - microchip,mcp3561
+ - microchip,mcp3562
+ - microchip,mcp3564
+ - microchip,mcp3561r
+ - microchip,mcp3562r
+ - microchip,mcp3564r
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 20000000
+
+ spi-cpha: true
+
+ spi-cpol: true
+
+ clocks:
+ description:
+ Phandle and clock identifier for external sampling clock.
+ If not specified, the internal crystal oscillator will be used.
+ maxItems: 1
+
+ interrupts:
+ description: IRQ line of the ADC
+ maxItems: 1
+
+ drive-open-drain:
+ description:
+ Whether to drive the IRQ signal as push-pull (default) or open-drain. Note
+ that the device requires this pin to become "high", otherwise it will stop
+ converting.
+ type: boolean
+
+ vref-supply:
+ description:
+ Some devices have a specific reference voltage supplied on a different
+ pin to the other supplies. Needed to be able to establish channel scaling
+ unless there is also an internal reference available (e.g. mcp3564r). In
+ case of "r" devices (e. g. mcp3564r), if it does not exists the internal
+ reference will be used.
+
+ microchip,hw-device-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 3
+ description:
+ The address is set on a per-device basis by fuses in the factory,
+ configured on request. If not requested, the fuses are set for 0x1.
+ The device address is part of the device markings to avoid
+ potential confusion. This address is coded on two bits, so four possible
+ addresses are available when multiple devices are present on the same
+ SPI bus with only one Chip Select line for all devices.
+ Each device communication starts by a CS falling edge, followed by the
+ clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
+ which is first one on the wire).
+
+ "#io-channel-cells":
+ const: 1
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+dependencies:
+ spi-cpol: [ spi-cpha ]
+ spi-cpha: [ spi-cpol ]
+
+patternProperties:
+ "^channel@([0-9]|([1-7][0-9]))$":
+ $ref: adc.yaml
+ type: object
+ description: Represents the external channels which are connected to the ADC.
+
+ properties:
+ reg:
+ description: The channel number in single-ended and differential mode.
+ minimum: 0
+ maximum: 79
+
+ diff-channels: true
+
+ required:
+ - reg
+
+required:
+ - compatible
+ - reg
+ - microchip,hw-device-address
+ - spi-max-frequency
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+ - # External vref, no internal reference
+ if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,mcp3461
+ - microchip,mcp3462
+ - microchip,mcp3464
+ - microchip,mcp3561
+ - microchip,mcp3562
+ - microchip,mcp3564
+ then:
+ required:
+ - vref-supply
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "microchip,mcp3564r";
+ reg = <0>;
+ vref-supply = <&vref_reg>;
+ spi-cpha;
+ spi-cpol;
+ spi-max-frequency = <10000000>;
+ microchip,hw-device-address = <1>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ /* CH0 to AGND */
+ reg = <0>;
+ };
+
+ channel@1 {
+ /* CH1 to AGND */
+ reg = <1>;
+ };
+
+ /* diff-channels */
+ channel@11 {
+ reg = <11>;
+
+ /* CN0, CN1 */
+ diff-channels = <0 1>;
+ };
+
+ channel@22 {
+ reg = <0x22>;
+
+ /* CN1, CN2 */
+ diff-channels = <1 2>;
+ };
+
+ channel@23 {
+ reg = <0x23>;
+
+ /* CN1, CN3 */
+ diff-channels = <1 3>;
+ };
+ };
+ };
+...
--
2.34.1
Hey,
On Fri, Jul 14, 2023 at 06:00:50PM +0300, [email protected] wrote:
> From: Marius Cristea <[email protected]>
>
> This is the device tree schema for iio driver for
> Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
> Delta-Sigma ADCs with an SPI interface (Microchip's
> MCP3461, MCP3462, MCP3464, MCP3461R, MCP3462R,
> MCP3464R, MCP3561, MCP3562, MCP3564, MCP3561R,
> MCP3562R and MCP3564R analog to digital converters).
>
> Signed-off-by: Marius Cristea <[email protected]>
This looks good to me, other than the custom property, for which I can't
tell if a consensus was reached on last time around.
> + microchip,hw-device-address:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 3
> + description:
> + The address is set on a per-device basis by fuses in the factory,
> + configured on request. If not requested, the fuses are set for 0x1.
> + The device address is part of the device markings to avoid
> + potential confusion. This address is coded on two bits, so four possible
> + addresses are available when multiple devices are present on the same
> + SPI bus with only one Chip Select line for all devices.
> + Each device communication starts by a CS falling edge, followed by the
> + clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> + which is first one on the wire).
On the last version, the last comment I could find on lore was
https://lore.kernel.org/all/[email protected]/
where Jonathan and Rob were discussing whether or not a spi-mux type of
thing could work, but it does not seem to have ended conclusively.
Rob or Jonathan, would you mind commenting on that?
There was also a comment from Jonathan:
> > + vref-supply:
> > + description:
> > + Some devices have a specific reference voltage supplied on a different
> > + pin to the other supplies. Needed to be able to establish channel scaling
> > + unless there is also an internal reference available (e.g. mcp3564r)
> > +
>
> From a quick glance at a random datasheet, looks like there additional power supplies
> that should be required.
>
> If this is required for some devices, I'd expect to see the binding enforce
> that with some required entries conditioned on the compatibles rather than as
> documentation. If there are devices where it isn't even optional then the binding
> should enforce that as well.
The binding does now enforce the vref supply where relevant, but it
sounds like you were looking more supplies to be documented Jonathan?
(AVdd, DVdd etc)
Thanks,
Conor.
On Sat, 15 Jul 2023 11:28:03 +0100
Conor Dooley <[email protected]> wrote:
> Hey,
>
> On Fri, Jul 14, 2023 at 06:00:50PM +0300, [email protected] wrote:
> > From: Marius Cristea <[email protected]>
> >
> > This is the device tree schema for iio driver for
> > Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
> > Delta-Sigma ADCs with an SPI interface (Microchip's
> > MCP3461, MCP3462, MCP3464, MCP3461R, MCP3462R,
> > MCP3464R, MCP3561, MCP3562, MCP3564, MCP3561R,
> > MCP3562R and MCP3564R analog to digital converters).
> >
> > Signed-off-by: Marius Cristea <[email protected]>
>
> This looks good to me, other than the custom property, for which I can't
> tell if a consensus was reached on last time around.
>
> > + microchip,hw-device-address:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 3
> > + description:
> > + The address is set on a per-device basis by fuses in the factory,
> > + configured on request. If not requested, the fuses are set for 0x1.
> > + The device address is part of the device markings to avoid
> > + potential confusion. This address is coded on two bits, so four possible
> > + addresses are available when multiple devices are present on the same
> > + SPI bus with only one Chip Select line for all devices.
> > + Each device communication starts by a CS falling edge, followed by the
> > + clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > + which is first one on the wire).
>
> On the last version, the last comment I could find on lore was
> https://lore.kernel.org/all/[email protected]/
> where Jonathan and Rob were discussing whether or not a spi-mux type of
> thing could work, but it does not seem to have ended conclusively.
>
> Rob or Jonathan, would you mind commenting on that?
Sure - as far as I'm concerned - it looks like it should be possible to do something
generic, but without a prototype it's hard to be sure how fiddly that will be.
+CC Mark Brown who might be able to give a more informed answer to whether such a
thing would work / be easy to implement.
I've no idea how common this trick is. If it's a one off, may not be worth the bother
of a more generic mux like binding whether that is the more elegant solution or
not.
>
> There was also a comment from Jonathan:
> > > + vref-supply:
> > > + description:
> > > + Some devices have a specific reference voltage supplied on a different
> > > + pin to the other supplies. Needed to be able to establish channel scaling
> > > + unless there is also an internal reference available (e.g. mcp3564r)
> > > +
> >
> > From a quick glance at a random datasheet, looks like there additional power supplies
> > that should be required.
> >
> > If this is required for some devices, I'd expect to see the binding enforce
> > that with some required entries conditioned on the compatibles rather than as
> > documentation. If there are devices where it isn't even optional then the binding
> > should enforce that as well.
>
> The binding does now enforce the vref supply where relevant, but it
> sounds like you were looking more supplies to be documented Jonathan?
> (AVdd, DVdd etc)
Exactly.
>
> Thanks,
> Conor.
On 14/07/2023 17:00, [email protected] wrote:
> From: Marius Cristea <[email protected]>
>
> This is the device tree schema for iio driver for
> Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
...
> +
> +dependencies:
> + spi-cpol: [ spi-cpha ]
> + spi-cpha: [ spi-cpol ]
Put dependencies after patternProperties:, before required:.
> +
> +patternProperties:
> + "^channel@([0-9]|([1-7][0-9]))$":
> + $ref: adc.yaml
> + type: object
Missing unevaluatedProperties: false.
Open other bindings and look how it is done there.
> + description: Represents the external channels which are connected to the ADC.
> +
> + properties:
> + reg:
> + description: The channel number in single-ended and differential mode.
> + minimum: 0
> + maximum: 79
> +
> + diff-channels: true
Why? Drop, unless you want to say there all other ADC properties are
invalid for this type of device (device, not driver!).
> +
> + required:
> + - reg
Best regards,
Krzysztof
Hey Conor,
On Sat, 2023-07-15 at 11:28 +0100, Conor Dooley wrote:
> Hey,
>
> On Fri, Jul 14, 2023 at 06:00:50PM +0300,
> [email protected] wrote:
> > From: Marius Cristea <[email protected]>
> >
> > This is the device tree schema for iio driver for
> > Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
> > Delta-Sigma ADCs with an SPI interface (Microchip's
> > MCP3461, MCP3462, MCP3464, MCP3461R, MCP3462R,
> > MCP3464R, MCP3561, MCP3562, MCP3564, MCP3561R,
> > MCP3562R and MCP3564R analog to digital converters).
> >
> > Signed-off-by: Marius Cristea <[email protected]>
>
> This looks good to me, other than the custom property, for which I
> can't
> tell if a consensus was reached on last time around.
>
I don't think a consensus related to "custom property" was reached
last time around. I was aiming to fix all other review comments first
and leave the "details" at the end.
I still think is a good idea to have some custom properties that will
impact only a certain range of devices and leave the user to
decide/choose how to use that configuration setting (to better suite
his needs). If the application doesn't recognize the property it will
be configured with the default value and it should not broke anything.
If we decide that we need to take out the custom properties, then I
could move some of them into the Device Tree.
> > + microchip,hw-device-address:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 3
> > + description:
> > + The address is set on a per-device basis by fuses in the
> > factory,
> > + configured on request. If not requested, the fuses are set
> > for 0x1.
> > + The device address is part of the device markings to avoid
> > + potential confusion. This address is coded on two bits, so
> > four possible
> > + addresses are available when multiple devices are present on
> > the same
> > + SPI bus with only one Chip Select line for all devices.
> > + Each device communication starts by a CS falling edge,
> > followed by the
> > + clocking of the device address (BITS[7:6] - top two bits of
> > COMMAND BYTE
> > + which is first one on the wire).
>
> On the last version, the last comment I could find on lore was
> https://lore.kernel.org/all/[email protected]/
> where Jonathan and Rob were discussing whether or not a spi-mux type
> of
> thing could work, but it does not seem to have ended conclusively.
>
> Rob or Jonathan, would you mind commenting on that?
>
I think in case of a single device on bus, we could avoid using the
spi-mux.
> There was also a comment from Jonathan:
> > > + vref-supply:
> > > + description:
> > > + Some devices have a specific reference voltage supplied on
> > > a different
> > > + pin to the other supplies. Needed to be able to establish
> > > channel scaling
> > > + unless there is also an internal reference available (e.g.
> > > mcp3564r)
> > > +
> >
> > From a quick glance at a random datasheet, looks like there
> > additional power supplies
> > that should be required.
> >
> > If this is required for some devices, I'd expect to see the binding
> > enforce
> > that with some required entries conditioned on the compatibles
> > rather than as
> > documentation. If there are devices where it isn't even optional
> > then the binding
> > should enforce that as well.
>
> The binding does now enforce the vref supply where relevant, but it
> sounds like you were looking more supplies to be documented Jonathan?
> (AVdd, DVdd etc)
>
All other supply (like AVdd, DVdd etc) for this particular chip
doesn't have any direct impact (way of working, resolution, any
configuration setup), so I'm not sure if we need to add anything more
here.
> Thanks,
> Conor.
Hi Krzysztof,
> > +
> > +patternProperties:
> > + "^channel@([0-9]|([1-7][0-9]))$":
> > + $ref: adc.yaml
> > + type: object
>
> Missing unevaluatedProperties: false.
>
> Open other bindings and look how it is done there.
>
> > + description: Represents the external channels which are
> > connected to the ADC.
> > +
> > + properties:
> > + reg:
> > + description: The channel number in single-ended and
> > differential mode.
> > + minimum: 0
> > + maximum: 79
> > +
> > + diff-channels: true
>
> Why? Drop, unless you want to say there all other ADC properties are
> invalid for this type of device (device, not driver!).
>
> > +
> > + required:
> > + - reg
>
>
All other ADC properties are valid. Here I was trying to add some
properties for each the channel (ADC channel) used by user on this ADC.
The channel could be single ended (Channel to ground) or
"diff-channels" where I need to know the pins/channel used.
Maybe I'm missing something but I was trying to follow the binding
from: Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
Best regards,
Marius
On 19/07/2023 17:40, [email protected] wrote:
> Hi Krzysztof,
>
>>> +
>>> +patternProperties:
>>> + "^channel@([0-9]|([1-7][0-9]))$":
>>> + $ref: adc.yaml
>>> + type: object
>>
>> Missing unevaluatedProperties: false.
>>
>> Open other bindings and look how it is done there.
>>
>>> + description: Represents the external channels which are
>>> connected to the ADC.
>>> +
>>> + properties:
>>> + reg:
>>> + description: The channel number in single-ended and
>>> differential mode.
>>> + minimum: 0
>>> + maximum: 79
>>> +
>>> + diff-channels: true
>>
>> Why? Drop, unless you want to say there all other ADC properties are
>> invalid for this type of device (device, not driver!).
>>
>>> +
>>> + required:
>>> + - reg
>>
>>
>
> All other ADC properties are valid.
So drop what I questioned.
Best regards,
Krzysztof
On Tue, 18 Jul 2023 09:24:58 +0000
<[email protected]> wrote:
> Hey Conor,
>
>
> On Sat, 2023-07-15 at 11:28 +0100, Conor Dooley wrote:
> > Hey,
> >
> > On Fri, Jul 14, 2023 at 06:00:50PM +0300,
> > [email protected] wrote:
> > > From: Marius Cristea <[email protected]>
> > >
> > > This is the device tree schema for iio driver for
> > > Microchip family of 153.6 ksps, Low-Noise 16/24-Bit
> > > Delta-Sigma ADCs with an SPI interface (Microchip's
> > > MCP3461, MCP3462, MCP3464, MCP3461R, MCP3462R,
> > > MCP3464R, MCP3561, MCP3562, MCP3564, MCP3561R,
> > > MCP3562R and MCP3564R analog to digital converters).
> > >
> > > Signed-off-by: Marius Cristea <[email protected]>
> >
> > This looks good to me, other than the custom property, for which I
> > can't
> > tell if a consensus was reached on last time around.
> >
>
> I don't think a consensus related to "custom property" was reached
> last time around. I was aiming to fix all other review comments first
> and leave the "details" at the end.
That's fair enough as a way to move things forward - just make sure to
mention open questions in your cover letter / patch descriptions or
under the ---
>
> I still think is a good idea to have some custom properties that will
> impact only a certain range of devices and leave the user to
> decide/choose how to use that configuration setting (to better suite
> his needs). If the application doesn't recognize the property it will
> be configured with the default value and it should not broke anything.
>
> If we decide that we need to take out the custom properties, then I
> could move some of them into the Device Tree.
>
> > > + microchip,hw-device-address:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + minimum: 0
> > > + maximum: 3
> > > + description:
> > > + The address is set on a per-device basis by fuses in the
> > > factory,
> > > + configured on request. If not requested, the fuses are set
> > > for 0x1.
> > > + The device address is part of the device markings to avoid
> > > + potential confusion. This address is coded on two bits, so
> > > four possible
> > > + addresses are available when multiple devices are present on
> > > the same
> > > + SPI bus with only one Chip Select line for all devices.
> > > + Each device communication starts by a CS falling edge,
> > > followed by the
> > > + clocking of the device address (BITS[7:6] - top two bits of
> > > COMMAND BYTE
> > > + which is first one on the wire).
> >
> > On the last version, the last comment I could find on lore was
> > https://lore.kernel.org/all/[email protected]/
> > where Jonathan and Rob were discussing whether or not a spi-mux type
> > of
> > thing could work, but it does not seem to have ended conclusively.
> >
> > Rob or Jonathan, would you mind commenting on that?
> >
>
> I think in case of a single device on bus, we could avoid using the
> spi-mux.
>
That's a fair point. I think key here is how common this is, and
I have no idea.
> > > If this is required for some devices, I'd expect to see the binding
> > > enforce
> > > that with some required entries conditioned on the compatibles
> > > rather than as
> > > documentation. If there are devices where it isn't even optional
> > > then the binding
> > > should enforce that as well.
> >
> > The binding does now enforce the vref supply where relevant, but it
> > sounds like you were looking more supplies to be documented Jonathan?
> > (AVdd, DVdd etc)
> >
>
> All other supply (like AVdd, DVdd etc) for this particular chip
> doesn't have any direct impact (way of working, resolution, any
> configuration setup), so I'm not sure if we need to add anything more
> here.
>
Pretty big impact if they aren't turned on ;) Expectation is the driver
will just ensure they are and it can only do that if it knows they exist.
Jonathan