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