On Sun, Jan 4, 2015 at 6:27 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On 21/12/14 00:42, 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>
> Some really trivial suggests inline. Looks good even without it being
> useful for later stuff ;)
>> ---
>> drivers/iio/accel/bmc150-accel.c | 269 ++++++++++++++++-----------------------
>> 1 file changed, 113 insertions(+), 156 deletions(-)
>>
>> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
>> index 92f1d2b..53d1d1d 100644
>> --- a/drivers/iio/accel/bmc150-accel.c
>> +++ b/drivers/iio/accel/bmc150-accel.c
>> @@ -144,6 +144,13 @@ struct bmc150_accel_chip_info {
>> const struct bmc150_scale_info scale_table[4];
>> };
>>
> I'd be tempted to define this at the place where you fill them below...
Done.
<snip>
>> +
>> +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],
> Maybe define a matching enum so the indexes are obvious.
> Then is there a lot of point in having these wrappers? I'd just call
> it directly so you'd get something like.
>
The wrappers go away in one of the subsequent patches. I have kept
them here for now to make the review a bit simpler.
In another subsequent patch the interrupts are integrated to the
trigger structure and with that we can remove the ugly indexes as
well.
--
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
