[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <7f39410a-72fc-463e-b41c-64674ab4f129@app.fastmail.com>
Date: Mon, 02 Oct 2023 20:55:23 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Frank Li" <Frank.Li@....com>, "Vinod Koul" <vkoul@...nel.org>
Cc: "Baoquan He" <bhe@...hat.com>, dmaengine@...r.kernel.org,
"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
imx@...ts.linux.dev, linux-kernel@...r.kernel.org,
"kernel test robot" <lkp@...el.com>, oe-kbuild-all@...ts.linux.dev,
"Rafael J . Wysocki" <rafael@...nel.org>
Subject: Re: [PATCH v5 1/3] debugfs_create_regset32() support 8/16 bit width registers
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@....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.
- 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
- 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.
Arnd
Powered by blists - more mailing lists