linux-kernel - Re: [PATCH v5 2/2] iio: light: ROHM BH1745 colour 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]

Date: Tue, 25 Jun 2024 20:25:00 +0100
From: Mudit Sharma <muditsharma.info@...il.com>
To: Javier Carrasco <javier.carrasco.cruz@...il.com>, jic23@...nel.org,
 lars@...afoo.de, krzk+dt@...nel.org, conor+dt@...nel.org, robh@...nel.org
Cc: ivan.orlov0322@...il.com, linux-kernel@...r.kernel.org,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v5 2/2] iio: light: ROHM BH1745 colour sensor

On 24/06/2024 23:27, Javier Carrasco wrote:
> 
>> +static int bh1745_set_trigger_state(struct iio_trigger *trig, bool state)
>> +{
>> +	int ret;
> 
> Why is value initialized here? If regmap returns an error, you will not
> use value anyway. I caught my eye because it is initialized here, and
> not in the other functions where you use the same pattern.

Hi Javier,

Thank you for the review on this.

'value' is initialized here for case when we un-set the trigger. In that 
case, 'state' will be false and 'value' of 0 (default value for 
BH1745_INTR register) will be written.

Best regards,
Mudit Sharma
> 
>> +	int value = 0;
>> +	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>> +	struct bh1745_data *data = iio_priv(indio_dev);
>> +
>> +	guard(mutex)(&data->lock);
>> +	if (state) {
>> +		ret = regmap_read(data->regmap, BH1745_INTR, &value);
>> +		if (ret)
>> +			return ret;
>> +		// Latch is always set when enabling interrupt
>> +		value |= BH1745_INT_ENABLE |
>> +			FIELD_PREP(BH1745_INT_SIGNAL_LATCHED, 1) |
>> +			FIELD_PREP(BH1745_INT_SOURCE_MASK, data->int_src);
>> +		return regmap_write(data->regmap, BH1745_INTR, value);
>> +	}
>> +
>> +	return regmap_write(data->regmap, BH1745_INTR, value);
>> +}
> 
> 
> Best regards,
> Javier Carrasco
>