mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Daniel Almeida" <daniel.almeida@collabora.com>
Cc: "FUJITA Tomonori" <fujita.tomonori@gmail.com>,
	<a.hindborg@kernel.org>, <alex.gaynor@gmail.com>,
	<ojeda@kernel.org>, <aliceryhl@google.com>,
	<anna-maria@linutronix.de>, <bjorn3_gh@protonmail.com>,
	<boqun.feng@gmail.com>, <frederic@kernel.org>, <gary@garyguo.net>,
	<jstultz@google.com>, <linux-kernel@vger.kernel.org>,
	<lossin@kernel.org>, <lyude@redhat.com>,
	<rust-for-linux@vger.kernel.org>, <sboyd@kernel.org>,
	<tglx@linutronix.de>, <tmgross@umich.edu>, <acourbot@nvidia.com>
Subject: Re: [PATCH v1 2/2] rust: Add read_poll_timeout_atomic function
Date: Wed, 27 Aug 2025 14:19:11 +0200	[thread overview]
Message-ID: <DCD7DP6A72A8.2HAYR7K7Z14UO@kernel.org> (raw)
In-Reply-To: <D8CE958C-508F-4A09-96BA-985B5A7C2BA7@collabora.com>

On Wed Aug 27, 2025 at 2:14 PM CEST, Daniel Almeida wrote:
> Hi Danilo,
>
> […}
>
>> 
>> Actually, let me put it in other words:
>> 
>> let val = read_poll_timeout_atomic(
>>     || {
>>         // Fetch the offset to read from from the HW.
>>         let offset = io.read32(0x1000);
>> 
>>         // HW needs a break for some odd reason.
>>         udelay(100);
>> 
>>         // Read the actual value.
>>         io.try_read32(offset)
>>     },
>>     |val: &u32| *val == HW_READY,
>>     Delta::from_micros(0),      // No delay, keep spinning.
>>     Delta::from_millis(10),     // Timeout after 10ms.
>> )?;
>> 
>> Seems like a fairly reasonable usage without knowing the implementation details
>> of read_poll_timeout_atomic(), right?
>> 
>> Except that if the hardware does not become ready, this will spin for 16.67
>> *minutes* -- in atomic context. Instead of the 10ms the user would expect.
>> 
>> This would be way less error prone if we do not provide a timeout value, but a
>> retry count.
>> 
>>> Instead, I think it makes much more sense to provide a retry count as function
>>> argument, such that the user can specify "I want a dealy of 100us, try it 100
>>> times".
>>> 
>>> This way it is transparent to the caller that the timeout may be significantly
>>> more than 10ms depending on the user's implementation.
>>> 
>>> As for doing this in C vs Rust: I don't think things have to align in every
>>> implementation detail. If we can improve things on the Rust side from the
>>> get-go, we should not stop ourselves from doing so, just because a similar C
>>> implementation is hard to refactor, due to having a lot of users already.
>
> I must say I do not follow. Can you expand yet some more on this?

Sure, but it would help if you could clarify which aspect you want me to expand
on. :)

  reply	other threads:[~2025-08-27 12:19 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-21  3:57 [PATCH v1 0/2] Add read_poll_timeout_atomic support FUJITA Tomonori
2025-08-21  3:57 ` [PATCH v1 1/2] rust: add udelay() function FUJITA Tomonori
2025-08-26  9:09   ` Andreas Hindborg
2025-08-26 11:59     ` FUJITA Tomonori
2025-08-26 18:03       ` Miguel Ojeda
2025-08-27  7:12         ` Andreas Hindborg
2025-08-26 12:44   ` Daniel Almeida
2025-08-27  2:43     ` FUJITA Tomonori
2025-08-21  3:57 ` [PATCH v1 2/2] rust: Add read_poll_timeout_atomic function FUJITA Tomonori
2025-08-26 14:02   ` Daniel Almeida
2025-08-27  0:35     ` FUJITA Tomonori
2025-08-27  4:32       ` FUJITA Tomonori
2025-08-26 14:12   ` Danilo Krummrich
2025-08-26 16:59     ` Daniel Almeida
2025-08-26 17:15       ` Danilo Krummrich
2025-08-27  0:14     ` FUJITA Tomonori
2025-08-27  9:00       ` Danilo Krummrich
2025-08-27 10:29         ` Danilo Krummrich
2025-08-27 12:14           ` Daniel Almeida
2025-08-27 12:19             ` Danilo Krummrich [this message]
2025-08-27 12:22               ` Daniel Almeida
2025-08-27 12:36                 ` Danilo Krummrich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DCD7DP6A72A8.2HAYR7K7Z14UO@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=anna-maria@linutronix.de \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=daniel.almeida@collabora.com \
    --cc=frederic@kernel.org \
    --cc=fujita.tomonori@gmail.com \
    --cc=gary@garyguo.net \
    --cc=jstultz@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sboyd@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®