Hello Rob, Thank you for the review. On Wed, Jul 25, 2018 at 11:51:17AM -0600, Rob Herring wrote: > On Tue, Jul 24, 2018 at 08:30:03PM +0200, Marcus Folkesson wrote: > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@xxxxxxxxx> > > Signed-off-by: Kent Gustavsson <kent@xxxxxxxxxx> > > --- > > > > Notes: > > v2: > > - drop channel width > > - drop `external_vref` > > - replace `external-clock` with proper clock bindings > > > > .../devicetree/bindings/iio/adc/mcp3911.txt | 28 ++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > new file mode 100644 > > index 000000000000..af5472f51a84 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > @@ -0,0 +1,28 @@ > > +* Microchip MCP3911 Dual channel analog front end (ADC) > > + > > +Required properties: > > + - compatible: Should be "microchip,mcp3911" > > + - reg: SPI chip select number for the device > > + > > +Recommended properties: > > + - spi-max-frequency: Definition as per > > + Documentation/devicetree/bindings/spi/spi-bus.txt. > > + Max frequency for this chip is 20MHz. > > + > > +Optional properties: > > + - device-addr: Device address when multiple MCP3911 chips are present on the > > + same SPI bus. Valid values are 0-3. Defaults to 0. > > Isn't this the reg value? > The reg value defines which chip select the chip hangs on. The chip has support to connect up to four chips on the same SPI bus, including the same chip select signal. The chips are separated by the device-addr as it is part of the communication protocol. When reading the description for device-addr, I agree that it fits well for the reg property as well.. > > + - vref-supply: Phandle to the external reference voltage supply. > > + - clocks: Phandle and clock identifier (see clock-names) > > + - clock-names: "adc_clk" for the ADC (sampling) clock > > Datasheet calls this clki (or mclk internally). Or just drop clock-names > as it is pointless > when there is only 1 clock. I will drop the clock-names, thanks! > > Also DR handling as an IRQ is missing. I have considered using the DR signal, but as we not support buffering yet I did not see any point in using it. From the datasheet, section 7.1 :: These registers [channel registers] are latched when an ADC read communication occurs. When a data ready event occurs during a read communication, the most current ADC data is also latched to avoid data corruption issues. The three bytes of each channel are updated synchronously at a DRCLK rate. The three bytes can be accessed separately if needed but are refreshed synchronously. So it should be safe to read the values independently of the DR signal. Or maybe I'm missing something. Best regards Marcus Folkesson
Attachment:
signature.asc
Description: PGP signature
