soc: aspeed: Add BMC and host driver for PCIe BMC device

The ast2600 was using the ast2500 vuart compatible string. Make it possible to have ast2600-specific properties.

Signed-off-by Grégoire Layet <[email protected]>

---
 .../devicetree/bindings/serial/8250.yaml      | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

Documentation/devicetree/bindings/serial/8250.yaml +7 -1

index bb7b9c87a807..8f22121381fc 100644
@@ -23,7 +23,10 @@ allOf:
     then:
       properties:
         compatible:
-          const: aspeed,ast2500-vuart
+          contains:
+            enum:
+              - aspeed,ast2500-vuart
+              - aspeed,ast2600-vuart
   - if:
       properties:
         compatible:

@@ -106,6 +109,9 @@ properties:
       - const: ns16850
       - const: aspeed,ast2400-vuart
       - const: aspeed,ast2500-vuart
+      - items:
+          - const: aspeed,ast2600-vuart
+          - const: aspeed,ast2500-vuart
       - const: intel,xscale-uart
       - const: mrvl,pxa-uart
       - const: nuvoton,wpcm450-uart

Quoted 2 lines · Grégoire Layet

The ast2600 was using the ast2500 vuart compatible string. Make it possible to have ast2600-specific properties.

Then add these properties here as well. Adding a new device is one
commit: its compatible and its properties.

Documentation/devicetree/bindings/serial/8250.yaml properties: · line 296

Quoted 8 lines

aspeed,sirq-polarity-sense:
$ref: /schemas/types.yaml#/definitions/phandle-array
description: |

Documentation/devicetree/bindings/serial/8250.yaml +1 -4

-      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

This is a deprecated property, so it cannot apply to a new device - aspeed,ast2600-vuart. You cannot use deprecated code for new bindings or device support.

The ASPEED AST2600 has 2 Virtual UARTs accessible over PCI. The ASPEED AST2600 can be used as a PCI device.
2 Virtual UART can be exposed to the host via this PCI device. These are 8250-compatible register sets and can be used to have UART communication between the PCI BMC and the host.

This boolean can be set to specify if a VUART is used over PCI. A VUART over PCI needs a syscon phandle. The syscon gives a regmap to the SCU, which is used to set the enable bits of the PCI device.
On the aspeed's chips, the clocks phandle already points at the SCU node. But deriving the regmap from the clock provider would misuse the clocks binding. An explicit syscon phandle states the dependency directly.

Signed-off-by Grégoire Layet <[email protected]>

---
 .../devicetree/bindings/serial/8250.yaml      | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

Documentation/devicetree/bindings/serial/8250.yaml +0 -0

index 8f22121381fc..2ea8981bba80 100644
@@ -27,6 +27,16 @@ allOf:
             enum:
               - aspeed,ast2500-vuart
               - aspeed,ast2600-vuart

Documentation/devicetree/bindings/serial/8250.yaml +11 -0

+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to the SCU syscon, used to enable the VUART over the BMC PCI
+      device. Only applicable to aspeed,ast2600-vuart.
+
   dmas:
     minItems: 1
     maxItems: 4

@@ -316,6 +332,12 @@ 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
+    description:
+      Enable the VUART over the BMC PCI device. Only applicable to
+      aspeed,ast2600-vuart.
+
 required:
   - reg
   - interrupts

Signature · 1 line

2.54.0

Quoted 11 lines

...

Documentation/devicetree/bindings/serial/8250.yaml +6 -0

@@ -27,6 +27,16 @@ allOf:
             enum:
               - aspeed,ast2500-vuart
               - aspeed,ast2600-vuart
+  - if:
+      required:
+        - aspeed,vuart-over-pci
+    then:
+      required:
+        - syscon

Documentation/devicetree/bindings/serial/8250.yaml allOf: · line 39

Quoted 4 lines

Documentation/devicetree/bindings/serial/8250.yaml +4 -0

+      properties:
+        compatible:
+          contains:
+            const: aspeed,ast2600-vuart

else:
...
disallow new properties (see example-schema)

Quoted 8 lines

Documentation/devicetree/bindings/serial/8250.yaml +0 -0

   - if:
       properties:
         compatible:

@@ -223,6 +233,12 @@ properties:
           - const: uartclk
           - const: reg
 

Documentation/devicetree/bindings/serial/8250.yaml properties: · line 236

Quoted 1 line

Documentation/devicetree/bindings/serial/8250.yaml +1 -0

+  syscon:

That's explicitly forbidden by writing bindings (and mentioned as the antipattern in my older talks about DT).

Documentation/devicetree/bindings/serial/8250.yaml properties: · line 241

Quoted 6 lines

Documentation/devicetree/bindings/serial/8250.yaml +5 -0

+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to the SCU syscon, used to enable the VUART over the BMC PCI
+      device. Only applicable to aspeed,ast2600-vuart.
+
...

Quoted 8 lines

drivers/tty/serial/8250/8250_aspeed_vuart.c +8 -0

+	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.

drivers/tty/serial/8250/8250_aspeed_vuart.c static int aspeed_vuart_map_irq_polarity(u32 dt) · line 483

Quoted 3 lines

drivers/tty/serial/8250/8250_aspeed_vuart.c +3 -0

+					 */
+					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.

Quoted 12 lines · Andrew Lunn

Quoted 11 lines

drivers/tty/serial/8250/8250_aspeed_vuart.c +11 -0

+    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);

Quoted 4 lines

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.

drivers/tty/serial/8250/8250_aspeed_vuart.c +12 -0

+#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)
+{

On 08. 07. 26, 17:35, Grégoire Layet wrote:
...

Quoted 6 lines

drivers/tty/serial/8250/8250_aspeed_vuart.c +0 -0

@@ -32,6 +32,26 @@
  #define ASPEED_VUART_DEFAULT_SIRQ   4
  #define ASPEED_VUART_DEFAULT_SIRQ_POLARITY  IRQ_TYPE_LEVEL_LOW
 

drivers/tty/serial/8250/8250_aspeed_vuart.c line 40

Quoted 6 lines

drivers/tty/serial/8250/8250_aspeed_vuart.c +6 -0

+#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)
?

drivers/tty/serial/8250/8250_aspeed_vuart.c line 42

Quoted 3 lines

drivers/tty/serial/8250/8250_aspeed_vuart.c +2 -0

+#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?

drivers/tty/serial/8250/8250_aspeed_vuart.c line 44

Quoted 1 line

drivers/tty/serial/8250/8250_aspeed_vuart.c +1 -0

+#define ASPEED_SCU_PCIE_CONF_CTRL    0xC20

Hmm, should these go before 0xC24?

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.

drivers/tty/serial/8250/8250_aspeed_vuart.c +8 -0

+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 |

drivers/tty/serial/8250/8250_aspeed_vuart.c +19 -0

+		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;
+	}

Add support for VUART over PCIe between BMC and host. Add the host side driver.
Support only the AST2600.

Taken from ASPEED 6.18 Kernel SDK and trimmed down.

drivers/soc/aspeed/aspeed-host-bmc-dev.c new file +8 -0

@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (C) ASPEED Technology Inc.
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/pci.h>

drivers/soc/aspeed/aspeed-host-bmc-dev.c new file +11 -0

+struct aspeed_pci_bmc_dev {
+	unsigned long message_bar_base;
+
+	struct uart_8250_port uart[2];
+	int uart_line[2];
+};
+
+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;

drivers/soc/aspeed/aspeed-host-bmc-dev.c new file +22 -0

+	uart->port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_SHARE_IRQ;
+	uart->port.uartclk = 115200 * 16;
+	uart->port.irq = pci_irq_vector(pdev, vuart_msi_index[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;
+}

drivers/soc/aspeed/aspeed-host-bmc-dev.c new file +8 -0

+		.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,

drivers/soc/aspeed/aspeed-host-bmc-dev.c line 18

Quoted 4 lines

...

drivers/soc/aspeed/aspeed-host-bmc-dev.c new file +3 -0

+
+enum aspeed_platform_id {
+     ASPEED,

What is this good for?

drivers/soc/aspeed/aspeed-host-bmc-dev.c line 22

Quoted 5 lines

drivers/soc/aspeed/aspeed-host-bmc-dev.c new file +4 -0

+};
+
+static const int vuart_msi_index[2] = { 16, 17 };
+static const int vuart_port_addr[2] = {0x3f8, 0x2f8};

Sort of inconsistent spaces. Both arrays should be unsigned anyway. And for the latter, u16 should be enough.

drivers/soc/aspeed/aspeed-host-bmc-dev.c line 44

Quoted 5 lines

...

drivers/soc/aspeed/aspeed-host-bmc-dev.c new file +4 -0

+
+     vuart_ioport = vuart_port_addr[idx];
+     /* ASPEED BMC device shift addresses by 2 to the left */
+     vuart_ioport = vuart_ioport << 2;

Simply:
vuart_ioport <<= 2;
? Or join the two lines?

drivers/soc/aspeed/aspeed-host-bmc-dev.c line 156

Quoted 4 lines

...

drivers/soc/aspeed/aspeed-host-bmc-dev.c new file +3 -0

+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,

PCI_CLASS_OTHERS << 16

Quoted 1 line

...

thanks,
--
js
suse labs

Add support for VUART over PCIe between BMC and host. Add the host side driver.
Support only the AST2600.

Taken from ASPEED 6.18 Kernel SDK and trimmed down.

The host can't detect the VUART addresses, so force them to 0x3f8 and 0x2f8, as in the initial ASPEED driver.

Change the MSI vector index of 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.

This module is added in soc/aspeed as it's very soc specific. This is not added as a PCI 8250 UART device as this host module can be expanded upon for IPMI over KCS. It can also be used in the future for custom BMC<->host communication with shared memory and doorbell.

This host module should be the entry point for setting up all features related to an AST2600 present on the PCI bus.

Signed-off-by Jacky Chou <[email protected]>

Signed-off-by aspeedyh <[email protected]>

Signed-off-by Grégoire Layet <[email protected]>

drivers/soc/aspeed/Kconfig +0 -0

index f579ee0b5afa..147a9033bdc4 100644

drivers/soc/aspeed/aspeed-host-bmc-dev.c new file +9 -0

+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");

Use the ast2600 compatible string.
Make it more precise and enable specific ast2600 properties. Still use the ast2500 compatible string as a fallback.

Signed-off-by Grégoire Layet <[email protected]>

arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +0 -0

index 56bb3b0444f7..7c02633f2bd6 100644