Re: [PATCH] [PATCH V4] Invensense MPU6050 Device Driver.

[Jonathan Cameron <jic23@kernel.org>]

On 01/14/2013 06:40 PM, Ge Gao wrote:
> Dear Jonanthan,
> 
>         I submitted another version of driver for Invensense MPU6050last week. It basically fixed everything you said in
> the comments. It also removed the inv_mpu_misc file, which is not useful while taking too much spaces.However, only one
> thing I am not surewhat to do with your comments.In the trigger part, as the red part shown below, what that mean and
> what I am supposed to do to make a“callback”. Thanks.

see validate_trigger in struct iio_info and how it's used in say
drivers/iio/adc/ad_sigma_delta.c to enforce the use of a trigger by the device
that created it.  Note there is also a validate_device callback in the
struct iio_trigger_ops to handle the opposite.

Sometimes a particular device will only work with the trigger in question
(say a data ready signal) but nothing prevents other devices from using
that trigger.  In other cases the trigger may only be used by a particular
device.

So to enforce one trigger to one device you would need both.  Not sure
what enforcing you need here (can't remember from when I last looked
at your code and don't have enough time to take a close look today).

It might be a few days before I get round to another review of your
driver as have a few other bits that hit me earlier to catch up with.
(and it's big so you get jumped by the trivial stuff ;)

Jonathan
> 
> Best Regards,
> 
> Ge GAO
> 
>> +static int inv_mpu_data_rdy_trigger_set_state(struct iio_trigger *trig,
> 
>> +                                             bool state)
> 
>> +{
> 
>> +     struct iio_dev *indio_dev = trig->private_data;
> 
>> +
> 
>> +     return set_inv_enable(indio_dev, state);
> 
> Combine the above into one line.
> 
>> +}
> 
>> +
> 
>> +static const struct iio_trigger_ops inv_mpu_trigger_ops = {
> 
>> +     .owner = THIS_MODULE,
> 
>> +     .set_trigger_state = &inv_mpu_data_rdy_trigger_set_state,
> 
> You need some callbacks to prevent this being attached to another
> 
> trigger...
> 
>> +};
> 
>> +
> 
>> +int inv_mpu_probe_trigger(struct iio_dev *indio_dev)
> 
>> +{
> 
>> +     int ret;
> 
>> +     struct inv_mpu_iio_s *st = iio_priv(indio_dev);
> 
>> +
> 
>> +     st->trig = iio_trigger_alloc("%s-dev%d",
>