Re: [PATCH v2 1/2] soc: aspeed: add BMC-side PCIe BMC device driver
Andrew Jeffery <[email protected]> Tue, 16 Jun 2026 09:46:24 +0930
| Newsgroups | org.ozlabs.lists.linux-aspeed,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <e5822b82e99f8c2f81deecfeaea2192b223dfbec.camel@codeconstruct.com.au> |
On Fri, 2026-06-12 at 11:21 +0200, Grégoire Layet wrote:
> Hello Andrew,
>
> Anirudh Srinivasan and I have found that IPMI over KCS using the
> PCI worked with the stock ASPEED bmc driver (the bmc driver in the V1)
> but not with the trimmed-down version in the V2. I have apparently removed
> a bit too much from the V2 , but that's not what I want to focus on.
Sure.
>
> This brings back the question of where we should put the registers
> configuration,
> considering that two different functionalities depend on it.
>
> > It is also possible to put the SCU initialisation on the
> > 8250_aspeed_vuart driver
> > directly. This could be activated with a specific flag added to VUART nodes
> > ('pcie2vuart' for example) on the DeviceTree.
The concept sounds reasonable to me. There's probably some bikeshedding
to do on the devicetree property though.
>
> Similarly to this idea, we could include have the necessary configuration in the
> 'kcs_bmc_aspeed' driver. This could be activated using a similar flag
> ,such as 'pci2lpc'
> or 'pci2kcs' directly. However, this would result in a lot of code
> duplication for most
> of the configuration.
Can you outline the duplication you're concerned about? I think it's a
matter of resolving the SCU syscon to its regmap, then performing the
necessary accesses?
>
> The issue for me is that, two drivers configuring the same registers
> is not a good idea.
I think it's not as bad as you make it out to be. The SCU's regmap
protects updates to individual registers under a lock, so concurrent
modification isn't a concern. The hardware design choices make all of
this slightly awkward for any related software design. As an
alternative you could implement a mini subsystem that relevant drivers
could call through to set the bits, but I currently think that's
unnecessary work.
Andrew