On 05/02/15 17:06, Srinivas Pandruvada wrote:
> On Sat, 2015-01-31 at 02:00 +0200, Octavian Purdila wrote:
>> This patch combines the any motion and new data interrupts function
>> into a single, generic, interrupt enable function. On top of this, we
>> can later refactor triggers to make it easier to add new triggers.
>>
>> Signed-off-by: Octavian Purdila <octavian.purdila@xxxxxxxxx>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
Worthwhile little refactor on it's own. Again would ideally have been
in a precursor series, but anyhow, applied to the togreg branch of iio.git.
Thanks,
>> ---
>> drivers/iio/accel/bmc150-accel.c | 272 ++++++++++++++++-----------------------
>> 1 file changed, 113 insertions(+), 159 deletions(-)
>>
>> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
>> index 2b6b80d..0873925 100644
>> --- a/drivers/iio/accel/bmc150-accel.c
>> +++ b/drivers/iio/accel/bmc150-accel.c
>> @@ -359,137 +359,6 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
>> return 0;
>> }
>>
>> -static int bmc150_accel_setup_any_motion_interrupt(
>> - struct bmc150_accel_data *data,
>> - bool status)
>> -{
>> - int ret;
>> -
>> - /* Enable/Disable INT1 mapping */
>> - ret = i2c_smbus_read_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_MAP_0);
>> - if (ret < 0) {
>> - dev_err(&data->client->dev, "Error reading reg_int_map_0\n");
>> - return ret;
>> - }
>> - if (status)
>> - ret |= BMC150_ACCEL_INT_MAP_0_BIT_SLOPE;
>> - else
>> - ret &= ~BMC150_ACCEL_INT_MAP_0_BIT_SLOPE;
>> -
>> - ret = i2c_smbus_write_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_MAP_0,
>> - ret);
>> - if (ret < 0) {
>> - dev_err(&data->client->dev, "Error writing reg_int_map_0\n");
>> - return ret;
>> - }
>> -
>> - if (status) {
>> - /*
>> - * New data interrupt is always non-latched,
>> - * which will have higher priority, so no need
>> - * to set latched mode, we will be flooded anyway with INTR
>> - */
>> - if (!data->dready_trigger_on) {
>> - ret = i2c_smbus_write_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_RST_LATCH,
>> - BMC150_ACCEL_INT_MODE_LATCH_INT |
>> - BMC150_ACCEL_INT_MODE_LATCH_RESET);
>> - if (ret < 0) {
>> - dev_err(&data->client->dev,
>> - "Error writing reg_int_rst_latch\n");
>> - return ret;
>> - }
>> - }
>> -
>> - ret = i2c_smbus_write_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_EN_0,
>> - BMC150_ACCEL_INT_EN_BIT_SLP_X |
>> - BMC150_ACCEL_INT_EN_BIT_SLP_Y |
>> - BMC150_ACCEL_INT_EN_BIT_SLP_Z);
>> - } else
>> - ret = i2c_smbus_write_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_EN_0,
>> - 0);
>> -
>> - if (ret < 0) {
>> - dev_err(&data->client->dev, "Error writing reg_int_en_0\n");
>> - return ret;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> -static int bmc150_accel_setup_new_data_interrupt(struct bmc150_accel_data *data,
>> - bool status)
>> -{
>> - int ret;
>> -
>> - /* Enable/Disable INT1 mapping */
>> - ret = i2c_smbus_read_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_MAP_1);
>> - if (ret < 0) {
>> - dev_err(&data->client->dev, "Error reading reg_int_map_1\n");
>> - return ret;
>> - }
>> - if (status)
>> - ret |= BMC150_ACCEL_INT_MAP_1_BIT_DATA;
>> - else
>> - ret &= ~BMC150_ACCEL_INT_MAP_1_BIT_DATA;
>> -
>> - ret = i2c_smbus_write_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_MAP_1,
>> - ret);
>> - if (ret < 0) {
>> - dev_err(&data->client->dev, "Error writing reg_int_map_1\n");
>> - return ret;
>> - }
>> -
>> - if (status) {
>> - /*
>> - * Set non latched mode interrupt and clear any latched
>> - * interrupt
>> - */
>> - ret = i2c_smbus_write_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_RST_LATCH,
>> - BMC150_ACCEL_INT_MODE_NON_LATCH_INT |
>> - BMC150_ACCEL_INT_MODE_LATCH_RESET);
>> - if (ret < 0) {
>> - dev_err(&data->client->dev,
>> - "Error writing reg_int_rst_latch\n");
>> - return ret;
>> - }
>> -
>> - ret = i2c_smbus_write_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_EN_1,
>> - BMC150_ACCEL_INT_EN_BIT_DATA_EN);
>> -
>> - } else {
>> - /* Restore default interrupt mode */
>> - ret = i2c_smbus_write_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_RST_LATCH,
>> - BMC150_ACCEL_INT_MODE_LATCH_INT |
>> - BMC150_ACCEL_INT_MODE_LATCH_RESET);
>> - if (ret < 0) {
>> - dev_err(&data->client->dev,
>> - "Error writing reg_int_rst_latch\n");
>> - return ret;
>> - }
>> -
>> - ret = i2c_smbus_write_byte_data(data->client,
>> - BMC150_ACCEL_REG_INT_EN_1,
>> - 0);
>> - }
>> -
>> - if (ret < 0) {
>> - dev_err(&data->client->dev, "Error writing reg_int_en_1\n");
>> - return ret;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> static int bmc150_accel_get_bw(struct bmc150_accel_data *data, int *val,
>> int *val2)
>> {
>> @@ -547,6 +416,105 @@ static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
>> }
>> #endif
>>
>> +static const struct bmc150_accel_interrupt_info {
>> + u8 map_reg;
>> + u8 map_bitmask;
>> + u8 en_reg;
>> + u8 en_bitmask;
>> +} bmc150_accel_interrupts[] = {
>> + { /* data ready interrupt */
>> + .map_reg = BMC150_ACCEL_REG_INT_MAP_1,
>> + .map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_DATA,
>> + .en_reg = BMC150_ACCEL_REG_INT_EN_1,
>> + .en_bitmask = BMC150_ACCEL_INT_EN_BIT_DATA_EN,
>> + },
>> + { /* motion interrupt */
>> + .map_reg = BMC150_ACCEL_REG_INT_MAP_0,
>> + .map_bitmask = BMC150_ACCEL_INT_MAP_0_BIT_SLOPE,
>> + .en_reg = BMC150_ACCEL_REG_INT_EN_0,
>> + .en_bitmask = BMC150_ACCEL_INT_EN_BIT_SLP_X |
>> + BMC150_ACCEL_INT_EN_BIT_SLP_Y |
>> + BMC150_ACCEL_INT_EN_BIT_SLP_Z
>> + },
>> +};
>> +
>> +static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data,
>> + const struct bmc150_accel_interrupt_info *info,
>> + bool state)
>> +{
>> + int ret;
>> +
>> + /*
>> + * We will expect the enable and disable to do operation in
>> + * in reverse order. This will happen here anyway as our
>> + * resume operation uses sync mode runtime pm calls, the
>> + * suspend operation will be delayed by autosuspend delay
>> + * So the disable operation will still happen in reverse of
>> + * enable operation. When runtime pm is disabled the mode
>> + * is always on so sequence doesn't matter
>> + */
>> + ret = bmc150_accel_set_power_state(data, state);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* map the interrupt to the appropriate pins */
>> + ret = i2c_smbus_read_byte_data(data->client, info->map_reg);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error reading reg_int_map\n");
>> + goto out_fix_power_state;
>> + }
>> + if (state)
>> + ret |= info->map_bitmask;
>> + else
>> + ret &= ~info->map_bitmask;
>> +
>> + ret = i2c_smbus_write_byte_data(data->client, info->map_reg,
>> + ret);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error writing reg_int_map\n");
>> + goto out_fix_power_state;
>> + }
>> +
>> + /* enable/disable the interrupt */
>> + ret = i2c_smbus_read_byte_data(data->client, info->en_reg);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error reading reg_int_en\n");
>> + goto out_fix_power_state;
>> + }
>> +
>> + if (state)
>> + ret |= info->en_bitmask;
>> + else
>> + ret &= ~info->en_bitmask;
>> +
>> + ret = i2c_smbus_write_byte_data(data->client, info->en_reg, ret);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error writing reg_int_en\n");
>> + goto out_fix_power_state;
>> + }
>> +
>> + return 0;
>> +
>> +out_fix_power_state:
>> + bmc150_accel_set_power_state(data, false);
>> + return ret;
>> +}
>> +
>> +static int bmc150_accel_setup_any_motion_interrupt(
>> + struct bmc150_accel_data *data,
>> + bool status)
>> +{
>> + return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[1],
>> + status);
>> +}
>> +
>> +static int bmc150_accel_setup_new_data_interrupt(struct bmc150_accel_data *data,
>> + bool status)
>> +{
>> + return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[0],
>> + status);
>> +}
>> +
>> static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
>> {
>> int ret, i;
>> @@ -791,25 +759,8 @@ static int bmc150_accel_write_event_config(struct iio_dev *indio_dev,
>> return 0;
>> }
>>
>> - /*
>> - * We will expect the enable and disable to do operation in
>> - * in reverse order. This will happen here anyway as our
>> - * resume operation uses sync mode runtime pm calls, the
>> - * suspend operation will be delayed by autosuspend delay
>> - * So the disable operation will still happen in reverse of
>> - * enable operation. When runtime pm is disabled the mode
>> - * is always on so sequence doesn't matter
>> - */
>> -
>> - ret = bmc150_accel_set_power_state(data, state);
>> - if (ret < 0) {
>> - mutex_unlock(&data->mutex);
>> - return ret;
>> - }
>> -
>> ret = bmc150_accel_setup_any_motion_interrupt(data, state);
>> if (ret < 0) {
>> - bmc150_accel_set_power_state(data, false);
>> mutex_unlock(&data->mutex);
>> return ret;
>> }
>> @@ -1039,16 +990,6 @@ static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
>> return 0;
>> }
>>
>> - /*
>> - * Refer to comment in bmc150_accel_write_event_config for
>> - * enable/disable operation order
>> - */
>> - ret = bmc150_accel_set_power_state(data, state);
>> - if (ret < 0) {
>> - mutex_unlock(&data->mutex);
>> - return ret;
>> - }
>> -
>> if (data->motion_trig == trig) {
>> ret = bmc150_accel_update_slope(data);
>> if (!ret)
>> @@ -1058,7 +999,6 @@ static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
>> ret = bmc150_accel_setup_new_data_interrupt(data, state);
>> }
>> if (ret < 0) {
>> - bmc150_accel_set_power_state(data, false);
>> mutex_unlock(&data->mutex);
>> return ret;
>> }
>> @@ -1244,6 +1184,20 @@ static int bmc150_accel_probe(struct i2c_client *client,
>> if (ret)
>> return ret;
>>
>> + /*
>> + * Set latched mode interrupt. While certain interrupts are
>> + * non-latched regardless of this settings (e.g. new data) we
>> + * want to use latch mode when we can to prevent interrupt
>> + * flooding.
>> + */
>> + ret = i2c_smbus_write_byte_data(data->client,
>> + BMC150_ACCEL_REG_INT_RST_LATCH,
>> + BMC150_ACCEL_INT_MODE_LATCH_RESET);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
>> + return ret;
>> + }
>> +
>> data->dready_trig = devm_iio_trigger_alloc(&client->dev,
>> "%s-dev%d",
>> indio_dev->name,
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
