Re: [PATCH v2 1/2] dt-bindings: iio: adc: adding MCP3564 ADC

[Conor Dooley]

Hey,

On Fri, Jul 14, 2023 at 06:00:50PM +0300, marius.cristea@xxxxxxxxxxxxx wrote:
> From: Marius Cristea <marius.cristea@xxxxxxxxxxxxx>
> 
> 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 <marius.cristea@xxxxxxxxxxxxx>

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/20230609184149.00002766@xxxxxxxxxx/
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.

Attachment: signature.asc
Description: PGP signature