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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e89813c7-696a-bdb1-8452-3aa2886d2d9a@posteo.de>
Date:   Fri, 10 Nov 2017 23:35:04 +0100
From:   Martin Kepplinger <martink@...teo.de>
To:     Harinath Nampally <harinath922@...il.com>, jic23@...nel.org
Cc:     knaack.h@....de, lars@...afoo.de, pmeerw@...erw.net,
        gregkh@...uxfoundation.org, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org, amsfield22@...il.com
Subject: Re: [PATCH] iio: accel: mma8452: Add single pulse/tap event detection

On 2017-11-09 04:12, Harinath Nampally wrote:
> This patch adds following related changes:
> - defines pulse event related registers
> - enables and handles single pulse interrupt for fxls8471
> - handles IIO_EV_DIR_EITHER in read/write callbacks (because
>   event direction for pulse is either rising or falling)
> - configures read/write event value for pulse latency register
>   using IIO_EV_INFO_HYSTERESIS
> - adds multiple events like pulse and tranient event spec
>   as elements of event_spec array named 'mma8452_accel_events'
> 
> Except mma8653 chip all other chips like mma845x and
> fxls8471 have single tap detection feature.
> Tested thoroughly using iio_event_monitor application on
> imx6ul-evk board which has fxls8471.
> 
> Signed-off-by: Harinath Nampally <harinath922@...il.com>
> ---

What tree is this written against? It doesn't apply to the current -next
anyways.

>  drivers/iio/accel/mma8452.c | 156 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 151 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 43c3a6b..36f1b56 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -72,6 +72,19 @@
>  #define  MMA8452_TRANSIENT_THS_MASK		GENMASK(6, 0)
>  #define MMA8452_TRANSIENT_COUNT			0x20
>  #define MMA8452_TRANSIENT_CHAN_SHIFT 1
> +#define MMA8452_PULSE_CFG			0x21
> +#define MMA8452_PULSE_CFG_CHAN(chan)		BIT(chan * 2)
> +#define MMA8452_PULSE_CFG_ELE			BIT(6)
> +#define MMA8452_PULSE_SRC			0x22
> +#define MMA8452_PULSE_SRC_XPULSE		BIT(4)
> +#define MMA8452_PULSE_SRC_YPULSE		BIT(5)
> +#define MMA8452_PULSE_SRC_ZPULSE		BIT(6)
> +#define MMA8452_PULSE_THS			0x23
> +#define MMA8452_PULSE_THS_MASK			GENMASK(6, 0)
> +#define MMA8452_PULSE_COUNT			0x26
> +#define MMA8452_PULSE_CHAN_SHIFT		2
> +#define MMA8452_PULSE_LTCY			0x27
> +
>  #define MMA8452_CTRL_REG1			0x2a
>  #define  MMA8452_CTRL_ACTIVE			BIT(0)
>  #define  MMA8452_CTRL_DR_MASK			GENMASK(5, 3)
> @@ -91,6 +104,7 @@
>  
>  #define  MMA8452_INT_DRDY			BIT(0)
>  #define  MMA8452_INT_FF_MT			BIT(2)
> +#define  MMA8452_INT_PULSE			BIT(3)
>  #define  MMA8452_INT_TRANS			BIT(5)
>  
I think the defintions would deserve to be in a separate patch, but
that's debatable.

>  #define MMA8451_DEVICE_ID			0x1a
> @@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
>  		.ev_count = MMA8452_TRANSIENT_COUNT,
>  };
>  
> +static const struct mma8452_event_regs pulse_ev_regs = {
> +		.ev_cfg = MMA8452_PULSE_CFG,
> +		.ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
> +		.ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
> +		.ev_src = MMA8452_PULSE_SRC,
> +		.ev_ths = MMA8452_PULSE_THS,
> +		.ev_ths_mask = MMA8452_PULSE_THS_MASK,
> +		.ev_count = MMA8452_PULSE_COUNT,
> +};
> +
>  /**
>   * struct mma_chip_info - chip specific data
>   * @chip_id:			WHO_AM_I register's value
> @@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
>  		case IIO_EV_DIR_FALLING:
>  			*ev_reg = &ff_mt_ev_regs;
>  			return 0;
> +		case IIO_EV_DIR_EITHER:
> +			if (!(data->chip_info->all_events
> +					& MMA8452_INT_PULSE) ||
> +				!(data->chip_info->enabled_events
> +					& MMA8452_INT_PULSE))
> +				return -EINVAL;
> +			*ev_reg = &pulse_ev_regs;
> +			return 0;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev *indio_dev,
>  				return ret;
>  		}
>  
> +	case IIO_EV_INFO_HYSTERESIS:
> +		if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
> +			!(data->chip_info->enabled_events & MMA8452_INT_PULSE))
> +			return -EINVAL;
> +
> +		ret = i2c_smbus_read_byte_data(data->client,
> +						MMA8452_PULSE_LTCY);
> +		if (ret < 0)
> +			return ret;
> +
> +		power_mode = mma8452_get_power_mode(data);
> +		if (power_mode < 0)
> +			return power_mode;
> +
> +		us = ret * mma8452_time_step_us[power_mode][
> +				mma8452_get_odr_index(data)];
> +		*val = us / USEC_PER_SEC;
> +		*val2 = us % USEC_PER_SEC;
> +
>  		return IIO_VAL_INT_PLUS_MICRO;
>  
>  	default:
> @@ -908,6 +959,24 @@ static int mma8452_write_event_value(struct iio_dev *indio_dev,
>  
>  		return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, reg);
>  
> +	case IIO_EV_INFO_HYSTERESIS:
> +		if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
> +			!(data->chip_info->enabled_events & MMA8452_INT_PULSE))
> +			return -EINVAL;
> +
> +		ret = mma8452_get_power_mode(data);
> +		if (ret < 0)
> +			return ret;
> +
> +		steps = (val * USEC_PER_SEC + val2) /
> +				mma8452_time_step_us[ret][
> +					mma8452_get_odr_index(data)];
> +
> +		if (steps < 0 || steps > 0xff)
> +			return -EINVAL;
> +
> +		return mma8452_change_config(data, MMA8452_PULSE_LTCY, steps);
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -937,6 +1006,14 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev,
>  
>  		return !!(ret & BIT(chan->scan_index +
>  				ev_regs->ev_cfg_chan_shift));
> +	case IIO_EV_DIR_EITHER:
> +		ret = i2c_smbus_read_byte_data(data->client,
> +				ev_regs->ev_cfg);
> +		if (ret < 0)
> +			return ret;
> +
> +		return !!(ret & BIT(chan->scan_index *
> +				ev_regs->ev_cfg_chan_shift));
>  	default:
>  		return -EINVAL;
>  	}
> @@ -988,6 +1065,25 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
>  		val |= ev_regs->ev_cfg_ele;
>  
>  		return mma8452_change_config(data, ev_regs->ev_cfg, val);
> +
> +	case IIO_EV_DIR_EITHER:
> +		val = i2c_smbus_read_byte_data(data->client, ev_regs->ev_cfg);
> +		if (val < 0)
> +			return val;
> +
> +		if (state) {
> +			val &= ~BIT(chan->scan_index *
> +					ev_regs->ev_cfg_chan_shift);
> +			val |= BIT(chan->scan_index *
> +					ev_regs->ev_cfg_chan_shift);
> +		} else {
> +			val &= ~BIT(chan->scan_index *
> +					ev_regs->ev_cfg_chan_shift);
> +		}
> +
> +		val |= ev_regs->ev_cfg_ele;
> +
> +		return mma8452_change_config(data, ev_regs->ev_cfg, val);
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1025,6 +1121,38 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
>  			       ts);
>  }
>  
> +static void mma8452_pulse_interrupt(struct iio_dev *indio_dev)
> +{
> +	struct mma8452_data *data = iio_priv(indio_dev);
> +	s64 ts = iio_get_time_ns(indio_dev);
> +	int src;
> +
> +	src = i2c_smbus_read_byte_data(data->client, MMA8452_PULSE_SRC);
> +	if (src < 0)
> +		return;
> +
> +	if (src & MMA8452_PULSE_SRC_XPULSE)
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
> +						  IIO_EV_TYPE_MAG,
> +						  IIO_EV_DIR_EITHER),
> +			       ts);
> +
> +	if (src & MMA8452_PULSE_SRC_YPULSE)
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
> +						  IIO_EV_TYPE_MAG,
> +						  IIO_EV_DIR_EITHER),
> +			       ts);
> +
> +	if (src & MMA8452_PULSE_SRC_ZPULSE)
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
> +						  IIO_EV_TYPE_MAG,
> +						  IIO_EV_DIR_EITHER),
> +			       ts);
> +}
> +
>  static irqreturn_t mma8452_interrupt(int irq, void *p)
>  {
>  	struct iio_dev *indio_dev = p;
> @@ -1063,6 +1191,11 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
>  		ret = IRQ_HANDLED;
>  	}
>  
> +	if (src & MMA8452_INT_PULSE) {
> +		mma8452_pulse_interrupt(indio_dev);
> +		ret = IRQ_HANDLED;
> +	}
> +
>  	return ret;
>  }
>  
> @@ -1130,8 +1263,10 @@ static const struct iio_event_spec mma8652_freefall_event[] = {
>  	},
>  };
>  
> -static const struct iio_event_spec mma8452_transient_event[] = {
> +
> +static const struct iio_event_spec mma8452_accel_events[] = {

I would prefer having this renaming in a separate patch too - with a
good expanation about why. (Don't be afraid of longer commit messages)

>  	{
> +		//trasient event

typo?

>  		.type = IIO_EV_TYPE_MAG,
>  		.dir = IIO_EV_DIR_RISING,
>  		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
> @@ -1139,6 +1274,15 @@ static const struct iio_event_spec mma8452_transient_event[] = {
>  					BIT(IIO_EV_INFO_PERIOD) |
>  					BIT(IIO_EV_INFO_HIGH_PASS_FILTER_3DB)
>  	},
> +	{
> +		//pulse event
> +		.type = IIO_EV_TYPE_MAG,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
> +		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
> +					BIT(IIO_EV_INFO_PERIOD) |
> +					BIT(IIO_EV_INFO_HYSTERESIS)
> +	},
>  };
>  
>  static const struct iio_event_spec mma8452_motion_event[] = {
> @@ -1202,8 +1346,8 @@ static struct attribute_group mma8452_event_attribute_group = {
>  		.shift = 16 - (bits), \
>  		.endianness = IIO_BE, \
>  	}, \
> -	.event_spec = mma8452_transient_event, \
> -	.num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
> +	.event_spec = mma8452_accel_events, \
> +	.num_event_specs = ARRAY_SIZE(mma8452_accel_events), \

that would go in the mentioned separate renaming-patch

>  }
>  
>  #define MMA8652_CHANNEL(axis, idx, bits) { \
> @@ -1368,9 +1512,11 @@ static const struct mma_chip_info mma_chip_info_table[] = {
>  		 */
>  		.all_events = MMA8452_INT_DRDY |
>  					MMA8452_INT_TRANS |
> -					MMA8452_INT_FF_MT,
> +					MMA8452_INT_FF_MT |
> +					MMA8452_INT_PULSE,
>  		.enabled_events = MMA8452_INT_TRANS |
> -					MMA8452_INT_FF_MT,
> +					MMA8452_INT_FF_MT |
> +					MMA8452_INT_PULSE,
>  	},
>  };
>  
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ