All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11 0/4] rust: platform: add Io support
@ 2025-07-01 14:34 Daniel Almeida
  2025-07-01 14:34 ` [PATCH v11 1/4] rust: io: add resource abstraction Daniel Almeida
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Daniel Almeida @ 2025-07-01 14:34 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, Benno Lossin
  Cc: linux-kernel, rust-for-linux, Fiona Behrens, Daniel Almeida

Changes in v11:
- Rebased on top of driver-core-next (to get the latest Devres changes)
- Changed the order between requesting the resource and mapping it
  (Danilo)
- Link to v10: https://lore.kernel.org/r/20250623-topics-tyr-platform_iomem-v10-0-ed860a562940@collabora.com

rust: platform: add Io support

Changes in v10:
- Rebased on driver-core-next
- Fixed examples (they were still using try_access())
- Removed map_err() from the examples, as it was not needed.
- Added a pub use for Resource in io.rs
- Reworked the platform code to make use of the pub use above
- Link to v9: https://lore.kernel.org/r/20250603-topics-tyr-platform_iomem-v9-0-a27e04157e3e@collabora.com

Changes in v9:
- Rebased on top of nova-next (for Devres::access())
- Reworked the documentation to add more markdown.
- Converted to &raw mut instead of addr_of_mut!()
- Renamed 'from_ptr' to 'as_ref' for consistency
- Changed the IoMem examples to use the signature for probe()
- Changed resource() to resource_by_index(). It's a better fit given
  the existance of resource_by_name().
- Created a separate patch for the resource accessors above.
- Moved the accessors into the generic impl block, so they work with all
  Device contexts.
- Take Device<Bound> where applicable
- Renamed "ioremap_*" to "iomap_*", in order to be consistent with the code
  in pci.rs
- Switched to Devres::access()
- Link to v8: https://lore.kernel.org/r/20250509-topics-tyr-platform_iomem-v8-0-e9f1725a40da@collabora.com

rust: platform: add Io support

Changes in v8:
- Rebased on driver-core-next
- Opted to wait for 'rust/revocable: add try_with() convenience method' to
  land instead of using the suggested closure (let me know if we should
  just switch to the closure anyways)
- Cc'd more people
- Link to v7: https://lore.kernel.org/r/20250318-topics-tyr-platform_iomem-v7-0-7438691d9ef7@collabora.com

Changes in v7:

- Rebased on top of rust-next
- Fixed a few Clippy lints
- Fixed typos (Thanks Daniel!)
- "struct Flags" now contains a usize (thanks Daniel)
- Fixed "Doc list without indentation" warning (thanks, Guangbo)

Thanks, Fiona {
- Removed RequestFn, as all functions simply used request_region and RequestFn
  had issues. Only request_region() is exposed now.
- Gated iomem_resource on CONFIG_HAS_IOMEM
- Require that the name argument be 'static
}

- Correctly check for IORESOURCE_MEM_NONPOSTED. We now call ioremap_np if that
  is set (thanks, Lina!)
- Remove #[dead_code] attribute from ExclusiveIoMem::region.

Changes in v6:

- Added Fiona as co-developer in the first patch, as I merged part of her code
from the LED driver series (thanks, Fiona)

- (Fiona) added the ResourceSize type, thereby fixing the u32 vs u64 issues
  pointed out by Christian

- Moved the request_region, release_region and friends to resource.rs

- Added the Region type. This type represents a resource returned by
  `request_region` and friends. It is also owned, representing the fact
  that the region remains marked as busy until release_region is called on
  drop. (Thanks Alice, for pointing out this pattern)

- Rewrote the IoMem abstraction to implement a separate type for exclusive
  access to an underlying region. I really disliked the `EXCLUSIVE` const
  generic, as it was definitely not ergonomic, i.e.:

  `IoMem<0, false>`

  ...doesn't really say much. In fact, I believe that boolean parameters
  hurt readability in general.

  This new approach lets users build either regular IoMem's, which basically
  call ioremap under the covers, and ExclusiveIoMem's , which also call request_region
  via the Region type.

- Added access to the ioresource_port and ioresource_mem globals.

Link to v5: https://lore.kernel.org/rust-for-linux/20250116125632.65017-1-daniel.almeida@collabora.com/

Changes in v5:

- resend v5, as the r4l list was not cc'd
- use srctree where applicable in the docs (Alice)
- Remove 'mut' in Resource::from_ptr() (Alice)
- Add 'invariants' section for Resource (Alice)
- Fix typos in mem.rs (Alice)
- Turn 'exclusive' into a const generic (Alice)
- Fix example in platform.rs (Alice)
- Rework the resource.is_null() check (Alice)
- Refactor IoMem::new() to return DevRes<IoMem> directly (Danilo)

link to v4: https://lore.kernel.org/rust-for-linux/20250109133057.243751-1-daniel.almeida@collabora.com/

Changes in v4:

- Rebased on top of driver-core-next
- Split series in multiple patches (Danilo)
- Move IoMem and Resource into its own files (Danilo)
- Fix a missing "if exclusive {...}" check (Danilo)
- Fixed the example, since it was using the old API (Danilo)
- Use Opaque in `Resource`, instead of NonNull and PhantomData (Boqun)
- Highlight that non-exclusive access to the iomem might be required in some cases
- Fixed the safety comment in IoMem::deref()

Link to v3: https://lore.kernel.org/rust-for-linux/20241211-topic-panthor-rs-platform_io_support-v3-1-08ba707e5e3b@collabora.com/

Changes in v3:
- Rebased on top of v5 for the PCI/Platform abstractions
- platform_get_resource is now called only once when calling ioremap
- Introduced a platform::Resource type, which is bound to the lifetime of the
 platform Device
- Allow retrieving resources from the platform device either by index or
 name
- Make request_mem_region() optional
- Use resource.name() in request_mem_region
- Reword the example to remove an unaligned, out-of-bounds offset
- Update the safety requirements of platform::IoMem

Changes in v2:
- reworked the commit message
- added missing request_mem_region call (Thanks Alice, Danilo)
- IoMem::new() now takes the platform::Device, the resource number and
 the name, instead of an address and a size (thanks, Danilo)
- Added a new example for both sized and unsized versions of IoMem.
- Compiled the examples using kunit.py (thanks for the tip, Alice!)
- Removed instances of `foo as _`. All `as` casts now spell out the actual
 type.
- Now compiling with CLIPPY=1 (I realized I had forgotten, sorry)
- Rebased on top of rust-next to check for any warnings given the new
 unsafe lints.

Daniel Almeida (3):
  rust: io: add resource abstraction
  rust: io: mem: add a generic iomem abstraction
  rust: platform: allow ioremap of platform resources

 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/io.c               |  36 +++++
 rust/kernel/io.rs               |   3 +
 rust/kernel/io/mem.rs           | 125 ++++++++++++++++
 rust/kernel/io/resource.rs      | 252 ++++++++++++++++++++++++++++++++
 rust/kernel/platform.rs         | 123 +++++++++++++++-
 6 files changed, 539 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/io/mem.rs
 create mode 100644 rust/kernel/io/resource.rs

--
2.48.0

---
Daniel Almeida (4):
      rust: io: add resource abstraction
      rust: io: mem: add a generic iomem abstraction
      rust: platform: add resource accessors
      rust: platform: allow ioremap of platform resources

 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/io.c               |  41 ++++++++
 rust/kernel/io.rs               |   5 +
 rust/kernel/io/mem.rs           | 147 ++++++++++++++++++++++++++
 rust/kernel/io/resource.rs      | 222 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/platform.rs         | 163 ++++++++++++++++++++++++++++-
 6 files changed, 578 insertions(+), 1 deletion(-)
---
base-commit: f5d3ef25d238901a76fe0277787afa44f7714739
change-id: 20250318-topics-tyr-platform_iomem-1710a177e1df

Best regards,
-- 
Daniel Almeida <daniel.almeida@collabora.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v11 1/4] rust: io: add resource abstraction
  2025-07-01 14:34 [PATCH v11 0/4] rust: platform: add Io support Daniel Almeida
@ 2025-07-01 14:34 ` Daniel Almeida
  2025-07-02  7:44   ` Miguel Ojeda
  2025-07-02 10:21   ` Danilo Krummrich
  2025-07-01 14:34 ` [PATCH v11 2/4] rust: io: mem: add a generic iomem abstraction Daniel Almeida
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Daniel Almeida @ 2025-07-01 14:34 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, Benno Lossin
  Cc: linux-kernel, rust-for-linux, Fiona Behrens, Daniel Almeida

In preparation for ioremap support, add a Rust abstraction for struct
resource.

A future commit will introduce the Rust API to ioremap a resource from a
platform device. The current abstraction, therefore, adds only the
minimum API needed to get that done.

Co-developed-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/io.c               |  36 +++++++
 rust/kernel/io.rs               |   4 +
 rust/kernel/io/resource.rs      | 222 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 263 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 7e8f2285064797d5bbac5583990ff823b76c6bdc..5f795e60e889b9fc887013743c81b1cf92a52adb 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -53,6 +53,7 @@
 #include <linux/file.h>
 #include <linux/firmware.h>
 #include <linux/fs.h>
+#include <linux/ioport.h>
 #include <linux/jiffies.h>
 #include <linux/jump_label.h>
 #include <linux/mdio.h>
diff --git a/rust/helpers/io.c b/rust/helpers/io.c
index 15ea187c5466256effd07efe6f6995a1dd95a967..404776cf6717c8570c7600a24712ce6e4623d3c6 100644
--- a/rust/helpers/io.c
+++ b/rust/helpers/io.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/io.h>
+#include <linux/ioport.h>
 
 void __iomem *rust_helper_ioremap(phys_addr_t offset, size_t size)
 {
@@ -99,3 +100,38 @@ void rust_helper_writeq_relaxed(u64 value, void __iomem *addr)
 	writeq_relaxed(value, addr);
 }
 #endif
+
+resource_size_t rust_helper_resource_size(struct resource *res)
+{
+	return resource_size(res);
+}
+
+struct resource *rust_helper_request_mem_region(resource_size_t start,
+						resource_size_t n,
+						const char *name)
+{
+	return request_mem_region(start, n, name);
+}
+
+void rust_helper_release_mem_region(resource_size_t start, resource_size_t n)
+{
+	release_mem_region(start, n);
+}
+
+struct resource *rust_helper_request_region(resource_size_t start,
+					    resource_size_t n, const char *name)
+{
+	return request_region(start, n, name);
+}
+
+struct resource *rust_helper_request_muxed_region(resource_size_t start,
+						  resource_size_t n,
+						  const char *name)
+{
+	return request_muxed_region(start, n, name);
+}
+
+void rust_helper_release_region(resource_size_t start, resource_size_t n)
+{
+	release_region(start, n);
+}
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 72d80a6f131e3e826ecd9d2c3bcf54e89aa60cc3..7b70d5b5477e57d6d0f24bcd26bd8b0071721bc0 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -7,6 +7,10 @@
 use crate::error::{code::EINVAL, Result};
 use crate::{bindings, build_assert};
 
+pub mod resource;
+
+pub use resource::Resource;
+
 /// Raw representation of an MMIO region.
 ///
 /// By itself, the existence of an instance of this structure does not provide any guarantees that
diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs
new file mode 100644
index 0000000000000000000000000000000000000000..611eca8cc693d83bccc2c384db2ff22d11e7ac46
--- /dev/null
+++ b/rust/kernel/io/resource.rs
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for system resources.
+//!
+//! C header: [`include/linux/ioport.h`](srctree/include/linux/ioport.h)
+
+use core::ops::Deref;
+use core::ptr::NonNull;
+
+use crate::str::CStr;
+use crate::types::Opaque;
+
+#[cfg(CONFIG_HAS_IOPORT)]
+/// Returns a reference to the global `ioport_resource` variable.
+pub fn ioport_resource() -> &'static Resource {
+    // SAFETY: `bindings::ioport_resoure` has global lifetime and is of type Resource.
+    unsafe { Resource::as_ref(&raw mut bindings::ioport_resource) }
+}
+
+#[cfg(CONFIG_HAS_IOMEM)]
+/// Returns a reference to the global `iomem_resource` variable.
+pub fn iomem_resource() -> &'static Resource {
+    // SAFETY: `bindings::iomem_resoure` has global lifetime and is of type Resource.
+    unsafe { Resource::as_ref(&raw mut bindings::iomem_resource) }
+}
+
+/// Resource Size type.
+///
+/// This is a type alias to `u64` depending on the config option
+/// `CONFIG_PHYS_ADDR_T_64BIT`.
+#[cfg(CONFIG_PHYS_ADDR_T_64BIT)]
+pub type ResourceSize = u64;
+
+/// Resource Size type.
+///
+/// This is a type alias to `u32` depending on the config option
+/// `CONFIG_PHYS_ADDR_T_64BIT`.
+#[cfg(not(CONFIG_PHYS_ADDR_T_64BIT))]
+pub type ResourceSize = u32;
+
+/// A region allocated from a parent [`Resource`].
+///
+/// # Invariants
+///
+/// - `self.0` points to a valid `bindings::resource` that was obtained through
+///   `bindings::__request_region`.
+pub struct Region(NonNull<bindings::resource>);
+
+impl Deref for Region {
+    type Target = Resource;
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: Safe as per the invariant of `Region`
+        unsafe { Resource::as_ref(self.0.as_ptr()) }
+    }
+}
+
+impl Drop for Region {
+    fn drop(&mut self) {
+        // SAFETY: Safe as per the invariant of `Region`
+        let res = unsafe { Resource::as_ref(self.0.as_ptr()) };
+        let flags = res.flags();
+
+        let release_fn = if flags.contains(flags::IORESOURCE_MEM) {
+            bindings::release_mem_region
+        } else {
+            bindings::release_region
+        };
+
+        // SAFETY: Safe as per the invariant of `Region`
+        unsafe { release_fn(res.start(), res.size()) };
+    }
+}
+
+// SAFETY: `Region` only holds a pointer to a C `struct resource`, which is safe to be used from
+// any thread.
+unsafe impl Send for Region {}
+
+// SAFETY: `Region` only holds a pointer to a C `struct resource`, references to which are
+// safe to be used from any thread.
+unsafe impl Sync for Region {}
+
+/// A resource abstraction.
+///
+/// # Invariants
+///
+/// [`Resource`] is a transparent wrapper around a valid `bindings::resource`.
+#[repr(transparent)]
+pub struct Resource(Opaque<bindings::resource>);
+
+impl Resource {
+    /// Creates a reference to a [`Resource`] from a valid pointer.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that for the duration of 'a, the pointer will
+    /// point at a valid `bindings::resource`.
+    ///
+    /// The caller must also ensure that the [`Resource`] is only accessed via the
+    /// returned reference for the duration of 'a.
+    pub(crate) const unsafe fn as_ref<'a>(ptr: *mut bindings::resource) -> &'a Self {
+        // SAFETY: Self is a transparent wrapper around `Opaque<bindings::resource>`.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Requests a resource region.
+    ///
+    /// Exclusive access will be given and the region will be marked as busy.
+    /// Further calls to [`Self::request_region`] will return [`None`] if
+    /// the region, or a part of it, is already in use.
+    pub fn request_region(
+        &self,
+        start: ResourceSize,
+        size: ResourceSize,
+        name: &'static CStr,
+        flags: Flags,
+    ) -> Option<Region> {
+        // SAFETY: Safe as per the invariant of `Resource`
+        let region = unsafe {
+            bindings::__request_region(
+                self.0.get(),
+                start,
+                size,
+                name.as_char_ptr(),
+                flags.0 as i32,
+            )
+        };
+
+        Some(Region(NonNull::new(region)?))
+    }
+
+    /// Returns the size of the resource.
+    pub fn size(&self) -> ResourceSize {
+        let inner = self.0.get();
+        // SAFETY: safe as per the invariants of `Resource`
+        unsafe { bindings::resource_size(inner) }
+    }
+
+    /// Returns the start address of the resource.
+    pub fn start(&self) -> u64 {
+        let inner = self.0.get();
+        // SAFETY: safe as per the invariants of `Resource`
+        unsafe { *inner }.start
+    }
+
+    /// Returns the name of the resource.
+    pub fn name(&self) -> &'static CStr {
+        let inner = self.0.get();
+        // SAFETY: safe as per the invariants of `Resource`
+        unsafe { CStr::from_char_ptr((*inner).name) }
+    }
+
+    /// Returns the flags associated with the resource.
+    pub fn flags(&self) -> Flags {
+        let inner = self.0.get();
+        // SAFETY: safe as per the invariants of `Resource`
+        let flags = unsafe { *inner }.flags;
+
+        Flags(flags)
+    }
+}
+
+// SAFETY: `Resource` only holds a pointer to a C `struct resource`, which is safe to be used from
+// any thead.
+unsafe impl Send for Resource {}
+
+// SAFETY: `Resource` only holds a pointer to a C `struct resource`, references to which are
+// safe to be used from any thead.
+unsafe impl Sync for Resource {}
+
+/// Resource flags as stored in the C `struct resource::flags` field.
+///
+/// They can be combined with the operators `|`, `&`, and `!`.
+///
+/// Values can be used from the [`flags`] module.
+#[derive(Clone, Copy, PartialEq)]
+pub struct Flags(usize);
+
+impl Flags {
+    /// Check whether `flags` is contained in `self`.
+    pub fn contains(self, flags: Flags) -> bool {
+        (self & flags) == flags
+    }
+}
+
+impl core::ops::BitOr for Flags {
+    type Output = Self;
+    fn bitor(self, rhs: Self) -> Self::Output {
+        Self(self.0 | rhs.0)
+    }
+}
+
+impl core::ops::BitAnd for Flags {
+    type Output = Self;
+    fn bitand(self, rhs: Self) -> Self::Output {
+        Self(self.0 & rhs.0)
+    }
+}
+
+impl core::ops::Not for Flags {
+    type Output = Self;
+    fn not(self) -> Self::Output {
+        Self(!self.0)
+    }
+}
+
+/// Resource flags as stored in the `struct resource::flags` field.
+pub mod flags {
+    use super::Flags;
+
+    /// PCI/ISA I/O ports.
+    pub const IORESOURCE_IO: Flags = Flags(bindings::IORESOURCE_IO as usize);
+
+    /// Resource is software muxed.
+    pub const IORESOURCE_MUXED: Flags = Flags(bindings::IORESOURCE_MUXED as usize);
+
+    /// Resource represents a memory region.
+    pub const IORESOURCE_MEM: Flags = Flags(bindings::IORESOURCE_MEM as usize);
+
+    /// Resource represents a memory region that must be ioremaped using `ioremap_np`.
+    pub const IORESOURCE_MEM_NONPOSTED: Flags = Flags(bindings::IORESOURCE_MEM_NONPOSTED as usize);
+}

-- 
2.50.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v11 2/4] rust: io: mem: add a generic iomem abstraction
  2025-07-01 14:34 [PATCH v11 0/4] rust: platform: add Io support Daniel Almeida
  2025-07-01 14:34 ` [PATCH v11 1/4] rust: io: add resource abstraction Daniel Almeida
@ 2025-07-01 14:34 ` Daniel Almeida
  2025-07-01 14:34 ` [PATCH v11 3/4] rust: platform: add resource accessors Daniel Almeida
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Daniel Almeida @ 2025-07-01 14:34 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, Benno Lossin
  Cc: linux-kernel, rust-for-linux, Daniel Almeida

Add a generic iomem abstraction to safely read and write ioremapped
regions.

The reads and writes are done through IoRaw, and are thus checked either
at compile-time, if the size of the region is known at that point, or at
runtime otherwise.

Non-exclusive access to the underlying memory region is made possible to
cater to cases where overlapped regions are unavoidable.

Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/helpers/io.c     |   5 ++
 rust/kernel/io.rs     |   1 +
 rust/kernel/io/mem.rs | 147 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+)

diff --git a/rust/helpers/io.c b/rust/helpers/io.c
index 404776cf6717c8570c7600a24712ce6e4623d3c6..c475913c69e647b1042e8e7d66b9148d892947a1 100644
--- a/rust/helpers/io.c
+++ b/rust/helpers/io.c
@@ -8,6 +8,11 @@ void __iomem *rust_helper_ioremap(phys_addr_t offset, size_t size)
 	return ioremap(offset, size);
 }
 
+void __iomem *rust_helper_ioremap_np(phys_addr_t offset, size_t size)
+{
+	return ioremap_np(offset, size);
+}
+
 void rust_helper_iounmap(void __iomem *addr)
 {
 	iounmap(addr);
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 7b70d5b5477e57d6d0f24bcd26bd8b0071721bc0..b7fc759f8b5d3c3ac6f33f5a66e9f619c58b7405 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -7,6 +7,7 @@
 use crate::error::{code::EINVAL, Result};
 use crate::{bindings, build_assert};
 
+pub mod mem;
 pub mod resource;
 
 pub use resource::Resource;
diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
new file mode 100644
index 0000000000000000000000000000000000000000..16bd2d6752ff720cb791eaff633adbc1c0964f03
--- /dev/null
+++ b/rust/kernel/io/mem.rs
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Generic memory-mapped IO.
+
+use core::ops::Deref;
+
+use crate::device::Bound;
+use crate::device::Device;
+use crate::devres::Devres;
+use crate::io;
+use crate::io::resource::Region;
+use crate::io::resource::Resource;
+use crate::io::Io;
+use crate::io::IoRaw;
+use crate::prelude::*;
+
+/// An exclusive memory-mapped IO region.
+///
+/// # Invariants
+///
+/// - [`ExclusiveIoMem`] has exclusive access to the underlying [`IoMem`].
+#[pin_data]
+pub struct ExclusiveIoMem<const SIZE: usize> {
+    /// The underlying `IoMem` instance.
+    iomem: IoMem<SIZE>,
+
+    /// The region abstraction. This represents exclusive access to the
+    /// range represented by the underlying `iomem`.
+    ///
+    /// This field is needed for ownership of the region.
+    _region: Region,
+}
+
+impl<const SIZE: usize> ExclusiveIoMem<SIZE> {
+    /// Creates a new `ExclusiveIoMem` instance.
+    pub(crate) fn ioremap(resource: &Resource) -> Result<Self> {
+        let start = resource.start();
+        let size = resource.size();
+        let name = resource.name();
+
+        let region = resource
+            .request_region(start, size, name, io::resource::flags::IORESOURCE_MEM)
+            .ok_or(EBUSY)?;
+
+        let iomem = IoMem::ioremap(resource)?;
+
+        let iomem = ExclusiveIoMem {
+            iomem,
+            _region: region,
+        };
+
+        Ok(iomem)
+    }
+
+    pub(crate) fn new<'a>(
+        resource: &Resource,
+        device: &'a Device<Bound>,
+    ) -> Result<impl PinInit<Devres<Self>, Error> + 'a> {
+        let iomem = Self::ioremap(resource)?;
+        let devres = Devres::new(device, iomem);
+
+        Ok(devres)
+    }
+}
+
+impl<const SIZE: usize> Deref for ExclusiveIoMem<SIZE> {
+    type Target = Io<SIZE>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.iomem
+    }
+}
+
+/// A generic memory-mapped IO region.
+///
+/// Accesses to the underlying region is checked either at compile time, if the
+/// region's size is known at that point, or at runtime otherwise.
+///
+/// # Invariants
+///
+/// [`IoMem`] always holds an [`IoRaw`] instance that holds a valid pointer to the
+/// start of the I/O memory mapped region.
+pub struct IoMem<const SIZE: usize = 0> {
+    io: IoRaw<SIZE>,
+}
+
+impl<const SIZE: usize> IoMem<SIZE> {
+    fn ioremap(resource: &Resource) -> Result<Self> {
+        let size = resource.size();
+        if size == 0 {
+            return Err(EINVAL);
+        }
+
+        let res_start = resource.start();
+
+        let addr = if resource
+            .flags()
+            .contains(io::resource::flags::IORESOURCE_MEM_NONPOSTED)
+        {
+            // SAFETY:
+            // - `res_start` and `size` are read from a presumably valid `struct resource`.
+            // - `size` is known not to be zero at this point.
+            unsafe { bindings::ioremap_np(res_start, size as usize) }
+        } else {
+            // SAFETY:
+            // - `res_start` and `size` are read from a presumably valid `struct resource`.
+            // - `size` is known not to be zero at this point.
+            unsafe { bindings::ioremap(res_start, size as usize) }
+        };
+
+        if addr.is_null() {
+            return Err(ENOMEM);
+        }
+
+        let io = IoRaw::new(addr as usize, size as usize)?;
+        let io = IoMem { io };
+
+        Ok(io)
+    }
+
+    /// Creates a new `IoMem` instance.
+    pub(crate) fn new<'a>(
+        resource: &Resource,
+        device: &'a Device<Bound>,
+    ) -> Result<impl PinInit<Devres<Self>, Error> + 'a> {
+        let io = Self::ioremap(resource)?;
+        let devres = Devres::new(device, io);
+
+        Ok(devres)
+    }
+}
+
+impl<const SIZE: usize> Drop for IoMem<SIZE> {
+    fn drop(&mut self) {
+        // SAFETY: Safe as by the invariant of `Io`.
+        unsafe { bindings::iounmap(self.io.addr() as *mut core::ffi::c_void) }
+    }
+}
+
+impl<const SIZE: usize> Deref for IoMem<SIZE> {
+    type Target = Io<SIZE>;
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: Safe as by the invariant of `IoMem`.
+        unsafe { Io::from_raw(&self.io) }
+    }
+}

-- 
2.50.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v11 3/4] rust: platform: add resource accessors
  2025-07-01 14:34 [PATCH v11 0/4] rust: platform: add Io support Daniel Almeida
  2025-07-01 14:34 ` [PATCH v11 1/4] rust: io: add resource abstraction Daniel Almeida
  2025-07-01 14:34 ` [PATCH v11 2/4] rust: io: mem: add a generic iomem abstraction Daniel Almeida
@ 2025-07-01 14:34 ` Daniel Almeida
  2025-07-01 14:34 ` [PATCH v11 4/4] rust: platform: allow ioremap of platform resources Daniel Almeida
  2025-07-02  7:52 ` [PATCH v11 0/4] rust: platform: add Io support Miguel Ojeda
  4 siblings, 0 replies; 12+ messages in thread
From: Daniel Almeida @ 2025-07-01 14:34 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, Benno Lossin
  Cc: linux-kernel, rust-for-linux, Daniel Almeida

A previous patch has exposed the abstractions struct resource.

Now make it possible to retrieve resources from platform devices in Rust
using either a name or an index.

Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/platform.rs | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 86f9d73c64b38ffe067be329a77b2fc04564c7fe..bafd3ccea6d15b7965d1a993deef3f58e03b3490 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -7,6 +7,7 @@
 use crate::{
     acpi, bindings, container_of, device, driver,
     error::{to_result, Result},
+    io::Resource,
     of,
     prelude::*,
     str::CStr,
@@ -211,6 +212,43 @@ impl<Ctx: device::DeviceContext> Device<Ctx> {
     fn as_raw(&self) -> *mut bindings::platform_device {
         self.0.get()
     }
+
+    /// Returns the resource at `index`, if any.
+    pub fn resource_by_index(&self, index: u32) -> Option<&Resource> {
+        // SAFETY: `self.as_raw()` returns a valid pointer to a `struct platform_device`.
+        let resource = unsafe {
+            bindings::platform_get_resource(self.as_raw(), bindings::IORESOURCE_MEM, index)
+        };
+
+        if resource.is_null() {
+            return None;
+        }
+
+        // SAFETY: `resource` is a valid pointer to a `struct resource` as
+        // returned by `platform_get_resource`.
+        Some(unsafe { Resource::as_ref(resource) })
+    }
+
+    /// Returns the resource with a given `name`, if any.
+    pub fn resource_by_name(&self, name: &CStr) -> Option<&Resource> {
+        // SAFETY: `self.as_raw()` returns a valid pointer to a `struct
+        // platform_device` and `name` points to a valid C string.
+        let resource = unsafe {
+            bindings::platform_get_resource_byname(
+                self.as_raw(),
+                bindings::IORESOURCE_MEM,
+                name.as_char_ptr(),
+            )
+        };
+
+        if resource.is_null() {
+            return None;
+        }
+
+        // SAFETY: `resource` is a valid pointer to a `struct resource` as
+        // returned by `platform_get_resource`.
+        Some(unsafe { Resource::as_ref(resource) })
+    }
 }
 
 // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic

-- 
2.50.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v11 4/4] rust: platform: allow ioremap of platform resources
  2025-07-01 14:34 [PATCH v11 0/4] rust: platform: add Io support Daniel Almeida
                   ` (2 preceding siblings ...)
  2025-07-01 14:34 ` [PATCH v11 3/4] rust: platform: add resource accessors Daniel Almeida
@ 2025-07-01 14:34 ` Daniel Almeida
  2025-07-02 10:36   ` Danilo Krummrich
  2025-07-02  7:52 ` [PATCH v11 0/4] rust: platform: add Io support Miguel Ojeda
  4 siblings, 1 reply; 12+ messages in thread
From: Daniel Almeida @ 2025-07-01 14:34 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, Benno Lossin
  Cc: linux-kernel, rust-for-linux, Daniel Almeida

The preceding patches added support for resources, and for a general
IoMem abstraction, but thus far there is no way to access said IoMem
from drivers, as its creation is unsafe and depends on a resource that
must be acquired from some device first.

Now, allow the ioremap of platform resources themselves, thereby making
the IoMem available to platform drivers.

Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/platform.rs | 127 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 125 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index bafd3ccea6d15b7965d1a993deef3f58e03b3490..b2c17fd6ab990c0b5ae6d6ed2e466b199eae2536 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -5,9 +5,14 @@
 //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
 
 use crate::{
-    acpi, bindings, container_of, device, driver,
+    acpi, bindings, container_of, device,
+    devres::Devres,
+    driver,
     error::{to_result, Result},
-    io::Resource,
+    io::{
+        mem::{ExclusiveIoMem, IoMem},
+        Resource,
+    },
     of,
     prelude::*,
     str::CStr,
@@ -251,6 +256,124 @@ pub fn resource_by_name(&self, name: &CStr) -> Option<&Resource> {
     }
 }
 
+impl Device<device::Bound> {
+    /// Maps a platform resource where the size is known at compile time.
+    ///
+    /// This uses the
+    /// [`ioremap()`](https://docs.kernel.org/driver-api/device-io.html#getting-access-to-the-device)
+    /// C API.
+    ///
+    /// # Examples
+    ///
+    /// ```no_run
+    /// # use kernel::{bindings, c_str, platform, of, device::Core};
+    /// # struct SampleDriver;
+    ///
+    /// impl platform::Driver for SampleDriver {
+    ///    # type IdInfo = ();
+    ///    # const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+    ///
+    ///    fn probe(
+    ///       pdev: &platform::Device<Core>,
+    ///       info: Option<&Self::IdInfo>,
+    ///    ) -> Result<Pin<KBox<Self>>> {
+    ///       let offset = 0; // Some offset.
+    ///
+    ///       // If the size is known at compile time, use [`Self::iomap_resource_sized`].
+    ///       //
+    ///       // No runtime checks will apply when reading and writing.
+    ///       let resource = pdev.resource_by_index(0).ok_or(ENODEV)?;
+    ///       let iomem = pdev.iomap_resource_sized::<42>(&resource)?;
+    ///       let iomem = KBox::pin_init(iomem, GFP_KERNEL)?;
+    ///
+    ///       let io = iomem.access(pdev.as_ref())?;
+    ///
+    ///       // Read and write a 32-bit value at `offset`.
+    ///       let data = io.read32_relaxed(offset);
+    ///
+    ///       io.write32_relaxed(data, offset);
+    ///
+    ///       # Ok(KBox::new(SampleDriver, GFP_KERNEL)?.into())
+    ///     }
+    /// }
+    /// ```
+    pub fn iomap_resource_sized<const SIZE: usize>(
+        &self,
+        resource: &Resource,
+    ) -> Result<impl PinInit<Devres<IoMem<SIZE>>, Error> + '_> {
+        IoMem::new(resource, self.as_ref())
+    }
+
+    /// Same as [`Self::iomap_resource_sized`] but with exclusive access to the
+    /// underlying region.
+    ///
+    /// This uses the
+    /// [`ioremap()`](https://docs.kernel.org/driver-api/device-io.html#getting-access-to-the-device)
+    /// C API.
+    pub fn iomap_resource_exclusive_sized<const SIZE: usize>(
+        &self,
+        resource: &Resource,
+    ) -> Result<impl PinInit<Devres<ExclusiveIoMem<SIZE>>, Error> + '_> {
+        ExclusiveIoMem::new(resource, self.as_ref())
+    }
+
+    /// Maps a platform resource through `iomap()`.
+    ///
+    /// This uses the
+    /// [`ioremap()`](https://docs.kernel.org/driver-api/device-io.html#getting-access-to-the-device)
+    /// C API.
+    ///
+    /// # Examples
+    ///
+    /// ```no_run
+    /// # use kernel::{bindings, c_str, platform, of, device::Core};
+    /// # struct SampleDriver;
+    ///
+    /// impl platform::Driver for SampleDriver {
+    ///    # type IdInfo = ();
+    ///    # const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+    ///
+    ///    fn probe(
+    ///       pdev: &platform::Device<Core>,
+    ///       info: Option<&Self::IdInfo>,
+    ///    ) -> Result<Pin<KBox<Self>>> {
+    ///       let offset = 0; // Some offset.
+    ///
+    ///       // Unlike [`Self::iomap_resource_sized`], here the size of the memory region
+    ///       // is not known at compile time, so only the `try_read*` and `try_write*`
+    ///       // family of functions should be used, leading to runtime checks on every
+    ///       // access.
+    ///       let resource = pdev.resource_by_index(0).ok_or(ENODEV)?;
+    ///       let iomem = pdev.iomap_resource(&resource)?;
+    ///       let iomem = KBox::pin_init(iomem, GFP_KERNEL)?;
+    ///
+    ///       let io = iomem.access(pdev.as_ref())?;
+    ///
+    ///       let data = io.try_read32_relaxed(offset)?;
+    ///
+    ///       io.try_write32_relaxed(data, offset)?;
+    ///
+    ///       # Ok(KBox::new(SampleDriver, GFP_KERNEL)?.into())
+    ///     }
+    /// }
+    /// ```
+    pub fn iomap_resource(
+        &self,
+        resource: &Resource,
+    ) -> Result<impl PinInit<Devres<IoMem<0>>, Error> + '_> {
+        self.iomap_resource_sized::<0>(resource)
+    }
+
+    /// Same as [`Self::iomap_resource`] but with exclusive access to the underlying
+    /// region.
+    pub fn iomap_resource_exclusive(
+        &self,
+        resource: &Resource,
+    ) -> Result<impl PinInit<Devres<ExclusiveIoMem<0>>, Error> + '_> {
+        self.iomap_resource_exclusive_sized::<0>(resource)
+    }
+}
+
 // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
 // argument.
 kernel::impl_device_context_deref!(unsafe { Device });

-- 
2.50.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 1/4] rust: io: add resource abstraction
  2025-07-01 14:34 ` [PATCH v11 1/4] rust: io: add resource abstraction Daniel Almeida
@ 2025-07-02  7:44   ` Miguel Ojeda
  2025-07-02 18:04     ` Daniel Almeida
  2025-07-02 10:21   ` Danilo Krummrich
  1 sibling, 1 reply; 12+ messages in thread
From: Miguel Ojeda @ 2025-07-02  7:44 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, linux-kernel,
	rust-for-linux, Fiona Behrens

Hi Daniel,

A couple nits Danilo can take care of them on apply.

On Tue, Jul 1, 2025 at 4:35 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
> +//! Abstractions for system resources.

Potential link:

    https://docs.kernel.org/core-api/kernel-api.html#resources-management

I think there are a couple kernel-doc includes missing in the `.rst`
for this, so it is not great.

> +/// This is a type alias to `u64` depending on the config option

"to `u32` or `u64`"? (also below)

> +    /// The caller must ensure that for the duration of 'a, the pointer will

`'a` (same below).

> +        // SAFETY: Safe as per the invariant of `Resource`

"Safe", periods at the end. (few instances)

The last patch on the series has nice examples, thanks! It could be
nice to have some here too (e.g. on how `Flags` and its operators) and
nowadays also KUnit tests, but that can be a good first issue for
later.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 0/4] rust: platform: add Io support
  2025-07-01 14:34 [PATCH v11 0/4] rust: platform: add Io support Daniel Almeida
                   ` (3 preceding siblings ...)
  2025-07-01 14:34 ` [PATCH v11 4/4] rust: platform: allow ioremap of platform resources Daniel Almeida
@ 2025-07-02  7:52 ` Miguel Ojeda
  4 siblings, 0 replies; 12+ messages in thread
From: Miguel Ojeda @ 2025-07-02  7:52 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, linux-kernel,
	rust-for-linux, Fiona Behrens

On Tue, Jul 1, 2025 at 4:35 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
> Daniel Almeida (4):
>       rust: io: add resource abstraction
>       rust: io: mem: add a generic iomem abstraction
>       rust: platform: add resource accessors
>       rust: platform: allow ioremap of platform resources

I think Danilo needs these -- I left a couple nits I noticed in the first one:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

(By the way, this cover letter is all over the place -- it has two
diffstats/footers, no intro, and there is even a title in the middle
of it.)

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 1/4] rust: io: add resource abstraction
  2025-07-01 14:34 ` [PATCH v11 1/4] rust: io: add resource abstraction Daniel Almeida
  2025-07-02  7:44   ` Miguel Ojeda
@ 2025-07-02 10:21   ` Danilo Krummrich
  2025-07-02 18:11     ` Daniel Almeida
  1 sibling, 1 reply; 12+ messages in thread
From: Danilo Krummrich @ 2025-07-02 10:21 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, Andrew Morton,
	Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, linux-kernel,
	rust-for-linux, Fiona Behrens

> +#[cfg(CONFIG_HAS_IOPORT)]
> +/// Returns a reference to the global `ioport_resource` variable.
> +pub fn ioport_resource() -> &'static Resource {
> +    // SAFETY: `bindings::ioport_resoure` has global lifetime and is of type Resource.
> +    unsafe { Resource::as_ref(&raw mut bindings::ioport_resource) }
> +}
> +
> +#[cfg(CONFIG_HAS_IOMEM)]
> +/// Returns a reference to the global `iomem_resource` variable.
> +pub fn iomem_resource() -> &'static Resource {
> +    // SAFETY: `bindings::iomem_resoure` has global lifetime and is of type Resource.
> +    unsafe { Resource::as_ref(&raw mut bindings::iomem_resource) }
> +}

This caught my attention, and I have a few questions:

   1) What do you need them for? I don't see any methods that would usually
      consume those.

   2) Why are they behind CONFIG_HAS_IOPORT and CONFIG_HAS_IOMEM, even though the
      C instances are not?

   3) What happens if we pass them to IoMem::new()? Is this really safe, or do we
      need them to be a special Resource type?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 4/4] rust: platform: allow ioremap of platform resources
  2025-07-01 14:34 ` [PATCH v11 4/4] rust: platform: allow ioremap of platform resources Daniel Almeida
@ 2025-07-02 10:36   ` Danilo Krummrich
  0 siblings, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2025-07-02 10:36 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, Andrew Morton,
	Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, linux-kernel,
	rust-for-linux

On 7/1/25 4:34 PM, Daniel Almeida wrote:
> +impl Device<device::Bound> {

<snip>

> +    pub fn iomap_resource_sized<const SIZE: usize>(
> +        &self,
> +        resource: &Resource,
> +    ) -> Result<impl PinInit<Devres<IoMem<SIZE>>, Error> + '_> {
> +        IoMem::new(resource, self.as_ref())
> +    }

Sorry that I did not catch this earlier, but what if I supply a Resource here
that has *not* been obtained by any of the platform::Device methods and from the
same platform::Device as you call this function for?

I think this also needs something similar to what we do in the IRQ abstraction,
e.g.:

	/// # Invariants
	///
	/// `res` has been obtained from `dev`.
	pub struct IoRequest<'a> {
	   dev: &'a Device<Bound>,
	   res: &'a Resource,
	}

Such that IoMem::new() can take an IoRequest instance instead.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 1/4] rust: io: add resource abstraction
  2025-07-02  7:44   ` Miguel Ojeda
@ 2025-07-02 18:04     ` Daniel Almeida
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Almeida @ 2025-07-02 18:04 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, linux-kernel,
	rust-for-linux, Fiona Behrens

Hi Miguel,

> 
> "Safe", periods at the end. (few instances)
> 


Is it only about the periods, or is there any issues with the word
“Safe” itself (and/or its capitalization?)

— Daniel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 1/4] rust: io: add resource abstraction
  2025-07-02 10:21   ` Danilo Krummrich
@ 2025-07-02 18:11     ` Daniel Almeida
  2025-07-02 18:16       ` Danilo Krummrich
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Almeida @ 2025-07-02 18:11 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, Andrew Morton,
	Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, linux-kernel,
	rust-for-linux, Fiona Behrens

Hi Danilo

> On 2 Jul 2025, at 07:21, Danilo Krummrich <dakr@kernel.org> wrote:
> 
>> +#[cfg(CONFIG_HAS_IOPORT)]
>> +/// Returns a reference to the global `ioport_resource` variable.
>> +pub fn ioport_resource() -> &'static Resource {
>> +    // SAFETY: `bindings::ioport_resoure` has global lifetime and is of type Resource.
>> +    unsafe { Resource::as_ref(&raw mut bindings::ioport_resource) }
>> +}
>> +
>> +#[cfg(CONFIG_HAS_IOMEM)]
>> +/// Returns a reference to the global `iomem_resource` variable.
>> +pub fn iomem_resource() -> &'static Resource {
>> +    // SAFETY: `bindings::iomem_resoure` has global lifetime and is of type Resource.
>> +    unsafe { Resource::as_ref(&raw mut bindings::iomem_resource) }
>> +}
> 
> This caught my attention, and I have a few questions:
> 
>  1) What do you need them for? I don't see any methods that would usually
>     consume those.
> 
>  2) Why are they behind CONFIG_HAS_IOPORT and CONFIG_HAS_IOMEM, even though the
>     C instances are not?
> 
>  3) What happens if we pass them to IoMem::new()? Is this really safe, or do we
>     need them to be a special Resource type?
> 

Good catch, actually.

I worked on this patch with Fiona and IIRC, she needed access to these two for
her LED abstractions. This patch has seen a few iterations already and this may
or may not be obsolete by now. I must say I've never used these before so I
don't really know how they work nor do I need this at all.

Fiona, do you still need these two accessors?

— Daniel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v11 1/4] rust: io: add resource abstraction
  2025-07-02 18:11     ` Daniel Almeida
@ 2025-07-02 18:16       ` Danilo Krummrich
  0 siblings, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2025-07-02 18:16 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, Andrew Morton,
	Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
	Mika Westerberg, Ying Huang, Benno Lossin, linux-kernel,
	rust-for-linux, Fiona Behrens

On Wed, Jul 02, 2025 at 03:11:04PM -0300, Daniel Almeida wrote:
> Hi Danilo
> 
> > On 2 Jul 2025, at 07:21, Danilo Krummrich <dakr@kernel.org> wrote:
> > 
> >> +#[cfg(CONFIG_HAS_IOPORT)]
> >> +/// Returns a reference to the global `ioport_resource` variable.
> >> +pub fn ioport_resource() -> &'static Resource {
> >> +    // SAFETY: `bindings::ioport_resoure` has global lifetime and is of type Resource.
> >> +    unsafe { Resource::as_ref(&raw mut bindings::ioport_resource) }
> >> +}
> >> +
> >> +#[cfg(CONFIG_HAS_IOMEM)]
> >> +/// Returns a reference to the global `iomem_resource` variable.
> >> +pub fn iomem_resource() -> &'static Resource {
> >> +    // SAFETY: `bindings::iomem_resoure` has global lifetime and is of type Resource.
> >> +    unsafe { Resource::as_ref(&raw mut bindings::iomem_resource) }
> >> +}
> > 
> > This caught my attention, and I have a few questions:
> > 
> >  1) What do you need them for? I don't see any methods that would usually
> >     consume those.
> > 
> >  2) Why are they behind CONFIG_HAS_IOPORT and CONFIG_HAS_IOMEM, even though the
> >     C instances are not?
> > 
> >  3) What happens if we pass them to IoMem::new()? Is this really safe, or do we
> >     need them to be a special Resource type?
> > 
> 
> Good catch, actually.
> 
> I worked on this patch with Fiona and IIRC, she needed access to these two for
> her LED abstractions. This patch has seen a few iterations already and this may
> or may not be obsolete by now. I must say I've never used these before so I
> don't really know how they work nor do I need this at all.

They serve as parent / root resource instances, e.g. if you want to create new
resource regions yourself.

> Fiona, do you still need these two accessors?

I'd say let's drop them for now, we can easily add them later on in the context
of an actual user.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-07-02 18:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-01 14:34 [PATCH v11 0/4] rust: platform: add Io support Daniel Almeida
2025-07-01 14:34 ` [PATCH v11 1/4] rust: io: add resource abstraction Daniel Almeida
2025-07-02  7:44   ` Miguel Ojeda
2025-07-02 18:04     ` Daniel Almeida
2025-07-02 10:21   ` Danilo Krummrich
2025-07-02 18:11     ` Daniel Almeida
2025-07-02 18:16       ` Danilo Krummrich
2025-07-01 14:34 ` [PATCH v11 2/4] rust: io: mem: add a generic iomem abstraction Daniel Almeida
2025-07-01 14:34 ` [PATCH v11 3/4] rust: platform: add resource accessors Daniel Almeida
2025-07-01 14:34 ` [PATCH v11 4/4] rust: platform: allow ioremap of platform resources Daniel Almeida
2025-07-02 10:36   ` Danilo Krummrich
2025-07-02  7:52 ` [PATCH v11 0/4] rust: platform: add Io support Miguel Ojeda

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.