linux-kernel - Re: [PATCH v4 3/3] iio: light: Add support for ltrf216a sensor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0231ff5d-9bdd-03f2-cdf4-2b7b9b25609c@collabora.com>
Date:   Fri, 13 May 2022 20:08:58 +0530
From:   Shreeya Patel <shreeya.patel@...labora.com>
To:     Dmitry Osipenko <digetx@...il.com>, jic23@...nel.org,
        lars@...afoo.de, robh+dt@...nel.org, Zhigang.Shi@...eon.com,
        krisman@...labora.com, sebastian.reichel@...labora.com
Cc:     linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel@...labora.com,
        alvaro.soliverez@...labora.com
Subject: Re: [PATCH v4 3/3] iio: light: Add support for ltrf216a sensor


On 13/05/22 19:10, Shreeya Patel wrote:
>
> On 13/05/22 05:24, Dmitry Osipenko wrote:
>
> Hi Dmitry,
>
>> 11.05.2022 12:40, Shreeya Patel пишет:
>>> +static int ltrf216a_init(struct iio_dev *indio_dev)
>>> +{
>>> +    int ret;
>>> +    struct ltrf216a_data *data = iio_priv(indio_dev);
>>> +
>>> +    ret = i2c_smbus_read_byte_data(data->client, LTRF216A_MAIN_CTRL);
>>> +    if (ret < 0) {
>>> +        dev_err(&data->client->dev, "Error reading 
>>> LTRF216A_MAIN_CTRL\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    /* enable sensor */
>>> +    ret |= FIELD_PREP(LTRF216A_ALS_ENABLE_MASK, 1);
>>> +    ret = i2c_smbus_write_byte_data(data->client, 
>>> LTRF216A_MAIN_CTRL, ret);
>>> +    if (ret < 0) {
>>> +        dev_err(&data->client->dev, "Error writing 
>>> LTRF216A_MAIN_CTRL\n");
>>> +        return ret;
>>> +    }
>> Couldn't you write "1" directly without reading?
>>
>> What about doing SW reset?
>
> I think we are doing a read here just to make sure device registers 
> are ready and accessible
> without any issues.
>
I just came to know that in I2C communication, writing a single bit 
requires reading the old value (whole byte),
modifying the result (i.e. set or clear the bit one is interested in) 
and then write it back. So the above code writes
the enable bit without modifying the other bits in the 
LTRF216A_MAIN_CTRL register. ( Thanks to Sebastian )

And you are right, we don't need to do a read here since we anyway want 
all other bits of LTRF216A_MAIN_CTRL
to be 0.

Thanks,
Shreeya Patel


> Also, why would we want to do a SW reset here?
>
> In the datasheet, I could see the following steps to enable the sensor
> Supply VDD to Sensor (Sensor in Standby Mode) ---> Wait 100 ms (min) - 
> initial startup time
> ---> I2C Command (Write) To enable sensor to Active Mode
>
> Thanks,
> Shreeya Patel
>
>>
>