On Wed, Jun 19, 2024 at 01:39:53AM +0200, Danilo Krummrich wrote: > I/O memory is typically either mapped through direct calls to ioremap() > or subsystem / bus specific ones such as pci_iomap(). > > Even though subsystem / bus specific functions to map I/O memory are > based on ioremap() / iounmap() it is not desirable to re-implement them > in Rust. Why not? > Instead, implement a base type for I/O mapped memory, which generically > provides the corresponding accessors, such as `Io::readb` or > `Io:try_readb`. It provides a subset of the existing accessors, one you might want to trim down for now, see below... > +/* io.h */ > +u8 rust_helper_readb(const volatile void __iomem *addr) > +{ > + return readb(addr); > +} > +EXPORT_SYMBOL_GPL(rust_helper_readb); <snip> You provide wrappers for a subset of what io.h provides, why that specific subset? Why not just add what you need, when you need it? I doubt you need all of these, and odds are you will need more. > +u32 rust_helper_readl_relaxed(const volatile void __iomem *addr) > +{ > + return readl_relaxed(addr); > +} > +EXPORT_SYMBOL_GPL(rust_helper_readl_relaxed); I know everyone complains about wrapper functions around inline functions, so I'll just say it again, this is horrid. And it's going to hurt performance, so any rust code people write is not on a level playing field here. Your call, but ick... > +#ifdef CONFIG_64BIT > +u64 rust_helper_readq_relaxed(const volatile void __iomem *addr) > +{ > + return readq_relaxed(addr); > +} > +EXPORT_SYMBOL_GPL(rust_helper_readq_relaxed); > +#endif Rust works on 32bit targets in the kernel now? > +macro_rules! define_read { > + ($(#[$attr:meta])* $name:ident, $try_name:ident, $type_name:ty) => { > + /// Read IO data from a given offset known at compile time. > + /// > + /// Bound checks are performed on compile time, hence if the offset is not known at compile > + /// time, the build will fail. offsets aren't know at compile time for many implementations, as it could be a dynamically allocated memory range. How is this going to work for that? Heck, how does this work for DT-defined memory ranges today? thanks, greg k-h