* Re: [PATCH v3 2/7] dt-bindings: serial: 8250: aspeed: add aspeed,vuart-over-pci bool prop
From: Krzysztof Kozlowski @ 2026-06-24 7:17 UTC (permalink / raw)
To: Grégoire Layet
Cc: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt, andrew, jacky_chou, yh_chung, ninad,
anirudhsriniv, linux-serial, linux-aspeed, linux-arm-kernel,
linux-kernel
In-Reply-To: <73b2bd81ce70814612e6d3cb689c3296de742aaf.1782224059.git.gregoire.layet@9elements.com>
On Tue, Jun 23, 2026 at 02:25:40PM +0000, Grégoire Layet wrote:
> The ASPEED AST2600 has 2 VUART accessible over PCI.
What does that mean? How UART can be accessible over PCI bus?
> This boolean can be set to specify if the VUART is used over PCI.
>
> Signed-off-by: Grégoire Layet <gregoire.layet@9elements.com>
> ---
> .../devicetree/bindings/serial/8250.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
> index 3cbd0f532e15..b03797f4674d 100644
> --- a/Documentation/devicetree/bindings/serial/8250.yaml
> +++ b/Documentation/devicetree/bindings/serial/8250.yaml
> @@ -26,6 +26,14 @@ allOf:
> anyOf:
> - const: aspeed,ast2500-vuart
> - const: aspeed,ast2600-vuart
> + - if:
> + anyOf:
> + - required:
> + - aspeed,vuart-over-pci
> + then:
> + properties:
> + compatible:
> + const: aspeed,ast2600-vuart
> - if:
> properties:
> compatible:
> @@ -312,6 +320,13 @@ properties:
> polarity (IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_LEVEL_HIGH). Only
> applicable to aspeed,ast2500-vuart and aspeed,ast2600-vuart.
>
> + aspeed,vuart-over-pci:
> + type: boolean
> + default: false
There is no such syntax. Please do not introduce own style. Instead,
look at other files how this is done.
> + description: |
Do not need '|' unless you need to preserve formatting.
> + Enable the VUART over the BMC PCI device. Only applicable to
> + aspeed,ast2600-vuart.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/7] dt-bindings: serial: 8250: aspeed: add compatible string for ast2600
From: Krzysztof Kozlowski @ 2026-06-24 7:15 UTC (permalink / raw)
To: Grégoire Layet
Cc: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt, andrew, jacky_chou, yh_chung, ninad,
anirudhsriniv, linux-serial, linux-aspeed, linux-arm-kernel,
linux-kernel
In-Reply-To: <80d983887dfdfc7e70a6db95f8cb95b7312f3044.1782224059.git.gregoire.layet@9elements.com>
On Tue, Jun 23, 2026 at 02:25:39PM +0000, Grégoire Layet wrote:
> The ast2600 was using the ast2500 vuart compatible string.
> This change makes it possible to have ast2600-specific properties.
>
> Signed-off-by: Grégoire Layet <gregoire.layet@9elements.com>
Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets. See also:
https://elixir.bootlin.com/linux/v6.16-rc2/source/Documentation/process/submitting-patches.rst#L830
> ---
> .../devicetree/bindings/serial/8250.yaml | 20 +++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
> index bb7b9c87a807..3cbd0f532e15 100644
> --- a/Documentation/devicetree/bindings/serial/8250.yaml
> +++ b/Documentation/devicetree/bindings/serial/8250.yaml
> @@ -23,7 +23,9 @@ allOf:
> then:
> properties:
> compatible:
> - const: aspeed,ast2500-vuart
> + anyOf:
This should be oneOf (by convention and actually more accurate meaning).
> + - const: aspeed,ast2500-vuart
> + - const: aspeed,ast2600-vuart
> - if:
> properties:
> compatible:
> @@ -287,17 +289,19 @@ properties:
> aspeed,sirq-polarity-sense:
> $ref: /schemas/types.yaml#/definitions/phandle-array
> description: |
> - Phandle to aspeed,ast2500-scu compatible syscon alongside register
> - offset and bit number to identify how the SIRQ polarity should be
> - configured. One possible data source is the LPC/eSPI mode bit. Only
> - applicable to aspeed,ast2500-vuart.
> + Phandle to aspeed,ast2500-scu or aspeed,ast2600-scu compatible syscon
> + alongside register offset and bit number to identify how the SIRQ
> + polarity should be configured. One possible data source is the LPC/eSPI
> + mode bit. Only applicable to aspeed,ast2500-vuart and
> + aspeed,ast2600-vuart.
> deprecated: true
>
> aspeed,lpc-io-reg:
> $ref: /schemas/types.yaml#/definitions/uint32-array
> maxItems: 1
> description: |
> - The VUART LPC address. Only applicable to aspeed,ast2500-vuart.
> + The VUART LPC address. Only applicable to aspeed,ast2500-vuart and
> + aspeed,ast2600-vuart.
>
> aspeed,lpc-interrupts:
> $ref: /schemas/types.yaml#/definitions/uint32-array
> @@ -305,8 +309,8 @@ properties:
> maxItems: 2
> description: |
> A 2-cell property describing the VUART SIRQ number and SIRQ
> - polarity (IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_LEVEL_HIGH). Only
> - applicable to aspeed,ast2500-vuart.
> + polarity (IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_LEVEL_HIGH). Only
> + applicable to aspeed,ast2500-vuart and aspeed,ast2600-vuart.
>
More important, where is documenting of the actual compatible?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5] serial: 8250: fix use-after-free in IRQ chain handling
From: Jiri Slaby @ 2026-06-24 3:31 UTC (permalink / raw)
To: Qiliang Yuan, Greg Kroah-Hartman, Anton Vorontsov, Alan Cox
Cc: linux-kernel, linux-serial, Wang Zhaolong
In-Reply-To: <20260624-bug-221579-8250-shared-irq-race-v5-1-15d841f89e1e@gmail.com>
On 24. 06. 26, 3:21, Qiliang Yuan wrote:
> serial_unlink_irq_chain() holds hash_mutex and calls free_irq() + kfree(i)
> when it sees an empty port list. serial_link_irq_chain() released
> hash_mutex after serial_get_or_create_irq_info() but before acquiring
> i->lock. This gap allowed a concurrent unlink to observe list_empty()
> as true while a new port was still being added, free i, and trigger a
> use-after-free.
>
> Dropping hash_mutex before request_irq() completes also allows another
> port sharing the same IRQ to join the chain and run the shared-IRQ THRE
> test while IRQ startup is still in progress, which can also trigger the
> "Unbalanced enable for IRQ" warning (kernel/irq/manage.c:774) because
> irq_shutdown() in the premature free_irq() path increments desc->depth,
> breaking the disable_irq/enable_irq pairing in serial8250_THRE_test().
>
> Fix by pulling hash_mutex into serial_link_irq_chain() and holding it
> across the first request_irq() completion (including the error path)
> so that no concurrent unlink or second-port join can race with IRQ
> setup or cleanup.
> serial_unlink_irq_chain() already holds hash_mutex throughout, so the
> race window is closed.
>
> Fixes: 768aec0b5bcc ("serial: 8250: fix shared interrupts issues with SMP and RT kernels")
> Reported-by: Wang Zhaolong <wangzhaolong@fnnas.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
> Signed-off-by: Qiliang Yuan <realwujing@gmail.com>
> ---
> V4 -> V5:
> - Add __must_hold(&hash_mutex) annotation to
> serial_get_or_create_irq_info() for static analysis.
>
> V3 -> V4:
> - Move cleanup under hash_mutex on request_irq() failure to prevent a
> second port from joining the chain before the irq_info is cleaned up.
> - Fix inaccurate description of irq_shutdown() in commit message.
>
> V2 -> V3:
> - Hold hash_mutex across the first request_irq() completion to prevent a
> second port from joining the chain and running the shared-IRQ THRE test
> while IRQ startup is still in progress.
>
> V1 -> V2:
> - Add Reported-by tag from Wang Zhaolong.
>
> v4: https://lore.kernel.org/r/20260529-bug-221579-8250-shared-irq-race-v4-1-cfda63b4420f@gmail.com
> v3: https://lore.kernel.org/r/20260529-bug-221579-8250-shared-irq-race-v3-1-fe4d430862a9@gmail.com
> v2: https://lore.kernel.org/r/20260528-bug-221579-8250-shared-irq-race-v2-1-06531202e54d@gmail.com
> v1: https://lore.kernel.org/r/20260528-bug-221579-8250-shared-irq-race-v1-1-30980cca02f3@gmail.com
> ---
> drivers/tty/serial/8250/8250_core.c | 56 ++++++++++++++++++++++++++++---------
> 1 file changed, 43 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index a428e88938eb7..a86505c3dcb67 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -131,10 +131,11 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
> * - allocate a new one, add it to the hashtable and return it.
> */
> static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
> + __must_hold(&hash_mutex)
> {
> struct irq_info *i;
>
> - guard(mutex)(&hash_mutex);
> + lockdep_assert_held(&hash_mutex);
>
> hash_for_each_possible(irq_lists, i, node, up->port.irq)
> if (i->irq == up->port.irq)
> @@ -151,31 +152,60 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
> return i;
> }
>
> +/*
> + * serial_link_irq_chain() hooks the given 8250 port into the IRQ chain.
> + *
> + * hash_mutex must be held from the hash lookup through the first
> + * request_irq() completion. Dropping it earlier allows a concurrent
> + * serial_unlink_irq_chain() to race in after i->head is published but
> + * before the IRQ is fully set up — another port sharing the IRQ can then
> + * join the chain and run the shared-IRQ THRE test while IRQ startup is
> + * still in progress, triggering an "Unbalanced enable for IRQ" warning
> + * in kernel/irq/manage.c.
> + */
> static int serial_link_irq_chain(struct uart_8250_port *up)
> {
> struct irq_info *i;
> int ret;
>
> + mutex_lock(&hash_mutex);
> +
> i = serial_get_or_create_irq_info(up);
> - if (IS_ERR(i))
> + if (IS_ERR(i)) {
> + mutex_unlock(&hash_mutex);
> return PTR_ERR(i);
> + }
>
> - scoped_guard(spinlock_irq, &i->lock) {
> - if (i->head) {
> - list_add(&up->list, i->head);
> -
> - return 0;
> - }
> + /*
> + * Serialise against the list manipulation in the interrupt handler
> + * and in serial_unlink_irq_chain(). hash_mutex is still held which
> + * prevents serial_unlink_irq_chain() from entering and freeing the
> + * irq_info until the first request_irq() completes.
> + */
> + spin_lock_irq(&i->lock);
> + if (i->head) {
> + list_add(&up->list, i->head);
> + spin_unlock_irq(&i->lock);
> + mutex_unlock(&hash_mutex);
So what is the reason to switch from guards to manual locking?
>
> - INIT_LIST_HEAD(&up->list);
> - i->head = &up->list;
> + return 0;
> }
>
> - ret = request_irq(up->port.irq, serial8250_interrupt, up->port.irqflags, up->port.name, i);
> - if (ret < 0)
> + INIT_LIST_HEAD(&up->list);
> + i->head = &up->list;
> + spin_unlock_irq(&i->lock);
> +
> + ret = request_irq(up->port.irq, serial8250_interrupt,
> + up->port.irqflags, up->port.name, i);
> + if (ret < 0) {
> serial_do_unlink(i, up);
> + mutex_unlock(&hash_mutex);
> + return ret;
> + }
>
> - return ret;
> + mutex_unlock(&hash_mutex);
> +
> + return 0;
> }
>
> static void serial_unlink_irq_chain(struct uart_8250_port *up)
>
> ---
> base-commit: eb3f4b7426cfd2b79d65b7d37155480b32259a11
> change-id: 20260528-bug-221579-8250-shared-irq-race-581e4900a178
>
> Best regards,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH] serial: 8250: serialize shared IRQ startup
From: Wang Zhaolong @ 2026-06-24 2:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Ilpo Järvinen, Xin Zhao,
Andy Shevchenko, Kees Cook, Ingo Molnar, Bing Fan, Guanbing Huang,
linux-kernel, linux-serial
Cc: stable
In-Reply-To: <20260527092052.2086342-1-wangzhaolong@fnnas.com>
On Wed, May 27, 2026 at 05:20:51PM +0800, Wang Zhaolong wrote:
> Concurrent startup of two 8250 ports sharing the same IRQ can trigger an
> IRQ core warning:
>
> Unbalanced enable for IRQ 3
> WARNING: CPU: 0 PID: 580 at kernel/irq/manage.c:774 __enable_irq+0x3b/0x60
> Call Trace:
> enable_irq+0x8d/0x120
> serial8250_do_startup+0x80d/0xa80
> uart_port_startup+0x13d/0x440
> uart_port_activate+0x5b/0xb0
> tty_port_open+0xa1/0x120
> uart_open+0x1e/0x30
> tty_open+0x140/0x7a0
>
> The second port can then run the shared-IRQ startup test while the IRQ core
> is still enabling the line for the first port. The local
> disable_irq_nosync()/enable_irq() pair is balanced, but the interleaving can
> still unbalance the IRQ core disable depth.
>
> That makes the QEMU legacy serial ports enter the shared-IRQ THRE test path:
>
> serial8250_do_startup()
> if (port->irqflags & IRQF_SHARED)
> disable_irq_nosync(port->irq)
> ...
> if (port->irqflags & IRQF_SHARED)
> enable_irq(port->irq)
>
> One possible interleaving is:
>
> CPU0, ttyS1 CPU1, ttyS3
>
> serial_link_irq_chain()
> hash_add(i)
> i->head = &ttyS1
> request_irq()
> serial_link_irq_chain()
> find i in irq_lists
> list_add(&ttyS3, i->head)
> serial8250_do_startup()
> disable_irq_nosync(irq)
> irq_startup()
> desc->depth = 0
> enable_irq(irq)
> WARN: Unbalanced enable for IRQ 3
>
> Keep hash_mutex held in serial_link_irq_chain() until the first request_irq()
> has completed. This prevents another 8250 port sharing the IRQ from joining
> the chain and running the THRE test while the IRQ core is still starting the
> interrupt.
>
> This was reproduced in QEMU with ttyS1 and ttyS3 sharing IRQ 3. With this
> change, 100000 synchronized open/close iterations on /dev/ttyS1 and /dev/ttyS3
> completed without the warning.
>
> Fixes: 64c79dfbc458 ("serial: 8250_pnp: Support configurable reg shift property")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
> Cc: stable@vger.kernel.org # 6.10+
> Assisted-by: Codex:gpt-5
> Signed-off-by: Wang Zhaolong <wangzhaolong@fnnas.com>
> ---
> drivers/tty/serial/8250/8250_core.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index a428e88938eb..64eed4dc343f 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -132,12 +132,10 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
> */
> static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
> {
> struct irq_info *i;
>
> - guard(mutex)(&hash_mutex);
> -
> hash_for_each_possible(irq_lists, i, node, up->port.irq)
> if (i->irq == up->port.irq)
> return i;
>
> i = kzalloc_obj(*i);
> @@ -154,10 +152,12 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
> static int serial_link_irq_chain(struct uart_8250_port *up)
> {
> struct irq_info *i;
> int ret;
>
> + guard(mutex)(&hash_mutex);
> +
> i = serial_get_or_create_irq_info(up);
> if (IS_ERR(i))
> return PTR_ERR(i);
>
> scoped_guard(spinlock_irq, &i->lock) {
> @@ -169,10 +169,15 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
>
> INIT_LIST_HEAD(&up->list);
> i->head = &up->list;
> }
>
> + /*
> + * Keep the shared-IRQ chain locked until the first handler is installed.
> + * Otherwise another UART can join early and run startup IRQ masking while
> + * the IRQ core is still enabling the line, unbalancing the disable depth.
> + */
> ret = request_irq(up->port.irq, serial8250_interrupt, up->port.irqflags, up->port.name, i);
> if (ret < 0)
> serial_do_unlink(i, up);
>
> return ret;
> --
> 2.54.0
Hi Maintainers,
Friendly ping on this patch.
This is a clean and simple one-line relocation fix for the shared IRQ race condition.
I noticed there is another ongoing thread attempting to address the same bug with a
much more complex approach, but it seems to miss the regression test cases.
Could you please take a look at this simpler alternative when you have time? Any
feedback or reviews would be highly appreciated.
Thanks,
Wang
^ permalink raw reply
* [PATCH v5] serial: 8250: fix use-after-free in IRQ chain handling
From: Qiliang Yuan @ 2026-06-24 1:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Anton Vorontsov, Alan Cox
Cc: linux-kernel, linux-serial, Wang Zhaolong, Qiliang Yuan
serial_unlink_irq_chain() holds hash_mutex and calls free_irq() + kfree(i)
when it sees an empty port list. serial_link_irq_chain() released
hash_mutex after serial_get_or_create_irq_info() but before acquiring
i->lock. This gap allowed a concurrent unlink to observe list_empty()
as true while a new port was still being added, free i, and trigger a
use-after-free.
Dropping hash_mutex before request_irq() completes also allows another
port sharing the same IRQ to join the chain and run the shared-IRQ THRE
test while IRQ startup is still in progress, which can also trigger the
"Unbalanced enable for IRQ" warning (kernel/irq/manage.c:774) because
irq_shutdown() in the premature free_irq() path increments desc->depth,
breaking the disable_irq/enable_irq pairing in serial8250_THRE_test().
Fix by pulling hash_mutex into serial_link_irq_chain() and holding it
across the first request_irq() completion (including the error path)
so that no concurrent unlink or second-port join can race with IRQ
setup or cleanup.
serial_unlink_irq_chain() already holds hash_mutex throughout, so the
race window is closed.
Fixes: 768aec0b5bcc ("serial: 8250: fix shared interrupts issues with SMP and RT kernels")
Reported-by: Wang Zhaolong <wangzhaolong@fnnas.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
Signed-off-by: Qiliang Yuan <realwujing@gmail.com>
---
V4 -> V5:
- Add __must_hold(&hash_mutex) annotation to
serial_get_or_create_irq_info() for static analysis.
V3 -> V4:
- Move cleanup under hash_mutex on request_irq() failure to prevent a
second port from joining the chain before the irq_info is cleaned up.
- Fix inaccurate description of irq_shutdown() in commit message.
V2 -> V3:
- Hold hash_mutex across the first request_irq() completion to prevent a
second port from joining the chain and running the shared-IRQ THRE test
while IRQ startup is still in progress.
V1 -> V2:
- Add Reported-by tag from Wang Zhaolong.
v4: https://lore.kernel.org/r/20260529-bug-221579-8250-shared-irq-race-v4-1-cfda63b4420f@gmail.com
v3: https://lore.kernel.org/r/20260529-bug-221579-8250-shared-irq-race-v3-1-fe4d430862a9@gmail.com
v2: https://lore.kernel.org/r/20260528-bug-221579-8250-shared-irq-race-v2-1-06531202e54d@gmail.com
v1: https://lore.kernel.org/r/20260528-bug-221579-8250-shared-irq-race-v1-1-30980cca02f3@gmail.com
---
drivers/tty/serial/8250/8250_core.c | 56 ++++++++++++++++++++++++++++---------
1 file changed, 43 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb7..a86505c3dcb67 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -131,10 +131,11 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
* - allocate a new one, add it to the hashtable and return it.
*/
static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
+ __must_hold(&hash_mutex)
{
struct irq_info *i;
- guard(mutex)(&hash_mutex);
+ lockdep_assert_held(&hash_mutex);
hash_for_each_possible(irq_lists, i, node, up->port.irq)
if (i->irq == up->port.irq)
@@ -151,31 +152,60 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
return i;
}
+/*
+ * serial_link_irq_chain() hooks the given 8250 port into the IRQ chain.
+ *
+ * hash_mutex must be held from the hash lookup through the first
+ * request_irq() completion. Dropping it earlier allows a concurrent
+ * serial_unlink_irq_chain() to race in after i->head is published but
+ * before the IRQ is fully set up — another port sharing the IRQ can then
+ * join the chain and run the shared-IRQ THRE test while IRQ startup is
+ * still in progress, triggering an "Unbalanced enable for IRQ" warning
+ * in kernel/irq/manage.c.
+ */
static int serial_link_irq_chain(struct uart_8250_port *up)
{
struct irq_info *i;
int ret;
+ mutex_lock(&hash_mutex);
+
i = serial_get_or_create_irq_info(up);
- if (IS_ERR(i))
+ if (IS_ERR(i)) {
+ mutex_unlock(&hash_mutex);
return PTR_ERR(i);
+ }
- scoped_guard(spinlock_irq, &i->lock) {
- if (i->head) {
- list_add(&up->list, i->head);
-
- return 0;
- }
+ /*
+ * Serialise against the list manipulation in the interrupt handler
+ * and in serial_unlink_irq_chain(). hash_mutex is still held which
+ * prevents serial_unlink_irq_chain() from entering and freeing the
+ * irq_info until the first request_irq() completes.
+ */
+ spin_lock_irq(&i->lock);
+ if (i->head) {
+ list_add(&up->list, i->head);
+ spin_unlock_irq(&i->lock);
+ mutex_unlock(&hash_mutex);
- INIT_LIST_HEAD(&up->list);
- i->head = &up->list;
+ return 0;
}
- ret = request_irq(up->port.irq, serial8250_interrupt, up->port.irqflags, up->port.name, i);
- if (ret < 0)
+ INIT_LIST_HEAD(&up->list);
+ i->head = &up->list;
+ spin_unlock_irq(&i->lock);
+
+ ret = request_irq(up->port.irq, serial8250_interrupt,
+ up->port.irqflags, up->port.name, i);
+ if (ret < 0) {
serial_do_unlink(i, up);
+ mutex_unlock(&hash_mutex);
+ return ret;
+ }
- return ret;
+ mutex_unlock(&hash_mutex);
+
+ return 0;
}
static void serial_unlink_irq_chain(struct uart_8250_port *up)
---
base-commit: eb3f4b7426cfd2b79d65b7d37155480b32259a11
change-id: 20260528-bug-221579-8250-shared-irq-race-581e4900a178
Best regards,
--
Jing Wu <realwujing@gmail.com>
^ permalink raw reply related
* Re: [PATCH v2 2/2] dt-bindings: Drop incorrect usage of double '::'
From: Andi Shyti @ 2026-06-23 20:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-clk, dri-devel, freedreno, linux-i2c,
linux-pm, linux-leds, linux-media, linux-mmc, linux-phy,
linux-gpio, linux-renesas-soc, linux-serial, linux-sound,
linux-usb
In-Reply-To: <20260623054842.21831-4-krzysztof.kozlowski@oss.qualcomm.com>
Hi Krzysztof,
On Tue, Jun 23, 2026 at 07:48:44AM +0200, Krzysztof Kozlowski wrote:
> There is no use of double colon '::' in YAML. OTOH, the literal style
> block, e.g. using '|' treats all characters as content [1] therefore
> single use of ':' in descriptions is perfectly fine, whenever '|' is
> used.
>
> Cleanup existing code, so the confusing style won't be re-used in new
> contributions.
>
> Link: https://yaml.org/spec/1.2.2/#literal-style [1]
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Acked-by: Alim Akhtar <alim.akhtar@samsung.com>
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> # renesas
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Acked-by: Andi Shyti <andi.shyti@kernel.org>
Thanks,
Andi
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: clock: Drop incorrect usage of double '::'
From: Andi Shyti @ 2026-06-23 20:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-clk, dri-devel, freedreno, linux-i2c,
linux-pm, linux-leds, linux-media, linux-mmc, linux-phy,
linux-gpio, linux-renesas-soc, linux-serial, linux-sound,
linux-usb
In-Reply-To: <20260623054842.21831-3-krzysztof.kozlowski@oss.qualcomm.com>
Hi Krzysztof,
On Tue, Jun 23, 2026 at 07:48:43AM +0200, Krzysztof Kozlowski wrote:
> There is no use of double colon '::' in YAML. OTOH, the literal style
> block, e.g. using '|' treats all characters as content [1] therefore
> single use of ':' in descriptions is perfectly fine, whenever '|' is
> used.
>
> Cleanup existing code, so the confusing style won't be re-used in new
> contributions.
>
> Link: https://yaml.org/spec/1.2.2/#literal-style [1]
> Acked-by: Alim Akhtar <alim.akhtar@samsung.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Acked-by: Andi Shyti <andi.shyti@kernel.org>
Thanks,
Andi
^ permalink raw reply
* Re: [PATCH 2/2] serial: sc16is7xx: set TX FIFO trigger level to half FIFO to prevent underruns
From: David Laight @ 2026-06-23 18:42 UTC (permalink / raw)
To: Paul Mbewe
Cc: linux-serial, linux-kernel, gregkh, jirislaby, hvilleneuve,
stable, tobias.gannert, joachim.knorr
In-Reply-To: <20260623171328.153735-1-paultyson.mbewe@ziehl-abegg.de>
On Tue, 23 Jun 2026 19:13:28 +0200
Paul Mbewe <paultyson.mbewe@ziehl-abegg.de> wrote:
> Hi David,
>
> On the test system, the relevant threads already run with RT priority:
>
> irq/134-spi2.0 SCHED_FIFO priority 50
> sc16is7xx SCHED_FIFO priority 50
>
> So this is not caused by the IRQ thread running as a normal SCHED_OTHER
> task. That does not remove all latency sources, of course; on this
> single-core SPI system the threaded IRQ can still be affected by other
> RT/kernel activity, IRQ/preemption-disabled sections, SPI transfer time,
> or lock contention.
>
> I agree, changing the TX trigger from 8 to 32 free spaces reduces the
> per-interrupt time-to-empty margin. With an 8-space trigger the FIFO
> still contains 56 bytes when THRI asserts, while with a 32-space trigger
> it contains 32 bytes. So the v1 commit message is misleading when it
> describes this as increasing the refill window.
>
> The observed effect is instead that the 8-space trigger causes many
> small TX refill events. Each event has roughly the same cost, as you
> said. If the handler runs in time, it can catch up by seeing more than
> 8 free spaces and writing more data. The failure happens when one event
> is delayed long enough for the FIFO to drain.
>
> Using a 32-space trigger reduces the number of refill events and the
> associated IRQ/SPI load. It also reduces the chance that one delayed
> event lets the FIFO drain completely. On the tested setup this reduced
> irq/134-spi2.0 CPU usage from about 15-17% to about 5%, sys CPU from
> about 51-61% to about 19-28%, and load average from about 2.0-2.2 to
> about 0.65-1.3. With that change, the observed TX gaps disappeared.
Even with those figures (and the cpu % differences seem reasonable)
I still don't see why it stops the underruns.
An interesting exercise would be to call to ftrace_stop() when the
tx fifo is unexpectedly nearly empty.
Then enable all the ftrace scheduler and interrupt events (they don't
normally slow things down that much) and run the test with ftrace enabled.
When the trace gets stopped (you can just 'cat tracing_on' to find out
when it has been stopped) just 'less trace' to see what happened.
I think that will show that the isr thread is scheduled but doesn't
run for a while because something else is 'hogging' the cpu.
I've used this set of events - seemed to be useful.
Clear old events
echo >set_event
System call entry and exit:
echo 'syscalls:*' >>set_event
IRQ entry and exit:
echo irq:irq_handler_entry irq:irq_handler_exit >>set_event
echo irq:softirq_entry irq:softirq_exit >>set_event
Scheduler process switch events:
echo sched:sched_switch >>set_event
echo sched:sched_wakeup sched:sched_wakeup_new >>set_event
echo sched:sched_migrate_task >>set_event
David
>
> So I agree the commit message should be reworked to describe this as
> reducing TX refill events and IRQ/SPI load, not as increasing the
> per-interrupt latency margin.
>
> If changing the default trigger globally is considered too broad, I can
> also look at making the TX trigger configurable or limiting the change to
> SPI-backed devices.
>
> Thanks
> Paul
>
> _______________________________________
>
> ZIEHL-ABEGG
>
> Executive Board: Joachim Ley (Chairman), Marco Altherr, Wolfgang Mayer
> Supervisory Board: Dennis Ziehl (Chairman)
>
> Court of Registry: District Court Stuttgart HRB 746188
> Company Seat: Künzelsau, Germany
>
> Der Inhalt dieser E-Mail und/oder jegliche Anhänge können vertrauliche Mitteilungen enthalten und sind ausschließlich für den bezeichneten Adressaten bestimmt.
> Wenn Sie nicht der vorgesehene Adressat dieser E-Mail oder dessen Vertreter sein sollten, so beachten Sie bitte, dass jede Form der Kenntnisnahme, Veröffentlichung,
> Vervielfältigung oder Weitergabe des Inhalts dieser E-Mail einschließlich Anhängen unzulässig ist.
> Wir bitten Sie, sich in diesem Fall mit dem Absender der E-Mail in Verbindung zu setzen.
>
> The content of this e-mail and/or attachments may contain confidential information and is intended solely for the named recipient.
> If you are not the intended recipient of this e-mail or on its distribution list, please note that any type of disclosure, publication,
> copying or distribution of the content of this e-mail including attachments is strictly forbidden.
> In this case, we would kindly ask you to notify the sender of the e-mail.
^ permalink raw reply
* Re: [PATCH 2/2] serial: sc16is7xx: set TX FIFO trigger level to half FIFO to prevent underruns
From: Paul Mbewe @ 2026-06-23 17:13 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-serial, linux-kernel, gregkh, jirislaby, hvilleneuve,
stable, tobias.gannert, joachim.knorr, Paul Mbewe
In-Reply-To: <20260623160759.506f456e@pumpkin>
Hi David,
On the test system, the relevant threads already run with RT priority:
irq/134-spi2.0 SCHED_FIFO priority 50
sc16is7xx SCHED_FIFO priority 50
So this is not caused by the IRQ thread running as a normal SCHED_OTHER
task. That does not remove all latency sources, of course; on this
single-core SPI system the threaded IRQ can still be affected by other
RT/kernel activity, IRQ/preemption-disabled sections, SPI transfer time,
or lock contention.
I agree, changing the TX trigger from 8 to 32 free spaces reduces the
per-interrupt time-to-empty margin. With an 8-space trigger the FIFO
still contains 56 bytes when THRI asserts, while with a 32-space trigger
it contains 32 bytes. So the v1 commit message is misleading when it
describes this as increasing the refill window.
The observed effect is instead that the 8-space trigger causes many
small TX refill events. Each event has roughly the same cost, as you
said. If the handler runs in time, it can catch up by seeing more than
8 free spaces and writing more data. The failure happens when one event
is delayed long enough for the FIFO to drain.
Using a 32-space trigger reduces the number of refill events and the
associated IRQ/SPI load. It also reduces the chance that one delayed
event lets the FIFO drain completely. On the tested setup this reduced
irq/134-spi2.0 CPU usage from about 15-17% to about 5%, sys CPU from
about 51-61% to about 19-28%, and load average from about 2.0-2.2 to
about 0.65-1.3. With that change, the observed TX gaps disappeared.
So I agree the commit message should be reworked to describe this as
reducing TX refill events and IRQ/SPI load, not as increasing the
per-interrupt latency margin.
If changing the default trigger globally is considered too broad, I can
also look at making the TX trigger configurable or limiting the change to
SPI-backed devices.
Thanks
Paul
^ permalink raw reply
* Re: [PATCH] tty: serial: pch_uart: add check for pci_get_slot()
From: Andy Shevchenko @ 2026-06-23 16:03 UTC (permalink / raw)
To: Haoxiang Li
Cc: gregkh, jirislaby, fourier.thomas, 2426767509, kees, linux-kernel,
linux-serial, stable
In-Reply-To: <20260623140539.2272473-1-haoxiang_li2024@163.com>
On Tue, Jun 23, 2026 at 10:05:39PM +0800, Haoxiang Li wrote:
> Add check for pci_get_slot() to prevent a potetial
> null pointer dereference in pch_request_dma().
Not that it's quite possible, but probably you found this due to AI review
which might have considered some cases of manual binding a PCI device to any
PCI driver.
Given that is the case, I'm fine with the change.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 2/2] serial: sc16is7xx: set TX FIFO trigger level to half FIFO to prevent underruns
From: David Laight @ 2026-06-23 15:07 UTC (permalink / raw)
To: Paul Mbewe
Cc: linux-serial, linux-kernel, gregkh, jirislaby, hvilleneuve,
stable, tobias.gannert, joachim.knorr
In-Reply-To: <20260623140155.13258-1-paultyson.mbewe@ziehl-abegg.de>
On Tue, 23 Jun 2026 16:01:55 +0200
Paul Mbewe <paultyson.mbewe@ziehl-abegg.de> wrote:
> Hi David,
>
> Thanks for the detailed review.
>
> According to the SC16IS7xx datasheet, the TX trigger level is defined
> in terms of free FIFO spaces, not remaining data. So with the default
> configuration (FCR[5:4] = 00), the THRI interrupt fires when the FIFO
> has 8 free entries, i.e. when it still contains 56 bytes.
>
> While this in theory leaves enough data in the FIFO, in practice the
> system has to service many small refill cycles (~8 bytes per interrupt).
> On slow SPI hosts, each cycle involves threaded interrupt handling and
> multiple SPI transactions, and the cumulative latency plus scheduling
> jitter can exceed the available margin between refills under load.
But that cost/time is much the same regardless of the trigger level.
Changing the level from 8 to 32 significantly reduces the allowable
latency.
> Increasing the trigger level to 32 free spaces reduces the number of
> refill cycles significantly (from ~8 per FIFO load to ~2), and increases
> the amount of data written per cycle. This reduces scheduling pressure
> and, in practice, avoids the FIFO draining to empty between bursts.
But shouldn't it should all catch up.
The isr thread should start finding more than 8 bytes space in the fifo,
write enough bytes to fill it and the next interrupt should happen about
8 byte times after the previous one finishes.
That does rely on nothing 'going wrong' between the hardware interrupt
and the isr thread.
What priority does the isr thread run at?
If it isn't running at an RT priority then the scheduler might decide
to reduce its priority which could easily generate what you are seeing.
I'll agree that changing the threshold reduces system load - so should
give extra time for 'other work'.
But I don't really see why it be the correct way to stop underruns.
David
>
> The current commit message focuses too much on the "refill window" and
> does not explain this aspect clearly. I can rephrase it to better
> describe the interaction between trigger level, refill granularity and
> system latency.
>
> Thanks,
> Paul
>
> _______________________________________
>
> ZIEHL-ABEGG
>
> Executive Board: Joachim Ley (Chairman), Marco Altherr, Wolfgang Mayer
> Supervisory Board: Dennis Ziehl (Chairman)
>
> Court of Registry: District Court Stuttgart HRB 746188
> Company Seat: Künzelsau, Germany
>
> Der Inhalt dieser E-Mail und/oder jegliche Anhänge können vertrauliche Mitteilungen enthalten und sind ausschließlich für den bezeichneten Adressaten bestimmt.
> Wenn Sie nicht der vorgesehene Adressat dieser E-Mail oder dessen Vertreter sein sollten, so beachten Sie bitte, dass jede Form der Kenntnisnahme, Veröffentlichung,
> Vervielfältigung oder Weitergabe des Inhalts dieser E-Mail einschließlich Anhängen unzulässig ist.
> Wir bitten Sie, sich in diesem Fall mit dem Absender der E-Mail in Verbindung zu setzen.
>
> The content of this e-mail and/or attachments may contain confidential information and is intended solely for the named recipient.
> If you are not the intended recipient of this e-mail or on its distribution list, please note that any type of disclosure, publication,
> copying or distribution of the content of this e-mail including attachments is strictly forbidden.
> In this case, we would kindly ask you to notify the sender of the e-mail.
^ permalink raw reply
* Re: [PATCH v4] serial: 8250: fix use-after-free in IRQ chain handling
From: Jing Wu @ 2026-06-23 14:32 UTC (permalink / raw)
To: gregkh
Cc: jirislaby, avorontsov, alan, linux-kernel, linux-serial,
wangzhaolong
In-Reply-To: <2026061213-blinker-portable-a198@gregkh>
From: Qiliang Yuan <realwujing@gmail.com>
On Fri, Jun 12, 2026 at 11:49:51AM +0200, Greg Kroah-Hartman wrote:
> What real systems causes this to happen? How are you triggering this
> warning to happen? How was this tested?
The original report is Bugzilla #221579 from Wang Zhaolong. The bug
triggers on systems with multiple 8250 serial ports sharing an IRQ
(e.g. NAS-like devices). It can be reproduced by probing/removing
8250 serial ports that share an IRQ. Wang confirmed off-list that v3
fixes the reproducer on his setup.
> Shouldn't the function be marked as requiring this lock to be held?
> Just putting in this lockdep_assert will not catch the static analysis
> tools :(
Agreed, will add __must_hold(&hash_mutex) in v5.
Thanks,
Qiliang
^ permalink raw reply
* [PATCH v3 7/7] ARM: dts: aspeed: g6: add aspeed,vuart-over-pci prop to vuart3 and 4
From: Grégoire Layet @ 2026-06-23 14:25 UTC (permalink / raw)
To: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt
Cc: andrew, jacky_chou, yh_chung, ninad, anirudhsriniv, linux-serial,
linux-aspeed, linux-arm-kernel, linux-kernel, Grégoire Layet
In-Reply-To: <cover.1782224059.git.gregoire.layet@9elements.com>
The VUART 3 and 4 are VUART over PCI.
This flag indicates this information.
Signed-off-by: Grégoire Layet <gregoire.layet@9elements.com>
---
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
index 7c02633f2bd6..2a19463b4c21 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
@@ -723,6 +723,7 @@ vuart3: serial@1e787800 {
interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&syscon ASPEED_CLK_APB2>;
no-loopback-test;
+ aspeed,vuart-over-pci;
status = "disabled";
};
@@ -743,6 +744,7 @@ vuart4: serial@1e788800 {
interrupts = <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&syscon ASPEED_CLK_APB2>;
no-loopback-test;
+ aspeed,vuart-over-pci;
status = "disabled";
};
--
2.54.0
^ permalink raw reply related
* [PATCH v3 6/7] ARM: dts: aspeed: g6: Change vuart compatible string for ast2600
From: Grégoire Layet @ 2026-06-23 14:25 UTC (permalink / raw)
To: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt
Cc: andrew, jacky_chou, yh_chung, ninad, anirudhsriniv, linux-serial,
linux-aspeed, linux-arm-kernel, linux-kernel, Grégoire Layet
In-Reply-To: <cover.1782224059.git.gregoire.layet@9elements.com>
Use the ast2600 compatible string.
This makes it more precise and enables specific ast2600 properties.
Still use the ast2500 compatible string as a fallback.
Signed-off-by: Grégoire Layet <gregoire.layet@9elements.com>
---
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
index 56bb3b0444f7..7c02633f2bd6 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
@@ -707,7 +707,7 @@ emmc: sdhci@1e750100 {
};
vuart1: serial@1e787000 {
- compatible = "aspeed,ast2500-vuart";
+ compatible = "aspeed,ast2600-vuart", "aspeed,ast2500-vuart";
reg = <0x1e787000 0x40>;
reg-shift = <2>;
interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
@@ -717,7 +717,7 @@ vuart1: serial@1e787000 {
};
vuart3: serial@1e787800 {
- compatible = "aspeed,ast2500-vuart";
+ compatible = "aspeed,ast2600-vuart", "aspeed,ast2500-vuart";
reg = <0x1e787800 0x40>;
reg-shift = <2>;
interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
@@ -727,7 +727,7 @@ vuart3: serial@1e787800 {
};
vuart2: serial@1e788000 {
- compatible = "aspeed,ast2500-vuart";
+ compatible = "aspeed,ast2600-vuart", "aspeed,ast2500-vuart";
reg = <0x1e788000 0x40>;
reg-shift = <2>;
interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
@@ -737,7 +737,7 @@ vuart2: serial@1e788000 {
};
vuart4: serial@1e788800 {
- compatible = "aspeed,ast2500-vuart";
+ compatible = "aspeed,ast2600-vuart", "aspeed,ast2500-vuart";
reg = <0x1e788800 0x40>;
reg-shift = <2>;
interrupts = <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>;
--
2.54.0
^ permalink raw reply related
* [PATCH v3 5/7] soc: aspeed: add host-side PCIe BMC device driver
From: Grégoire Layet @ 2026-06-23 14:25 UTC (permalink / raw)
To: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt
Cc: andrew, jacky_chou, yh_chung, ninad, anirudhsriniv, linux-serial,
linux-aspeed, linux-arm-kernel, linux-kernel, Grégoire Layet
In-Reply-To: <cover.1782224059.git.gregoire.layet@9elements.com>
Add support for VUART over PCIe between BMC and host.
This add host side driver.
This only support the AST2600.
Taken from ASPEED 6.18 Kernel SDK and trimmed down.
The host can't detect the VUART adresses, they are forced
at 0x3f8 and 0x2f8, similar from the initial ASPEED driver.
The MSI vector index has been changed for the VUART2 from 15 to 17.
The index 15 used in the initial driver was not working.
Data path in both direction is tested on both VUART.
Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
Signed-off-by: aspeedyh <yh_chung@aspeedtech.com>
Signed-off-by: Grégoire Layet <gregoire.layet@9elements.com>
Tested-by: Grégoire Layet <gregoire.layet@9elements.com>
---
drivers/soc/aspeed/Kconfig | 8 +
drivers/soc/aspeed/Makefile | 1 +
drivers/soc/aspeed/aspeed-host-bmc-dev.c | 183 +++++++++++++++++++++++
3 files changed, 192 insertions(+)
create mode 100644 drivers/soc/aspeed/aspeed-host-bmc-dev.c
diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
index 63a656449a1a..ebd023b10701 100644
--- a/drivers/soc/aspeed/Kconfig
+++ b/drivers/soc/aspeed/Kconfig
@@ -4,6 +4,14 @@ if ARCH_ASPEED || COMPILE_TEST
menu "ASPEED SoC drivers"
+config ASPEED_HOST_BMC_DEV
+ tristate "ASPEED Host BMC Device"
+ depends on PCI
+ depends on SERIAL_8250
+ help
+ Enable support for the ASPEED AST2600 BMC Device on the Host.
+ This configure the PCIe and setup two 8250 compatible VUART ports.
+
config ASPEED_LPC_CTRL
tristate "ASPEED LPC firmware cycle control"
select REGMAP
diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
index b35d74592964..c515e163eab7 100644
--- a/drivers/soc/aspeed/Makefile
+++ b/drivers/soc/aspeed/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_ASPEED_HOST_BMC_DEV) += aspeed-host-bmc-dev.o
obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
obj-$(CONFIG_ASPEED_UART_ROUTING) += aspeed-uart-routing.o
diff --git a/drivers/soc/aspeed/aspeed-host-bmc-dev.c b/drivers/soc/aspeed/aspeed-host-bmc-dev.c
new file mode 100644
index 000000000000..3160b6aedb5b
--- /dev/null
+++ b/drivers/soc/aspeed/aspeed-host-bmc-dev.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (C) ASPEED Technology Inc.
+
+#include <linux/init.h>
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/pci.h>
+#include <linux/serial_core.h>
+#include <linux/serial_8250.h>
+
+#define BMC_MULTI_MSI 32
+#define PCI_BMC_DEVICE_ID 0x2402
+
+#define DRIVER_NAME "aspeed-host-bmc-dev"
+
+enum aspeed_platform_id {
+ ASPEED,
+};
+
+static int vuart_msi_index[2] = { 16, 17 };
+static int vuart_port_addr[2] = {0x3f8, 0x2f8};
+
+struct aspeed_pci_bmc_dev {
+ struct device *dev;
+ kernel_ulong_t driver_data;
+ int id;
+
+ unsigned long message_bar_base;
+
+ struct uart_8250_port uart[2];
+ int uart_line[2];
+
+ int *msi_idx_table;
+};
+
+static void aspeed_pci_setup_irq_resource(struct pci_dev *pdev)
+{
+ struct aspeed_pci_bmc_dev *pci_bmc_dev = pci_get_drvdata(pdev);
+
+ pci_bmc_dev->msi_idx_table = vuart_msi_index;
+
+ if (pci_alloc_irq_vectors(pdev, 1, BMC_MULTI_MSI, PCI_IRQ_INTX | PCI_IRQ_MSI) <= 1)
+ /* If pci_alloc fail, set all msi index to the first vector */
+ memset(pci_bmc_dev->msi_idx_table, 0, sizeof(vuart_msi_index));
+}
+
+static int aspeed_pci_bmc_device_setup_vuart(struct pci_dev *pdev, int idx)
+{
+ struct aspeed_pci_bmc_dev *pci_bmc_dev = pci_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+ struct uart_8250_port *uart = &pci_bmc_dev->uart[idx];
+ u16 vuart_ioport;
+ int ret;
+
+ /* Assign the line to non-exist device before everything is setup */
+ pci_bmc_dev->uart_line[idx] = -ENOENT;
+
+ vuart_ioport = vuart_port_addr[idx];
+ /* ASPEED BMC device shift adresses by 2 to the left */
+ vuart_ioport = vuart_ioport << 2;
+
+ uart->port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_SHARE_IRQ;
+ uart->port.uartclk = 115200 * 16;
+ uart->port.irq = pci_irq_vector(pdev, pci_bmc_dev->msi_idx_table[idx]);
+ uart->port.dev = dev;
+ uart->port.iotype = UPIO_MEM32;
+ uart->port.iobase = 0;
+ uart->port.mapbase = pci_bmc_dev->message_bar_base + vuart_ioport;
+ uart->port.membase = 0;
+ uart->port.type = PORT_16550A;
+ uart->port.flags |= (UPF_IOREMAP | UPF_FIXED_PORT | UPF_FIXED_TYPE);
+ uart->port.regshift = 2;
+
+ ret = serial8250_register_8250_port(&pci_bmc_dev->uart[idx]);
+ if (ret < 0) {
+ dev_err_probe(dev, ret, "Can't setup PCIe VUART%d\n", idx);
+ return ret;
+ }
+
+ pci_bmc_dev->uart_line[idx] = ret;
+
+ return 0;
+}
+
+static void aspeed_pci_host_bmc_device_release_vuart(struct pci_dev *pdev, int idx)
+{
+ struct aspeed_pci_bmc_dev *pci_bmc_dev = pci_get_drvdata(pdev);
+
+ if (pci_bmc_dev->uart_line[idx] >= 0)
+ serial8250_unregister_port(pci_bmc_dev->uart_line[idx]);
+}
+
+static int aspeed_pci_host_setup(struct pci_dev *pdev)
+{
+ struct aspeed_pci_bmc_dev *pci_bmc_dev = pci_get_drvdata(pdev);
+ int rc = 0;
+
+ pci_bmc_dev->message_bar_base = pci_resource_start(pdev, 1);
+
+ if (pdev->revision == 0x27) {
+ pr_err("AST2700 detected but not supported");
+ return -ENODEV;
+ }
+
+ rc = aspeed_pci_bmc_device_setup_vuart(pdev, 0);
+ if (rc)
+ return rc;
+
+ rc = aspeed_pci_bmc_device_setup_vuart(pdev, 1);
+ if (rc)
+ goto out_freeVUART1;
+
+ return 0;
+
+out_freeVUART1:
+ aspeed_pci_host_bmc_device_release_vuart(pdev, 0);
+
+ return rc;
+}
+
+static int aspeed_pci_host_bmc_device_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+ struct aspeed_pci_bmc_dev *pci_bmc_dev;
+ int rc = 0;
+
+ pci_bmc_dev = devm_kzalloc(&pdev->dev, sizeof(*pci_bmc_dev), GFP_KERNEL);
+ if (!pci_bmc_dev)
+ return -ENOMEM;
+
+ rc = pci_enable_device(pdev);
+ if (rc) {
+ dev_err(&pdev->dev, "pci_enable_device() returned error %d\n", rc);
+ return rc;
+ }
+
+ pci_set_master(pdev);
+ pci_set_drvdata(pdev, pci_bmc_dev);
+
+ aspeed_pci_setup_irq_resource(pdev);
+
+ /* Setup BMC PCI device */
+ rc = aspeed_pci_host_setup(pdev);
+ if (rc) {
+ dev_err(&pdev->dev, "ASPEED PCIe Host device returned error %d\n", rc);
+ pci_free_irq_vectors(pdev);
+ pci_disable_device(pdev);
+ return rc;
+ }
+
+ return 0;
+}
+
+static void aspeed_pci_host_bmc_device_remove(struct pci_dev *pdev)
+{
+ aspeed_pci_host_bmc_device_release_vuart(pdev, 0);
+ aspeed_pci_host_bmc_device_release_vuart(pdev, 1);
+
+ pci_free_irq_vectors(pdev);
+ pci_disable_device(pdev);
+}
+
+static struct pci_device_id aspeed_host_bmc_dev_pci_ids[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_ASPEED, PCI_BMC_DEVICE_ID), .class = 0xFF0000, .class_mask = 0xFFFF00,
+ .driver_data = ASPEED },
+ { 0 }
+};
+
+MODULE_DEVICE_TABLE(pci, aspeed_host_bmc_dev_pci_ids);
+
+static struct pci_driver aspeed_host_bmc_dev_driver = {
+ .name = DRIVER_NAME,
+ .id_table = aspeed_host_bmc_dev_pci_ids,
+ .probe = aspeed_pci_host_bmc_device_probe,
+ .remove = aspeed_pci_host_bmc_device_remove,
+};
+
+module_driver(aspeed_host_bmc_dev_driver, pci_register_driver, pci_unregister_driver);
+
+MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
+MODULE_DESCRIPTION("ASPEED Host BMC DEVICE Driver");
+MODULE_LICENSE("GPL");
--
2.54.0
^ permalink raw reply related
* [PATCH v3 4/7] serial: 8250_aspeed_vuart: add VUART over PCI
From: Grégoire Layet @ 2026-06-23 14:25 UTC (permalink / raw)
To: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt
Cc: andrew, jacky_chou, yh_chung, ninad, anirudhsriniv, linux-serial,
linux-aspeed, linux-arm-kernel, linux-kernel, Grégoire Layet
In-Reply-To: <cover.1782224059.git.gregoire.layet@9elements.com>
This patch enables the VUART over PCI possible for the AST2600. This is
only activated if the 'aspeed,vuart-over-pci' property flag is set on an
'ast2600-vuart' compatible node.
The AST2600 has 2 VUART that are usable over PCI. These are the VUART3
and VUART4 in the 'apseed-g6.dtsi'.
This code sets the BMC PCI device enables
bits, sets the PCI class code to MFD device and configures MSI interrupts.
There is no disable function. Removing this driver should not disable
the BMC PCI device, as other drivers could use it.
However, if all the drivers using it are removed, the
BMC PCI device will still be activated, which is not ideal. Realistically though, this is not a
use case for a BMC, the drivers will never be removed.
This is useful on PCIe BMC expansion cards that use the AST2600, such as the
ASUS Kommando IPMI Expansion Card.
Registers initialisation taken from ASPEED 6.18 Kernel SDK.
Return code checks were added to each register write.
The code has been simplified and macros have been added.
The ASPEED_SCUC24 regmap update is missing a macro for 'BIT(14)'. I was
unable to determine the purpose of this bit. In the AST2600 A3
datasheet it is marked as 'reserved'. It is only used on the other
revision. As I only have the AST2600A3, I was unable to try this code
path.
Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
Signed-off-by: aspeedyh <yh_chung@aspeedtech.com>
Signed-off-by: Grégoire Layet <gregoire.layet@9elements.com>
Tested-by: Grégoire Layet <gregoire.layet@9elements.com>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 86 +++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 6afa2f4057e1..e204e26fa173 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -32,6 +32,27 @@
#define ASPEED_VUART_DEFAULT_SIRQ 4
#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
+#define ASPEED_SCU_SILICON_REVISION_ID 0x04
+#define AST2600A3_REVISION_ID 0x05030303
+
+#define ASPEED_SCUC24 0xC24
+#define ASPEED_SCUC24_MSI_ROUTING_MASK GENMASK(11, 10)
+#define ASPEED_SCUC24_MSI_ROUTING_PCIe2LPC_PCIDEV1 (0x2 << 10)
+#define ASPEED_SCUC24_PCIDEV1_INTX_MSI_HOST2BMC_EN BIT(18)
+#define ASPEED_SCUC24_PCIDEV1_INTX_MSI_SCU560_EN BIT(17)
+
+
+#define ASPEED_SCU_PCIE_CONF_CTRL 0xC20
+#define SCU_PCIE_CONF_BMC_DEV_EN BIT(8)
+#define SCU_PCIE_CONF_BMC_DEV_EN_MMIO BIT(9)
+#define SCU_PCIE_CONF_BMC_DEV_EN_MSI BIT(11)
+#define SCU_PCIE_CONF_BMC_DEV_EN_IRQ BIT(13)
+#define SCU_PCIE_CONF_BMC_DEV_EN_PCIE_BUS_MASTER BIT(14)
+#define SCU_PCIE_CONF_BMC_DEV_EN_E2L BIT(15)
+#define SCU_PCIE_CONF_BMC_DEV_EN_LPC_DECODE BIT(21)
+
+#define ASPEED_SCU_BMC_DEV_CLASS 0xC68
+
struct aspeed_vuart {
struct device *dev;
int line;
@@ -412,6 +433,62 @@ static int aspeed_vuart_map_irq_polarity(u32 dt)
}
}
+static int aspeed_ast2600_vuart_over_pci_set_enabled(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ u32 silicon_revision_id;
+ struct regmap *scu;
+ int rc;
+
+ u32 pcie_config_ctl = SCU_PCIE_CONF_BMC_DEV_EN_IRQ |
+ SCU_PCIE_CONF_BMC_DEV_EN_MMIO |
+ SCU_PCIE_CONF_BMC_DEV_EN_MSI |
+ SCU_PCIE_CONF_BMC_DEV_EN_PCIE_BUS_MASTER |
+ SCU_PCIE_CONF_BMC_DEV_EN_E2L |
+ SCU_PCIE_CONF_BMC_DEV_EN_LPC_DECODE |
+ SCU_PCIE_CONF_BMC_DEV_EN;
+
+ scu = syscon_regmap_lookup_by_phandle(dev->of_node, "clocks");
+ if (IS_ERR(scu)) {
+ dev_err(&pdev->dev, "failed to find SCU regmap\n");
+ return PTR_ERR(scu);
+ }
+
+ /* update class code to be an MFD device */
+ if (regmap_write(scu, ASPEED_SCU_BMC_DEV_CLASS, 0xff000000)) {
+ dev_err(dev, "could not set PCI class code\n");
+ return -EIO;
+ }
+
+ if (regmap_update_bits(scu, ASPEED_SCU_PCIE_CONF_CTRL,
+ pcie_config_ctl, pcie_config_ctl)) {
+ dev_err(dev, "could not set PCIe configuration\n");
+ return -EIO;
+ }
+
+ if (regmap_read(scu, ASPEED_SCU_SILICON_REVISION_ID, &silicon_revision_id)) {
+ dev_err(dev, "could not read silicon revision\n");
+ return -EIO;
+ }
+
+ if (silicon_revision_id == AST2600A3_REVISION_ID)
+ rc = regmap_update_bits(scu, ASPEED_SCUC24,
+ ASPEED_SCUC24_PCIDEV1_INTX_MSI_HOST2BMC_EN | ASPEED_SCUC24_MSI_ROUTING_MASK,
+ ASPEED_SCUC24_PCIDEV1_INTX_MSI_HOST2BMC_EN | ASPEED_SCUC24_MSI_ROUTING_PCIe2LPC_PCIDEV1);
+ else
+ rc = regmap_update_bits(scu, ASPEED_SCUC24,
+ /* The bit 14 is reserved in the Datasheet, so we can't say what it does. This revision has not been tested */
+ ASPEED_SCUC24_PCIDEV1_INTX_MSI_SCU560_EN | BIT(14) | ASPEED_SCUC24_MSI_ROUTING_MASK,
+ ASPEED_SCUC24_PCIDEV1_INTX_MSI_SCU560_EN | BIT(14) | ASPEED_SCUC24_MSI_ROUTING_PCIe2LPC_PCIDEV1);
+ if (rc) {
+ dev_err(dev, "could not set PCI device 1 MSI interrupt routing\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+
static int aspeed_vuart_probe(struct platform_device *pdev)
{
struct of_phandle_args sirq_polarity_sense_args;
@@ -540,6 +617,15 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
aspeed_vuart_set_host_tx_discard(vuart, true);
platform_set_drvdata(pdev, vuart);
+ if (of_device_is_compatible(dev->of_node, "aspeed,ast2600-vuart") &&
+ of_property_read_bool(dev->of_node, "aspeed,vuart-over-pci")) {
+ rc = aspeed_ast2600_vuart_over_pci_set_enabled(pdev);
+ if (rc) {
+ dev_err(dev, "could not enable VUART over PCI\n");
+ return rc;
+ }
+ }
+
return 0;
err_sysfs_remove:
--
2.54.0
^ permalink raw reply related
* [PATCH v3 3/7] serial: 8250_aspeed_vuart: add aspeed,ast2600-vuart compatible string
From: Grégoire Layet @ 2026-06-23 14:25 UTC (permalink / raw)
To: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt
Cc: andrew, jacky_chou, yh_chung, ninad, anirudhsriniv, linux-serial,
linux-aspeed, linux-arm-kernel, linux-kernel, Grégoire Layet
In-Reply-To: <cover.1782224059.git.gregoire.layet@9elements.com>
Makes the driver compatible with the ast2600-vuart.
This enables specific configuration for the AST2600.
Signed-off-by: Grégoire Layet <gregoire.layet@9elements.com>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 26fc0464f1cc..6afa2f4057e1 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -560,6 +560,7 @@ static void aspeed_vuart_remove(struct platform_device *pdev)
static const struct of_device_id aspeed_vuart_table[] = {
{ .compatible = "aspeed,ast2400-vuart" },
{ .compatible = "aspeed,ast2500-vuart" },
+ { .compatible = "aspeed,ast2600-vuart" },
{ },
};
MODULE_DEVICE_TABLE(of, aspeed_vuart_table);
--
2.54.0
^ permalink raw reply related
* [PATCH v3 2/7] dt-bindings: serial: 8250: aspeed: add aspeed,vuart-over-pci bool prop
From: Grégoire Layet @ 2026-06-23 14:25 UTC (permalink / raw)
To: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt
Cc: andrew, jacky_chou, yh_chung, ninad, anirudhsriniv, linux-serial,
linux-aspeed, linux-arm-kernel, linux-kernel, Grégoire Layet
In-Reply-To: <cover.1782224059.git.gregoire.layet@9elements.com>
The ASPEED AST2600 has 2 VUART accessible over PCI.
This boolean can be set to specify if the VUART is used over PCI.
Signed-off-by: Grégoire Layet <gregoire.layet@9elements.com>
---
.../devicetree/bindings/serial/8250.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index 3cbd0f532e15..b03797f4674d 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -26,6 +26,14 @@ allOf:
anyOf:
- const: aspeed,ast2500-vuart
- const: aspeed,ast2600-vuart
+ - if:
+ anyOf:
+ - required:
+ - aspeed,vuart-over-pci
+ then:
+ properties:
+ compatible:
+ const: aspeed,ast2600-vuart
- if:
properties:
compatible:
@@ -312,6 +320,13 @@ properties:
polarity (IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_LEVEL_HIGH). Only
applicable to aspeed,ast2500-vuart and aspeed,ast2600-vuart.
+ aspeed,vuart-over-pci:
+ type: boolean
+ default: false
+ description: |
+ Enable the VUART over the BMC PCI device. Only applicable to
+ aspeed,ast2600-vuart.
+
required:
- reg
- interrupts
--
2.54.0
^ permalink raw reply related
* [PATCH v3 1/7] dt-bindings: serial: 8250: aspeed: add compatible string for ast2600
From: Grégoire Layet @ 2026-06-23 14:25 UTC (permalink / raw)
To: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt
Cc: andrew, jacky_chou, yh_chung, ninad, anirudhsriniv, linux-serial,
linux-aspeed, linux-arm-kernel, linux-kernel, Grégoire Layet
In-Reply-To: <cover.1782224059.git.gregoire.layet@9elements.com>
The ast2600 was using the ast2500 vuart compatible string.
This change makes it possible to have ast2600-specific properties.
Signed-off-by: Grégoire Layet <gregoire.layet@9elements.com>
---
.../devicetree/bindings/serial/8250.yaml | 20 +++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index bb7b9c87a807..3cbd0f532e15 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -23,7 +23,9 @@ allOf:
then:
properties:
compatible:
- const: aspeed,ast2500-vuart
+ anyOf:
+ - const: aspeed,ast2500-vuart
+ - const: aspeed,ast2600-vuart
- if:
properties:
compatible:
@@ -287,17 +289,19 @@ properties:
aspeed,sirq-polarity-sense:
$ref: /schemas/types.yaml#/definitions/phandle-array
description: |
- Phandle to aspeed,ast2500-scu compatible syscon alongside register
- offset and bit number to identify how the SIRQ polarity should be
- configured. One possible data source is the LPC/eSPI mode bit. Only
- applicable to aspeed,ast2500-vuart.
+ Phandle to aspeed,ast2500-scu or aspeed,ast2600-scu compatible syscon
+ alongside register offset and bit number to identify how the SIRQ
+ polarity should be configured. One possible data source is the LPC/eSPI
+ mode bit. Only applicable to aspeed,ast2500-vuart and
+ aspeed,ast2600-vuart.
deprecated: true
aspeed,lpc-io-reg:
$ref: /schemas/types.yaml#/definitions/uint32-array
maxItems: 1
description: |
- The VUART LPC address. Only applicable to aspeed,ast2500-vuart.
+ The VUART LPC address. Only applicable to aspeed,ast2500-vuart and
+ aspeed,ast2600-vuart.
aspeed,lpc-interrupts:
$ref: /schemas/types.yaml#/definitions/uint32-array
@@ -305,8 +309,8 @@ properties:
maxItems: 2
description: |
A 2-cell property describing the VUART SIRQ number and SIRQ
- polarity (IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_LEVEL_HIGH). Only
- applicable to aspeed,ast2500-vuart.
+ polarity (IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_LEVEL_HIGH). Only
+ applicable to aspeed,ast2500-vuart and aspeed,ast2600-vuart.
required:
- reg
--
2.54.0
^ permalink raw reply related
* [PATCH v3 0/7] soc: aspeed: Add BMC and host driver for PCIe BMC device
From: Grégoire Layet @ 2026-06-23 14:25 UTC (permalink / raw)
To: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
krzk+dt, conor+dt
Cc: andrew, jacky_chou, yh_chung, ninad, anirudhsriniv, linux-serial,
linux-aspeed, linux-arm-kernel, linux-kernel, Grégoire Layet
In-Reply-To: <cover.1780929570.git.gregoire.layet@9elements.com>
This is a v3 for upstreaming the VUART over PCIe BMC device driver.
The initial driver is from the ASPEED kernel SDK (master-v6.18) [1].
There are two drivers: a BMC-side driver and a host-side driver.
Together they enable host<->BMC VUART communication via PCIe.
The host cannot access the BMC's memory. Only the enabled features are
accessible. These are the KCS4 channel and 2 VUARTs. There is also some
mailbox register functionality also exist for a communication between
the host and the BMC. More information can be found here [2].
This v3 mainly modifies the BMC driver and focuses on VUART.
The BMC driver is now incorporated into the '8250_aspeed_vuart' driver.
A specific flag can be set to indicate that the VUART should be used
over PCI. Several changes have been made to the 8250 device tree binding
and the 'aspeed-g6.dtsi'.
Changes since v2 [3]:
- Add the aspeed,ast2600-vuart compatible entry to the '8250' DT binding
- Add the aspeed,ast2600-vuart compatible property in 'aspeed-g6.dtsi'
- Add the aspeed,vuart-over-pci boolean property to the '8250' DT binding,
only for the aspeed,ast2600-vuart
- Add the aspeed,vuart-over-pci flag to the vuart3 and vuart4
- Add the aspeed,ast2600-vuart compatible property to the
'8250_aspeed_vuart' driver
- Add the VUART over PCI code to the '8250_aspeed_vuart' driver
- The v2 review of the host-side BMC driver has been applied.
The host-side driver is still in /soc/aspeed/, as it is very specific to
this SoC for me. I didn't receive any feedback on where to put this
driver. I can, of course, change this to the relevant location.
It's important to consider that the host driver will do multiple
functions. The AST2600 also supports LPC over PCI, with a specific KCS
channel (KCS4). This driver should also be used to enable the IPMI
automatically via this KCS channel. The UART and the IPMI will depend on
the same PCI resource (BAR1), so this must be configured in one driver.
As with v2, VUART data flow and MSI interrupts have been verified
working on the test hardware.
Tested on:
BMC:
- Asus IPMI Kommando Card R1.01, AST2600 A3.
- OpenBMC
Host:
- Linux kernel v7.0.0
This v3 only supports AST2600; the AST2700 is not supported by this series.
I would like to know whether I should add the 'lpc-io-reg' and
'lpc-interrupt' values to the vuart3 and vuart4 nodes directly in the
'aspeed-g6.dtsi'. The host driver is not capable of finding the vuart
address on his own, so they are hardcoded to 0x3f8 and 0x2f8. It will
not work with other adresses, so perhaps they should be in the .dtsi to
ensure the correct configuration for the 2 vuart over PCI.
For the interrupt number, my test is working with interrupt = 0
for vuart3 and interrupt = 1 for vuart4. I don't fully understand how
the silicon routes MSI numbers to the VUART but the following
combination is working :
| host MSI idx | BMC lpc-interrupts |
VUART3 | 16 | 0 |
VUART4 | 17 | 1 |
The original ASPEED driver used MSI index 15 for the VUART4.
I tested every lpc-interrupts on the BMC from 0 to 15, but none of them
worked with the host MSI index set to 15.
For me, the silicon only routes the MSI index 16 to VUART3 and 17 to
VUART4, and the lpc-interrupt needs to match the 4 least significant bits.
I might be wrong on this explanation but the data path is working with
those numbers. There is no explanation for any of this in the datasheet.
[1]: https://github.com/AspeedTech-BMC/linux/tree/aspeed-master-v6.18/drivers/soc/aspeed
[2]: https://lore.kernel.org/linux-aspeed/CAFi2wKYOAotiezepDqaR5PZDqDaPKKDfAEnpx5EHC0mL39hy6w@mail.gmail.com/
[3]: https://lore.kernel.org/linux-aspeed/cover.1780929570.git.gregoire.layet@9elements.com/
Grégoire Layet (7):
dt-bindings: serial: 8250: aspeed: add compatible string for ast2600
dt-bindings: serial: 8250: aspeed: add aspeed,vuart-over-pci bool prop
serial: 8250_aspeed_vuart: add aspeed,ast2600-vuart compatible string
serial: 8250_aspeed_vuart: add VUART over PCI
soc: aspeed: add host-side PCIe BMC device driver
ARM: dts: aspeed: g6: Change vuart compatible string for ast2600
ARM: dts: aspeed: g6: add aspeed,vuart-over-pci prop to vuart3 and 4
.../devicetree/bindings/serial/8250.yaml | 35 +++-
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 10 +-
drivers/soc/aspeed/Kconfig | 8 +
drivers/soc/aspeed/Makefile | 1 +
drivers/soc/aspeed/aspeed-host-bmc-dev.c | 183 ++++++++++++++++++
drivers/tty/serial/8250/8250_aspeed_vuart.c | 87 +++++++++
6 files changed, 312 insertions(+), 12 deletions(-)
create mode 100644 drivers/soc/aspeed/aspeed-host-bmc-dev.c
--
2.54.0
^ permalink raw reply
* [PATCH] tty: serial: pch_uart: add check for pci_get_slot()
From: Haoxiang Li @ 2026-06-23 14:05 UTC (permalink / raw)
To: gregkh, jirislaby, andriy.shevchenko, fourier.thomas, 2426767509,
kees
Cc: linux-kernel, linux-serial, Haoxiang Li, stable
Add check for pci_get_slot() to prevent a potetial
null pointer dereference in pch_request_dma().
Fixes: 8368d6a2b739 ("pch_uart: don't hardcode PCI slot to get DMA device")
Cc: stable@vger.kernel.org
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
---
drivers/tty/serial/pch_uart.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index ba1fcd663fe2..6c9e596a14e7 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -678,6 +678,11 @@ static void pch_request_dma(struct uart_port *port)
/* Get DMA's dev information */
dma_dev = pci_get_slot(priv->pdev->bus,
PCI_DEVFN(PCI_SLOT(priv->pdev->devfn), 0));
+ if (!dma_dev) {
+ dev_err(priv->port.dev, "%s: failed to get DMA device\n",
+ __func__);
+ return;
+ }
/* Set Tx DMA */
param = &priv->param_tx;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 2/2] serial: sc16is7xx: set TX FIFO trigger level to half FIFO to prevent underruns
From: Paul Mbewe @ 2026-06-23 14:01 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-serial, linux-kernel, gregkh, jirislaby, hvilleneuve,
stable, tobias.gannert, joachim.knorr, Paul Mbewe
In-Reply-To: <20260623134536.24dca506@pumpkin>
Hi David,
Thanks for the detailed review.
According to the SC16IS7xx datasheet, the TX trigger level is defined
in terms of free FIFO spaces, not remaining data. So with the default
configuration (FCR[5:4] = 00), the THRI interrupt fires when the FIFO
has 8 free entries, i.e. when it still contains 56 bytes.
While this in theory leaves enough data in the FIFO, in practice the
system has to service many small refill cycles (~8 bytes per interrupt).
On slow SPI hosts, each cycle involves threaded interrupt handling and
multiple SPI transactions, and the cumulative latency plus scheduling
jitter can exceed the available margin between refills under load.
Increasing the trigger level to 32 free spaces reduces the number of
refill cycles significantly (from ~8 per FIFO load to ~2), and increases
the amount of data written per cycle. This reduces scheduling pressure
and, in practice, avoids the FIFO draining to empty between bursts.
The current commit message focuses too much on the "refill window" and
does not explain this aspect clearly. I can rephrase it to better
describe the interaction between trigger level, refill granularity and
system latency.
Thanks,
Paul
^ permalink raw reply
* Re: [PATCH 2/2] serial: sc16is7xx: set TX FIFO trigger level to half FIFO to prevent underruns
From: David Laight @ 2026-06-23 12:45 UTC (permalink / raw)
To: Paul Mbewe
Cc: linux-serial, linux-kernel, gregkh, jirislaby, hvilleneuve,
stable, Tobias Gannert, Joachim Knorr
In-Reply-To: <20260623112225.82386-3-paultyson.mbewe@ziehl-abegg.de>
On Tue, 23 Jun 2026 13:22:25 +0200
Paul Mbewe <paultyson.mbewe@ziehl-abegg.de> wrote:
> The THRI interrupt (IER[1]) fires when the TX FIFO free space reaches the
> configured threshold. With the reset default (TLR=0), the chip falls back
> to the FCR TX trigger of 8 free spaces (FCR[5:4]=00), causing THRI to
> assert after every 8 bytes drain from the FIFO.
>
> At 115200 baud 8N1, 8 bytes drain in 694 us. On slow single-core SPI
> hosts, the combined latency of an SPI IIR read, TXLVL read, and 8-byte
> THR write per interrupt, plus kthread scheduling jitter, can exceed this
> window on a loaded system. When the kthread cannot refill the FIFO within
> 694 us, the FIFO empties and produces an idle gap on the TX line.
That seems strange.
The earlier interrupt (8 free fifo spaces) ought to make it less likely
that the fifo will underrun.
Are you sure it isn't the other way around?
So the interrupt happens when there are 8 bytes left in the fifo.
With a 64 byte fifo each interrupt would fill at least 56 bytes.
Increasing it to 32 (bytes left in the fifo) gives more time for the
interrupt latency (etc), but reduces the number of bytes written by
each isr to at least 32.
The other possibility is some error passing the level sensitive 'fifo empty'
state through to scheduling the kthread.
It may well be that the system interrupt latency is small enough that you
can take the interrupt at 'half full', and that the associated reduction
in the number of interrupts makes the system behave better.
But this isn't what the commit message says.
David
>
> This violates the Modbus RTU specification, which treats any intra-frame
> silence longer than 1.5 character times (~130 us at 115200 baud) as a
> frame boundary, causing receivers to fragment frames and report CRC errors.
> Oscilloscope measurements confirmed a 757 us inter-burst gap during
> continuous transmission without this fix.
>
> Setting the TX trigger to 32 free spaces (half FIFO) via TLR[3:0]=8
> widens the refill window to 2778 us at 115200 baud, reducing THRI events
> per 256-byte frame from ~32 to ~8 and eliminating the underrun.
>
> Only TLR[3:0] is written; TLR[7:4] is left at zero, so the RX trigger
> retains its FCR default. Only TX interrupt timing is affected.
>
> While increasing the SPI clock would also reduce per-round-trip latency,
> the driver should work correctly regardless of SPI speed. The fix belongs
> in the driver.
>
> Tested on i.MX6ULL (ARM Cortex-A7, single-core) with SC16IS752IBS over
> SPI at 1 MHz, 115200 baud 8N1, 256-byte Modbus RTU frames under production
> load:
>
> IRQ thread (irq/134-spi2.0): ~15-17% -> ~5% (~67% reduction)
> sys CPU: ~51-61% -> ~19-28% (~55% reduction)
> load average: ~2.0-2.2 -> ~0.65-1.3
>
> No mid-frame gaps observed after fix. Without fix, oscilloscope confirmed
> 757 us inter-burst gaps causing Modbus frame fragmentation.
>
> Cc: stable@vger.kernel.org
> Reported-by: Tobias Gannert <tobias.gannert@ziehl-abegg.de>
> Tested-by: Tobias Gannert <tobias.gannert@ziehl-abegg.de>
> Reviewed-by: Joachim Knorr <joachim.knorr@ziehl-abegg.de>
> Signed-off-by: Paul Mbewe <paultyson.mbewe@ziehl-abegg.de>
> ---
> drivers/tty/serial/sc16is7xx.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 395a219280be..476e0dd3fa7f 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -1196,6 +1196,16 @@ static int sc16is7xx_startup(struct uart_port *port)
> SC16IS7XX_TCR_RX_RESUME(24) |
> SC16IS7XX_TCR_RX_HALT(48));
>
> + /*
> + * Set TX FIFO trigger level to 32 spaces (half FIFO) via TLR. The reset
> + * default (TLR=0) falls back to the FCR TX trigger of 8 free spaces,
> + * requiring ~8 SPI round-trips per 64-byte FIFO load. On slow single-core
> + * SPI hosts, this accumulated latency can cause a TX FIFO underrun gap
> + * between bursts.
> + */
> + sc16is7xx_port_write(port, SC16IS7XX_TLR_REG,
> + SC16IS7XX_TLR_TX_TRIGGER(32));
> +
> /* Disable TCR/TLR access */
> sc16is7xx_port_update(port, SC16IS7XX_MCR_REG, SC16IS7XX_MCR_TCRTLR_BIT, 0);
>
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: Drop incorrect usage of double '::'
From: Daniel Lezcano @ 2026-06-23 11:45 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peter Griffin, Alim Akhtar,
Michael Turquette, Stephen Boyd, Brian Masney, Sylwester Nawrocki,
Chanwoo Choi, Sam Protsenko, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Inki Dae, Seung-Woo Kim, Kyungmin Park,
Andi Shyti, Georgi Djakov, Lee Jones, Pavel Machek, Hans Verkuil,
Mauro Carvalho Chehab, Ulf Hansson, Peter Rosin, Vinod Koul,
Neil Armstrong, Linus Walleij, Geert Uytterhoeven, Magnus Damm,
Sebastian Reichel, Javier Martinez Canillas, Liam Girdwood,
Mark Brown, Greg Kroah-Hartman, Jiri Slaby, Srinivas Kandagatla,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Jonathan Marek, Taniya Das, Robert Marko,
Christian Marangi, Stephan Gerhold, Adam Skladowski,
Sireesh Kodali, Barnabas Czeman, Imran Shaik,
Sricharan Ramabadhran, Anusha Rao, Luo Jie, Tomasz Figa,
Chanho Park, Sunyeal Hong, Shin Son, Krishna Manikandan,
Jacek Anaszewski, Jaehoon Chung, Marek Szyprowski, Alina Yu,
Andy Gross, Niklas Söderlund, Wesley Cheng, linux-arm-msm,
devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc,
linux-clk, dri-devel, freedreno, linux-i2c, linux-pm, linux-leds,
linux-media, linux-mmc, linux-phy, linux-gpio, linux-renesas-soc,
linux-serial, linux-sound, linux-usb
In-Reply-To: <20260622101606.485961-4-krzysztof.kozlowski@oss.qualcomm.com>
On 6/22/26 12:16, Krzysztof Kozlowski wrote:
> There is no use of double colon '::' in YAML. OTOH, the literal style
> block, e.g. using '|' treats all characters as content [1] therefore
> single use of ':' in descriptions is perfectly fine, whenever '|' is
> used.
>
> Cleanup existing code, so the confusing style won't be re-used in new
> contributions.
>
> Link: https://yaml.org/spec/1.2.2/#literal-style [1]
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> ---
>
> Intention for this patch is to go via Rob's tree.
> ---
> .../devicetree/bindings/arm/qcom-soc.yaml | 4 ++--
> .../devicetree/bindings/arm/qcom.yaml | 4 ++--
> .../bindings/arm/samsung/samsung-soc.yaml | 4 ++--
> .../display/msm/dsi-controller-main.yaml | 20 +++++++++----------
> .../display/samsung/samsung,fimd.yaml | 4 ++--
> .../bindings/i2c/samsung,s3c2410-i2c.yaml | 2 +-
> .../interconnect/qcom,msm8998-bwmon.yaml | 2 +-
> .../interconnect/samsung,exynos-bus.yaml | 14 ++++++-------
> .../bindings/leds/qcom,pm8058-led.yaml | 4 ++--
> .../bindings/leds/skyworks,aat1290.yaml | 6 +++---
> .../bindings/media/cec/cec-gpio.yaml | 2 +-
> .../bindings/mmc/samsung,exynos-dw-mshc.yaml | 2 +-
> .../devicetree/bindings/mux/mux-consumer.yaml | 4 ++--
> .../bindings/phy/samsung,mipi-video-phy.yaml | 4 ++--
> .../bindings/phy/samsung,usb2-phy.yaml | 2 +-
> .../bindings/phy/samsung,usb3-drd-phy.yaml | 2 +-
> .../bindings/pinctrl/samsung,pinctrl.yaml | 2 +-
> .../bindings/power/renesas,rcar-sysc.yaml | 2 +-
> .../bindings/power/reset/restart-handler.yaml | 8 ++++----
> .../bindings/regulator/maxim,max77802.yaml | 4 ++--
> .../bindings/regulator/richtek,rtq2208.yaml | 2 +-
> .../bindings/serial/qcom,msm-uartdm.yaml | 2 +-
> .../devicetree/bindings/slimbus/slimbus.yaml | 4 ++--
> .../bindings/soc/qcom/qcom,apr-services.yaml | 2 +-
> .../bindings/soc/qcom/qcom,rpmh-rsc.yaml | 8 ++++----
> .../bindings/soc/qcom/qcom,wcnss.yaml | 2 +-
> .../bindings/soc/renesas/renesas-soc.yaml | 4 ++--
> .../bindings/sound/qcom,q6asm-dais.yaml | 2 +-
> .../thermal/samsung,exynos-thermal.yaml | 4 ++--
Acked-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com> # thermal
^ permalink raw reply
* [PATCH 2/2] serial: sc16is7xx: set TX FIFO trigger level to half FIFO to prevent underruns
From: Paul Mbewe @ 2026-06-23 11:22 UTC (permalink / raw)
To: linux-serial, linux-kernel
Cc: gregkh, jirislaby, hvilleneuve, Paul Mbewe, stable,
Tobias Gannert, Joachim Knorr
In-Reply-To: <20260623112225.82386-1-paultyson.mbewe@ziehl-abegg.de>
The THRI interrupt (IER[1]) fires when the TX FIFO free space reaches the
configured threshold. With the reset default (TLR=0), the chip falls back
to the FCR TX trigger of 8 free spaces (FCR[5:4]=00), causing THRI to
assert after every 8 bytes drain from the FIFO.
At 115200 baud 8N1, 8 bytes drain in 694 us. On slow single-core SPI
hosts, the combined latency of an SPI IIR read, TXLVL read, and 8-byte
THR write per interrupt, plus kthread scheduling jitter, can exceed this
window on a loaded system. When the kthread cannot refill the FIFO within
694 us, the FIFO empties and produces an idle gap on the TX line.
This violates the Modbus RTU specification, which treats any intra-frame
silence longer than 1.5 character times (~130 us at 115200 baud) as a
frame boundary, causing receivers to fragment frames and report CRC errors.
Oscilloscope measurements confirmed a 757 us inter-burst gap during
continuous transmission without this fix.
Setting the TX trigger to 32 free spaces (half FIFO) via TLR[3:0]=8
widens the refill window to 2778 us at 115200 baud, reducing THRI events
per 256-byte frame from ~32 to ~8 and eliminating the underrun.
Only TLR[3:0] is written; TLR[7:4] is left at zero, so the RX trigger
retains its FCR default. Only TX interrupt timing is affected.
While increasing the SPI clock would also reduce per-round-trip latency,
the driver should work correctly regardless of SPI speed. The fix belongs
in the driver.
Tested on i.MX6ULL (ARM Cortex-A7, single-core) with SC16IS752IBS over
SPI at 1 MHz, 115200 baud 8N1, 256-byte Modbus RTU frames under production
load:
IRQ thread (irq/134-spi2.0): ~15-17% -> ~5% (~67% reduction)
sys CPU: ~51-61% -> ~19-28% (~55% reduction)
load average: ~2.0-2.2 -> ~0.65-1.3
No mid-frame gaps observed after fix. Without fix, oscilloscope confirmed
757 us inter-burst gaps causing Modbus frame fragmentation.
Cc: stable@vger.kernel.org
Reported-by: Tobias Gannert <tobias.gannert@ziehl-abegg.de>
Tested-by: Tobias Gannert <tobias.gannert@ziehl-abegg.de>
Reviewed-by: Joachim Knorr <joachim.knorr@ziehl-abegg.de>
Signed-off-by: Paul Mbewe <paultyson.mbewe@ziehl-abegg.de>
---
drivers/tty/serial/sc16is7xx.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 395a219280be..476e0dd3fa7f 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1196,6 +1196,16 @@ static int sc16is7xx_startup(struct uart_port *port)
SC16IS7XX_TCR_RX_RESUME(24) |
SC16IS7XX_TCR_RX_HALT(48));
+ /*
+ * Set TX FIFO trigger level to 32 spaces (half FIFO) via TLR. The reset
+ * default (TLR=0) falls back to the FCR TX trigger of 8 free spaces,
+ * requiring ~8 SPI round-trips per 64-byte FIFO load. On slow single-core
+ * SPI hosts, this accumulated latency can cause a TX FIFO underrun gap
+ * between bursts.
+ */
+ sc16is7xx_port_write(port, SC16IS7XX_TLR_REG,
+ SC16IS7XX_TLR_TX_TRIGGER(32));
+
/* Disable TCR/TLR access */
sc16is7xx_port_update(port, SC16IS7XX_MCR_REG, SC16IS7XX_MCR_TCRTLR_BIT, 0);
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox