Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Infallible 64-bit I/O accessors (`read64`, `write64`) and typed 64-bit registers cannot be used on MMIO regions, and custom view types are unsupported by standard offsets. --
commit 09225b49c257719774237bd1b25407607dd9df87 Author: Gary Guo <[email protected]> rust: io: implement Mmio as view type Implement Mmio as view type and convert RelaxedMmio to view type as well. I/O implementations of MmioOwned are changed to delegate to the Mmio view type. > diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs > index f93be7f780694..8110b49aa4307 100644 > --- a/rust/kernel/io.rs > +++ b/rust/kernel/io.rs [ ... ] > @@ -847,17 +915,12 @@ unsafe fn io_write(self, value: $ty, address: usize) { > } > > // MMIO regions support 8, 16, and 32-bit accesses. > -impl_mmio_io_capable!(MmioOwned, u8, readb, writeb); > -impl_mmio_io_capable!(MmioOwned, u16, readw, writew); > -impl_mmio_io_capable!(MmioOwned, u32, readl, writel); > +impl_mmio_io_capable!(Mmio, u8, readb, writeb); > +impl_mmio_io_capable!(Mmio, u16, readw, writew); > +impl_mmio_io_capable!(Mmio, u32, readl, writel); > // MMIO regions on 64-bit systems also support 64-bit accesses. > -impl_mmio_io_capable!( > - MmioOwned, > - #[cfg(CONFIG_64BIT)] > - u64, > - readq, > - writeq > -); > +#[cfg(CONFIG_64BIT)] > +impl_mmio_io_capable!(Mmio, u64, readq, writeq); [Severity: High] Does this actually allow infallible 64-bit I/O accesses when the target is Region<SIZE>? Region<SIZE> has a MIN_ALIGN of 4, so infallible 64-bit accessors like read64() and write64() will fail the compile-time assertion in io_view_assert(): const_assert!( Alignment::of::<U>().as_usize() <= IO::Target::MIN_ALIGN.as_usize() ); For U = u64, the alignment is 8 on 64-bit architectures, which fails the assertion since 8 <= 4 is false. Additionally, while this patch introduces Mmio as a generic view type, can users create a custom target type (e.g. with align(8)) to work around this? Looking at impl_usize_ioloc in rust/kernel/io.rs and the IoLoc implementation in rust/kernel/io/register.rs, both usize offsets and typed registers are strictly implemented only for IoLoc<Region<SIZE>, T>. Because standard offsets and registers are restricted to Region<SIZE>, a custom view type cannot be used with them. Does this leave 64-bit infallible accesses and 64-bit typed registers unusable on MMIO regions? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
