currently the rust `Io` type maps to the c read{b, w, l, q}/write{b, w, l, q}
functions and have no support for port io.this is a problem for pci::Bar
because the pointer returned by pci_iomap is expected to accessed with
the ioread/iowrite api [0].

this patch series splits the `Io` type into `Io`, `PortIo` and `MMIo`.and,
updates pci::Bar, as suggested in the zulip[1], so that it is generic over
Io and, a user can optionally give a compile time hint about the type of io. 

Link: https://docs.kernel.org/6.11/driver-api/pci/pci.html#c.pci_iomap [0]
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/.60IoRaw.60.20and.20.60usize.60/near/514788730 [1]

Andrew Ballance (6):
  rust: io: add new Io type
  rust: io: add from_raw_cookie functions
  rust: pci: make Bar generic over Io
  samples: rust: rust_driver_pci: update to use new bar and io api
  gpu: nova-core: update to use the new bar and io api
  rust: devres: fix doctest

Fiona Behrens (5):
  rust: helpers: io: use macro to generate io accessor functions
  rust: io: Replace Io with MMIo using IoAccess trait
  rust: io: implement Debug for IoRaw and add some doctests
  rust: io: add PortIo
  io: move PIO_OFFSET to linux/io.h

 drivers/gpu/nova-core/driver.rs |   4 +-
 drivers/gpu/nova-core/regs.rs   |   1 +
 include/linux/io.h              |  13 +
 lib/iomap.c                     |  13 -
 rust/helpers/io.c               | 132 +++---
 rust/kernel/devres.rs           |   4 +-
 rust/kernel/io.rs               | 753 +++++++++++++++++++++++++-------
 rust/kernel/pci.rs              |  88 +++-
 samples/rust/rust_driver_pci.rs |   6 +-
 9 files changed, 731 insertions(+), 283 deletions(-)


base-commit: 92a09c47464d040866cf2b4cd052bc60555185fb
-- 
2.49.0

On Fri, May 9, 2025, at 05:15, Andrew Ballance wrote:
> currently the rust `Io` type maps to the c read{b, w, l, q}/write{b, w, l, q}
> functions and have no support for port io.this is a problem for pci::Bar
> because the pointer returned by pci_iomap is expected to accessed with
> the ioread/iowrite api [0].
>
> this patch series splits the `Io` type into `Io`, `PortIo` and `MMIo`.and,
> updates pci::Bar, as suggested in the zulip[1], so that it is generic over
> Io and, a user can optionally give a compile time hint about the type of io. 

Can you describe here why you want to support both "Io" and "PortIo"
cases separately? I don't think we need to micro-optimize for
legacy ISA devices any more, so I'd hope the "Io" path would be
sufficient to cover the common outliers (ata, uart, vga, ipmi, ne2000)
that need the iomap indirection and also the legacy devices that only
need port I/O (floppy, x86 platform devices, ...).

Ideally we'd only need one set of I/O accessors at all, but I suspect
there are x86 specific drivers that actually need readl/writel to be
inline for performance when accessing on-chip registers rather than
slow PCIe registers.

      Arnd

On Fri, May 09, 2025 at 07:53:31AM +0200, Arnd Bergmann wrote:
> Can you describe here why you want to support both "Io" and "PortIo"
> cases separately? I don't think we need to micro-optimize for
> legacy ISA devices any more, so I'd hope the "Io" path would be
> sufficient to cover the common outliers (ata, uart, vga, ipmi, ne2000)
> that need the iomap indirection and also the legacy devices that only
> need port I/O (floppy, x86 platform devices, ...).

Yeah, we probably don`t need the `PortIo` type and can rely on `Io` for
port io. I`ll remove it for the v2.

Best regards
Andrew Ballance

Hi Andrew,

On Thu, May 08, 2025 at 10:15:13PM -0500, Andrew Ballance wrote:
> currently the rust `Io` type maps to the c read{b, w, l, q}/write{b, w, l, q}
> functions and have no support for port io.this is a problem for pci::Bar
> because the pointer returned by pci_iomap is expected to accessed with
> the ioread/iowrite api [0].
> 
> this patch series splits the `Io` type into `Io`, `PortIo` and `MMIo`.and,
> updates pci::Bar, as suggested in the zulip[1], so that it is generic over
> Io and, a user can optionally give a compile time hint about the type of io. 
> 
> Link: https://docs.kernel.org/6.11/driver-api/pci/pci.html#c.pci_iomap [0]
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/.60IoRaw.60.20and.20.60usize.60/near/514788730 [1]
> 
> Andrew Ballance (6):
>   rust: io: add new Io type
>   rust: io: add from_raw_cookie functions
>   rust: pci: make Bar generic over Io
>   samples: rust: rust_driver_pci: update to use new bar and io api
>   gpu: nova-core: update to use the new bar and io api
>   rust: devres: fix doctest
> 
> Fiona Behrens (5):
>   rust: helpers: io: use macro to generate io accessor functions
>   rust: io: Replace Io with MMIo using IoAccess trait
>   rust: io: implement Debug for IoRaw and add some doctests
>   rust: io: add PortIo
>   io: move PIO_OFFSET to linux/io.h

Thanks for sending out the patch series.

I gave it a quick shot and the series breaks the build starting with patch 2. I
see that you have fixup commits later in the series, however in the kernel we
don't allow patches to intermediately introduce build failures, build warnings,
bugs, etc., see also [1]. You should still try to break things down logically as
good as possible.

From the current structure of your patches it seems to me that structure-wise
you should be good by going through them one by one and fix them up; your later
patches should become "noops" then. But feel free to re-organize things if you
feel that's not the best approach.

Can you please fix this up and resend right away? This should make the
subsequent review much easier.

[1] https://docs.kernel.org/process/submitting-patches.html#separate-your-changes