On Tue, 08 Oct 2024 17:43:41 +0200
Angelo Dureghello <adureghello@xxxxxxxxxxxx> wrote:
> From: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
>
> Add High Speed ad3552r platform driver.
>
> The ad3552r DAC is controlled by a custom (fpga-based) DAC IP
> through the current AXI backend, or similar alternative IIO backend.
>
> Compared to the existing driver (ad3552r.c), that is a simple SPI
> driver, this driver is coupled with a DAC IIO backend that finally
> controls the ad3552r by a fpga-based "QSPI+DDR" interface, to reach
> maximum transfer rate of 33MUPS using dma stream capabilities.
>
> All commands involving QSPI bus read/write are delegated to the backend
> through the provided APIs for bus read/write.
>
> Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
> ---
> drivers/iio/dac/Kconfig | 14 +
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/ad3552r-hs.c | 526 +++++++++++++++++++++++++++++++
> drivers/iio/dac/ad3552r.h | 7 +
> include/linux/platform_data/ad3552r-hs.h | 18 ++
I'd missed this before. No obvious reason to put the include in a 'global' location
when for now at least only includers re in drivers/iio/dac/
So please move it there.
Some stuff include/linux/platform_data/ is ancient things dating back to board file
days.
Other than that and things bit Nuno raised, this series looks good to me.
Please drop the first 2 patches from v6 as I've applied those now.
Jonathan
> diff --git a/include/linux/platform_data/ad3552r-hs.h b/include/linux/platform_data/ad3552r-hs.h
> new file mode 100644
> index 000000000000..4e3213a0c73b
> --- /dev/null
> +++ b/include/linux/platform_data/ad3552r-hs.h
This isn't what we'd think of as 'platform_data' normally and it is only used
in drivers/iio/dac/ so I would simply move the header to drivers/iio/dac/ad3552r-hs.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2010-2024 Analog Devices Inc.
Generous on the years. This one should probably just be new copyright given
the code is effectively new.
> + * Copyright (c) 2024 Baylibre, SAS
> + */
> +#ifndef __LINUX_PLATFORM_DATA_AD3552R_HS_H__
> +#define __LINUX_PLATFORM_DATA_AD3552R_HS_H__
> +
> +#include <linux/iio/backend.h>
For this use a forwards reference
struct iio_backend;
Nice to avoid nests of includes where we can easily do so.
> +
> +struct ad3552r_hs_platform_data {
> + int (*bus_reg_read)(struct iio_backend *back, u32 reg, u32 *val,
> + size_t data_size);
> + int (*bus_reg_write)(struct iio_backend *back, u32 reg, u32 val,
> + size_t data_size);
> +};
> +
> +#endif /* __LINUX_PLATFORM_DATA_AD3552R_HS_H__ */
>
