| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250201144517.56b0e636@jic23-huawei>
Date: Sat, 1 Feb 2025 14:45:17 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Robert Budai <robert.budai@...log.com>
Cc: <lars@...afoo.de>, <Michael.Hennerich@...log.com>, <nuno.sa@...log.com>,
<ramona.gradinariu@...log.com>, <antoniu.miclaus@...log.com>,
<robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
<corbet@....net>, <lanzano.alex@...il.com>, <linux-iio@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-doc@...r.kernel.org>
Subject: Re: [PATCH v5 1/6] iio: imu: adis: Add custom ops struct
On Wed, 29 Jan 2025 10:20:41 +0200
Robert Budai <robert.budai@...log.com> wrote:
> This patch introduces a custom ops struct letting users define
> custom read and write functions. Some adis devices might define
> a completely different spi protocol from the one used in the
> default implementation.
Wrap at 75 chars, not around 62
Otherwise this looks fine to me. Could have been merged
with patch 2 as that just adds another op, but that's not important.
>
> Co-developed-by: Ramona Gradinariu <ramona.gradinariu@...log.com>
> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@...log.com>
> Co-developed-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> Signed-off-by: Nuno Sá <nuno.sa@...log.com>
> Signed-off-by: Robert Budai <robert.budai@...log.com>
> ---
> drivers/iio/imu/adis.c | 16 +++++++++++++---
> include/linux/iio/imu/adis.h | 28 +++++++++++++++++++++-------
> 2 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 494171844812..54915c7a3e76 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -223,13 +223,13 @@ int __adis_update_bits_base(struct adis *adis, unsigned int reg, const u32 mask,
> int ret;
> u32 __val;
>
> - ret = __adis_read_reg(adis, reg, &__val, size);
> + ret = adis->ops->read(adis, reg, &__val, size);
> if (ret)
> return ret;
>
> __val = (__val & ~mask) | (val & mask);
>
> - return __adis_write_reg(adis, reg, __val, size);
> + return adis->ops->write(adis, reg, __val, size);
> }
> EXPORT_SYMBOL_NS_GPL(__adis_update_bits_base, "IIO_ADISLIB");
>
> @@ -468,7 +468,7 @@ int adis_single_conversion(struct iio_dev *indio_dev,
>
> guard(mutex)(&adis->state_lock);
>
> - ret = __adis_read_reg(adis, chan->address, &uval,
> + ret = adis->ops->read(adis, chan->address, &uval,
> chan->scan_type.storagebits / 8);
> if (ret)
> return ret;
> @@ -488,6 +488,11 @@ int adis_single_conversion(struct iio_dev *indio_dev,
> }
> EXPORT_SYMBOL_NS_GPL(adis_single_conversion, "IIO_ADISLIB");
>
> +static const struct adis_ops adis_default_ops = {
> + .read = __adis_read_reg,
> + .write = __adis_write_reg,
> +};
> +
> /**
> * adis_init() - Initialize adis device structure
> * @adis: The adis device
> @@ -517,6 +522,11 @@ int adis_init(struct adis *adis, struct iio_dev *indio_dev,
>
> adis->spi = spi;
> adis->data = data;
> + if (!adis->ops->write && !adis->ops->read)
> + adis->ops = &adis_default_ops;
> + else if (!adis->ops->write || !adis->ops->read)
> + return -EINVAL;
> +
> iio_device_set_drvdata(indio_dev, adis);
>
> if (data->has_paging) {
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index e6a75356567a..04140b36712a 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -94,6 +94,18 @@ struct adis_data {
> unsigned int burst_max_speed_hz;
> };
>
> +/**
> + * struct adis_ops: Custom ops for adis devices.
> + * @write: Custom spi write implementation.
> + * @read: Custom spi read implementation.
> + */
> +struct adis_ops {
> + int (*write)(struct adis *adis, unsigned int reg, unsigned int value,
> + unsigned int size);
> + int (*read)(struct adis *adis, unsigned int reg, unsigned int *value,
> + unsigned int size);
> +};
> +
> /**
> * struct adis - ADIS device instance data
> * @spi: Reference to SPI device which owns this ADIS IIO device
> @@ -102,6 +114,7 @@ struct adis_data {
> * @burst: ADIS burst transfer information
> * @burst_extra_len: Burst extra length. Should only be used by devices that can
> * dynamically change their burst mode length.
> + * @ops: ops struct for custom read and write functions
> * @state_lock: Lock used by the device to protect state
> * @msg: SPI message object
> * @xfer: SPI transfer objects to be used for a @msg
> @@ -117,6 +130,7 @@ struct adis {
>
> const struct adis_data *data;
> unsigned int burst_extra_len;
> + const struct adis_ops *ops;
> /**
> * The state_lock is meant to be used during operations that require
> * a sequence of SPI R/W in order to protect the SPI transfer
> @@ -169,7 +183,7 @@ int __adis_read_reg(struct adis *adis, unsigned int reg,
> static inline int __adis_write_reg_8(struct adis *adis, unsigned int reg,
> u8 val)
> {
> - return __adis_write_reg(adis, reg, val, 1);
> + return adis->ops->write(adis, reg, val, 1);
> }
>
> /**
> @@ -181,7 +195,7 @@ static inline int __adis_write_reg_8(struct adis *adis, unsigned int reg,
> static inline int __adis_write_reg_16(struct adis *adis, unsigned int reg,
> u16 val)
> {
> - return __adis_write_reg(adis, reg, val, 2);
> + return adis->ops->write(adis, reg, val, 2);
> }
>
> /**
> @@ -193,7 +207,7 @@ static inline int __adis_write_reg_16(struct adis *adis, unsigned int reg,
> static inline int __adis_write_reg_32(struct adis *adis, unsigned int reg,
> u32 val)
> {
> - return __adis_write_reg(adis, reg, val, 4);
> + return adis->ops->write(adis, reg, val, 4);
> }
>
> /**
> @@ -208,7 +222,7 @@ static inline int __adis_read_reg_16(struct adis *adis, unsigned int reg,
> unsigned int tmp;
> int ret;
>
> - ret = __adis_read_reg(adis, reg, &tmp, 2);
> + ret = adis->ops->read(adis, reg, &tmp, 2);
> if (ret == 0)
> *val = tmp;
>
> @@ -227,7 +241,7 @@ static inline int __adis_read_reg_32(struct adis *adis, unsigned int reg,
> unsigned int tmp;
> int ret;
>
> - ret = __adis_read_reg(adis, reg, &tmp, 4);
> + ret = adis->ops->read(adis, reg, &tmp, 4);
> if (ret == 0)
> *val = tmp;
>
> @@ -245,7 +259,7 @@ static inline int adis_write_reg(struct adis *adis, unsigned int reg,
> unsigned int val, unsigned int size)
> {
> guard(mutex)(&adis->state_lock);
> - return __adis_write_reg(adis, reg, val, size);
> + return adis->ops->write(adis, reg, val, size);
> }
>
> /**
> @@ -259,7 +273,7 @@ static int adis_read_reg(struct adis *adis, unsigned int reg,
> unsigned int *val, unsigned int size)
> {
> guard(mutex)(&adis->state_lock);
> - return __adis_read_reg(adis, reg, val, size);
> + return adis->ops->read(adis, reg, val, size);
> }
>
> /**
Powered by blists - more mailing lists