* [PATCH] iio: cros_ec: unify hw fifo attributes into the core file
@ 2020-09-23 13:03 Alexandru Ardelean
2020-09-25 12:33 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Alexandru Ardelean @ 2020-09-23 13:03 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: bleung, gwendal, enric.balletbo, groeck, jic23, Alexandru Ardelean
The intent here is to minimize the use of iio_buffer_set_attrs(). Since we
are planning to add support for multiple IIO buffers per IIO device, the
issue has to do with:
1. Accessing 'indio_dev->buffer' directly (as is done with
'iio_buffer_set_attrs(indio_dev->buffer, <attrs>)').
2. The way that the buffer attributes would get handled or expanded when
there are more buffers per IIO device. Current a sysfs kobj_type expands
into a 'device' object that expands into an 'iio_dev' object.
We will need to change this, so that the sysfs attributes for IIO
buffers expand into IIO buffers at some point.
Right now, the current IIO framework works fine for the
'1 IIO device == 1 IIO buffer' case (that is now).
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/accel/cros_ec_accel_legacy.c | 2 +-
.../iio/common/cros_ec_sensors/cros_ec_lid_angle.c | 3 ++-
drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 5 ++---
.../iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 11 ++++++++---
drivers/iio/light/cros_ec_light_prox.c | 5 ++---
drivers/iio/pressure/cros_ec_baro.c | 5 ++---
include/linux/iio/common/cros_ec_sensors_core.h | 4 ++--
7 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
index b6f3471b62dc..8f1232c38e0d 100644
--- a/drivers/iio/accel/cros_ec_accel_legacy.c
+++ b/drivers/iio/accel/cros_ec_accel_legacy.c
@@ -215,7 +215,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
return -ENOMEM;
ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
- cros_ec_sensors_capture, NULL);
+ cros_ec_sensors_capture, NULL, false);
if (ret)
return ret;
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
index af801e203623..752f59037715 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
@@ -97,7 +97,8 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
if (!indio_dev)
return -ENOMEM;
- ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL, NULL);
+ ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL,
+ NULL, false);
if (ret)
return ret;
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
index 130ab8ce0269..57038ca48d93 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -236,12 +236,11 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
cros_ec_sensors_capture,
- cros_ec_sensors_push_data);
+ cros_ec_sensors_push_data,
+ true);
if (ret)
return ret;
- iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
-
indio_dev->info = &ec_sensors_info;
state = iio_priv(indio_dev);
for (channel = state->channels, i = CROS_EC_SENSOR_X;
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index ea480c1d4349..0de800d41978 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -174,12 +174,11 @@ static ssize_t hwfifo_watermark_max_show(struct device *dev,
static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0);
-const struct attribute *cros_ec_sensor_fifo_attributes[] = {
+static const struct attribute *cros_ec_sensor_fifo_attributes[] = {
&iio_dev_attr_hwfifo_timeout.dev_attr.attr,
&iio_dev_attr_hwfifo_watermark_max.dev_attr.attr,
NULL,
};
-EXPORT_SYMBOL_GPL(cros_ec_sensor_fifo_attributes);
int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
s16 *data,
@@ -238,6 +237,7 @@ static void cros_ec_sensors_core_clean(void *arg)
* for backward compatibility.
* @push_data: function to call when cros_ec_sensorhub receives
* a sample for that sensor.
+ * @has_hw_fifo: Set true if this device has/uses a HW FIFO
*
* Return: 0 on success, -errno on failure.
*/
@@ -245,7 +245,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
struct iio_dev *indio_dev,
bool physical_device,
cros_ec_sensors_capture_t trigger_capture,
- cros_ec_sensorhub_push_data_cb_t push_data)
+ cros_ec_sensorhub_push_data_cb_t push_data,
+ bool has_hw_fifo)
{
struct device *dev = &pdev->dev;
struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
@@ -358,6 +359,10 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
NULL);
if (ret)
return ret;
+
+ if (has_hw_fifo)
+ iio_buffer_set_attrs(indio_dev->buffer,
+ cros_ec_sensor_fifo_attributes);
}
}
diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
index fed79ba27fda..75d6b5fcf2cc 100644
--- a/drivers/iio/light/cros_ec_light_prox.c
+++ b/drivers/iio/light/cros_ec_light_prox.c
@@ -182,12 +182,11 @@ static int cros_ec_light_prox_probe(struct platform_device *pdev)
ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
cros_ec_sensors_capture,
- cros_ec_sensors_push_data);
+ cros_ec_sensors_push_data,
+ true);
if (ret)
return ret;
- iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
-
indio_dev->info = &cros_ec_light_prox_info;
state = iio_priv(indio_dev);
state->core.type = state->core.resp->info.type;
diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
index f0938b6fbba0..aa043cb9ac42 100644
--- a/drivers/iio/pressure/cros_ec_baro.c
+++ b/drivers/iio/pressure/cros_ec_baro.c
@@ -139,12 +139,11 @@ static int cros_ec_baro_probe(struct platform_device *pdev)
ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
cros_ec_sensors_capture,
- cros_ec_sensors_push_data);
+ cros_ec_sensors_push_data,
+ true);
if (ret)
return ret;
- iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
-
indio_dev->info = &cros_ec_baro_info;
state = iio_priv(indio_dev);
state->core.type = state->core.resp->info.type;
diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
index caa8bb279a34..c9b80be82440 100644
--- a/include/linux/iio/common/cros_ec_sensors_core.h
+++ b/include/linux/iio/common/cros_ec_sensors_core.h
@@ -96,7 +96,8 @@ struct platform_device;
int cros_ec_sensors_core_init(struct platform_device *pdev,
struct iio_dev *indio_dev, bool physical_device,
cros_ec_sensors_capture_t trigger_capture,
- cros_ec_sensorhub_push_data_cb_t push_data);
+ cros_ec_sensorhub_push_data_cb_t push_data,
+ bool has_hw_fifo);
irqreturn_t cros_ec_sensors_capture(int irq, void *p);
int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
@@ -125,6 +126,5 @@ extern const struct dev_pm_ops cros_ec_sensors_pm_ops;
/* List of extended channel specification for all sensors. */
extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
-extern const struct attribute *cros_ec_sensor_fifo_attributes[];
#endif /* __CROS_EC_SENSORS_CORE_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: cros_ec: unify hw fifo attributes into the core file
2020-09-23 13:03 [PATCH] iio: cros_ec: unify hw fifo attributes into the core file Alexandru Ardelean
@ 2020-09-25 12:33 ` Jonathan Cameron
2020-09-25 15:26 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2020-09-25 12:33 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, bleung, gwendal, enric.balletbo, groeck
On Wed, 23 Sep 2020 16:03:39 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> The intent here is to minimize the use of iio_buffer_set_attrs(). Since we
> are planning to add support for multiple IIO buffers per IIO device, the
> issue has to do with:
> 1. Accessing 'indio_dev->buffer' directly (as is done with
> 'iio_buffer_set_attrs(indio_dev->buffer, <attrs>)').
> 2. The way that the buffer attributes would get handled or expanded when
> there are more buffers per IIO device. Current a sysfs kobj_type expands
> into a 'device' object that expands into an 'iio_dev' object.
> We will need to change this, so that the sysfs attributes for IIO
> buffers expand into IIO buffers at some point.
>
> Right now, the current IIO framework works fine for the
> '1 IIO device == 1 IIO buffer' case (that is now).
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Looks good to me, but I'll need a cros_ec ack for this one.
thanks
Jonathan
> ---
> drivers/iio/accel/cros_ec_accel_legacy.c | 2 +-
> .../iio/common/cros_ec_sensors/cros_ec_lid_angle.c | 3 ++-
> drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 5 ++---
> .../iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 11 ++++++++---
> drivers/iio/light/cros_ec_light_prox.c | 5 ++---
> drivers/iio/pressure/cros_ec_baro.c | 5 ++---
> include/linux/iio/common/cros_ec_sensors_core.h | 4 ++--
> 7 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> index b6f3471b62dc..8f1232c38e0d 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -215,7 +215,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> - cros_ec_sensors_capture, NULL);
> + cros_ec_sensors_capture, NULL, false);
> if (ret)
> return ret;
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> index af801e203623..752f59037715 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> @@ -97,7 +97,8 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
> if (!indio_dev)
> return -ENOMEM;
>
> - ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL, NULL);
> + ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL,
> + NULL, false);
> if (ret)
> return ret;
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> index 130ab8ce0269..57038ca48d93 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -236,12 +236,11 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
>
> ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> cros_ec_sensors_capture,
> - cros_ec_sensors_push_data);
> + cros_ec_sensors_push_data,
> + true);
> if (ret)
> return ret;
>
> - iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
> -
> indio_dev->info = &ec_sensors_info;
> state = iio_priv(indio_dev);
> for (channel = state->channels, i = CROS_EC_SENSOR_X;
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index ea480c1d4349..0de800d41978 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -174,12 +174,11 @@ static ssize_t hwfifo_watermark_max_show(struct device *dev,
>
> static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0);
>
> -const struct attribute *cros_ec_sensor_fifo_attributes[] = {
> +static const struct attribute *cros_ec_sensor_fifo_attributes[] = {
> &iio_dev_attr_hwfifo_timeout.dev_attr.attr,
> &iio_dev_attr_hwfifo_watermark_max.dev_attr.attr,
> NULL,
> };
> -EXPORT_SYMBOL_GPL(cros_ec_sensor_fifo_attributes);
>
> int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
> s16 *data,
> @@ -238,6 +237,7 @@ static void cros_ec_sensors_core_clean(void *arg)
> * for backward compatibility.
> * @push_data: function to call when cros_ec_sensorhub receives
> * a sample for that sensor.
> + * @has_hw_fifo: Set true if this device has/uses a HW FIFO
> *
> * Return: 0 on success, -errno on failure.
> */
> @@ -245,7 +245,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> struct iio_dev *indio_dev,
> bool physical_device,
> cros_ec_sensors_capture_t trigger_capture,
> - cros_ec_sensorhub_push_data_cb_t push_data)
> + cros_ec_sensorhub_push_data_cb_t push_data,
> + bool has_hw_fifo)
> {
> struct device *dev = &pdev->dev;
> struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
> @@ -358,6 +359,10 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> NULL);
> if (ret)
> return ret;
> +
> + if (has_hw_fifo)
> + iio_buffer_set_attrs(indio_dev->buffer,
> + cros_ec_sensor_fifo_attributes);
> }
> }
>
> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> index fed79ba27fda..75d6b5fcf2cc 100644
> --- a/drivers/iio/light/cros_ec_light_prox.c
> +++ b/drivers/iio/light/cros_ec_light_prox.c
> @@ -182,12 +182,11 @@ static int cros_ec_light_prox_probe(struct platform_device *pdev)
>
> ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> cros_ec_sensors_capture,
> - cros_ec_sensors_push_data);
> + cros_ec_sensors_push_data,
> + true);
> if (ret)
> return ret;
>
> - iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
> -
> indio_dev->info = &cros_ec_light_prox_info;
> state = iio_priv(indio_dev);
> state->core.type = state->core.resp->info.type;
> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> index f0938b6fbba0..aa043cb9ac42 100644
> --- a/drivers/iio/pressure/cros_ec_baro.c
> +++ b/drivers/iio/pressure/cros_ec_baro.c
> @@ -139,12 +139,11 @@ static int cros_ec_baro_probe(struct platform_device *pdev)
>
> ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> cros_ec_sensors_capture,
> - cros_ec_sensors_push_data);
> + cros_ec_sensors_push_data,
> + true);
> if (ret)
> return ret;
>
> - iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
> -
> indio_dev->info = &cros_ec_baro_info;
> state = iio_priv(indio_dev);
> state->core.type = state->core.resp->info.type;
> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> index caa8bb279a34..c9b80be82440 100644
> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> @@ -96,7 +96,8 @@ struct platform_device;
> int cros_ec_sensors_core_init(struct platform_device *pdev,
> struct iio_dev *indio_dev, bool physical_device,
> cros_ec_sensors_capture_t trigger_capture,
> - cros_ec_sensorhub_push_data_cb_t push_data);
> + cros_ec_sensorhub_push_data_cb_t push_data,
> + bool has_hw_fifo);
>
> irqreturn_t cros_ec_sensors_capture(int irq, void *p);
> int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
> @@ -125,6 +126,5 @@ extern const struct dev_pm_ops cros_ec_sensors_pm_ops;
>
> /* List of extended channel specification for all sensors. */
> extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
> -extern const struct attribute *cros_ec_sensor_fifo_attributes[];
>
> #endif /* __CROS_EC_SENSORS_CORE_H */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: cros_ec: unify hw fifo attributes into the core file
2020-09-25 12:33 ` Jonathan Cameron
@ 2020-09-25 15:26 ` Guenter Roeck
2020-09-26 15:04 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2020-09-25 15:26 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Alexandru Ardelean, linux-iio, linux-kernel, Benson Leung,
Gwendal Grignou, Enric Balletbo i Serra, Guenter Roeck
On Fri, Sep 25, 2020 at 5:33 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed, 23 Sep 2020 16:03:39 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > The intent here is to minimize the use of iio_buffer_set_attrs(). Since we
> > are planning to add support for multiple IIO buffers per IIO device, the
> > issue has to do with:
> > 1. Accessing 'indio_dev->buffer' directly (as is done with
> > 'iio_buffer_set_attrs(indio_dev->buffer, <attrs>)').
> > 2. The way that the buffer attributes would get handled or expanded when
> > there are more buffers per IIO device. Current a sysfs kobj_type expands
> > into a 'device' object that expands into an 'iio_dev' object.
> > We will need to change this, so that the sysfs attributes for IIO
> > buffers expand into IIO buffers at some point.
> >
> > Right now, the current IIO framework works fine for the
> > '1 IIO device == 1 IIO buffer' case (that is now).
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> Looks good to me, but I'll need a cros_ec ack for this one.
>
I like it.
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Guenter
> thanks
>
> Jonathan
>
> > ---
> > drivers/iio/accel/cros_ec_accel_legacy.c | 2 +-
> > .../iio/common/cros_ec_sensors/cros_ec_lid_angle.c | 3 ++-
> > drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 5 ++---
> > .../iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 11 ++++++++---
> > drivers/iio/light/cros_ec_light_prox.c | 5 ++---
> > drivers/iio/pressure/cros_ec_baro.c | 5 ++---
> > include/linux/iio/common/cros_ec_sensors_core.h | 4 ++--
> > 7 files changed, 19 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> > index b6f3471b62dc..8f1232c38e0d 100644
> > --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> > +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> > @@ -215,7 +215,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> > return -ENOMEM;
> >
> > ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > - cros_ec_sensors_capture, NULL);
> > + cros_ec_sensors_capture, NULL, false);
> > if (ret)
> > return ret;
> >
> > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > index af801e203623..752f59037715 100644
> > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > @@ -97,7 +97,8 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
> > if (!indio_dev)
> > return -ENOMEM;
> >
> > - ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL, NULL);
> > + ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL,
> > + NULL, false);
> > if (ret)
> > return ret;
> >
> > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > index 130ab8ce0269..57038ca48d93 100644
> > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > @@ -236,12 +236,11 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
> >
> > ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > cros_ec_sensors_capture,
> > - cros_ec_sensors_push_data);
> > + cros_ec_sensors_push_data,
> > + true);
> > if (ret)
> > return ret;
> >
> > - iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
> > -
> > indio_dev->info = &ec_sensors_info;
> > state = iio_priv(indio_dev);
> > for (channel = state->channels, i = CROS_EC_SENSOR_X;
> > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > index ea480c1d4349..0de800d41978 100644
> > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > @@ -174,12 +174,11 @@ static ssize_t hwfifo_watermark_max_show(struct device *dev,
> >
> > static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0);
> >
> > -const struct attribute *cros_ec_sensor_fifo_attributes[] = {
> > +static const struct attribute *cros_ec_sensor_fifo_attributes[] = {
> > &iio_dev_attr_hwfifo_timeout.dev_attr.attr,
> > &iio_dev_attr_hwfifo_watermark_max.dev_attr.attr,
> > NULL,
> > };
> > -EXPORT_SYMBOL_GPL(cros_ec_sensor_fifo_attributes);
> >
> > int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
> > s16 *data,
> > @@ -238,6 +237,7 @@ static void cros_ec_sensors_core_clean(void *arg)
> > * for backward compatibility.
> > * @push_data: function to call when cros_ec_sensorhub receives
> > * a sample for that sensor.
> > + * @has_hw_fifo: Set true if this device has/uses a HW FIFO
> > *
> > * Return: 0 on success, -errno on failure.
> > */
> > @@ -245,7 +245,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > struct iio_dev *indio_dev,
> > bool physical_device,
> > cros_ec_sensors_capture_t trigger_capture,
> > - cros_ec_sensorhub_push_data_cb_t push_data)
> > + cros_ec_sensorhub_push_data_cb_t push_data,
> > + bool has_hw_fifo)
> > {
> > struct device *dev = &pdev->dev;
> > struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
> > @@ -358,6 +359,10 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > NULL);
> > if (ret)
> > return ret;
> > +
> > + if (has_hw_fifo)
> > + iio_buffer_set_attrs(indio_dev->buffer,
> > + cros_ec_sensor_fifo_attributes);
> > }
> > }
> >
> > diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> > index fed79ba27fda..75d6b5fcf2cc 100644
> > --- a/drivers/iio/light/cros_ec_light_prox.c
> > +++ b/drivers/iio/light/cros_ec_light_prox.c
> > @@ -182,12 +182,11 @@ static int cros_ec_light_prox_probe(struct platform_device *pdev)
> >
> > ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > cros_ec_sensors_capture,
> > - cros_ec_sensors_push_data);
> > + cros_ec_sensors_push_data,
> > + true);
> > if (ret)
> > return ret;
> >
> > - iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
> > -
> > indio_dev->info = &cros_ec_light_prox_info;
> > state = iio_priv(indio_dev);
> > state->core.type = state->core.resp->info.type;
> > diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> > index f0938b6fbba0..aa043cb9ac42 100644
> > --- a/drivers/iio/pressure/cros_ec_baro.c
> > +++ b/drivers/iio/pressure/cros_ec_baro.c
> > @@ -139,12 +139,11 @@ static int cros_ec_baro_probe(struct platform_device *pdev)
> >
> > ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > cros_ec_sensors_capture,
> > - cros_ec_sensors_push_data);
> > + cros_ec_sensors_push_data,
> > + true);
> > if (ret)
> > return ret;
> >
> > - iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
> > -
> > indio_dev->info = &cros_ec_baro_info;
> > state = iio_priv(indio_dev);
> > state->core.type = state->core.resp->info.type;
> > diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> > index caa8bb279a34..c9b80be82440 100644
> > --- a/include/linux/iio/common/cros_ec_sensors_core.h
> > +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> > @@ -96,7 +96,8 @@ struct platform_device;
> > int cros_ec_sensors_core_init(struct platform_device *pdev,
> > struct iio_dev *indio_dev, bool physical_device,
> > cros_ec_sensors_capture_t trigger_capture,
> > - cros_ec_sensorhub_push_data_cb_t push_data);
> > + cros_ec_sensorhub_push_data_cb_t push_data,
> > + bool has_hw_fifo);
> >
> > irqreturn_t cros_ec_sensors_capture(int irq, void *p);
> > int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
> > @@ -125,6 +126,5 @@ extern const struct dev_pm_ops cros_ec_sensors_pm_ops;
> >
> > /* List of extended channel specification for all sensors. */
> > extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
> > -extern const struct attribute *cros_ec_sensor_fifo_attributes[];
> >
> > #endif /* __CROS_EC_SENSORS_CORE_H */
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: cros_ec: unify hw fifo attributes into the core file
2020-09-25 15:26 ` Guenter Roeck
@ 2020-09-26 15:04 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2020-09-26 15:04 UTC (permalink / raw)
To: Guenter Roeck
Cc: Alexandru Ardelean, linux-iio, linux-kernel, Benson Leung,
Gwendal Grignou, Enric Balletbo i Serra, Guenter Roeck
On Fri, 25 Sep 2020 08:26:23 -0700
Guenter Roeck <groeck@google.com> wrote:
> On Fri, Sep 25, 2020 at 5:33 AM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Wed, 23 Sep 2020 16:03:39 +0300
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> >
> > > The intent here is to minimize the use of iio_buffer_set_attrs(). Since we
> > > are planning to add support for multiple IIO buffers per IIO device, the
> > > issue has to do with:
> > > 1. Accessing 'indio_dev->buffer' directly (as is done with
> > > 'iio_buffer_set_attrs(indio_dev->buffer, <attrs>)').
> > > 2. The way that the buffer attributes would get handled or expanded when
> > > there are more buffers per IIO device. Current a sysfs kobj_type expands
> > > into a 'device' object that expands into an 'iio_dev' object.
> > > We will need to change this, so that the sysfs attributes for IIO
> > > buffers expand into IIO buffers at some point.
> > >
> > > Right now, the current IIO framework works fine for the
> > > '1 IIO device == 1 IIO buffer' case (that is now).
> > >
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > Looks good to me, but I'll need a cros_ec ack for this one.
> >
>
> I like it.
>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
Thanks,
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
>
> Guenter
>
> > thanks
> >
> > Jonathan
> >
> > > ---
> > > drivers/iio/accel/cros_ec_accel_legacy.c | 2 +-
> > > .../iio/common/cros_ec_sensors/cros_ec_lid_angle.c | 3 ++-
> > > drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 5 ++---
> > > .../iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 11 ++++++++---
> > > drivers/iio/light/cros_ec_light_prox.c | 5 ++---
> > > drivers/iio/pressure/cros_ec_baro.c | 5 ++---
> > > include/linux/iio/common/cros_ec_sensors_core.h | 4 ++--
> > > 7 files changed, 19 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> > > index b6f3471b62dc..8f1232c38e0d 100644
> > > --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> > > +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> > > @@ -215,7 +215,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> > > return -ENOMEM;
> > >
> > > ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > > - cros_ec_sensors_capture, NULL);
> > > + cros_ec_sensors_capture, NULL, false);
> > > if (ret)
> > > return ret;
> > >
> > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > > index af801e203623..752f59037715 100644
> > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > > @@ -97,7 +97,8 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
> > > if (!indio_dev)
> > > return -ENOMEM;
> > >
> > > - ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL, NULL);
> > > + ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL,
> > > + NULL, false);
> > > if (ret)
> > > return ret;
> > >
> > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > > index 130ab8ce0269..57038ca48d93 100644
> > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > > @@ -236,12 +236,11 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
> > >
> > > ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > > cros_ec_sensors_capture,
> > > - cros_ec_sensors_push_data);
> > > + cros_ec_sensors_push_data,
> > > + true);
> > > if (ret)
> > > return ret;
> > >
> > > - iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
> > > -
> > > indio_dev->info = &ec_sensors_info;
> > > state = iio_priv(indio_dev);
> > > for (channel = state->channels, i = CROS_EC_SENSOR_X;
> > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > index ea480c1d4349..0de800d41978 100644
> > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > @@ -174,12 +174,11 @@ static ssize_t hwfifo_watermark_max_show(struct device *dev,
> > >
> > > static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0);
> > >
> > > -const struct attribute *cros_ec_sensor_fifo_attributes[] = {
> > > +static const struct attribute *cros_ec_sensor_fifo_attributes[] = {
> > > &iio_dev_attr_hwfifo_timeout.dev_attr.attr,
> > > &iio_dev_attr_hwfifo_watermark_max.dev_attr.attr,
> > > NULL,
> > > };
> > > -EXPORT_SYMBOL_GPL(cros_ec_sensor_fifo_attributes);
> > >
> > > int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
> > > s16 *data,
> > > @@ -238,6 +237,7 @@ static void cros_ec_sensors_core_clean(void *arg)
> > > * for backward compatibility.
> > > * @push_data: function to call when cros_ec_sensorhub receives
> > > * a sample for that sensor.
> > > + * @has_hw_fifo: Set true if this device has/uses a HW FIFO
> > > *
> > > * Return: 0 on success, -errno on failure.
> > > */
> > > @@ -245,7 +245,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > > struct iio_dev *indio_dev,
> > > bool physical_device,
> > > cros_ec_sensors_capture_t trigger_capture,
> > > - cros_ec_sensorhub_push_data_cb_t push_data)
> > > + cros_ec_sensorhub_push_data_cb_t push_data,
> > > + bool has_hw_fifo)
> > > {
> > > struct device *dev = &pdev->dev;
> > > struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
> > > @@ -358,6 +359,10 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > > NULL);
> > > if (ret)
> > > return ret;
> > > +
> > > + if (has_hw_fifo)
> > > + iio_buffer_set_attrs(indio_dev->buffer,
> > > + cros_ec_sensor_fifo_attributes);
> > > }
> > > }
> > >
> > > diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> > > index fed79ba27fda..75d6b5fcf2cc 100644
> > > --- a/drivers/iio/light/cros_ec_light_prox.c
> > > +++ b/drivers/iio/light/cros_ec_light_prox.c
> > > @@ -182,12 +182,11 @@ static int cros_ec_light_prox_probe(struct platform_device *pdev)
> > >
> > > ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > > cros_ec_sensors_capture,
> > > - cros_ec_sensors_push_data);
> > > + cros_ec_sensors_push_data,
> > > + true);
> > > if (ret)
> > > return ret;
> > >
> > > - iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
> > > -
> > > indio_dev->info = &cros_ec_light_prox_info;
> > > state = iio_priv(indio_dev);
> > > state->core.type = state->core.resp->info.type;
> > > diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> > > index f0938b6fbba0..aa043cb9ac42 100644
> > > --- a/drivers/iio/pressure/cros_ec_baro.c
> > > +++ b/drivers/iio/pressure/cros_ec_baro.c
> > > @@ -139,12 +139,11 @@ static int cros_ec_baro_probe(struct platform_device *pdev)
> > >
> > > ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > > cros_ec_sensors_capture,
> > > - cros_ec_sensors_push_data);
> > > + cros_ec_sensors_push_data,
> > > + true);
> > > if (ret)
> > > return ret;
> > >
> > > - iio_buffer_set_attrs(indio_dev->buffer, cros_ec_sensor_fifo_attributes);
> > > -
> > > indio_dev->info = &cros_ec_baro_info;
> > > state = iio_priv(indio_dev);
> > > state->core.type = state->core.resp->info.type;
> > > diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> > > index caa8bb279a34..c9b80be82440 100644
> > > --- a/include/linux/iio/common/cros_ec_sensors_core.h
> > > +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> > > @@ -96,7 +96,8 @@ struct platform_device;
> > > int cros_ec_sensors_core_init(struct platform_device *pdev,
> > > struct iio_dev *indio_dev, bool physical_device,
> > > cros_ec_sensors_capture_t trigger_capture,
> > > - cros_ec_sensorhub_push_data_cb_t push_data);
> > > + cros_ec_sensorhub_push_data_cb_t push_data,
> > > + bool has_hw_fifo);
> > >
> > > irqreturn_t cros_ec_sensors_capture(int irq, void *p);
> > > int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
> > > @@ -125,6 +126,5 @@ extern const struct dev_pm_ops cros_ec_sensors_pm_ops;
> > >
> > > /* List of extended channel specification for all sensors. */
> > > extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
> > > -extern const struct attribute *cros_ec_sensor_fifo_attributes[];
> > >
> > > #endif /* __CROS_EC_SENSORS_CORE_H */
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-26 15:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23 13:03 [PATCH] iio: cros_ec: unify hw fifo attributes into the core file Alexandru Ardelean
2020-09-25 12:33 ` Jonathan Cameron
2020-09-25 15:26 ` Guenter Roeck
2020-09-26 15:04 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.