[PATCH RFC 0/2] ipmi: kcs_bmc_aspeed: Support multiple LPC controller instances

[PATCH RFC 0/2] ipmi: kcs_bmc_aspeed: Support multiple LPC controller instances

[Yu-Che Hsieh]

Hi,

Aspeed AST2700 SoC exposes more than one physical LPC controller
(e.g. lpc0@14c31000, lpc1@14c32000, plus pcie_lpc0@12c19000 and 
pcie_lpc1@12c19800 for the PCIe-facing path), each instantiating 
its own KCS1-KCS4 with identical IDR/ODR/STR register offsets:

	lpc0: lpc@14c31000 {
		compatible = "aspeed,ast2700-lpc", "simple-mfd", "syscon";
		lpc0_kcs1: lpc-kcs@24 {
			compatible = "aspeed,ast2600-kcs-bmc";
			...
		};
		...
	};

	lpc1: lpc@14c32000 {
		compatible = "aspeed,ast2700-lpc", "simple-mfd", "syscon";
		lpc1_kcs1: kcs@24 {
			compatible = "aspeed,ast2600-kcs-bmc";
			...
		};
		...
	};

kcs_bmc_device::channel currently serves two purposes at once:

  1. Selecting which HICR0/HICR2/HICRB bit-group to touch within a single
     LPC controller's register file (aspeed_kcs_of_get_channel() derives
     this purely from the KCS node's register offset, 1..4).

  2. Naming the misc chardev (/dev/ipmi-kcsN) exposed to userspace, which
     must be unique system-wide (kcs_bmc_cdev_ipmi.c uses "ipmi-kcs%u").

Both happen to be the same value only because this driver has only ever
had to support SoCs with a single LPC controller. On AST2700, KCS1 on
every LPC controller computes to the same channel number, and whichever
instance probes second fails outright.

This series keeps a driver-private `channel` (1..4) in
struct aspeed_kcs_bmc for register access, and computes a
globally-unique kcs_bmc_device::channel as `bank * KCS_CHANNEL_MAX +
channel`, where `bank` identifies which LPC controller instance a KCS
device belongs to.

Before settling on how `bank` is derived, I considered a few options
with different tradeoffs and no clearly "correct" answer from existing
driver conventions:

  (a) Dynamic discovery: keep a driver-global list (protected by a
      mutex), keyed by the LPC controller's device_node pointer, and
      assign bank indices in first-seen-during-probe order. No DT
      changes required, but bank numbering depends on probe order,
      which is not deterministic across boots/reprobes.

  (b) Tree walk by compatible: for_each_compatible_node() over the
      whole DT matching the LPC node's own compatible string, counting
      position. Deterministic and requires no DT changes, but AST2700's
      four LPC-compatible instances (lpc0/lpc1/pcie_lpc0/pcie_lpc1)
      don't share a common parent node and don't share a consistent
      child-node naming scheme, which also rules out parent-based or
      node-name-based tree walks I looked at.

  (c) of_alias_get_id(): require board DTs to declare
      /aliases { lpc0 = &lpc0; lpc1 = &lpc1; ... }, and read the index
      back from there. This is the pattern already used by mmc/i2c/spi/
      serial for "which instance is this" numbering, including the same
      "absent alias -> fall back to previous behaviour" compatibility
      guarantee for existing DTBs.

This series implements (c), since it puts the numbering under DT-author
control rather than driver inference, generalizes across however many
LPC-compatible instances a future SoC exposes (regardless of tree
placement), and existing single-LPC-controller boards need no DT
changes at all - of_alias_get_id() returns an error when no alias is
present and the driver falls back to bank 0, which is exactly today's
behaviour.

Patches:

  Patch 1: dt-bindings: mfd: aspeed-lpc: Document lpcN alias for
           multi-instance SoCs
  Patch 2: ipmi: kcs_bmc_aspeed: Support multiple LPC controller
           instances

Before dropping the RFC tag and posting the AST2700 follow-up patches, 
I'd particularly appreciate review feedback on the bank-numbering approach. 
In particular, does option (c) seem like an acceptable use of DT aliases 
for identifying LPC controller instances, or would the community prefer a 
different approach? 

Any guidance would be very helpful.

Signed-off-by: Yu-Che Hsieh <yc_hsieh@aspeedtech.com>
---
Yu-Che Hsieh (2):
      dt-bindings: mfd: aspeed-lpc: Document lpcN alias for multi-instance SoCs
      ipmi: kcs_bmc_aspeed: Support multiple LPC controller instances

 .../devicetree/bindings/mfd/aspeed-lpc.yaml        |  7 ++++
 drivers/char/ipmi/kcs_bmc_aspeed.c                 | 37 ++++++++++++++++------
 2 files changed, 34 insertions(+), 10 deletions(-)
---
base-commit: f0e6f20cb52b14c2c441f04e21cef0c95d498cac
change-id: 20260722-upstream_kcs_multiple_lpc-735942211508

Best regards,
-- 
Yu-Che Hsieh <yc_hsieh@aspeedtech.com>

[PATCH RFC 1/2] dt-bindings: mfd: aspeed-lpc: Document lpcN alias for multi-instance SoCs

[Yu-Che Hsieh]

Some SoCs (e.g. AST2700) expose more than one physical LPC controller
instance. Downstream devices such as KCS channels need a stable,
globally-unique index derived from which LPC controller instance they
belong to, so document the "lpcN" alias convention on the LPC
controller node that lets DT authors assign a fixed index per
instance, mirroring the existing alias-based numbering convention used
by mmc/i2c/spi/serial.

Signed-off-by: Yu-Che Hsieh <yc_hsieh@aspeedtech.com>
---
 Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml b/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
index cbc3a2485a2f..81c12e0fb312 100644
--- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
+++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
@@ -35,6 +35,13 @@ description:
   configuration, therefore the host portion of the controller is exposed as a
   syscon as a means to arbitrate access.
 
+  On SoCs that expose more than one LPC controller instance, it is possible to
+  assign a fixed index to a given instance (and thus to downstream devices
+  such as KCS channels that must be uniquely identified system-wide) by
+  defining an "lpcN" alias for it in the /aliases device tree node. If no such
+  alias is present, the SoC is treated as having a single LPC controller
+  instance.
+
 properties:
   compatible:
     items:

-- 
2.34.1

Re: [PATCH RFC 1/2] dt-bindings: mfd: aspeed-lpc: Document lpcN alias for multi-instance SoCs

[Lee Jones]

On Thu, 23 Jul 2026, Yu-Che Hsieh wrote:

> Some SoCs (e.g. AST2700) expose more than one physical LPC controller
> instance. Downstream devices such as KCS channels need a stable,
> globally-unique index derived from which LPC controller instance they
> belong to, so document the "lpcN" alias convention on the LPC
> controller node that lets DT authors assign a fixed index per
> instance, mirroring the existing alias-based numbering convention used
> by mmc/i2c/spi/serial.
> 
> Signed-off-by: Yu-Che Hsieh <yc_hsieh@aspeedtech.com>

Why is this RFC?  What's stopping me from applying this now?

> ---
>  Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml b/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
> index cbc3a2485a2f..81c12e0fb312 100644
> --- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
> +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
> @@ -35,6 +35,13 @@ description:
>    configuration, therefore the host portion of the controller is exposed as a
>    syscon as a means to arbitrate access.
>  
> +  On SoCs that expose more than one LPC controller instance, it is possible to
> +  assign a fixed index to a given instance (and thus to downstream devices
> +  such as KCS channels that must be uniquely identified system-wide) by
> +  defining an "lpcN" alias for it in the /aliases device tree node. If no such
> +  alias is present, the SoC is treated as having a single LPC controller
> +  instance.
> +
>  properties:
>    compatible:
>      items:
> 
> -- 
> 2.34.1
> 

-- 
Lee Jones

Re: [PATCH RFC 1/2] dt-bindings: mfd: aspeed-lpc: Document lpcN alias for multi-instance SoCs

[YC Hsieh]

On Thu, 23 Jul 2026, Yu-Che Hsieh wrote:

>> Some SoCs (e.g. AST2700) expose more than one physical LPC controller
>> instance. Downstream devices such as KCS channels need a stable,
>> globally-unique index derived from which LPC controller instance they
>> belong to, so document the "lpcN" alias convention on the LPC
>> controller node that lets DT authors assign a fixed index per
>> instance, mirroring the existing alias-based numbering convention used
>> by mmc/i2c/spi/serial.
>>
>> Signed-off-by: Yu-Che Hsieh <yc_hsieh@aspeedtech.com>

> Why is this RFC?  What's stopping me from applying this now?

Thanks, there is no blocker. 

I marked it as RFC to gather feedback on the alias-based 
numbering convention for LPC controllers before submitting 
it as a regular patch.

If the approach looks acceptable, I'm happy to resend it 
without the RFC tag.

>> ---
>>  Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml b/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
>> index cbc3a2485a2f..81c12e0fb312 100644
>> --- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
>> @@ -35,6 +35,13 @@ description:
>>    configuration, therefore the host portion of the controller is exposed as a
>>    syscon as a means to arbitrate access.
>> 
>> +  On SoCs that expose more than one LPC controller instance, it is possible to
>> +  assign a fixed index to a given instance (and thus to downstream devices
>> +  such as KCS channels that must be uniquely identified system-wide) by
>> +  defining an "lpcN" alias for it in the /aliases device tree node. If no such
> +  alias is present, the SoC is treated as having a single LPC controller
>> +  instance.
>> +
>>  properties:
>>    compatible:
>>      items:
>>
>> --
>> 2.34.1
>>
--

Yu-Che

[PATCH RFC 2/2] ipmi: kcs_bmc_aspeed: Support multiple LPC controller instances

[Yu-Che Hsieh]

Some SoCs (e.g. AST2700) expose more than one physical LPC controller,
each instantiating its own independent set of KCS0-KCS3 channels using
identical IDR/ODR/STR register offsets. kcs_bmc_device::channel is
currently overloaded to serve two distinct purposes: selecting which
HICR0/HICR2/HICRB bit-group to touch within a single LPC controller's
register file, and naming the misc chardev (/dev/ipmi-kcsN) exposed to
userspace, which must be unique system-wide. The two purposes happen
to share the same value only because the driver has so far only had
to support SoCs with a single LPC controller. On a multi-controller
SoC, KCS0 on every LPC controller computes to the same channel number,
and whichever instance probes second fails to register its chardev.

Split the two roles: keep a new driver-private `channel` field (1..4)
in struct aspeed_kcs_bmc for register access, leaving all the
HICR/IBFIE/LPCxE switch statements operating on it, and derive
kcs_bmc_device::channel as `bank * KCS_CHANNEL_MAX + channel`, where
`bank` identifies which LPC controller instance a KCS device belongs
to.

`bank` is obtained from an optional "lpcN" alias on the LPC controller
node via of_alias_get_id(), following the same convention already used
by mmc/i2c/spi/serial for instance numbering. SoCs with a single LPC
controller, and existing DTs that don't declare the alias, fall back
to bank 0, so /dev/ipmi-kcsN naming is unchanged for all existing
boards.

Signed-off-by: Yu-Che Hsieh <yc_hsieh@aspeedtech.com>
---
 drivers/char/ipmi/kcs_bmc_aspeed.c | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
index a13a3470c17a..591c11a44503 100644
--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
@@ -125,6 +125,8 @@ struct aspeed_kcs_bmc {
 		bool remove;
 		struct timer_list timer;
 	} obe;
+
+	u32 channel;
 };
 
 static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc_device *kcs_bmc)
@@ -167,7 +169,7 @@ static void aspeed_kcs_outb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 data)
 	if (priv->upstream_irq.mode != aspeed_kcs_irq_serirq)
 		return;
 
-	switch (kcs_bmc->channel) {
+	switch (priv->channel) {
 	case 1:
 		switch (priv->upstream_irq.id) {
 		case 12:
@@ -232,7 +234,7 @@ static int aspeed_kcs_set_address(struct kcs_bmc_device *kcs_bmc, u32 addrs[2],
 	if (WARN_ON(nr_addrs < 1 || nr_addrs > 2))
 		return -EINVAL;
 
-	switch (priv->kcs_bmc.channel) {
+	switch (priv->channel) {
 	case 1:
 		regmap_update_bits(priv->map, LPC_HICR4, LPC_HICR4_LADR12AS, 0);
 		regmap_write(priv->map, LPC_LADR12H, addrs[0] >> 8);
@@ -315,7 +317,7 @@ static int aspeed_kcs_config_upstream_irq(struct aspeed_kcs_bmc *priv, u32 id, u
 	priv->upstream_irq.mode = aspeed_kcs_irq_serirq;
 	priv->upstream_irq.id = id;
 
-	switch (priv->kcs_bmc.channel) {
+	switch (priv->channel) {
 	case 1:
 		/* Needs IRQxE1 rather than (ID1IRQX, SEL1IRQX, IRQXE1) before AST2600 A3 */
 		break;
@@ -347,7 +349,7 @@ static int aspeed_kcs_config_upstream_irq(struct aspeed_kcs_bmc *priv, u32 id, u
 	default:
 		dev_warn(priv->kcs_bmc.dev,
 			 "SerIRQ configuration not supported on KCS channel %d\n",
-			 priv->kcs_bmc.channel);
+			 priv->channel);
 		return -EINVAL;
 	}
 
@@ -358,7 +360,7 @@ static void aspeed_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enabl
 {
 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
 
-	switch (kcs_bmc->channel) {
+	switch (priv->channel) {
 	case 1:
 		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC1E, enable * LPC_HICR0_LPC1E);
 		return;
@@ -374,7 +376,7 @@ static void aspeed_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enabl
 		regmap_update_bits(priv->map, LPC_HICRB, LPC_HICRB_LPC4E, enable * LPC_HICRB_LPC4E);
 		return;
 	default:
-		pr_warn("%s: Unsupported channel: %d", __func__, kcs_bmc->channel);
+		pr_warn("%s: Unsupported channel: %d", __func__, priv->channel);
 		return;
 	}
 }
@@ -435,7 +437,7 @@ static void aspeed_kcs_irq_mask_update(struct kcs_bmc_device *kcs_bmc, u8 mask,
 	if (mask & KCS_BMC_EVENT_TYPE_IBF) {
 		const bool enable = !!(state & KCS_BMC_EVENT_TYPE_IBF);
 
-		switch (kcs_bmc->channel) {
+		switch (priv->channel) {
 		case 1:
 			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIE1,
 					   enable * LPC_HICR2_IBFIE1);
@@ -453,7 +455,7 @@ static void aspeed_kcs_irq_mask_update(struct kcs_bmc_device *kcs_bmc, u8 mask,
 					   enable * LPC_HICRB_IBFIE4);
 			return;
 		default:
-			pr_warn("%s: Unsupported channel: %d", __func__, kcs_bmc->channel);
+			pr_warn("%s: Unsupported channel: %d", __func__, priv->channel);
 			return;
 		}
 	}
@@ -526,6 +528,17 @@ static int aspeed_kcs_of_get_channel(struct platform_device *pdev)
 	return -EINVAL;
 }
 
+static int aspeed_kcs_of_get_bank(struct device_node *lpc_np)
+{
+	int id;
+
+	id = of_alias_get_id(lpc_np, "lpc");
+	if (id < 0)
+		return 0;
+
+	return id;
+}
+
 static int
 aspeed_kcs_of_get_io_address(struct platform_device *pdev, u32 addrs[2])
 {
@@ -559,7 +572,7 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
 	struct device_node *np;
 	bool have_upstream_irq;
 	u32 upstream_irq[2];
-	int rc, channel;
+	int rc, channel, bank;
 	int nr_addrs;
 	u32 addrs[2];
 
@@ -575,6 +588,8 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
 	if (channel < 0)
 		return channel;
 
+	bank = aspeed_kcs_of_get_bank(np);
+
 	nr_addrs = aspeed_kcs_of_get_io_address(pdev, addrs);
 	if (nr_addrs < 0)
 		return nr_addrs;
@@ -590,9 +605,11 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	priv->channel = channel;
+
 	kcs_bmc = &priv->kcs_bmc;
 	kcs_bmc->dev = &pdev->dev;
-	kcs_bmc->channel = channel;
+	kcs_bmc->channel = bank * KCS_CHANNEL_MAX + channel;
 	kcs_bmc->ioreg = ast_kcs_bmc_ioregs[channel - 1];
 	kcs_bmc->ops = &aspeed_kcs_ops;
 

-- 
2.34.1