Messages in this thread | | | Date | Mon, 2 Oct 2023 15:07:06 -0400 | From | Frank Li <> | Subject | Re: [PATCH v5 1/3] debugfs_create_regset32() support 8/16 bit width registers |
| |
On Mon, Oct 02, 2023 at 08:55:23PM +0200, Arnd Bergmann wrote: > On Mon, Oct 2, 2023, at 20:37, Frank Li wrote: > > Enhance the flexibility of `debugfs_create_regset32()` to support registers > > of various bit widths. The key changes are as follows: > > > > 1. Renamed '*reg32' and '*regset32' to '*reg' and '*regset' in relevant > > code to reflect that the register width is not limited to 32 bits. > > > > 2. Added 'size' and 'bigendian' fields to the `struct debugfs_reg` to allow > > for specifying the size and endianness of registers. These additions > > enable `debugfs_create_regset()` to support a wider range of register > > types. > > > > 3. When 'size' is set to 0, it signifies a 32-bit register. This change > > maintains compatibility with existing code that assumes 32-bit > > registers. > > > > Improve the versatility of `debugfs_create_regset()` and enable it to > > handle registers of different sizes and endianness, offering greater > > flexibility for debugging and monitoring. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > This version looks correct to me, I see no fundamental problems with it. > In fact, you could list "support for ioport_map() output" to the features > above. > > A few other thoughts from my side, all of which could be ignored: > > - if the ioport access is not an important feature, we can instead > support 64-bit readl() as I commented in a previous email. We just > can't easily have both.
We will get 64bit dma edma soon. So I can test and upstream it when I get it.
> > - instead of treating every value of "regs->size" other than 1 and 2 > as meaning '32-bit read', I would explicitly check for 0 and 4 > here
Yes, I will update next version.
> > - Another more complicated but also more featureful variant would > be to use the 'regmap' infrastructure as the abstraction, this would > also provide access to big-endian, variable register width > (including 64-bit), and pio, along with additional features and > other bus types. Not sure it's worth it, but could be interesting > to try out.
Yes, debugfs_create_regset may need big change or create new version for regmap. It is out of scope this patches. I will try later.
> > Arnd
| |