linux-kernel - Re: [PATCH] iio: accel: kx022a: Improve reset delay

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: <f4cf3437-87ea-4d0f-8dab-890d89f85605@gmail.com>
Date: Thu, 14 Nov 2024 11:54:33 +0200
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Nuno Sá <noname.nuno@...il.com>,
 Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Cc: Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen
 <lars@...afoo.de>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: accel: kx022a: Improve reset delay

On 14/11/2024 11:43, Nuno Sá wrote:
> On Thu, 2024-11-14 at 08:57 +0200, Matti Vaittinen wrote:
>> All the sensors supported by kx022a driver seemed to require some delay
>> after software reset to be operational again. More or less a random
>> msleep(1) was added to cause the driver to go to sleep so the sensor has
>> time to become operational again.
>>
>> Now we have official docuumentation available:
>> https://fscdn.rohm.com/kionix/en/document/AN010_KX022ACR-Z_Power-on_Procedure_E.pdf
>> https://fscdn.rohm.com/kionix/en/document/TN027-Power-On-Procedure.pdf
>> https://fscdn.rohm.com/kionix/en/document/AN011_KX134ACR-LBZ_Power-on_Procedure_E.pdf
>>
>> stating the required time is 2 ms.
>>
>> Due to the nature of the current msleep implementation, the msleep(1) is
>> likely to be sleeping more than 2ms already - but the value "1" is
>> misleading in case someone needs to optimize the start time and change
>> the msleep to a more accurate delay. Hence it is better for
>> "documentation" purposes to use value which actually reflects the
>> specified 2ms wait time.
>>
>> Change the value of delay after software reset to match the
>> specifications and add links to the power-on procedure specifications.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
>> ---
>> Sorry for not including this to the KX134ACR-LBZ series I sent
>> yesterday. It was only half an hour after I had sent the KX134ACR-LBZ
>> support when I was notified about the existence of the KX022ACR-Z
>> start-up procedure specification... Hence this lone patch to code which
>> I just sent a miscallaneous series for before.
>>
>>   drivers/iio/accel/kionix-kx022a.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-
>> kx022a.c
>> index 32387819995d..ccabe2e3b130 100644
>> --- a/drivers/iio/accel/kionix-kx022a.c
>> +++ b/drivers/iio/accel/kionix-kx022a.c
>> @@ -1121,10 +1121,15 @@ static int kx022a_chip_init(struct kx022a_data *data)
>>   		return ret;
>>   
>>   	/*
>> -	 * I've seen I2C read failures if we poll too fast after the sensor
>> -	 * reset. Slight delay gives I2C block the time to recover.
>> +	 * According to the power-on procedure documents, there is (at least)
>> +	 * 2ms delay required after the software reset. This should be same
>> for
>> +	 * all, KX022ACR-Z, KX132-1211, KX132ACR-LBZ and KX134ACR-LBZ.
>> +	 *
>> +	 *
>> https://fscdn.rohm.com/kionix/en/document/AN010_KX022ACR-Z_Power-on_Procedure_E.pdf
>> +	 *
>> https://fscdn.rohm.com/kionix/en/document/TN027-Power-On-Procedure.pdf
>> +	 *
>> https://fscdn.rohm.com/kionix/en/document/AN011_KX134ACR-LBZ_Power-on_Procedure_E.pdf
>>   	 */
>> -	msleep(1);
>> +	msleep(2);
> 
> msleep() is not advisable for something lower than 20ms. Maybe take the
> opportunity and change it to fsleep()?

Thank you for the suggestion Nuno. I did originally consider using the 
usleep_range() since the checkpatch knows to warn about msleep with 
small times.

However, there should be no rush to power-on the sensor at startup. It 
usually does not matter if the sleep is 2 or 20 milli seconds, as long 
as it is long enough. I wonder if interrupting the system with hrtimers 
for _all_ smallish delays (when the longer delay would not really hurt) 
is a the best design choice. Hence I'd rather keep the msleep when we 
don't need to guarantee delay to be short instead of defaulting to 
hrtimers or even busy-loop when it is not required.

Do you think I am mistaken?

Yours,
	-- Matti