All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lorenzo Bianconi <lorenzo@kernel.org>
To: Sean Nyekjaer <sean@geanix.com>
Cc: linux-iio@vger.kernel.org, jic23@kernel.org,
	lorenzo.bianconi83@gmail.com, denis.ciocca@st.com,
	mario.tesi@st.com, armando.visconti@st.com, martin@geanix.com
Subject: Re: [PATCH v9 6/6] iio: imu: st_lsm6dsx: filter motion events in driver
Date: Mon, 16 Sep 2019 15:32:55 +0200	[thread overview]
Message-ID: <20190916133255.GE16063@localhost.localdomain> (raw)
In-Reply-To: <20190916123456.1742253-6-sean@geanix.com>

[-- Attachment #1: Type: text/plain, Size: 4604 bytes --]

> Do not report non enabled motion events.
> Wakeup will still be on all channels as it's not possible to do the
> filtering in hw.

I do not see why we need a different patch to take care of this, I think this
patch should be squashed with patch 2/6 and 5/6

> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  2 +-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 38 +++++++++++++++-----
>  2 files changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index 0a782af9445b..fd02d0e184f3 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -360,7 +360,7 @@ struct st_lsm6dsx_hw {
>  	u8 sip;
>  
>  	u8 event_threshold;
> -	bool enable_event;
> +	u8 enable_event;

this should be merged in patch 2/6

>  	struct st_lsm6dsx_reg irq_routing;
>  
>  	u8 *buff;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 6b88d93dca2a..92fee4555dd5 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1202,7 +1202,7 @@ static int st_lsm6dsx_read_oneshot(struct st_lsm6dsx_sensor *sensor,
>  	if (err < 0)
>  		return err;
>  
> -	if (!hw->enable_event)
> +	if (0 == hw->enable_event)

why do we need this?

>  		st_lsm6dsx_sensor_set_enable(sensor, false);
>  
>  	*val = (s16)le16_to_cpu(data);
> @@ -1360,7 +1360,10 @@ static int st_lsm6dsx_read_event_config(struct iio_dev *iio_dev,
>  	if (type != IIO_EV_TYPE_THRESH)
>  		return -EINVAL;
>  
> -	return hw->enable_event;
> +	if (hw->enable_event & BIT(chan->channel2))
> +		return 1;
> +
> +	return 0;

please just do:

	return !!(hw->enable_event & BIT(chan->channel2));

>  }
>  
>  static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
> @@ -1371,13 +1374,28 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
>  {
>  	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	u8 enable_event;
>  	int err = 0;
>  
>  	if (type != IIO_EV_TYPE_THRESH)
>  		return -EINVAL;
>  
> -	/* do not enable events if they are already enabled */
> -	if (state && hw->enable_event)
> +	if (state) {
> +		enable_event = hw->enable_event | BIT(chan->channel2);
> +
> +		/* do not enable events if they are already enabled */
> +		if (hw->enable_event)
> +			goto out;
> +	} else {
> +		enable_event = hw->enable_event & ~BIT(chan->channel2);
> +
> +		/* only turn off sensor if no events is enabled */
> +		if (enable_event)
> +			goto out;
> +	}
> +
> +	/* stop here if no changes have been made */
> +	if (hw->enable_event == enable_event)
>  		return 0;
>  
>  	err = st_lsm6dsx_event_setup(hw, state);
> @@ -1388,7 +1406,8 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
>  	if (err < 0)
>  		return err;
>  
> -	hw->enable_event = state;
> +out:
> +	hw->enable_event = enable_event;
>  
>  	return 0;
>  }
> @@ -1745,7 +1764,8 @@ void st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw, int data)
>  {
>  	s64 timestamp = iio_get_time_ns(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
>  
> -	if (data & hw->settings->event_settings.wakeup_src_z_mask)
> +	if ((data & hw->settings->event_settings.wakeup_src_z_mask) &&
> +	    (hw->enable_event & BIT(IIO_MOD_Z)))
>  		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,

this should be merged in patch 5/6

>  						  0,
> @@ -1754,7 +1774,8 @@ void st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw, int data)
>  						  IIO_EV_DIR_EITHER),
>  						  timestamp);
>  
> -	if (data & hw->settings->event_settings.wakeup_src_y_mask)
> +	if ((data & hw->settings->event_settings.wakeup_src_y_mask) &&
> +	    (hw->enable_event & BIT(IIO_MOD_Y)))
>  		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
> @@ -1763,7 +1784,8 @@ void st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw, int data)
>  						  IIO_EV_DIR_EITHER),
>  						  timestamp);
>  
> -	if (data & hw->settings->event_settings.wakeup_src_x_mask)
> +	if ((data & hw->settings->event_settings.wakeup_src_x_mask) &&
> +	    (hw->enable_event & BIT(IIO_MOD_X)))
>  		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
> -- 
> 2.23.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

      reply	other threads:[~2019-09-16 13:33 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-16 12:34 [PATCH v9 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
2019-09-16 12:34 ` [PATCH v9 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
2019-09-16 12:34 ` [PATCH v9 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
2019-09-16 12:34 ` [PATCH v9 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
2019-09-16 12:34 ` [PATCH v9 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
2019-09-16 12:34 ` [PATCH v9 6/6] iio: imu: st_lsm6dsx: filter motion events in driver Sean Nyekjaer
2019-09-16 13:32   ` Lorenzo Bianconi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190916133255.GE16063@localhost.localdomain \
    --to=lorenzo@kernel.org \
    --cc=armando.visconti@st.com \
    --cc=denis.ciocca@st.com \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=lorenzo.bianconi83@gmail.com \
    --cc=mario.tesi@st.com \
    --cc=martin@geanix.com \
    --cc=sean@geanix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.