Re: [PATCH v4 1/6] nova-core: bitfield: Move bitfield-specific code from register! into new macro

Greg KH
2025-09-24T12:04:18Z

On Wed, Sep 24, 2025 at 01:28:18PM +0200, Danilo Krummrich wrote:
> On Wed Sep 24, 2025 at 12:52 PM CEST, Greg KH wrote:
> > Ok, great, but right now it's not doing that from what I am seeing when
> > reading the code. Shouldn't IoMem::new() take that as an argument?
>
> That's correct, neither IoMem nor pci::Bar do consider it yet; it's on the list
> of things that still need to be done.
>
> > But, that feels odd as our current iomem api in C doesn't care about
> > endian issues at all because it "assumes" that the caller has already
> > handle this properly and all that the caller "wants" is to write/read to
> > some memory chunk and not twiddle bits.
>
> Yet it seems to be the correct place to deal with it. As mentioned below, regmap
> could just become part of an I/O backend implementation to do exactly that.
>
> >> (Actually, we could even implement an I/O backend that uses regmap.)
> >
> > That would probably be best to do eventually as most platform drivers
> > use regmap today as it's the sanest api we have at the moment.
>
> I agree it's what we should do eventually.
>
> >> So, I think the register!() stuff is rather orthogonal.
> >
> > I think it's very relevant as people seem to just be "assuming" that all
> > the world (hardware and cpus) are little-endian, while in reality, they
> > are anything but. As proof, the code that uses this register!() logic
> > today totally ignores endian issues and just assumes that it is both
> > running on a little-endian system, AND the hardware is little-endian.
> >
> > As a crazy example, look at the USB host controllers that at runtime,
> > have to be queried to determine what endian they are running on and the
> > kernel drivers have to handle this "on the fly". Yes, one can argue
> > that the hardware developers who came up with that should be forced to
> > write the drivers as penance for such sins, but in the end, it's us that
> > has to deal with it...
> >
> > So ignoring it will get us quite a ways forward with controlling sane
> > hardware on sane systems, but when s390 finally realizes they can be
> > writing their drivers in rust, we are going to have to have these
> > conversations again :)
>
> I think it's not really that anyone is ignoring it (intentionally). It's two
> different things that should be addressed here; yet they are related:
>
> (1) Implementation of an abstract representation of a register that drivers
> can interact with.
>
> (2) The I/O layer that lays out the raw data on the physcial bus.
>
> The register!() macro intends to provide an abstract representation of a
> register for drivers to interact with. Think of it as an abstract box, where the
> memory layout does not matter at all -- could be anything.
>
> Theoretically, this abstraction could even store every single field of a
> register in its own u32 or u64, etc. Of course, that's a waste of memory, which
> is why we're using this bitfield thing instead.
>
> The only thing that matters is that there is a contract between the struct
> representing a register (generated by the register!() macro) and the I/O backend
> layer that lays out the raw value on the bus.
>
> This works attempts to address (1), whereas you are (rightfully) asking for (2).
> And I think the answer for (2) simply is, we still have to address it.

Ok, fair enough, you've convinced me, I'll let you all argue which side
the "0" should be on (left or right) :)

Let's wait for the first platform drivers to start showing up that want
to use regmap, hopefully that will be soon as almost all of the pieces
are there to do so.

thanks,

greg k-h