Re: [PATCH v7 3/3] rust: platform: allow ioremap of platform resources

[Benno Lossin]

On Wed Mar 19, 2025 at 12:22 PM CET, Danilo Krummrich wrote:
> On Wed, Mar 19, 2025 at 12:48:00AM +0000, Benno Lossin wrote:
>> On Tue Mar 18, 2025 at 7:22 PM CET, Daniel Almeida wrote:
>> > On 18 Mar 2025, at 14:43, Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
>> >> On Tue, Mar 18, 2025 at 02:20:43PM -0300, Daniel Almeida wrote:
>> >>> +    ///     // Read and write a 32-bit value at `offset`. Calling `try_access()` on
>> >>> +    ///     // the `Devres` makes sure that the resource is still valid.
>> >>> +    ///     let data = iomem.try_access().ok_or(ENODEV)?.readl(offset);
>> >>> +    ///
>> >>> +    ///     iomem.try_access().ok_or(ENODEV)?.writel(data, offset);
>> >> 
>> >> I'd probably write this as
>> >> 
>> >> || -> Result {
>> >> let iomem = iomem.try_access().ok_or(ENODEV)?;
>> >> 
>> >> iomem.read32(offset);
>> >> iomem.write32(data, offset);
>> >> 
>> >> Ok(())
>> >> }()?;
>> 
>> Why use a closure here?
>
> Calling try_access() only once and not having the closure is fine too.
>
> But I also think it would be good practice for an example to explicitly limit
> the scope of the corresponding guard.

Ah you're using the closure to limit the lifetime of the guard. You
don't need a closure, braces suffice.

> Ideally, it uses [1], once available.
>
> [1] https://lore.kernel.org/rust-for-linux/20250313-try_with-v1-1-adcae7ed98a9@xxxxxxxxxx/

Yeah that sounds best.

---
Cheers,
Benno