Mon, Jun 05, 2023 at 09:07:55PM +0800, Kim Seer Paller kirjoitti:
> The MAX14001 is configurable, isolated 10-bit ADCs for multi-range
> binary inputs.
First question, why don't you use regmap SPI?
...
> +static int max14001_read(void *context, unsigned int reg_addr,
> + unsigned int *data)
Strange indentation.
> +{
> + struct max14001_state *st = context;
> + u16 tx = 0;
Redundant assignment.
> + int ret;
> +
> + struct spi_transfer xfers[] = {
> + {
> + .tx_buf = &st->spi_tx_buffer,
> + .len = 2,
> + .cs_change = 1,
> + }, {
> + .rx_buf = &st->spi_rx_buffer,
> + .len = 2,
> + },
> + };
> +
> + tx = FIELD_PREP(MAX14001_ADDR_MASK, reg_addr);
> + st->spi_tx_buffer = bitrev16(cpu_to_be16(tx));
This is strange. Why this monsteur construct with bitrev16() is used?
According to the datasheet, the bit stream is LE16, where 10 LSB is data,
5 MSB is address and bit 11 is R/W.
> + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> + if (ret)
> + return ret;
> + *data = bitrev16(be16_to_cpu(st->spi_rx_buffer)) & MAX14001_DATA_MASK;
Ditto.
> + return 0;
> +}
> +static int max14001_write(void *context, unsigned int reg_addr,
> + unsigned int data)
> +{
> + struct max14001_state *st = context;
> + u16 tx = 0;
Redundant assignment.
> + tx = FIELD_PREP(MAX14001_ADDR_MASK, reg_addr);
> + tx |= FIELD_PREP(MAX14001_SET_WRITE_BIT, 1);
> + tx |= FIELD_PREP(MAX14001_DATA_MASK, data);
> +
> + st->spi_tx_buffer = bitrev16(cpu_to_be16(tx));
> +
> + return spi_write(st->spi, &st->spi_tx_buffer, 2);
sizeof() ?
> +}
...
> + return dev_err_probe(&spi->dev, PTR_ERR(vref),
> + "Failed to get vref regulator");
With
struct device *dev = &spi->dev;
this and other calls in this function can be made neater.
--
With Best Regards,
Andy Shevchenko
