Re: [PATCH v2 1/2] soc: aspeed: add BMC-side PCIe BMC device driver

[Andrew Jeffery <andrew@codeconstruct.com.au>]

Hi Grégoire,

On Wed, 2026-06-17 at 08:40 +0200, Grégoire Layet wrote:
> Hello Andrew,
> 
> > The concept sounds reasonable to me. There's probably some bikeshedding
> > to do on the devicetree property though.
> 
> Yes, having looked at how it's done, I would say :
> 'aspeed,vuart-over-pci' and 'aspeed,kcs-over-pci' flags would be
> better.
> 
> > 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?
> 
> Both drivers will need to set :
> - Enable PCI BMC Device MMIO
> - Enable PCI BMC Device IRQ
> - Enable PCI BMC Device MSI rooting over PCI Device 1 (BAR1)
> - Enable Host 2 BMC MSI interrupts
> - PCI device class to 0xff000000 to be identified as a MFD device. The
> reset default is 0x0C070100 which is an IPMI KCS device, but that
> causes issues as it is detected by ipmi_si but can't be loaded because
> of non default KCS address.
> 
> Sorry for my errors, there is not that much. But both drivers will do
> almost the same initialisation. That was my code duplication concern.

I think it's valid to be concerned, but perhaps not for the reason of
code duplication. If there are multiple consumers then we need to
ensure consistency of configuration and correctness wrt to enabling /
disabling the capability based on the number of consumers.

> 
> > 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.
> 
> You are right it's not as bad as I thought.
> For now, I will focus on the VUART until the solution has been
> validated. Then I will easily do the same for the KCS over PCI.

I think it's a good step to at least solve one thing at a time, so long
as we're not precluding making those future steps.

> 
> So I'll do for the V3 of the BMC side driver:
> - modify the device tree binding to have 'aspeed,ast2600-vuart' and
> add the 'aspeed,vuart-over-pci' boolean flag, only for the ast2600.

Just to confirm, you're proposing modifying the 8250 binding?

> - modify the '8250_aspeed_vuart' driver to add 'aspeed,ast2600-vuart' support.
> - add vuart over pci enable and disable code to the '8250_aspeed_vuart' driver.
> 

Sounds like a reasonable start to me.

Andrew