Enable the VUART over PCI for the AST2600. Activate it only 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 already defined as
the VUART3 and VUART4 in the 'aspeed-g6.dtsi'.
Sets the BMC PCI device enable bits, sets the PCI class code to
unassgined/device specific, 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. But in reality,
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.
Register initialisation taken from ASPEED 6.18 Kernel SDK.
Add return code checks to each register write.
Simplify the code and add macros.
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. This BIT14 was set in the ASPEED SDK so I kept it.
I can remove it and the untested path if necessary.
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>
---
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..4d09c04cb972 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -32,6 +32,26 @@
#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 +432,63 @@ 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, "syscon");
+ if (IS_ERR(scu)) {
+ dev_err(&pdev->dev, "failed to find SCU regmap\n");
+ return PTR_ERR(scu);
+ }
+
+ /* update class code to be an Unassigned/device specific class 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.
+ */
+ 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 < 0) {
+ dev_err_probe(dev, rc, "could not enable VUART over PCI\n");
+ goto err_sysfs_remove;
+ }
+ }
+
return 0;
err_sysfs_remove:
--
2.54.0
> Enable the VUART over PCI for the AST2600. Activate it only 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 already defined as
> the VUART3 and VUART4 in the 'aspeed-g6.dtsi'.
>
> Sets the BMC PCI device enable bits, sets the PCI class code to
> unassgined/device specific, 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. But in reality,
> 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.
>
> Register initialisation taken from ASPEED 6.18 Kernel SDK.
> Add return code checks to each register write.
> Simplify the code and add macros.
>
> 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. This BIT14 was set in the ASPEED SDK so I kept it.
> I can remove it and the untested path if necessary.
There's not even a reference in the datasheet changelog, so I suspect that it
has always been reserved.
I'd be in favour of removing it.
>
> 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>
>
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index 6afa2f4057e1..4d09c04cb972 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
*snip*
> +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, "syscon");
> + if (IS_ERR(scu)) {
> + dev_err(&pdev->dev, "failed to find SCU regmap\n");
dev_err(dev, ...) instead of &pdev->dev (defined above).
> + return PTR_ERR(scu);
> + }
> +
> + /* update class code to be an Unassigned/device specific class 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.
> + */
Even in the oldest datasheet that I've got (which was for A2 apparently) it is
marked as reserved.
Maybe some ASPEED folks can enlighten us what Bits 14 till 16 were used for on
A0-A2, and if they are relevant for PCIe INTx/MSI routing on the older silicons.
Tan
--
Tan Siewert <tan.siewert@9elements.com>
On 08. 07. 26, 17:35, Grégoire Layet wrote:
...
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -32,6 +32,26 @@
> #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)
So is this
FIELD_PREP(ASPEED_SCUC24_MSI_ROUTING_MASK, 2)
?
> +#define ASPEED_SCUC24_PCIDEV1_INTX_MSI_HOST2BMC_EN BIT(18)
> +#define ASPEED_SCUC24_PCIDEV1_INTX_MSI_SCU560_EN BIT(17)
Perhaps switch the two (to be in asc order)? And define 14 as _RESERVED
as well?
> +#define ASPEED_SCU_PCIE_CONF_CTRL 0xC20
Hmm, should these go before 0xC24?
> +#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 +432,63 @@ static int aspeed_vuart_map_irq_polarity(u32 dt)
> }
> }
>
> +static int aspeed_ast2600_vuart_over_pci_set_enabled(struct platform_device *pdev)
> +{
...
> + 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.
> + */
If you defined reserved as suggested above, no need for the comment.
> + 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;
> +}
> +
thanks,
--
js
suse labs
Hi Jiri, On Thu, 9 Jul 2026 at 07:17, Jiri Slaby <jirislaby@kernel.org> wrote: > > On 08. 07. 26, 17:35, Grégoire Layet wrote: > ... > > --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c > > +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c > > @@ -32,6 +32,26 @@ > > #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) > > So is this > FIELD_PREP(ASPEED_SCUC24_MSI_ROUTING_MASK, 2) > ? Yes, replaced > > > +#define ASPEED_SCUC24_PCIDEV1_INTX_MSI_HOST2BMC_EN BIT(18) > > +#define ASPEED_SCUC24_PCIDEV1_INTX_MSI_SCU560_EN BIT(17) > > Perhaps switch the two (to be in asc order)? And define 14 as _RESERVED > as well? Acknowledged. I have also removed the comment afterwards. > > +#define ASPEED_SCU_PCIE_CONF_CTRL 0xC20 > > Hmm, should these go before 0xC24? Yes Thanks for the feedback, I have made the changes for the next revision. Regards, Grégoire
> + 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. > + */ > + 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); checkpatch should be warning about these long lines. Traditionally, the limit is 80 character lines, but recently 100 has been accepted by some subsystems. The exception is when wrapping the lines will make them less readable, but i don't think that applies here. Andrew
On 08. 07. 26, 18:46, Andrew Lunn wrote: >> + 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. >> + */ >> + 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); > > checkpatch should be warning about these long lines. Traditionally, > the limit is 80 character lines, but recently 100 has been accepted by > some subsystems. The exception is when wrapping the lines will make > them less readable, but i don't think that applies here. FWIW 100 is fine by me. 80 is too ancient limit. -- js suse labs
On Thu, 9 Jul 2026 at 07:19, Jiri Slaby <jirislaby@kernel.org> wrote: > > On 08. 07. 26, 18:46, Andrew Lunn wrote: > >> + 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. > >> + */ > >> + 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); > > > > checkpatch should be warning about these long lines. Traditionally, > > the limit is 80 character lines, but recently 100 has been accepted by > > some subsystems. The exception is when wrapping the lines will make > > them less readable, but i don't think that applies here. > > FWIW 100 is fine by me. 80 is too ancient limit. Modified to fit 100 characters-per-line limit. No checkpatch warning. Regards, Grégoire
© 2016 - 2026 Red Hat, Inc.