<snip>
>>>>> + * 2. ALS integration time: 100ms
>>>>> + * 3. ALS gain: 1
>>>>> + */
>>>>> +static int vl6180_init(struct vl6180_data *data)
>>>>> +{
>>>>> + struct i2c_client *client = data->client;
>>>>> + int ret;
>>>>> +
>>>>> + ret = vl6180_read_byte(client, VL6180_MODEL_ID);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + if (ret != VL6180_MODEL_ID_VAL) {
>>>>> + dev_err(&client->dev, "invalid model ID %02x\n", ret);
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +
>>>>> + ret = vl6180_hold(data, true);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + ret = vl6180_read_byte(client, VL6180_OUT_OF_RESET);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + if (ret != 0x01) {
>>>>> + dev_err(&client->dev, "device is not fresh out of reset\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>> Why would it necessarily be fresh out of reset? If it is in some fashion
>>>> supposed to be guaranteed here, please add a comment explaining why.
>>> Device sometimes suffers from false reset condition. Hence, this check ensures that
>>> the device always starts up in the fresh reset condition.
>>> Will add a comment here.
>> Ensures it or checks that it is the case?
>>
>> What about a module remove and reprobe? Should work in that case as well.
> If the device is not fresh out of reset it won't get registered, by
> this way we can prevent
> the device to run in false reset condition.
Remove and probe needs to work if at all possible. It's a common enough
thing to do for one reason or another.
A classic option would be to allow provision of a gpios to manually perform
the reset. Sounds like that's not an option on your board however.
If not, is it possible to get the device to a known state by simply writing
all it's registers? If it's really impossible to continue in this state
we need detailed explanation of why here in comments in the code.
>
> When the module is removed and installed again without resetting the
> device, this check
> will get into act.
>>>>> +
>>>>> + ret = vl6180_write_byte(client, VL6180_INTR_CONFIG,
>>>>> + VL6180_ALS_READY | VL6180_RANGE_READY);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + ret = vl6180_write_word(client, VL6180_ALS_IT, VL6180_ALS_IT_100);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + ret = vl6180_write_byte(client, VL6180_ALS_GAIN, VL6180_ALS_GAIN_1);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + ret = vl6180_write_byte(client, VL6180_OUT_OF_RESET, 0x00);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + return vl6180_hold(data, false);
>>>>> +}
>>>>> +
>>>>> +static int vl6180_probe(struct i2c_client *client,
>>>>> + const struct i2c_device_id *id)
>>>>> +{
>>>>> + struct vl6180_data *data;
>>>>> + struct iio_dev *indio_dev;
>>>>> + int ret;
>>>>> +
>>>>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>>>>> + if (!indio_dev)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + data = iio_priv(indio_dev);
>>>>> + i2c_set_clientdata(client, indio_dev);
>>>>> + data->client = client;
>>>>> + mutex_init(&data->lock);
>>>>> +
>>>>> + indio_dev->dev.parent = &client->dev;
>>>>> + indio_dev->info = &vl6180_info;
>>>>> + indio_dev->channels = vl6180_channels;
>>>>> + indio_dev->num_channels = ARRAY_SIZE(vl6180_channels);
>>>>> + indio_dev->name = VL6180_DRV_NAME;
>>>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>>>> +
>>>>> + ret = vl6180_init(data);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + ret = devm_iio_device_register(&client->dev, indio_dev);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>> return devm_iio_device_register directly.
>>>>
>>>> The static checkers will fire on this anyway, but best to fix
>>>> it now.
>>> Ack
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static const struct of_device_id vl6180_of_match[] = {
>>>>> + { .compatible = "st,vl6180", },
>>>>> + { },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, vl6180_of_match);
>>>>> +
>>>>> +static const struct i2c_device_id vl6180_id[] = {
>>>>> + { "vl6180", 0 },
>>>>> + { }
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(i2c, vl6180_id);
>>>>> +
>>>>> +static struct i2c_driver vl6180_driver = {
>>>>> + .driver = {
>>>>> + .name = VL6180_DRV_NAME,
>>>>> + .of_match_table = of_match_ptr(vl6180_of_match),
>>>>> + },
>>>>> + .probe = vl6180_probe,
>>>>> + .id_table = vl6180_id,
>>>>> +};
>>>>> +
>>>>> +module_i2c_driver(vl6180_driver);
>>>>> +
>>>>> +MODULE_AUTHOR("Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx>");
>>>>> +MODULE_AUTHOR("Manivannan Sadhasivam <manivannanece23@xxxxxxxxx>");
>>>>> +MODULE_DESCRIPTION("STMicro VL6180 ALS and proximity sensor driver");
>>>>> +MODULE_LICENSE("GPL");
>>>>>
>>>>
>>
> --
> 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
