Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <c18c9fee-9cc1-526c-3d4a-23b317d9bbff@collabora.com>
Date:   Tue, 17 May 2022 16:24:54 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH v4 3/3] iio: light: Add support for ltrf216a sensor
Content-Language: en-US
To:     Dmitry Osipenko <digetx@gmail.com>, jic23@kernel.org,
        lars@metafoo.de, robh+dt@kernel.org, Zhigang.Shi@liteon.com,
        krisman@collabora.com
Cc:     linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel@collabora.com,
        alvaro.soliverez@collabora.com
References: <20220511094024.175994-1-shreeya.patel@collabora.com>
 <20220511094024.175994-4-shreeya.patel@collabora.com>
 <a54012e3-291b-42e8-752a-9abc9a88ad6c@gmail.com>
From:   Shreeya Patel <shreeya.patel@collabora.com>
In-Reply-To: <a54012e3-291b-42e8-752a-9abc9a88ad6c@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 13/05/22 06:00, Dmitry Osipenko wrote:
> 11.05.2022 12:40, Shreeya Patel пишет:
>> +static int ltrf216a_set_int_time(struct ltrf216a_data *data, int itime)
>> +{
>> +	int i, ret, index = -1;
>> +	u8 reg_val;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(ltrf216a_int_time_available); i++) {
>> +		if (ltrf216a_int_time_available[i][1] == itime) {
>> +			index = i;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (index < 0)
>> +		return -EINVAL;
>> +
>> +	reg_val = ltrf216a_int_time_reg[index][1];
>> +	data->int_time_fac = ltrf216a_int_time_reg[index][0];
>> +
>> +	ret = i2c_smbus_write_byte_data(data->client, LTRF216A_ALS_MEAS_RES, reg_val);
>> +	if (ret < 0)
>> +		return ret;
> Should the data->int_time_fac be updated only if I2C write was successful?

Yes, thanks for pointing it out. It should be updated only if write is 
successful.

> I'm not sure what reading of LTRF216A_CLEAR_DATA reg does, but if it
> clears the measured data, then shouldn't the data be cleared after
> changing the config?

LTRF216A_CLEAR_DATA isn't used for clearing the measured data. Name is a
bit confusing and even I assumed it is being used for clearing the data. 
But from the
datasheet, it seems that clear data registers provides light intensity 
data related to infrared
and ultravoilet.
We are currently not using this anywhere in the driver so we could 
remove the cleardata readings
for now.


Thanks,
Shreeya Patel
>> +	data->int_time = itime;
>> +
>> +	return 0;
>> +}
>