lkml.org 
[lkml]   [2022]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 2/2] iio: adc: ad4130: add AD4130 driver
From


On 6/20/22 21:29, Andy Shevchenko wrote:
> On Mon, Jun 20, 2022 at 6:27 PM Cosmin Tanislav <demonsingur@gmail.com> wrote:
>>
>> AD4130-8 is an ultra-low power, high precision, measurement solution for
>> low bandwidth battery operated applications.
>>
>> The fully integrated AFE (Analog Front-End) includes a multiplexer for up
>> to 16 single-ended or 8 differential inputs, PGA (Programmable Gain
>> Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator,
>> selectable filter options, smart sequencer, sensor biasing and excitation
>> options, diagnostics, and a FIFO buffer.
>
> ...
>
>> +KernelVersion: 5.18
>
> Are you sure?
>
> ...
>
>> +struct ad4130_state {
>> + struct regmap *regmap;
>> + struct spi_device *spi;
>> + struct clk *mclk;
>> + struct regulator_bulk_data regulators[4];
>> + u32 irq_trigger;
>> + u32 inv_irq_trigger;
>> +
>> + /*
>> + * Synchronize access to members of driver state, and ensure atomicity
>
> the driver
>
>> + * of consecutive regmap operations.
>> + */
>> + struct mutex lock;
>> + struct completion completion;
>> +
>> + struct iio_chan_spec chans[AD4130_MAX_CHANNELS];
>> + struct ad4130_chan_info chans_info[AD4130_MAX_CHANNELS];
>> + struct ad4130_slot_info slots_info[AD4130_MAX_SETUPS];
>> + enum ad4130_pin_function pins_fn[AD4130_MAX_ANALOG_PINS];
>> + u32 vbias_pins[AD4130_MAX_ANALOG_PINS];
>> + u32 num_vbias_pins;
>> + int scale_tbls[AD4130_REF_SEL_MAX][AD4130_MAX_PGA][2];
>> + struct gpio_chip gc;
>> + unsigned int gpio_offsets[AD4130_MAX_GPIOS];
>> + unsigned int num_gpios;
>> +
>> + u32 int_pin_sel;
>> + u32 int_ref_uv;
>> + u32 mclk_sel;
>> + bool int_ref_en;
>> + bool bipolar;
>> +
>> + unsigned int num_enabled_channels;
>> + unsigned int effective_watermark;
>> + unsigned int watermark;
>> +
>> + struct spi_message fifo_msg;
>> + struct spi_transfer fifo_xfer[2];
>> +
>> + /*
>> + * DMA (thus cache coherency maintenance) requires the
>> + * transfer buffers to live in their own cache lines.
>> + */
>
> This is a good comment, but what fields does it apply to?

Whatever is below it, grouped together. This is not hard to
understand.

>
>> + u8 reset_buf[AD4130_RESET_BUF_SIZE] __aligned(IIO_DMA_MINALIGN);
>> + u8 reg_write_tx_buf[4];
>> + u8 reg_read_tx_buf[1];
>> + u8 reg_read_rx_buf[3];
>> + u8 fifo_tx_buf[2];
>> + u8 fifo_rx_buf[AD4130_FIFO_SIZE *
>> + AD4130_FIFO_MAX_SAMPLE_SIZE];
>> +};
>
> ...
>
>> +static IIO_DEVICE_ATTR(hwfifo_watermark, 0444, ad4130_get_fifo_watermark, NULL, 0);
>> +static IIO_DEVICE_ATTR(hwfifo_enabled, 0444, ad4130_get_fifo_enabled, NULL, 0);
>
> IIO_DEVICE_ATTR_RO()
>
> ...
>
>> + for (i = 0; i < st->num_gpios; i++)
>> + val |= BIT(st->gpio_offsets[i]);
>
> This might overflow.

No it might not. num_gpios is at max AD4130_MAX_GPIOS, which is 4.

>
> ...
>
>> + for (i = 0; i < st->num_vbias_pins; i++)
>> + val |= BIT(st->vbias_pins[i]);
>
> Ditto.

No it might not. num_vbias_pins is at max AD4130_MAX_ANALOG_PINS, which
is 16.

>
> ...
>
>> + st->regmap = devm_regmap_init(dev, NULL, st, &ad4130_regmap_config);
>> + if (IS_ERR(st->regmap))
>> + return PTR_ERR(st->regmap);
>
> Can it use regular regmap SPI?

No. I'm pretty sure I've already talked about this.

>

\
 
 \ /
  Last update: 2022-06-23 17:27    [W:0.155 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site