| Message ID | 20230110042533.12894-1-clayc@hpe.com |
|---|---|
| Headers | show |
| Series | ARM: Add GXP SROM Support | expand |
On Mon, Jan 16, 2023, at 14:42, Clay Chang wrote: > On Thu, Jan 12, 2023 at 02:37:53PM +0100, Arnd Bergmann wrote: >> On Thu, Jan 12, 2023, at 14:16, Clay Chang wrote: >> For the user interface side, I don't really like the idea of >> having a hardware register directly exposed as driver in >> drivers/soc, this generally makes it impossible to have portable >> userspace that works across implementations of multiple SoC >> vendors, and it makes it too easy to come up with an ad-hoc >> interface to make a chip work for a particular use case when >> a more general solution would be better. >> > > I agree with you. I have one question though: if we create a 'hpe' > directory under drivers/soc, and put all HPE BMC specific drivers there, > do you think this proper? It certainly wouldn't be right to put "all HPE BMC specific drivers" in there. Most drivers will fit into some existing subsystem, and should be moved there instead. drivers/soc is used primarily for drivers using soc_device_register() to provide information about the soc, and we also use it as a place for drivers that just export soc-specific helper functions that can be used by other drivers. >> Again, it's hard for me to tell why this even needs to be runtime >> configurable, please try to describe what type of application >> would access the sysfs interface here, and why this can't just >> be set to a fixed value by bootloader or kernel without user >> interaction. > > The register is used for communication and synchronization between the > BMC and the host. During runtime, user-space daemons configures the > value of the register for interactions. That does not sound very specific. What is the subsystem on the host that this communicates with? Can you put the driver into the same subsystem? Arnd
On Thu, Jan 19, 2023, at 08:39, Clay Chang wrote: > On Mon, Jan 16, 2023 at 04:18:59PM +0100, Arnd Bergmann wrote: > > Sorry for not saying it clearly. I meant to put those HPE BMC related > drivers that are "not specific" to a particular subsystem in > drivers/soc/hpe. For those fit into some existing subsystems go to their > designated places. Ok, that makes sense. I'm just trying to reduce the number of drivers that fit into this category. >> >> Again, it's hard for me to tell why this even needs to be runtime >> >> configurable, please try to describe what type of application >> >> would access the sysfs interface here, and why this can't just >> >> be set to a fixed value by bootloader or kernel without user >> >> interaction. >> > >> > The register is used for communication and synchronization between the >> > BMC and the host. During runtime, user-space daemons configures the >> > value of the register for interactions. >> >> That does not sound very specific. What is the subsystem on the >> host that this communicates with? Can you put the driver into the >> same subsystem? > > This is a control register in the BMC chip that partially controls host > boot behaviors. When writing to the register, privileged mode is > required. That's why we rely on a kernel driver for writing to the > control register. And, there is no corresponding subsystem in the host > OS. For this case, is it acceptable to put this driver under > drivers/soc/hpe? I see. So this sounds like it might be generic BMC feature, which would be nice to handle consistently for all BMC families. We had some discussions about other BMC features in the past, but don't remember what the overall consensus was, so I'm adding the openbmc list and as well as aspeed and npcm maintainers to Cc. The part I'm still missing is what type of userspace makes this decision on the BMC side, and whether that might already have a generalized interface. If there is, maybe part of that interface can be abstracted in the kernel. Arnd
Hi Andrew, Thank you for taking time, and sorry for my late response. On Fri, Jan 20, 2023 at 12:51:54PM +1030, Andrew Jeffery wrote: > > > On Tue, 10 Jan 2023, at 14:55, clayc@hpe.com wrote: > > From: Clay Chang <clayc@hpe.com> > > > > The GXP SROM control register can be used to configure LPC related > > legacy I/O registers. Currently only the SROM RAM Offset Register > > (vromoff) is exported. > > What exact behaviour does vromoff influence? You mention I/O registers, > but RAM offset feels like it may be related to MEM or FWH LPC cycles > instead? > Sorry for my previous inaccurate description about the vromoff register. You are right, it is not related to I/O but memory LPC cycles. This register defines the offset and size to BMC's memory for the system rom. BMC uses it for system rom related operations. One way to access this is through the memory LPC cycles. > I'm trying to understand whether we can find some common ground with > controlling e.g. Aspeed's BMCs LPC peripherals based on Arnd's query[1], > but the description is a bit too vague right now for me to be able to do > that. > > [1] https://lore.kernel.org/all/66ef9643-b47e-428d-892d-7c1cbd358a5d@app.fastmail.com/ > > Andrew Thanks, Clay
Hi Andrew, On Tue, Jan 31, 2023 at 09:46:42PM +0800, Clay Chang wrote: > > I'm trying to understand whether we can find some common ground with > > controlling e.g. Aspeed's BMCs LPC peripherals based on Arnd's query[1], > > but the description is a bit too vague right now for me to be able to do > > that. > > > > [1] https://lore.kernel.org/all/66ef9643-b47e-428d-892d-7c1cbd358a5d@app.fastmail.com/ > > > > Andrew I briefly studied driver/soc/aspeed/aspeed-lpc-ctrl.c, and IMO the similarity between HPE GXP driver and Aspeed's could be that we both expose the LPC memory addresses or registers for configuration purposes. However, the functions to be configured could vary. There are a few sets of registers that HPE wants to expose for configuration in the future. So, do you think there could be some common grounds for genralization? Please let me know if you need more information. Thanks, Clay
On Thu, Feb 02, 2023 at 11:42:49AM +1030, Andrew Jeffery wrote: > > > On Wed, 1 Feb 2023, at 23:58, Clay Chang wrote: > > Hi Andrew, > > > > On Tue, Jan 31, 2023 at 09:46:42PM +0800, Clay Chang wrote: > >> > I'm trying to understand whether we can find some common ground with > >> > controlling e.g. Aspeed's BMCs LPC peripherals based on Arnd's query[1], > >> > but the description is a bit too vague right now for me to be able to do > >> > that. > >> > > >> > [1] https://lore.kernel.org/all/66ef9643-b47e-428d-892d-7c1cbd358a5d@app.fastmail.com/ > >> > > >> > Andrew > > > > I briefly studied driver/soc/aspeed/aspeed-lpc-ctrl.c, and IMO the > > similarity between HPE GXP driver and Aspeed's could be that we both > > expose the LPC memory addresses or registers for configuration purposes. > > However, the functions to be configured could vary. There are a few sets > > of registers that HPE wants to expose for configuration in the future. > > The talk of "exposing registers" feels concerning - we're trying not to > do that directly. Instead we want to lift out an API that constrains > the behaviour a bit but works for both of us if there's overlap in > functionality. > Let me describe it more clearly. We don't expose the registers directly to the user. We describe those registers in the device tree, and then the driver exposes them though sysfs interfaces. Users access the registers through the device attributes defined under the sysfs structure (e.g. /sys/class/soc/srom/vromoff) exposed by the driver. In the show/store function pair, we get or set the regmap or memory, validate input, do sanity check, synchronization, and so on. I learned that in the drivers/soc/aspeed/aspeed-lpc-ctrl.c, both mmap and ioctl were used, and revelant ioctl commands were defined in include/uapi/linux/aspeed-lpc-ctrl.h. Is this what you mean an API for aspeed? And we are trying to see if there are commonalities among us? If yes, yeah I think it is good to see a common abstraction for BMC chips. Accroding to [1], I'd comment that, in terms of flash update, what we want to do is similar to what was described in [1]. The SROM driver I am working on partially layouts the registers for flash update for HPE GXP hardware. BTW, the way for user to access the registers is flexible - ioctl should work as well. > Can you point to any documentation of the behaviour of your hardware? > It's still a little vague to me. > > Andrew I wish I could, but there is not yet a technical document or datasheet available to the public. Sorry. [1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?h=v6.2-rc4&id=6c4e976785011dfbe461821d0bfc58cfd60eac56 Thanks, Clay
From: Clay Chang <clayc@hpe.com> The GXP SROM control register can be used to configure LPC related legacy I/O registers. Currently only the SROM RAM Offset Register (vromoff) is exported. Clay Chang (5): soc: hpe: Add GXP SROM Control Register Driver dt-bindings: soc: hpe: hpe,gxp-srom.yaml ARM: dts: hpe: Add SROM Driver ARM: multi_v7_defconfig: Add GXP SROM Driver MAINTAINERS: Add maintainer of GXP SROM support .../bindings/soc/hpe/hpe,gxp-srom.yaml | 36 +++++ MAINTAINERS | 8 + arch/arm/boot/dts/hpe-gxp.dtsi | 41 ++--- arch/arm/configs/multi_v7_defconfig | 2 + drivers/soc/Kconfig | 1 + drivers/soc/Makefile | 1 + drivers/soc/hpe/Kconfig | 29 ++++ drivers/soc/hpe/Makefile | 2 + drivers/soc/hpe/gxp-soclib.c | 17 +++ drivers/soc/hpe/gxp-soclib.h | 9 ++ drivers/soc/hpe/gxp-srom.c | 141 ++++++++++++++++++ 11 files changed, 269 insertions(+), 18 deletions(-) create mode 100644 Documentation/devicetree/bindings/soc/hpe/hpe,gxp-srom.yaml create mode 100644 drivers/soc/hpe/Kconfig create mode 100644 drivers/soc/hpe/Makefile create mode 100644 drivers/soc/hpe/gxp-soclib.c create mode 100644 drivers/soc/hpe/gxp-soclib.h create mode 100644 drivers/soc/hpe/gxp-srom.c