On Thu, Aug 2, 2018 at 10:15 PM, Marcus Folkesson
<marcus.folkesson@xxxxxxxxx> 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>
What is this? Why it's here (presense and location in the message)?
> + * Copyright (C) 2018 Kent Gustavsson <kent@xxxxxxxxxx>
> + *
Redundant.
> +static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
> +{
> + int ret;
> +
> + reg = MCP3911_REG_READ(reg, adc->dev_addr);
> + ret = spi_write_then_read(adc->spi, ®, 1, val, len);
> + if (ret < 0)
> + return ret;
> +
> + be32_to_cpus(val);
> + *val >>= ((4 - len) * 8);
> + dev_dbg(&adc->spi->dev, "reading 0x%x from register 0x%x\n", *val,
> + reg>>1);
> + return ret;
> +}
> +
> +static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
> +{
> + dev_dbg(&adc->spi->dev, "writing 0x%x to register 0x%x\n", val, reg);
> +
> + val <<= (3 - len) * 8;
> + cpu_to_be32s(&val);
> + val |= MCP3911_REG_WRITE(reg, adc->dev_addr);
> +
> + return spi_write(adc->spi, &val, len + 1);
> +}
Can't you use REGMAP_SPI ?
> + of_property_read_u32(of_node, "device-addr", &adc->dev_addr);
> + if (adc->dev_addr > 3) {
> + dev_err(&adc->spi->dev,
> + "invalid device address (%i). Must be in range 0-3.\n",
> + adc->dev_addr);
> + return -EINVAL;
> + }
> + dev_dbg(&adc->spi->dev, "use device address %i\n", adc->dev_addr);
Isn't what we called CS (chip select)?
> + if (IS_ERR(adc->vref)) {
> +
Redundant.
> + if (adc->clki)
Seems to be redundant if clock is optional (it should be NULL and API
is NULL-aware).
> + clk_disable_unprepare(adc->clki);
> + if (adc->clki)
> + clk_disable_unprepare(adc->clki);
Ditto.
> +#if defined(CONFIG_OF)
This prevents playing with the device on ACPI enabled platforms.
> + .of_match_table = of_match_ptr(mcp3911_dt_ids),
Ditto for macro.
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
