[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VcBJkQ+CwyoDaTJ_AD+mv9d0tEd_txqHwkPRy4-xvnyKg@mail.gmail.com>
Date: Mon, 20 Jun 2022 20:29:43 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Cosmin Tanislav <demonsingur@...il.com>
Cc: Jonathan Cameron <jic23@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>,
linux-iio <linux-iio@...r.kernel.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
devicetree <devicetree@...r.kernel.org>,
Cosmin Tanislav <cosmin.tanislav@...log.com>
Subject: Re: [PATCH v5 2/2] iio: adc: ad4130: add AD4130 driver
On Mon, Jun 20, 2022 at 6:27 PM Cosmin Tanislav <demonsingur@...il.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?
> + 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.
...
> + for (i = 0; i < st->num_vbias_pins; i++)
> + val |= BIT(st->vbias_pins[i]);
Ditto.
...
> + 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?
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists