On Thu, 27 Jun 2024 13:25:18 +0300
Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> wrote:
> The ADXL380/ADXL382 is a low noise density, low power, 3-axis
> accelerometer with selectable measurement ranges. The ADXL380 supports
> the +/-4 g, +/-8 g, and +/-16 g ranges, and the ADXL382 supports
> +/-15 g, +/-30 g and +/-60 g ranges.
> The ADXL380/ADXL382 offers industry leading noise, enabling precision
> applications with minimal calibration. The low noise, and low power
> ADXL380/ADXL382 enables accurate measurement in an environment with
> high vibration, heart sounds and audio.
>
> In addition to its low power consumption, the ADXL380/ADXL382 has many
> features to enable true system level performance. These include a
> built-in micropower temperature sensor, single / double / triple tap
> detection and a state machine to prevent a false triggering. In
> addition, the ADXL380/ADXL382 has provisions for external control of
> the sampling time and/or an external clock.
>
> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@xxxxxxxxxx>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
Hi Antoniu, Ramona,
Some trivial stuff that I'd just have cleaned up, but one thing I don't
understand which is which entries of your iio_priv() structure you've
marked with __aligned(IIO_DMA_MINALIGN). That's beyond the level of thing
I'll tweak whilst applying.
I might be missing a path in which they are used for DMA transfers though!
Jonathan
> ---
> changes in v3:
> - rearrange includes in alphabetical order.
> - rework defines clearly stating which are registers.
> - use BIT() and FIELD_GET() where possible.
> - convert register related enums into macro definitions.
> - add spacings after { and before } for arrays.
> - reorder the `adxl380_state` structure members.
> - mark structure members with __aligned(IIO_DMA_MINALIGN) where required.
This change has me confused because I'm not sure why it is required
for the ones you've marked (you do need one such marking at least though!)
> - drop redundant brackets.
> - use min() function where it applies.
> - use put_unaligned_be24() where it applies.
> - wrap lines where indicated by the reviewers.
> - assign directly instead of using get_unaligned_be16 where not necessary.
> - rework error handling for irq_handler.
> - add missing error check.
> - drop != IIO_ACCEL for IIO_CHAN_INFO_SCALE
> - use MICRO where possible.
> - return iio_format_value() directly.
> - check for negative values aswell.
> - reorder local declarations.
> - improve dev_err_probe string description.
> - drop _ from functions naming where possible.
> - rework chip/part id handling.
> - improve comment for the delay required after reset.
> - add regulators implementation for the supplies.
> - handle both irqs.
> - use i2c_get_match_data.
> - use spi_get_device_match_data.
> - include mod_devicetable.h.
> diff --git a/drivers/iio/accel/adxl380.c b/drivers/iio/accel/adxl380.c
> new file mode 100644
> index 000000000000..b569265190e6
> --- /dev/null
> +++ b/drivers/iio/accel/adxl380.c
...
> +struct adxl380_state {
...
> + int irq;
> + int int_map[2];
> + int lpf_tbl[4] __aligned(IIO_DMA_MINALIGN);
> + int hpf_tbl[7][2] __aligned(IIO_DMA_MINALIGN);
Unless you allow the driver to write these two tables at the
time dma is going on with the other one you shoudn't need
to force them into separate cachelines.
I'm more curious though - why these two?
