* [PATCH v4 2/8] drivers/soc: Add Aspeed XDMA Engine Driver
From: Eddie James @ 2019-07-01 19:53 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1562010839-1113-1-git-send-email-eajames@linux.ibm.com>
The XDMA engine embedded in the AST2500 SOC performs PCI DMA operations
between the SOC (acting as a BMC) and a host processor in a server.
This commit adds a driver to control the XDMA engine and adds functions
to initialize the hardware and memory and start DMA operations.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
MAINTAINERS | 9 +
drivers/soc/aspeed/Kconfig | 8 +
drivers/soc/aspeed/Makefile | 1 +
drivers/soc/aspeed/aspeed-xdma.c | 528 +++++++++++++++++++++++++++++++++++++++
include/uapi/linux/aspeed-xdma.h | 26 ++
5 files changed, 572 insertions(+)
create mode 100644 drivers/soc/aspeed/aspeed-xdma.c
create mode 100644 include/uapi/linux/aspeed-xdma.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 0685c8a..b1a4b50 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2612,6 +2612,15 @@ S: Maintained
F: drivers/media/platform/aspeed-video.c
F: Documentation/devicetree/bindings/media/aspeed-video.txt
+ASPEED XDMA ENGINE DRIVER
+M: Eddie James <eajames@linux.ibm.com>
+L: linux-aspeed at lists.ozlabs.org (moderated for non-subscribers)
+L: linux-kernel at vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/misc/aspeed,xdma.txt
+F: drivers/soc/aspeed/aspeed-xdma.c
+F: include/uapi/linux/aspeed-xdma.h
+
ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
M: Corentin Chary <corentin.chary@gmail.com>
L: acpi4asus-user at lists.sourceforge.net
diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
index 323e177..522b079 100644
--- a/drivers/soc/aspeed/Kconfig
+++ b/drivers/soc/aspeed/Kconfig
@@ -29,4 +29,12 @@ config ASPEED_P2A_CTRL
ioctl()s, the driver also provides an interface for userspace mappings to
a pre-defined region.
+config ASPEED_XDMA
+ tristate "Aspeed XDMA Engine Driver"
+ depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON && HAS_DMA
+ help
+ Enable support for the Aspeed XDMA Engine found on the Aspeed AST2500
+ SOC. The XDMA engine can perform automatic PCI DMA operations between
+ the AST2500 (acting as a BMC) and a host processor.
+
endmenu
diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
index b64be47..977b046 100644
--- a/drivers/soc/aspeed/Makefile
+++ b/drivers/soc/aspeed/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
+obj-$(CONFIG_ASPEED_XDMA) += aspeed-xdma.o
diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
new file mode 100644
index 0000000..a8dabcb
--- /dev/null
+++ b/drivers/soc/aspeed/aspeed-xdma.c
@@ -0,0 +1,528 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright IBM Corp 2019
+
+#include <linux/aspeed-xdma.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/fs.h>
+#include <linux/genalloc.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/list.h>
+#include <linux/mfd/syscon.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/poll.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
+#include <linux/wait.h>
+
+#define DEVICE_NAME "aspeed-xdma"
+
+#define SCU_STRAP 0x070
+#define SCU_STRAP_VGA_MEM GENMASK(3, 2)
+
+#define SCU_PCIE_CONF 0x180
+#define SCU_PCIE_CONF_VGA_EN BIT(0)
+#define SCU_PCIE_CONF_VGA_EN_MMIO BIT(1)
+#define SCU_PCIE_CONF_VGA_EN_LPC BIT(2)
+#define SCU_PCIE_CONF_VGA_EN_MSI BIT(3)
+#define SCU_PCIE_CONF_VGA_EN_MCTP BIT(4)
+#define SCU_PCIE_CONF_VGA_EN_IRQ BIT(5)
+#define SCU_PCIE_CONF_VGA_EN_DMA BIT(6)
+#define SCU_PCIE_CONF_BMC_EN BIT(8)
+#define SCU_PCIE_CONF_BMC_EN_MMIO BIT(9)
+#define SCU_PCIE_CONF_BMC_EN_MSI BIT(11)
+#define SCU_PCIE_CONF_BMC_EN_MCTP BIT(12)
+#define SCU_PCIE_CONF_BMC_EN_IRQ BIT(13)
+#define SCU_PCIE_CONF_BMC_EN_DMA BIT(14)
+#define SCU_PCIE_CONF_RSVD GENMASK(19, 18)
+
+#define SCU_BMC_CLASS_REV 0x19c
+#define SCU_BMC_CLASS_REV_XDMA 0xff000001
+
+#define SDMC_CONF 0x004
+#define SDMC_CONF_MEM GENMASK(1, 0)
+#define SDMC_REMAP 0x008
+#define SDMC_REMAP_MAGIC GENMASK(17, 16)
+
+#define XDMA_CMD_SIZE 4
+#define XDMA_CMDQ_SIZE PAGE_SIZE
+#define XDMA_BYTE_ALIGN 16
+#define XDMA_MAX_LINE_SIZE BIT(10)
+#define XDMA_NUM_CMDS \
+ (XDMA_CMDQ_SIZE / sizeof(struct aspeed_xdma_cmd))
+#define XDMA_NUM_DEBUGFS_REGS 28
+
+/* Aspeed specification requires 10ms after switching the reset line */
+#define XDMA_RESET_TIME_MS 10
+
+#define XDMA_CMD_BMC_CHECK BIT(0)
+#define XDMA_CMD_BMC_ADDR GENMASK(29, 4)
+#define XDMA_CMD_BMC_DIR_US BIT(31)
+
+#define XDMA_CMD_COMM1_HI_HOST_PITCH GENMASK(14, 3)
+#define XDMA_CMD_COMM1_HI_BMC_PITCH GENMASK(30, 19)
+
+#define XDMA_CMD_CONF_CHECK BIT(1)
+#define XDMA_CMD_CONF_LINE_SIZE GENMASK(14, 4)
+#define XDMA_CMD_CONF_IRQ_BMC BIT(15)
+#define XDMA_CMD_CONF_NUM_LINES GENMASK(27, 16)
+#define XDMA_CMD_CONF_IRQ BIT(31)
+
+#define XDMA_CMD_ID_UPDIR GENMASK(17, 16)
+#define XDMA_CMD_ID_UPDIR_BMC 0
+#define XDMA_CMD_ID_UPDIR_HOST 1
+#define XDMA_CMD_ID_UPDIR_VGA 2
+
+#define XDMA_DS_PCIE_REQ_SIZE_128 0
+#define XDMA_DS_PCIE_REQ_SIZE_256 1
+#define XDMA_DS_PCIE_REQ_SIZE_512 2
+#define XDMA_DS_PCIE_REQ_SIZE_1K 3
+#define XDMA_DS_PCIE_REQ_SIZE_2K 4
+#define XDMA_DS_PCIE_REQ_SIZE_4K 5
+
+#define XDMA_HOST_CMD_QUEUE_ADDR0 0x00
+#define XDMA_HOST_CMD_QUEUE_ENDP 0x04
+#define XDMA_HOST_CMD_QUEUE_WRITEP 0x08
+#define XDMA_HOST_CMD_QUEUE_READP 0x0c
+#define XDMA_BMC_CMD_QUEUE_ADDR 0x10
+#define XDMA_BMC_CMD_QUEUE_ENDP 0x14
+#define XDMA_BMC_CMD_QUEUE_WRITEP 0x18
+#define XDMA_BMC_CMD_QUEUE_READP 0x1c
+#define XDMA_BMC_CMD_QUEUE_READP_MAGIC 0xee882266
+#define XDMA_CTRL 0x20
+#define XDMA_CTRL_US_COMP BIT(4)
+#define XDMA_CTRL_DS_COMP BIT(5)
+#define XDMA_CTRL_DS_DIRTY BIT(6)
+#define XDMA_CTRL_DS_PCIE_REQ_SIZE GENMASK(19, 17)
+#define XDMA_CTRL_DS_DATA_TIMEOUT BIT(28)
+#define XDMA_CTRL_DS_CHECK_ID BIT(29)
+#define XDMA_STATUS 0x24
+#define XDMA_STATUS_US_COMP BIT(4)
+#define XDMA_STATUS_DS_COMP BIT(5)
+#define XDMA_DS_FRAME_SIZE 0x28
+#define XDMA_PROBE_DS_PCIE 0x30
+#define XDMA_PROBE_US_PCIE 0x34
+#define XDMA_INPRG_DS_CMD1 0x38
+#define XDMA_INPRG_DS_CMD2 0x3c
+#define XDMA_INPRG_US_CMD00 0x40
+#define XDMA_INPRG_US_CMD01 0x44
+#define XDMA_INPRG_US_CMD10 0x48
+#define XDMA_INPRG_US_CMD11 0x4c
+#define XDMA_INPRG_US_CMD20 0x50
+#define XDMA_INPRG_US_CMD21 0x54
+#define XDMA_HOST_CMD_QUEUE_ADDR1 0x60
+#define XDMA_VGA_CMD_QUEUE_ADDR0 0x64
+#define XDMA_VGA_CMD_QUEUE_ENDP 0x68
+#define XDMA_VGA_CMD_QUEUE_WRITEP 0x6c
+#define XDMA_VGA_CMD_QUEUE_READP 0x70
+#define XDMA_VGA_CMD_STATUS 0x74
+#define XDMA_VGA_CMD_QUEUE_ADDR1 0x78
+
+enum {
+ XDMA_IN_PRG,
+ XDMA_UPSTREAM,
+};
+
+struct aspeed_xdma_cmd {
+ u32 host_addr_lo;
+ u32 host_addr_hi;
+ u32 bmc_addr;
+ u32 comm1_hi;
+ u32 conf;
+ u32 id;
+ u32 resv0;
+ u32 resv1;
+};
+
+struct aspeed_xdma_client;
+
+struct aspeed_xdma {
+ struct device *dev;
+ void __iomem *base;
+ struct regmap *scu;
+ struct reset_control *reset;
+
+ unsigned long flags;
+ unsigned int cmd_idx;
+ wait_queue_head_t wait;
+ struct aspeed_xdma_client *current_client;
+
+ u32 vga_phys;
+ u32 vga_size;
+ void *cmdq;
+ void __iomem *vga_virt;
+ dma_addr_t cmdq_vga_phys;
+ void *cmdq_vga_virt;
+ struct gen_pool *vga_pool;
+};
+
+struct aspeed_xdma_client {
+ struct aspeed_xdma *ctx;
+
+ unsigned long flags;
+ void *virt;
+ dma_addr_t phys;
+ u32 size;
+};
+
+static const u32 aspeed_xdma_bmc_pcie_conf = SCU_PCIE_CONF_BMC_EN |
+ SCU_PCIE_CONF_BMC_EN_MSI | SCU_PCIE_CONF_BMC_EN_MCTP |
+ SCU_PCIE_CONF_BMC_EN_IRQ | SCU_PCIE_CONF_BMC_EN_DMA |
+ SCU_PCIE_CONF_RSVD;
+
+static const u32 aspeed_xdma_vga_pcie_conf = SCU_PCIE_CONF_VGA_EN |
+ SCU_PCIE_CONF_VGA_EN_MSI | SCU_PCIE_CONF_VGA_EN_MCTP |
+ SCU_PCIE_CONF_VGA_EN_IRQ | SCU_PCIE_CONF_VGA_EN_DMA |
+ SCU_PCIE_CONF_RSVD;
+
+static void aspeed_scu_pcie_write(struct aspeed_xdma *ctx, u32 conf)
+{
+ u32 v = 0;
+
+ regmap_write(ctx->scu, SCU_PCIE_CONF, conf);
+ regmap_read(ctx->scu, SCU_PCIE_CONF, &v);
+
+ dev_dbg(ctx->dev, "write scu pcie_conf[%08x]\n", v);
+}
+
+static u32 aspeed_xdma_reg_read(struct aspeed_xdma *ctx, u32 reg)
+{
+ u32 v = readl(ctx->base + reg);
+
+ dev_dbg(ctx->dev, "read %02x[%08x]\n", reg, v);
+ return v;
+}
+
+static void aspeed_xdma_reg_write(struct aspeed_xdma *ctx, u32 reg, u32 val)
+{
+ writel(val, ctx->base + reg);
+ dev_dbg(ctx->dev, "write %02x[%08x]\n", reg, readl(ctx->base + reg));
+}
+
+static void aspeed_xdma_init_eng(struct aspeed_xdma *ctx)
+{
+ const u32 ctrl = XDMA_CTRL_US_COMP | XDMA_CTRL_DS_COMP |
+ XDMA_CTRL_DS_DIRTY | FIELD_PREP(XDMA_CTRL_DS_PCIE_REQ_SIZE,
+ XDMA_DS_PCIE_REQ_SIZE_256) |
+ XDMA_CTRL_DS_DATA_TIMEOUT | XDMA_CTRL_DS_CHECK_ID;
+
+ aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_ENDP,
+ XDMA_CMD_SIZE * XDMA_NUM_CMDS);
+ aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_READP,
+ XDMA_BMC_CMD_QUEUE_READP_MAGIC);
+ aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_WRITEP, 0);
+ aspeed_xdma_reg_write(ctx, XDMA_CTRL, ctrl);
+
+ aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_ADDR,
+ ctx->cmdq_vga_phys);
+
+ ctx->cmd_idx = 0;
+ ctx->flags = 0;
+}
+
+static void aspeed_xdma_reset(struct aspeed_xdma *ctx)
+{
+ reset_control_assert(ctx->reset);
+
+ msleep(XDMA_RESET_TIME_MS);
+
+ reset_control_deassert(ctx->reset);
+
+ msleep(XDMA_RESET_TIME_MS);
+
+ aspeed_xdma_init_eng(ctx);
+}
+
+static void aspeed_xdma_start(struct aspeed_xdma *ctx,
+ struct aspeed_xdma_op *op, u32 bmc_addr)
+{
+ u32 conf = XDMA_CMD_CONF_CHECK | XDMA_CMD_CONF_IRQ_BMC |
+ XDMA_CMD_CONF_IRQ;
+ unsigned int line_size = op->len / XDMA_BYTE_ALIGN;
+ unsigned int num_lines = 1;
+ unsigned int nidx = (ctx->cmd_idx + 1) % XDMA_NUM_CMDS;
+ unsigned int pitch = 1;
+ struct aspeed_xdma_cmd *cmd =
+ &(((struct aspeed_xdma_cmd *)ctx->cmdq)[ctx->cmd_idx]);
+
+ if (line_size > XDMA_MAX_LINE_SIZE) {
+ unsigned int rem;
+ unsigned int total;
+
+ num_lines = line_size / XDMA_MAX_LINE_SIZE;
+ total = XDMA_MAX_LINE_SIZE * num_lines;
+ rem = line_size - total;
+ line_size = XDMA_MAX_LINE_SIZE;
+ pitch = line_size;
+
+ if (rem) {
+ unsigned int offs = total * XDMA_BYTE_ALIGN;
+ u32 r_bmc_addr = bmc_addr + offs;
+ u64 r_host_addr = op->host_addr + (u64)offs;
+ struct aspeed_xdma_cmd *r_cmd =
+ &(((struct aspeed_xdma_cmd *)ctx->cmdq)[nidx]);
+
+ r_cmd->host_addr_lo =
+ (u32)(r_host_addr & 0xFFFFFFFFULL);
+ r_cmd->host_addr_hi = (u32)(r_host_addr >> 32ULL);
+ r_cmd->bmc_addr = (r_bmc_addr & XDMA_CMD_BMC_ADDR) |
+ XDMA_CMD_BMC_CHECK |
+ (op->upstream ? XDMA_CMD_BMC_DIR_US : 0);
+ r_cmd->conf = conf |
+ FIELD_PREP(XDMA_CMD_CONF_LINE_SIZE, rem) |
+ FIELD_PREP(XDMA_CMD_CONF_NUM_LINES, 1);
+ r_cmd->comm1_hi =
+ FIELD_PREP(XDMA_CMD_COMM1_HI_HOST_PITCH, 1) |
+ FIELD_PREP(XDMA_CMD_COMM1_HI_BMC_PITCH, 1);
+
+ /* do not trigger IRQ for first command */
+ conf = XDMA_CMD_CONF_CHECK;
+
+ nidx = (nidx + 1) % XDMA_NUM_CMDS;
+ }
+
+ /* undocumented formula to get required number of lines */
+ num_lines = (num_lines * 2) - 1;
+ }
+
+ /* ctrl == 0 indicates engine hasn't started properly; restart it */
+ if (!aspeed_xdma_reg_read(ctx, XDMA_CTRL))
+ aspeed_xdma_reset(ctx);
+
+ cmd->host_addr_lo = (u32)(op->host_addr & 0xFFFFFFFFULL);
+ cmd->host_addr_hi = (u32)(op->host_addr >> 32ULL);
+ cmd->bmc_addr = (bmc_addr & XDMA_CMD_BMC_ADDR) | XDMA_CMD_BMC_CHECK |
+ (op->upstream ? XDMA_CMD_BMC_DIR_US : 0);
+ cmd->conf = conf |
+ FIELD_PREP(XDMA_CMD_CONF_LINE_SIZE, line_size) |
+ FIELD_PREP(XDMA_CMD_CONF_NUM_LINES, num_lines);
+ cmd->comm1_hi = FIELD_PREP(XDMA_CMD_COMM1_HI_HOST_PITCH, pitch) |
+ FIELD_PREP(XDMA_CMD_COMM1_HI_BMC_PITCH, pitch);
+
+ memcpy(ctx->cmdq_vga_virt, ctx->cmdq, XDMA_CMDQ_SIZE);
+
+ if (op->upstream)
+ set_bit(XDMA_UPSTREAM, &ctx->flags);
+ else
+ clear_bit(XDMA_UPSTREAM, &ctx->flags);
+
+ set_bit(XDMA_IN_PRG, &ctx->flags);
+
+ aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_WRITEP,
+ nidx * XDMA_CMD_SIZE);
+ ctx->cmd_idx = nidx;
+}
+
+static void aspeed_xdma_done(struct aspeed_xdma *ctx)
+{
+ if (ctx->current_client) {
+ clear_bit(XDMA_IN_PRG, &ctx->current_client->flags);
+
+ ctx->current_client = NULL;
+ }
+
+ clear_bit(XDMA_IN_PRG, &ctx->flags);
+ wake_up_interruptible_all(&ctx->wait);
+}
+
+static irqreturn_t aspeed_xdma_irq(int irq, void *arg)
+{
+ struct aspeed_xdma *ctx = arg;
+ u32 status = aspeed_xdma_reg_read(ctx, XDMA_STATUS);
+
+ if (status & XDMA_STATUS_US_COMP) {
+ if (test_bit(XDMA_UPSTREAM, &ctx->flags))
+ aspeed_xdma_done(ctx);
+ }
+
+ if (status & XDMA_STATUS_DS_COMP) {
+ if (!test_bit(XDMA_UPSTREAM, &ctx->flags))
+ aspeed_xdma_done(ctx);
+ }
+
+ aspeed_xdma_reg_write(ctx, XDMA_STATUS, status);
+
+ return IRQ_HANDLED;
+}
+
+static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
+{
+ int rc;
+ u32 scu_conf = 0;
+ u32 mem_size = 0x20000000;
+ const u32 mem_sizes[4] = { 0x8000000, 0x10000000, 0x20000000,
+ 0x40000000 };
+ const u32 vga_sizes[4] = { 0x800000, 0x1000000, 0x2000000, 0x4000000 };
+ void __iomem *sdmc_base = ioremap(0x1e6e0000, 0x100);
+
+ aspeed_scu_pcie_write(ctx, aspeed_xdma_bmc_pcie_conf);
+
+ regmap_write(ctx->scu, SCU_BMC_CLASS_REV, SCU_BMC_CLASS_REV_XDMA);
+
+ /*
+ * Calculate the VGA memory size and physical address from the SCU and
+ * memory controller registers.
+ */
+ regmap_read(ctx->scu, SCU_STRAP, &scu_conf);
+ ctx->vga_size = vga_sizes[FIELD_GET(SCU_STRAP_VGA_MEM, scu_conf)];
+
+ if (sdmc_base) {
+ u32 sdmc = readl(sdmc_base + SDMC_CONF);
+ u32 remap = readl(sdmc_base + SDMC_REMAP);
+
+ remap |= SDMC_REMAP_MAGIC;
+ writel(remap, sdmc_base + SDMC_REMAP);
+ remap = readl(sdmc_base + SDMC_REMAP);
+
+ mem_size = mem_sizes[sdmc & SDMC_CONF_MEM];
+ iounmap(sdmc_base);
+ }
+
+ ctx->vga_phys = (mem_size - ctx->vga_size) + 0x80000000;
+
+ ctx->cmdq = devm_kzalloc(ctx->dev, XDMA_CMDQ_SIZE, GFP_KERNEL);
+ if (!ctx->cmdq) {
+ dev_err(ctx->dev, "Failed to allocate command queue.\n");
+ return -ENOMEM;
+ }
+
+ ctx->vga_virt = ioremap(ctx->vga_phys, ctx->vga_size);
+ if (!ctx->vga_virt) {
+ dev_err(ctx->dev, "Failed to ioremap VGA memory.\n");
+ return -ENOMEM;
+ }
+
+ rc = gen_pool_add_virt(ctx->vga_pool, (unsigned long)ctx->vga_virt,
+ ctx->vga_phys, ctx->vga_size, -1);
+ if (rc) {
+ dev_err(ctx->dev, "Failed to add memory to genalloc pool.\n");
+ iounmap(ctx->vga_virt);
+ return rc;
+ }
+
+ ctx->cmdq_vga_virt = gen_pool_dma_alloc(ctx->vga_pool, XDMA_CMDQ_SIZE,
+ &ctx->cmdq_vga_phys);
+ if (!ctx->cmdq_vga_virt) {
+ dev_err(ctx->dev, "Failed to genalloc cmdq.\n");
+ iounmap(ctx->vga_virt);
+ return -ENOMEM;
+ }
+
+ dev_dbg(ctx->dev, "VGA mapped at phys[%08x], size[%08x].\n",
+ ctx->vga_phys, ctx->vga_size);
+
+ return 0;
+}
+
+static int aspeed_xdma_probe(struct platform_device *pdev)
+{
+ int irq;
+ int rc;
+ struct resource *res;
+ struct device *dev = &pdev->dev;
+ struct aspeed_xdma *ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->dev = dev;
+ platform_set_drvdata(pdev, ctx);
+ init_waitqueue_head(&ctx->wait);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ ctx->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(ctx->base)) {
+ dev_err(dev, "Unable to ioremap registers.\n");
+ return PTR_ERR(ctx->base);
+ }
+
+ irq = irq_of_parse_and_map(dev->of_node, 0);
+ if (!irq) {
+ dev_err(dev, "Unable to find IRQ.\n");
+ return -ENODEV;
+ }
+
+ rc = devm_request_irq(dev, irq, aspeed_xdma_irq, IRQF_SHARED,
+ DEVICE_NAME, ctx);
+ if (rc < 0) {
+ dev_err(dev, "Unable to request IRQ %d.\n", irq);
+ return rc;
+ }
+
+ ctx->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
+ if (IS_ERR(ctx->scu)) {
+ dev_err(ctx->dev, "Unable to grab SCU regs.\n");
+ return PTR_ERR(ctx->scu);
+ }
+
+ ctx->reset = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(ctx->reset)) {
+ dev_err(dev, "Unable to request reset control.\n");
+ return PTR_ERR(ctx->reset);
+ }
+
+ ctx->vga_pool = devm_gen_pool_create(dev, ilog2(PAGE_SIZE), -1, NULL);
+ if (!ctx->vga_pool) {
+ dev_err(dev, "Unable to setup genalloc pool.\n");
+ return -ENOMEM;
+ }
+
+ reset_control_deassert(ctx->reset);
+
+ msleep(XDMA_RESET_TIME_MS);
+
+ rc = aspeed_xdma_init_mem(ctx);
+ if (rc) {
+ reset_control_assert(ctx->reset);
+ return rc;
+ }
+
+ aspeed_xdma_init_eng(ctx);
+
+ return 0;
+}
+
+static int aspeed_xdma_remove(struct platform_device *pdev)
+{
+ struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
+
+ gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
+ XDMA_CMDQ_SIZE);
+ iounmap(ctx->vga_virt);
+ reset_control_assert(ctx->reset);
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_xdma_match[] = {
+ { .compatible = "aspeed,ast2500-xdma" },
+ { },
+};
+
+static struct platform_driver aspeed_xdma_driver = {
+ .probe = aspeed_xdma_probe,
+ .remove = aspeed_xdma_remove,
+ .driver = {
+ .name = DEVICE_NAME,
+ .of_match_table = aspeed_xdma_match,
+ },
+};
+
+module_platform_driver(aspeed_xdma_driver);
+
+MODULE_AUTHOR("Eddie James");
+MODULE_DESCRIPTION("Aspeed XDMA Engine Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/uapi/linux/aspeed-xdma.h b/include/uapi/linux/aspeed-xdma.h
new file mode 100644
index 0000000..998459e
--- /dev/null
+++ b/include/uapi/linux/aspeed-xdma.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright IBM Corp 2019 */
+
+#ifndef _UAPI_LINUX_ASPEED_XDMA_H_
+#define _UAPI_LINUX_ASPEED_XDMA_H_
+
+#include <linux/types.h>
+
+/*
+ * aspeed_xdma_op
+ *
+ * host_addr: the DMA address on the host side, typically configured by PCI
+ * subsystem
+ *
+ * len: the size of the transfer in bytes; it should be a multiple of 16 bytes
+ *
+ * upstream: boolean indicating the direction of the DMA operation; upstream
+ * means a transfer from the BMC to the host
+ */
+struct aspeed_xdma_op {
+ __u64 host_addr;
+ __u32 len;
+ __u32 upstream;
+};
+
+#endif /* _UAPI_LINUX_ASPEED_XDMA_H_ */
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 1/8] dt-bindings: soc: Add Aspeed XDMA engine binding documentation
From: Eddie James @ 2019-07-01 19:53 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1562010839-1113-1-git-send-email-eajames@linux.ibm.com>
Document the bindings.
Reviewed-by: Andrew Jeffrey <andrew@aj.id.au>
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
.../devicetree/bindings/soc/aspeed/xdma.txt | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/aspeed/xdma.txt
diff --git a/Documentation/devicetree/bindings/soc/aspeed/xdma.txt b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
new file mode 100644
index 0000000..85e82ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
@@ -0,0 +1,23 @@
+* Device tree bindings for the Aspeed XDMA Engine
+
+The XDMA Engine embedded in the AST2500 SOC can perform automatic DMA
+operations over PCI between the AST2500 (acting as a BMC) and a host processor.
+
+Required properties:
+
+ - compatible "aspeed,ast2500-xdma"
+ - reg contains the offset and length of the memory region
+ assigned to the XDMA registers
+ - resets reset specifier for the syscon reset associated with
+ the XDMA engine
+ - interrupts the interrupt associated with the XDMA engine on this
+ platform
+
+Example:
+
+ xdma at 1e6e7000 {
+ compatible = "aspeed,ast2500-xdma";
+ reg = <0x1e6e7000 0x100>;
+ resets = <&syscon ASPEED_RESET_XDMA>;
+ interrupts = <6>;
+ };
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 0/8] drivers/soc: Add Aspeed XDMA Engine Driver
From: Eddie James @ 2019-07-01 19:53 UTC (permalink / raw)
To: linux-aspeed
The XDMA engine embedded in the AST2500 SOC performs PCI DMA operations
between the SOC (acting as a BMC) and a host processor in a server.
This series adds a driver to control the XDMA engine in order to easily
perform DMA operations to and from the host processor.
Changes since v3:
- Added a few comments.
- Change DMA reservation/allocation/mmap to iomap operations.
- Change miscdevice to "aspeed-xdma".
- Change default PCI device to the BMC device, not the VGA.
- Use length-limited string functions (strncpy, strncasecmp)
- Added more debugfs registers
Changes since v2:
- Switch to one pci device config sysfs file
- Add documentation for pci device config sysfs file
- Add module parameter for pci device config
- Switch to genalloc instead of custom allocator
- Fix alignment of user structure
- Cleaned up debugfs functions
I previously sent this series v1 to drivers/misc, but I'm now fairly certain
it belongs in drivers/soc, especially since the other Aspeed drivers have been
moved to soc.
Changes since v1:
- Correct the XDMA command pitch
- Don't use packed for the aspeed_xdma_op structure
- Correct the SCU PCI config change
Eddie James (8):
dt-bindings: soc: Add Aspeed XDMA engine binding documentation
drivers/soc: Add Aspeed XDMA Engine Driver
drivers/soc: xdma: Add user interface
Documentation: ABI: Add aspeed-xdma sysfs documentation
drivers/soc: xdma: Add PCI device configuration sysfs
drivers/soc: xdma: Add debugfs entries
ARM: dts: aspeed: Add XDMA Engine
ARM: dts: aspeed: witherspoon: Enable XDMA Engine
.../ABI/testing/sysfs-devices-platform-aspeed-xdma | 11 +
.../devicetree/bindings/soc/aspeed/xdma.txt | 23 +
MAINTAINERS | 9 +
arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 4 +
arch/arm/boot/dts/aspeed-g5.dtsi | 8 +
drivers/soc/aspeed/Kconfig | 8 +
drivers/soc/aspeed/Makefile | 1 +
drivers/soc/aspeed/aspeed-xdma.c | 953 +++++++++++++++++++++
include/uapi/linux/aspeed-xdma.h | 26 +
9 files changed, 1043 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma
create mode 100644 Documentation/devicetree/bindings/soc/aspeed/xdma.txt
create mode 100644 drivers/soc/aspeed/aspeed-xdma.c
create mode 100644 include/uapi/linux/aspeed-xdma.h
--
1.8.3.1
^ permalink raw reply
* [PATCH v3 5/8] drivers/soc: xdma: Add PCI device configuration sysfs
From: Eddie James @ 2019-07-01 18:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190531034502.GH17772@u40b0340c692b58f6553c.ant.amazon.com>
On 5/30/19 10:45 PM, Eduardo Valentin wrote:
> On Wed, May 29, 2019 at 01:10:05PM -0500, Eddie James wrote:
>> The AST2500 has two PCI devices embedded. The XDMA engine can use either
>> device to perform DMA transfers. Users need the capability to choose
>> which device to use. This commit therefore adds two sysfs files that
>> toggle the AST2500 and XDMA engine between the two PCI devices.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>> drivers/soc/aspeed/aspeed-xdma.c | 103 +++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 100 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
>> index 39f6545..ddd5e1e 100644
>> --- a/drivers/soc/aspeed/aspeed-xdma.c
>> +++ b/drivers/soc/aspeed/aspeed-xdma.c
>> @@ -143,6 +143,7 @@ struct aspeed_xdma {
>> void *cmdq_vga_virt;
>> struct gen_pool *vga_pool;
>>
>> + char pcidev[4];
>> struct miscdevice misc;
>> };
>>
>> @@ -165,6 +166,10 @@ struct aspeed_xdma_client {
>> SCU_PCIE_CONF_VGA_EN_IRQ | SCU_PCIE_CONF_VGA_EN_DMA |
>> SCU_PCIE_CONF_RSVD;
>>
>> +static char *_pcidev = "vga";
>> +module_param_named(pcidev, _pcidev, charp, 0600);
>> +MODULE_PARM_DESC(pcidev, "Default PCI device used by XDMA engine for DMA ops");
>> +
>> static void aspeed_scu_pcie_write(struct aspeed_xdma *ctx, u32 conf)
>> {
>> u32 v = 0;
>> @@ -512,7 +517,7 @@ static int aspeed_xdma_release(struct inode *inode, struct file *file)
>> .release = aspeed_xdma_release,
>> };
>>
>> -static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
>> +static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx, u32 conf)
>> {
>> int rc;
>> u32 scu_conf = 0;
>> @@ -522,7 +527,7 @@ static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
>> const u32 vga_sizes[4] = { 0x800000, 0x1000000, 0x2000000, 0x4000000 };
>> void __iomem *sdmc_base = ioremap(0x1e6e0000, 0x100);
>>
>> - aspeed_scu_pcie_write(ctx, aspeed_xdma_vga_pcie_conf);
>> + aspeed_scu_pcie_write(ctx, conf);
>>
>> regmap_read(ctx->scu, SCU_STRAP, &scu_conf);
>> ctx->vga_size = vga_sizes[FIELD_GET(SCU_STRAP_VGA_MEM, scu_conf)];
>> @@ -598,10 +603,91 @@ static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
>> return rc;
>> }
>>
>> +static int aspeed_xdma_change_pcie_conf(struct aspeed_xdma *ctx, u32 conf)
>> +{
>> + int rc;
>> +
>> + mutex_lock(&ctx->start_lock);
>> + rc = wait_event_interruptible_timeout(ctx->wait,
>> + !test_bit(XDMA_IN_PRG,
>> + &ctx->flags),
>> + msecs_to_jiffies(1000));
>> + if (rc < 0) {
>> + mutex_unlock(&ctx->start_lock);
>> + return -EINTR;
>> + }
>> +
>> + /* previous op didn't complete, wake up waiters anyway */
>> + if (!rc)
>> + wake_up_interruptible_all(&ctx->wait);
>> +
>> + reset_control_assert(ctx->reset);
>> + msleep(10);
>> +
>> + aspeed_scu_pcie_write(ctx, conf);
>> + msleep(10);
>> +
>> + reset_control_deassert(ctx->reset);
>> + msleep(10);
>> +
>> + aspeed_xdma_init_eng(ctx);
>> +
>> + mutex_unlock(&ctx->start_lock);
>> +
>> + return 0;
>> +}
>> +
>> +static int aspeed_xdma_pcidev_to_conf(struct aspeed_xdma *ctx,
>> + const char *pcidev, u32 *conf)
>> +{
>> + if (!strcasecmp(pcidev, "vga")) {
>> + *conf = aspeed_xdma_vga_pcie_conf;
>> + return 0;
>> + }
>> +
>> + if (!strcasecmp(pcidev, "bmc")) {
>> + *conf = aspeed_xdma_bmc_pcie_conf;
>> + return 0;
>> + }
> strncasecmp()?
Yes, will change, and below.
>
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static ssize_t aspeed_xdma_show_pcidev(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct aspeed_xdma *ctx = dev_get_drvdata(dev);
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%s", ctx->pcidev);
>> +}
>> +
>> +static ssize_t aspeed_xdma_store_pcidev(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + u32 conf;
>> + struct aspeed_xdma *ctx = dev_get_drvdata(dev);
>> + int rc = aspeed_xdma_pcidev_to_conf(ctx, buf, &conf);
>> +
>> + if (rc)
>> + return rc;
>> +
>> + rc = aspeed_xdma_change_pcie_conf(ctx, conf);
>> + if (rc)
>> + return rc;
>> +
>> + strcpy(ctx->pcidev, buf);
> should we use strncpy() instead?
>
>> + return count;
>> +}
>> +static DEVICE_ATTR(pcidev, 0644, aspeed_xdma_show_pcidev,
>> + aspeed_xdma_store_pcidev);
>> +
>> static int aspeed_xdma_probe(struct platform_device *pdev)
>> {
>> int irq;
>> int rc;
>> + u32 conf;
>> struct resource *res;
>> struct device *dev = &pdev->dev;
>> struct aspeed_xdma *ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> @@ -657,7 +743,14 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
>>
>> msleep(10);
>>
>> - rc = aspeed_xdma_init_mem(ctx);
>> + if (aspeed_xdma_pcidev_to_conf(ctx, _pcidev, &conf)) {
>> + conf = aspeed_xdma_vga_pcie_conf;
>> + strcpy(ctx->pcidev, "vga");
>> + } else {
>> + strcpy(ctx->pcidev, _pcidev);
>> + }
> same...
>
>> +
>> + rc = aspeed_xdma_init_mem(ctx, conf);
>> if (rc) {
>> reset_control_assert(ctx->reset);
>> return rc;
>> @@ -682,6 +775,8 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
>> return rc;
>> }
>>
>> + device_create_file(dev, &dev_attr_pcidev);
> Should we consider using one of the default attributes here instead of device_create_file()?
> http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
Doesn't seem to be any way to create attributes per device with that
method. Setting the device->groups in probe() doesn't do it.
>
> BTW, was this ABI documented? Is this the same file documented in patch 2?
Patch 4, but yes.
>
>> +
>> return 0;
>> }
>>
>> @@ -689,6 +784,8 @@ static int aspeed_xdma_remove(struct platform_device *pdev)
>> {
>> struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
>>
>> + device_remove_file(ctx->dev, &dev_attr_pcidev);
>> +
>> misc_deregister(&ctx->misc);
>> gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
>> XDMA_CMDQ_SIZE);
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* [PATCH v2 2/8] dt-bindings: pinctrl: aspeed: Convert AST2400 bindings to json-schema
From: Rob Herring @ 2019-06-28 15:46 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-3-andrew@aj.id.au>
On Thu, Jun 27, 2019 at 8:39 PM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> Convert ASPEED pinctrl bindings to DT schema format using json-schema
>
> Cc: Johnny Huang <johnny_huang@aspeedtech.com>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> In v2:
>
> * Enforce function/group names in bindings
> * Move description above properties
> * Simplify specification of compatible
> * Cleanup license specification
>
> .../pinctrl/aspeed,ast2400-pinctrl.txt | 80 ------------------
> .../pinctrl/aspeed,ast2400-pinctrl.yaml | 81 +++++++++++++++++++
> 2 files changed, 81 insertions(+), 80 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
> create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v2 3/8] dt-bindings: pinctrl: aspeed: Convert AST2500 bindings to json-schema
From: Rob Herring @ 2019-06-28 15:46 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-4-andrew@aj.id.au>
On Thu, Jun 27, 2019 at 8:39 PM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> Convert ASPEED pinctrl bindings to DT schema format using json-schema.
>
> Cc: Johnny Huang <johnny_huang@aspeedtech.com>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> In v2:
>
> * Enforce function/group names in bindings
> * Move description above properties
> * Simplify specification of compatible
> * Cleanup license specification
>
> .../pinctrl/aspeed,ast2500-pinctrl.txt | 119 ----------------
> .../pinctrl/aspeed,ast2500-pinctrl.yaml | 134 ++++++++++++++++++
> 2 files changed, 134 insertions(+), 119 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.txt
> create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v3 2/8] drivers/soc: Add Aspeed XDMA Engine Driver
From: Eddie James @ 2019-06-28 15:43 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190531033117.GG17772@u40b0340c692b58f6553c.ant.amazon.com>
On 5/30/19 10:31 PM, Eduardo Valentin wrote:
> On Wed, May 29, 2019 at 01:10:02PM -0500, Eddie James wrote:
>> The XDMA engine embedded in the AST2500 SOC performs PCI DMA operations
>> between the SOC (acting as a BMC) and a host processor in a server.
>>
>> This commit adds a driver to control the XDMA engine and adds functions
>> to initialize the hardware and memory and start DMA operations.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>> MAINTAINERS | 10 +
>> drivers/soc/aspeed/Kconfig | 8 +
>> drivers/soc/aspeed/Makefile | 1 +
>> drivers/soc/aspeed/aspeed-xdma.c | 520 +++++++++++++++++++++++++++++++++++++++
>> include/uapi/linux/aspeed-xdma.h | 26 ++
>> 5 files changed, 565 insertions(+)
>> create mode 100644 drivers/soc/aspeed/aspeed-xdma.c
>> create mode 100644 include/uapi/linux/aspeed-xdma.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7e09dda..84e2b62 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2584,6 +2584,16 @@ S: Maintained
>> F: drivers/media/platform/aspeed-video.c
>> F: Documentation/devicetree/bindings/media/aspeed-video.txt
>>
>> +ASPEED XDMA ENGINE DRIVER
>> +M: Eddie James <eajames@linux.ibm.com>
>> +L: linux-aspeed at lists.ozlabs.org (moderated for non-subscribers)
>> +L: linux-kernel at vger.kernel.org
>> +S: Maintained
>> +F: Documentation/devicetree/bindings/misc/aspeed,xdma.txt
>> +F: Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma
>> +F: drivers/soc/aspeed/aspeed-xdma.c
>> +F: include/uapi/linux/aspeed-xdma.h
>> +
>> ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
>> M: Corentin Chary <corentin.chary@gmail.com>
>> L: acpi4asus-user at lists.sourceforge.net
>> diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
>> index 323e177..8b08310 100644
>> --- a/drivers/soc/aspeed/Kconfig
>> +++ b/drivers/soc/aspeed/Kconfig
>> @@ -29,4 +29,12 @@ config ASPEED_P2A_CTRL
>> ioctl()s, the driver also provides an interface for userspace mappings to
>> a pre-defined region.
>>
>> +config ASPEED_XDMA
>> + tristate "Aspeed XDMA Engine Driver"
>> + depends on SOC_ASPEED && REGMAP && MFD_SYSCON && HAS_DMA
>> + help
>> + Enable support for the Aspeed XDMA Engine found on the Aspeed AST2500
>> + SOC. The XDMA engine can perform automatic PCI DMA operations between
>> + the AST2500 (acting as a BMC) and a host processor.
>> +
>> endmenu
>> diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
>> index b64be47..977b046 100644
>> --- a/drivers/soc/aspeed/Makefile
>> +++ b/drivers/soc/aspeed/Makefile
>> @@ -2,3 +2,4 @@
>> obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
>> obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
>> obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
>> +obj-$(CONFIG_ASPEED_XDMA) += aspeed-xdma.o
>> diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
>> new file mode 100644
>> index 0000000..3dc0ce4
>> --- /dev/null
>> +++ b/drivers/soc/aspeed/aspeed-xdma.c
>> @@ -0,0 +1,520 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +// Copyright IBM Corp 2019
>> +
>> +#include <linux/aspeed-xdma.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/fs.h>
>> +#include <linux/genalloc.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/list.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/poll.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +#include <linux/string.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/wait.h>
>> +
>> +#define DEVICE_NAME "aspeed-xdma"
>> +
>> +#define SCU_STRAP 0x070
>> +#define SCU_STRAP_VGA_MEM GENMASK(3, 2)
>> +
>> +#define SCU_PCIE_CONF 0x180
>> +#define SCU_PCIE_CONF_VGA_EN BIT(0)
>> +#define SCU_PCIE_CONF_VGA_EN_MMIO BIT(1)
>> +#define SCU_PCIE_CONF_VGA_EN_LPC BIT(2)
>> +#define SCU_PCIE_CONF_VGA_EN_MSI BIT(3)
>> +#define SCU_PCIE_CONF_VGA_EN_MCTP BIT(4)
>> +#define SCU_PCIE_CONF_VGA_EN_IRQ BIT(5)
>> +#define SCU_PCIE_CONF_VGA_EN_DMA BIT(6)
>> +#define SCU_PCIE_CONF_BMC_EN BIT(8)
>> +#define SCU_PCIE_CONF_BMC_EN_MMIO BIT(9)
>> +#define SCU_PCIE_CONF_BMC_EN_MSI BIT(11)
>> +#define SCU_PCIE_CONF_BMC_EN_MCTP BIT(12)
>> +#define SCU_PCIE_CONF_BMC_EN_IRQ BIT(13)
>> +#define SCU_PCIE_CONF_BMC_EN_DMA BIT(14)
>> +#define SCU_PCIE_CONF_RSVD GENMASK(19, 18)
>> +
>> +#define SDMC_CONF 0x004
>> +#define SDMC_CONF_MEM GENMASK(1, 0)
>> +#define SDMC_REMAP 0x008
>> +#define SDMC_REMAP_MAGIC GENMASK(17, 16)
>> +
>> +#define XDMA_CMD_SIZE 4
>> +#define XDMA_CMDQ_SIZE PAGE_SIZE
>> +#define XDMA_BYTE_ALIGN 16
>> +#define XDMA_MAX_LINE_SIZE BIT(10)
>> +#define XDMA_NUM_CMDS \
>> + (XDMA_CMDQ_SIZE / sizeof(struct aspeed_xdma_cmd))
>> +#define XDMA_NUM_DEBUGFS_REGS 6
>> +
>> +#define XDMA_CMD_BMC_CHECK BIT(0)
>> +#define XDMA_CMD_BMC_ADDR GENMASK(29, 4)
>> +#define XDMA_CMD_BMC_DIR_US BIT(31)
>> +
>> +#define XDMA_CMD_COMM1_HI_HOST_PITCH GENMASK(14, 3)
>> +#define XDMA_CMD_COMM1_HI_BMC_PITCH GENMASK(30, 19)
>> +
>> +#define XDMA_CMD_CONF_CHECK BIT(1)
>> +#define XDMA_CMD_CONF_LINE_SIZE GENMASK(14, 4)
>> +#define XDMA_CMD_CONF_IRQ_BMC BIT(15)
>> +#define XDMA_CMD_CONF_NUM_LINES GENMASK(27, 16)
>> +#define XDMA_CMD_CONF_IRQ BIT(31)
>> +
>> +#define XDMA_CMD_ID_UPDIR GENMASK(17, 16)
>> +#define XDMA_CMD_ID_UPDIR_BMC 0
>> +#define XDMA_CMD_ID_UPDIR_HOST 1
>> +#define XDMA_CMD_ID_UPDIR_VGA 2
>> +
>> +#define XDMA_DS_PCIE_REQ_SIZE_128 0
>> +#define XDMA_DS_PCIE_REQ_SIZE_256 1
>> +#define XDMA_DS_PCIE_REQ_SIZE_512 2
>> +#define XDMA_DS_PCIE_REQ_SIZE_1K 3
>> +#define XDMA_DS_PCIE_REQ_SIZE_2K 4
>> +#define XDMA_DS_PCIE_REQ_SIZE_4K 5
>> +
>> +#define XDMA_BMC_CMD_QUEUE_ADDR 0x10
>> +#define XDMA_BMC_CMD_QUEUE_ENDP 0x14
>> +#define XDMA_BMC_CMD_QUEUE_WRITEP 0x18
>> +#define XDMA_BMC_CMD_QUEUE_READP 0x1c
>> +#define XDMA_BMC_CMD_QUEUE_READP_MAGIC 0xee882266
>> +#define XDMA_CTRL 0x20
>> +#define XDMA_CTRL_US_COMP BIT(4)
>> +#define XDMA_CTRL_DS_COMP BIT(5)
>> +#define XDMA_CTRL_DS_DIRTY BIT(6)
>> +#define XDMA_CTRL_DS_PCIE_REQ_SIZE GENMASK(19, 17)
>> +#define XDMA_CTRL_DS_DATA_TIMEOUT BIT(28)
>> +#define XDMA_CTRL_DS_CHECK_ID BIT(29)
>> +#define XDMA_STATUS 0x24
>> +#define XDMA_STATUS_US_COMP BIT(4)
>> +#define XDMA_STATUS_DS_COMP BIT(5)
>> +
>> +enum {
>> + XDMA_IN_PRG,
>> + XDMA_UPSTREAM,
>> +};
>> +
>> +struct aspeed_xdma_cmd {
>> + u32 host_addr_lo;
>> + u32 host_addr_hi;
>> + u32 bmc_addr;
>> + u32 comm1_hi;
>> + u32 conf;
>> + u32 id;
>> + u32 resv0;
>> + u32 resv1;
>> +};
>> +
>> +struct aspeed_xdma_client;
>> +
>> +struct aspeed_xdma {
>> + struct device *dev;
>> + void __iomem *base;
>> + struct regmap *scu;
>> + struct reset_control *reset;
>> +
>> + unsigned long flags;
> interesting.. why do we need a long size flags field when we just toggle two bits?
> From a quick glance, looks like we use this to check for XDMA_IN_PRG and XDMA_UPSTREAM only..
That's correct. More flags could be added in the future. Are you worried
about memory space?
>
>> + unsigned int cmd_idx;
>> + wait_queue_head_t wait;
>> + struct aspeed_xdma_client *current_client;
>> +
>> + u32 vga_phys;
>> + u32 vga_size;
>> + dma_addr_t vga_dma;
>> + void *cmdq;
>> + void *vga_virt;
>> + dma_addr_t cmdq_vga_phys;
>> + void *cmdq_vga_virt;
>> + struct gen_pool *vga_pool;
>> +};
>> +
>> +struct aspeed_xdma_client {
>> + struct aspeed_xdma *ctx;
>> +
>> + unsigned long flags;
> same
>
>> + void *virt;
>> + dma_addr_t phys;
>> + u32 size;
>> +};
>> +
>> +static const u32 aspeed_xdma_bmc_pcie_conf = SCU_PCIE_CONF_BMC_EN |
>> + SCU_PCIE_CONF_BMC_EN_MSI | SCU_PCIE_CONF_BMC_EN_MCTP |
>> + SCU_PCIE_CONF_BMC_EN_IRQ | SCU_PCIE_CONF_BMC_EN_DMA |
>> + SCU_PCIE_CONF_RSVD;
>> +
>> +static const u32 aspeed_xdma_vga_pcie_conf = SCU_PCIE_CONF_VGA_EN |
>> + SCU_PCIE_CONF_VGA_EN_MSI | SCU_PCIE_CONF_VGA_EN_MCTP |
>> + SCU_PCIE_CONF_VGA_EN_IRQ | SCU_PCIE_CONF_VGA_EN_DMA |
>> + SCU_PCIE_CONF_RSVD;
>> +
>> +static void aspeed_scu_pcie_write(struct aspeed_xdma *ctx, u32 conf)
>> +{
>> + u32 v = 0;
>> +
>> + regmap_write(ctx->scu, SCU_PCIE_CONF, conf);
>> + regmap_read(ctx->scu, SCU_PCIE_CONF, &v);
>> +
>> + dev_dbg(ctx->dev, "write scu pcie_conf[%08x]\n", v);
>> +}
>> +
>> +static u32 aspeed_xdma_reg_read(struct aspeed_xdma *ctx, u32 reg)
>> +{
>> + u32 v = readl(ctx->base + reg);
>> +
>> + dev_dbg(ctx->dev, "read %02x[%08x]\n", reg, v);
>> + return v;
>> +}
>> +
>> +static void aspeed_xdma_reg_write(struct aspeed_xdma *ctx, u32 reg, u32 val)
>> +{
>> + writel(val, ctx->base + reg);
>> + dev_dbg(ctx->dev, "write %02x[%08x]\n", reg, readl(ctx->base + reg));
>> +}
>> +
>> +static void aspeed_xdma_init_eng(struct aspeed_xdma *ctx)
>> +{
>> + const u32 ctrl = XDMA_CTRL_US_COMP | XDMA_CTRL_DS_COMP |
>> + XDMA_CTRL_DS_DIRTY | FIELD_PREP(XDMA_CTRL_DS_PCIE_REQ_SIZE,
>> + XDMA_DS_PCIE_REQ_SIZE_256) |
>> + XDMA_CTRL_DS_DATA_TIMEOUT | XDMA_CTRL_DS_CHECK_ID;
>> +
>> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_ENDP,
>> + XDMA_CMD_SIZE * XDMA_NUM_CMDS);
>> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_READP,
>> + XDMA_BMC_CMD_QUEUE_READP_MAGIC);
>> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_WRITEP, 0);
>> + aspeed_xdma_reg_write(ctx, XDMA_CTRL, ctrl);
>> +
>> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_ADDR,
>> + ctx->cmdq_vga_phys);
>> +
>> + ctx->cmd_idx = 0;
>> + ctx->flags = 0;
>> +}
>> +
>> +static void aspeed_xdma_reset(struct aspeed_xdma *ctx)
>> +{
>> + reset_control_assert(ctx->reset);
>> +
>> + msleep(10);
>> +
>> + reset_control_deassert(ctx->reset);
>> +
>> + msleep(10);
> Why 10ms?
I will add a comment. It's from the specification.
>
>> +
>> + aspeed_xdma_init_eng(ctx);
>> +}
>> +
>> +static void aspeed_xdma_start(struct aspeed_xdma *ctx,
>> + struct aspeed_xdma_op *op, u32 bmc_addr)
>> +{
>> + u32 conf = XDMA_CMD_CONF_CHECK | XDMA_CMD_CONF_IRQ_BMC |
>> + XDMA_CMD_CONF_IRQ;
>> + unsigned int line_size = op->len / XDMA_BYTE_ALIGN;
>> + unsigned int num_lines = 1;
>> + unsigned int nidx = (ctx->cmd_idx + 1) % XDMA_NUM_CMDS;
>> + unsigned int pitch = 1;
>> + struct aspeed_xdma_cmd *cmd =
>> + &(((struct aspeed_xdma_cmd *)ctx->cmdq)[ctx->cmd_idx]);
>> +
>> + if (line_size > XDMA_MAX_LINE_SIZE) {
>> + unsigned int rem;
>> + unsigned int total;
>> +
>> + num_lines = line_size / XDMA_MAX_LINE_SIZE;
>> + total = XDMA_MAX_LINE_SIZE * num_lines;
>> + rem = line_size - total;
>> + line_size = XDMA_MAX_LINE_SIZE;
>> + pitch = line_size;
>> +
>> + if (rem) {
>> + unsigned int offs = total * XDMA_BYTE_ALIGN;
>> + u32 r_bmc_addr = bmc_addr + offs;
>> + u64 r_host_addr = op->host_addr + (u64)offs;
>> + struct aspeed_xdma_cmd *r_cmd =
>> + &(((struct aspeed_xdma_cmd *)ctx->cmdq)[nidx]);
>> +
>> + r_cmd->host_addr_lo =
>> + (u32)(r_host_addr & 0xFFFFFFFFULL);
>> + r_cmd->host_addr_hi = (u32)(r_host_addr >> 32ULL);
>> + r_cmd->bmc_addr = (r_bmc_addr & XDMA_CMD_BMC_ADDR) |
>> + XDMA_CMD_BMC_CHECK |
>> + (op->upstream ? XDMA_CMD_BMC_DIR_US : 0);
>> + r_cmd->conf = conf |
>> + FIELD_PREP(XDMA_CMD_CONF_LINE_SIZE, rem) |
>> + FIELD_PREP(XDMA_CMD_CONF_NUM_LINES, 1);
>> + r_cmd->comm1_hi =
>> + FIELD_PREP(XDMA_CMD_COMM1_HI_HOST_PITCH, 1) |
>> + FIELD_PREP(XDMA_CMD_COMM1_HI_BMC_PITCH, 1);
>> +
>> + /* do not trigger IRQ for first command */
>> + conf = XDMA_CMD_CONF_CHECK;
>> +
>> + nidx = (nidx + 1) % XDMA_NUM_CMDS;
>> + }
>> +
>> + /* undocumented formula to get required number of lines */
>> + num_lines = (num_lines * 2) - 1;
>> + }
>> +
>> + /* ctrl == 0 indicates engine hasn't started properly; restart it */
>> + if (!aspeed_xdma_reg_read(ctx, XDMA_CTRL))
>> + aspeed_xdma_reset(ctx);
>> +
>> + cmd->host_addr_lo = (u32)(op->host_addr & 0xFFFFFFFFULL);
>> + cmd->host_addr_hi = (u32)(op->host_addr >> 32ULL);
>> + cmd->bmc_addr = (bmc_addr & XDMA_CMD_BMC_ADDR) | XDMA_CMD_BMC_CHECK |
>> + (op->upstream ? XDMA_CMD_BMC_DIR_US : 0);
>> + cmd->conf = conf |
>> + FIELD_PREP(XDMA_CMD_CONF_LINE_SIZE, line_size) |
>> + FIELD_PREP(XDMA_CMD_CONF_NUM_LINES, num_lines);
>> + cmd->comm1_hi = FIELD_PREP(XDMA_CMD_COMM1_HI_HOST_PITCH, pitch) |
>> + FIELD_PREP(XDMA_CMD_COMM1_HI_BMC_PITCH, pitch);
>> +
>> + memcpy(ctx->cmdq_vga_virt, ctx->cmdq, XDMA_CMDQ_SIZE);
>> +
>> + if (op->upstream)
>> + set_bit(XDMA_UPSTREAM, &ctx->flags);
>> + else
>> + clear_bit(XDMA_UPSTREAM, &ctx->flags);
>> +
>> + set_bit(XDMA_IN_PRG, &ctx->flags);
>> +
>> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_WRITEP,
>> + nidx * XDMA_CMD_SIZE);
>> + ctx->cmd_idx = nidx;
>> +}
>> +
>> +static void aspeed_xdma_done(struct aspeed_xdma *ctx)
>> +{
>> + if (ctx->current_client) {
>> + clear_bit(XDMA_IN_PRG, &ctx->current_client->flags);
>> +
>> + ctx->current_client = NULL;
>> + }
>> +
>> + clear_bit(XDMA_IN_PRG, &ctx->flags);
>> + wake_up_interruptible_all(&ctx->wait);
>> +}
>> +
>> +static irqreturn_t aspeed_xdma_irq(int irq, void *arg)
>> +{
>> + struct aspeed_xdma *ctx = arg;
>> + u32 status = aspeed_xdma_reg_read(ctx, XDMA_STATUS);
>> +
>> + if (status & XDMA_STATUS_US_COMP) {
>> + if (test_bit(XDMA_UPSTREAM, &ctx->flags))
>> + aspeed_xdma_done(ctx);
>> + }
>> +
>> + if (status & XDMA_STATUS_DS_COMP) {
>> + if (!test_bit(XDMA_UPSTREAM, &ctx->flags))
>> + aspeed_xdma_done(ctx);
>> + }
>> +
>> + aspeed_xdma_reg_write(ctx, XDMA_STATUS, status);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
>> +{
>> + int rc;
>> + u32 scu_conf = 0;
>> + u32 mem_size = 0x20000000;
>> + const u32 mem_sizes[4] = { 0x8000000, 0x10000000, 0x20000000,
>> + 0x40000000 };
>> + const u32 vga_sizes[4] = { 0x800000, 0x1000000, 0x2000000, 0x4000000 };
>> + void __iomem *sdmc_base = ioremap(0x1e6e0000, 0x100);
>> +
> Should these come from fw specification? Say device tree mem reserved nodes?
I'm not sure. I quite like finding it dynamically here rather than
having to specify a new memory node in every system that will use the
driver. In addition it's a little bit awkward to get the physical
address from the reserved memory subsystem; we don't actually need to
grab the reserved memory for this device, it's already reserved by the
CPU/VGA. What do you think?
Thanks for the review!
Eddie
>
>> + aspeed_scu_pcie_write(ctx, aspeed_xdma_vga_pcie_conf);
>> +
>> + regmap_read(ctx->scu, SCU_STRAP, &scu_conf);
>> + ctx->vga_size = vga_sizes[FIELD_GET(SCU_STRAP_VGA_MEM, scu_conf)];
>> +
>> + if (sdmc_base) {
>> + u32 sdmc = readl(sdmc_base + SDMC_CONF);
>> + u32 remap = readl(sdmc_base + SDMC_REMAP);
>> +
>> + remap |= SDMC_REMAP_MAGIC;
>> + writel(remap, sdmc_base + SDMC_REMAP);
>> + remap = readl(sdmc_base + SDMC_REMAP);
>> +
>> + mem_size = mem_sizes[sdmc & SDMC_CONF_MEM];
>> + iounmap(sdmc_base);
>> + }
>> +
>> + ctx->vga_phys = (mem_size - ctx->vga_size) + 0x80000000;
>> +
>> + ctx->cmdq = devm_kzalloc(ctx->dev, XDMA_CMDQ_SIZE, GFP_KERNEL);
>> + if (!ctx->cmdq) {
>> + dev_err(ctx->dev, "Failed to allocate command queue.\n");
>> + return -ENOMEM;
>> + }
>> +
>> + rc = dma_set_mask_and_coherent(ctx->dev, DMA_BIT_MASK(32));
>> + if (rc) {
>> + dev_err(ctx->dev, "Failed to set DMA mask: %d.\n", rc);
>> + return rc;
>> + }
>> +
>> + rc = dma_declare_coherent_memory(ctx->dev, ctx->vga_phys,
>> + ctx->vga_phys, ctx->vga_size);
>> + if (rc) {
>> + dev_err(ctx->dev, "Failed to declare coherent memory: %d.\n",
>> + rc);
>> + return rc;
>> + }
>> +
>> + ctx->vga_virt = dma_alloc_coherent(ctx->dev, ctx->vga_size,
>> + &ctx->vga_dma, GFP_KERNEL);
>> + if (!ctx->vga_virt) {
>> + dev_err(ctx->dev, "Failed to allocate DMA.\n");
>> + rc = -ENOMEM;
>> + goto err_dma;
>> + }
>> +
>> + rc = gen_pool_add_virt(ctx->vga_pool, (unsigned long)ctx->vga_virt,
>> + ctx->vga_phys, ctx->vga_size, -1);
>> + if (rc) {
>> + dev_err(ctx->dev, "Failed to add memory to genalloc pool.\n");
>> + goto err_genalloc;
>> + }
>> +
>> + ctx->cmdq_vga_virt = gen_pool_dma_alloc(ctx->vga_pool, XDMA_CMDQ_SIZE,
>> + &ctx->cmdq_vga_phys);
>> + if (!ctx->cmdq_vga_virt) {
>> + dev_err(ctx->dev, "Failed to genalloc cmdq.\n");
>> + rc = -ENOMEM;
>> + goto err_genalloc;
>> + }
>> +
>> + dev_dbg(ctx->dev, "VGA mapped at phys[%08x], size[%08x].\n",
>> + ctx->vga_phys, ctx->vga_size);
>> +
>> + return 0;
>> +
>> +err_dma:
>> + dma_release_declared_memory(ctx->dev);
>> +
>> +err_genalloc:
>> + dma_free_coherent(ctx->dev, ctx->vga_size, ctx->vga_virt,
>> + ctx->vga_dma);
>> + return rc;
>> +}
>> +
>> +static int aspeed_xdma_probe(struct platform_device *pdev)
>> +{
>> + int irq;
>> + int rc;
>> + struct resource *res;
>> + struct device *dev = &pdev->dev;
>> + struct aspeed_xdma *ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> +
>> + if (!ctx)
>> + return -ENOMEM;
>> +
>> + ctx->dev = dev;
>> + platform_set_drvdata(pdev, ctx);
>> + init_waitqueue_head(&ctx->wait);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + ctx->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(ctx->base)) {
>> + dev_err(dev, "Unable to ioremap registers.\n");
>> + return PTR_ERR(ctx->base);
>> + }
>> +
>> + irq = irq_of_parse_and_map(dev->of_node, 0);
>> + if (!irq) {
>> + dev_err(dev, "Unable to find IRQ.\n");
>> + return -ENODEV;
>> + }
>> +
>> + rc = devm_request_irq(dev, irq, aspeed_xdma_irq, IRQF_SHARED,
>> + DEVICE_NAME, ctx);
>> + if (rc < 0) {
>> + dev_err(dev, "Unable to request IRQ %d.\n", irq);
>> + return rc;
>> + }
>> +
>> + ctx->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
>> + if (IS_ERR(ctx->scu)) {
>> + dev_err(ctx->dev, "Unable to grab SCU regs.\n");
>> + return PTR_ERR(ctx->scu);
>> + }
>> +
>> + ctx->reset = devm_reset_control_get_exclusive(dev, NULL);
>> + if (IS_ERR(ctx->reset)) {
>> + dev_err(dev, "Unable to request reset control.\n");
>> + return PTR_ERR(ctx->reset);
>> + }
>> +
>> + ctx->vga_pool = devm_gen_pool_create(dev, ilog2(PAGE_SIZE), -1, NULL);
>> + if (!ctx->vga_pool) {
>> + dev_err(dev, "Unable to setup genalloc pool.\n");
>> + return -ENOMEM;
>> + }
>> +
>> + reset_control_deassert(ctx->reset);
>> +
>> + msleep(10);
> Why 10ms again? :-)
>
>> +
>> + rc = aspeed_xdma_init_mem(ctx);
>> + if (rc) {
>> + reset_control_assert(ctx->reset);
>> + return rc;
>> + }
>> +
>> + aspeed_xdma_init_eng(ctx);
>> +
>> + return 0;
>> +}
>> +
>> +static int aspeed_xdma_remove(struct platform_device *pdev)
>> +{
>> + struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
>> +
>> + gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
>> + XDMA_CMDQ_SIZE);
>> + dma_free_coherent(ctx->dev, ctx->vga_size, ctx->vga_virt,
>> + ctx->vga_dma);
>> + dma_release_declared_memory(ctx->dev);
>> + reset_control_assert(ctx->reset);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id aspeed_xdma_match[] = {
>> + { .compatible = "aspeed,ast2500-xdma" },
>> + { },
>> +};
>> +
>> +static struct platform_driver aspeed_xdma_driver = {
>> + .probe = aspeed_xdma_probe,
>> + .remove = aspeed_xdma_remove,
>> + .driver = {
>> + .name = DEVICE_NAME,
>> + .of_match_table = aspeed_xdma_match,
>> + },
>> +};
>> +
>> +module_platform_driver(aspeed_xdma_driver);
>> +
>> +MODULE_AUTHOR("Eddie James");
>> +MODULE_DESCRIPTION("Aspeed XDMA Engine Driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/uapi/linux/aspeed-xdma.h b/include/uapi/linux/aspeed-xdma.h
>> new file mode 100644
>> index 0000000..998459e
>> --- /dev/null
>> +++ b/include/uapi/linux/aspeed-xdma.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/* Copyright IBM Corp 2019 */
>> +
>> +#ifndef _UAPI_LINUX_ASPEED_XDMA_H_
>> +#define _UAPI_LINUX_ASPEED_XDMA_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/*
>> + * aspeed_xdma_op
>> + *
>> + * host_addr: the DMA address on the host side, typically configured by PCI
>> + * subsystem
>> + *
>> + * len: the size of the transfer in bytes; it should be a multiple of 16 bytes
>> + *
>> + * upstream: boolean indicating the direction of the DMA operation; upstream
>> + * means a transfer from the BMC to the host
>> + */
>> +struct aspeed_xdma_op {
>> + __u64 host_addr;
>> + __u32 len;
>> + __u32 upstream;
>> +};
>> +
>> +#endif /* _UAPI_LINUX_ASPEED_XDMA_H_ */
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* [PATCH v2 8/8] pinctrl: aspeed: Add implementation-related documentation
From: Andrew Jeffery @ 2019-06-28 2:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-1-andrew@aj.id.au>
The ASPEED pinctrl driver implementations make heavy use of macros to
minimise tedium of implementation and maximise the chance that the
compiler will catch errors in defining signal and pin configurations.
While the goal of minimising errors is achieved, it is at the cost of
the complexity of the macros.
Document examples of the expanded form of pin declarations to
demonstrate the operation of the macros.
Cc: Johnny Huang <johnny_huang@aspeedtech.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/pinctrl/aspeed/pinmux-aspeed.h | 204 ++++++++++++++++++++++++-
1 file changed, 200 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/aspeed/pinmux-aspeed.h b/drivers/pinctrl/aspeed/pinmux-aspeed.h
index a036ce8f1571..329d54d48667 100644
--- a/drivers/pinctrl/aspeed/pinmux-aspeed.h
+++ b/drivers/pinctrl/aspeed/pinmux-aspeed.h
@@ -18,7 +18,8 @@
* priority level are frequently not the same (i.e. cannot just flip a bit to
* change from a high to low priority signal), or even in the same register.
* Further, not all signals can be unmuxed, as some expressions depend on
- * values in the hardware strapping register (which is treated as read-only).
+ * values in the hardware strapping register (which may be treated as
+ * read-only).
*
* SoC Multi-function Pin Expression Examples
* ------------------------------------------
@@ -172,9 +173,9 @@
* * A signal expression is the smallest set of signal descriptors whose
* comparisons must evaluate 'true' for a signal to be enabled on a pin.
*
- * * A function's signal is active on a pin if evaluating all signal
- * descriptors in the pin's signal expression for the function yields a 'true'
- * result
+ * * A signal participating in a function is active on a pin if evaluating all
+ * signal descriptors in the pin's signal expression for the function yields
+ * a 'true' result
*
* * A signal at a given priority on a given pin is active if any of the
* functions in which the signal participates are active, and no higher
@@ -221,6 +222,201 @@
* well as pins) required for the group's configuration will already be in use,
* likely in a way that's inconsistent with the requirements of the failed
* group.
+ *
+ * Implementation
+ * --------------
+ *
+ * Beyond the documentation below the various structures and helper macros that
+ * allow the implementation to hang together are defined. The macros are fairly
+ * dense, so below we walk through some raw examples of the configuration
+ * tables in an effort to clarify the concepts.
+ *
+ * The complexity of configuring the mux combined with the scale of the pins
+ * and functions was a concern, so the table design along with the macro jungle
+ * is an attempt to address it. The rough principles of the approach are:
+ *
+ * 1. Use a data-driven solution rather than embedding state into code
+ * 2. Minimise editing to the specifics of the given mux configuration
+ * 3. Detect as many errors as possible at compile time
+ *
+ * Addressing point 3 leads to naming of symbols in terms of the four
+ * properties associated with a given mux configuration: The pin, the signal,
+ * the group and the function. In this way copy/paste errors cause duplicate
+ * symbols to be defined, which prevents successful compilation. Failing to
+ * properly parent the tables leads to unused symbol warnings, and use of
+ * designated initialisers and additional warnings ensures that there are
+ * no override errors in the pin, group and function arrays.
+ *
+ * Addressing point 2 drives the development of the macro jungle, as it
+ * centralises the definition noise at the cost of taking some time to
+ * understand.
+ *
+ * Here's a complete, concrete "pre-processed" example of the table structures
+ * used to describe the D6 ball from the examples above:
+ *
+ * ```
+ * static const struct aspeed_sig_desc sig_descs_MAC1LINK_MAC1LINK[] = {
+ * {
+ * .ip = ASPEED_IP_SCU,
+ * .reg = 0x80,
+ * .mask = BIT(0),
+ * .enable = 1,
+ * .disable = 0
+ * },
+ * };
+ *
+ * static const struct aspeed_sig_expr sig_expr_MAC1LINK_MAC1LINK = {
+ * .signal = "MAC1LINK",
+ * .function = "MAC1LINK",
+ * .ndescs = ARRAY_SIZE(sig_descs_MAC1LINK_MAC1LINK),
+ * .descs = &(sig_descs_MAC1LINK_MAC1LINK)[0],
+ * };
+ *
+ * static const struct aspeed_sig_expr *sig_exprs_MAC1LINK_MAC1LINK[] = {
+ * &sig_expr_MAC1LINK_MAC1LINK,
+ * NULL,
+ * };
+ *
+ * static const struct aspeed_sig_desc sig_descs_GPIOA0_GPIOA0[] = { };
+ *
+ * static const struct aspeed_sig_expr sig_expr_GPIOA0_GPIOA0 = {
+ * .signal = "GPIOA0",
+ * .function = "GPIOA0",
+ * .ndescs = ARRAY_SIZE(sig_descs_GPIOA0_GPIOA0),
+ * .descs = &(sig_descs_GPIOA0_GPIOA0)[0],
+ * };
+ *
+ * static const struct aspeed_sig_expr *sig_exprs_GPIOA0_GPIOA0[] = {
+ * &sig_expr_GPIOA0_GPIOA0,
+ * NULL
+ * };
+ *
+ * static const struct aspeed_sig_expr **pin_exprs_0[] = {
+ * sig_exprs_MAC1LINK_MAC1LINK,
+ * sig_exprs_GPIOA0_GPIOA0,
+ * NULL
+ * };
+ *
+ * static const struct aspeed_pin_desc pin_0 = { "0", (&pin_exprs_0[0]) };
+ * static const int group_pins_MAC1LINK[] = { 0 };
+ * static const char *func_groups_MAC1LINK[] = { "MAC1LINK" };
+ *
+ * static struct pinctrl_pin_desc aspeed_g4_pins[] = {
+ * [0] = { .number = 0, .name = "D6", .drv_data = &pin_0 },
+ * };
+ *
+ * static const struct aspeed_pin_group aspeed_g4_groups[] = {
+ * {
+ * .name = "MAC1LINK",
+ * .pins = &(group_pins_MAC1LINK)[0],
+ * .npins = ARRAY_SIZE(group_pins_MAC1LINK),
+ * },
+ * };
+ *
+ * static const struct aspeed_pin_function aspeed_g4_functions[] = {
+ * {
+ * .name = "MAC1LINK",
+ * .groups = &func_groups_MAC1LINK[0],
+ * .ngroups = ARRAY_SIZE(func_groups_MAC1LINK),
+ * },
+ * };
+ * ```
+ *
+ * At the end of the day much of the above code is compressed into the
+ * following two lines:
+ *
+ * ```
+ * #define D6 0
+ * SSSF_PIN_DECL(D6, GPIOA0, MAC1LINK, SIG_DESC_SET(SCU80, 0));
+ * ```
+ *
+ * The two examples below show just the differences from the example above.
+ *
+ * Ball E18 demonstrates a function, EXTRST, that requires multiple descriptors
+ * be set for it to be muxed:
+ *
+ * ```
+ * static const struct aspeed_sig_desc sig_descs_EXTRST_EXTRST[] = {
+ * {
+ * .ip = ASPEED_IP_SCU,
+ * .reg = 0x3C,
+ * .mask = BIT(3),
+ * .enable = 1,
+ * .disable = 0
+ * },
+ * {
+ * .ip = ASPEED_IP_SCU,
+ * .reg = 0x80,
+ * .mask = BIT(15),
+ * .enable = 1,
+ * .disable = 0
+ * },
+ * {
+ * .ip = ASPEED_IP_SCU,
+ * .reg = 0x90,
+ * .mask = BIT(31),
+ * .enable = 0,
+ * .disable = 1
+ * },
+ * };
+ *
+ * static const struct aspeed_sig_expr sig_expr_EXTRST_EXTRST = {
+ * .signal = "EXTRST",
+ * .function = "EXTRST",
+ * .ndescs = ARRAY_SIZE(sig_descs_EXTRST_EXTRST),
+ * .descs = &(sig_descs_EXTRST_EXTRST)[0],
+ * };
+ * ...
+ * ```
+ *
+ * For ball E19, we have multiple functions enabling a single signal, LPCRST#.
+ * The data structures look like:
+ *
+ * static const struct aspeed_sig_desc sig_descs_LPCRST_LPCRST[] = {
+ * {
+ * .ip = ASPEED_IP_SCU,
+ * .reg = 0x80,
+ * .mask = BIT(12),
+ * .enable = 1,
+ * .disable = 0
+ * },
+ * };
+ *
+ * static const struct aspeed_sig_expr sig_expr_LPCRST_LPCRST = {
+ * .signal = "LPCRST",
+ * .function = "LPCRST",
+ * .ndescs = ARRAY_SIZE(sig_descs_LPCRST_LPCRST),
+ * .descs = &(sig_descs_LPCRST_LPCRST)[0],
+ * };
+ *
+ * static const struct aspeed_sig_desc sig_descs_LPCRST_LPCRSTS[] = {
+ * {
+ * .ip = ASPEED_IP_SCU,
+ * .reg = 0x70,
+ * .mask = BIT(14),
+ * .enable = 1,
+ * .disable = 0
+ * },
+ * };
+ *
+ * static const struct aspeed_sig_expr sig_expr_LPCRST_LPCRSTS = {
+ * .signal = "LPCRST",
+ * .function = "LPCRSTS",
+ * .ndescs = ARRAY_SIZE(sig_descs_LPCRST_LPCRSTS),
+ * .descs = &(sig_descs_LPCRST_LPCRSTS)[0],
+ * };
+ *
+ * static const struct aspeed_sig_expr *sig_exprs_LPCRST_LPCRST[] = {
+ * &sig_expr_LPCRST_LPCRST,
+ * &sig_expr_LPCRST_LPCRSTS,
+ * NULL,
+ * };
+ * ...
+ * ```
+ *
+ * Both expressions listed in the sig_exprs_LPCRST_LPCRST array need to be set
+ * to disabled for the associated GPIO to be muxed.
+ *
*/
#define ASPEED_IP_SCU 0
--
2.20.1
^ permalink raw reply related
* [PATCH v2 7/8] pinctrl: aspeed: Split out pinmux from general pinctrl
From: Andrew Jeffery @ 2019-06-28 2:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-1-andrew@aj.id.au>
ASPEED have completely rearranged the System Control Unit register
layout with the AST2600. The existing code took advantage of the fact
that the AST2400 and AST2500 had layouts that were similar enough to
have little impact on the pinmux infrastructure (though there is a wart
with read-modify-write vs write-1-clear semantics of the hardware
strapping registers between the two).
Given that any similarity has been thrown out with the AST2600, separate
out the function applying an expression state to be driver-specific.
With it, extract out the pinmux macro jungle to its own header and
implementation so the pieces can be composed without dependency cycles.
Cc: Johnny Huang <johnny_huang@aspeedtech.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/pinctrl/aspeed/Makefile | 2 +-
drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c | 94 +++-
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 123 ++++-
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 250 ++--------
drivers/pinctrl/aspeed/pinctrl-aspeed.h | 548 +--------------------
drivers/pinctrl/aspeed/pinmux-aspeed.c | 96 ++++
drivers/pinctrl/aspeed/pinmux-aspeed.h | 539 ++++++++++++++++++++
7 files changed, 892 insertions(+), 760 deletions(-)
create mode 100644 drivers/pinctrl/aspeed/pinmux-aspeed.c
create mode 100644 drivers/pinctrl/aspeed/pinmux-aspeed.h
diff --git a/drivers/pinctrl/aspeed/Makefile b/drivers/pinctrl/aspeed/Makefile
index 068729bf4f86..ea8962645e49 100644
--- a/drivers/pinctrl/aspeed/Makefile
+++ b/drivers/pinctrl/aspeed/Makefile
@@ -2,6 +2,6 @@
# Aspeed pinctrl support
ccflags-y += $(call cc-option,-Woverride-init)
-obj-$(CONFIG_PINCTRL_ASPEED) += pinctrl-aspeed.o
+obj-$(CONFIG_PINCTRL_ASPEED) += pinctrl-aspeed.o pinmux-aspeed.o
obj-$(CONFIG_PINCTRL_ASPEED_G4) += pinctrl-aspeed-g4.o
obj-$(CONFIG_PINCTRL_ASPEED_G5) += pinctrl-aspeed-g5.o
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
index 73e2c9c0e549..384396cbb22d 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
@@ -18,8 +18,34 @@
#include "../core.h"
#include "../pinctrl-utils.h"
+#include "pinmux-aspeed.h"
#include "pinctrl-aspeed.h"
+/*
+ * The "Multi-function Pins Mapping and Control" table in the SoC datasheet
+ * references registers by the device/offset mnemonic. The register macros
+ * below are named the same way to ease transcription and verification (as
+ * opposed to naming them e.g. PINMUX_CTRL_[0-9]). Further, signal expressions
+ * reference registers beyond those dedicated to pinmux, such as the system
+ * reset control and MAC clock configuration registers.
+ */
+#define SCU2C 0x2C /* Misc. Control Register */
+#define SCU3C 0x3C /* System Reset Control/Status Register */
+#define SCU48 0x48 /* MAC Interface Clock Delay Setting */
+#define HW_STRAP1 0x70 /* AST2400 strapping is 33 bits, is split */
+#define HW_REVISION_ID 0x7C /* Silicon revision ID register */
+#define SCU80 0x80 /* Multi-function Pin Control #1 */
+#define SCU84 0x84 /* Multi-function Pin Control #2 */
+#define SCU88 0x88 /* Multi-function Pin Control #3 */
+#define SCU8C 0x8C /* Multi-function Pin Control #4 */
+#define SCU90 0x90 /* Multi-function Pin Control #5 */
+#define SCU94 0x94 /* Multi-function Pin Control #6 */
+#define SCUA0 0xA0 /* Multi-function Pin Control #7 */
+#define SCUA4 0xA4 /* Multi-function Pin Control #8 */
+#define SCUA8 0xA8 /* Multi-function Pin Control #9 */
+#define SCUAC 0xAC /* Multi-function Pin Control #10 */
+#define HW_STRAP2 0xD0 /* Strapping */
+
/*
* Uses undefined macros for symbol naming and references, eg GPIOA0, MAC1LINK,
* TIMER3 etc.
@@ -2386,13 +2412,73 @@ static const struct aspeed_pin_config aspeed_g4_configs[] = {
{ PIN_CONFIG_INPUT_DEBOUNCE, { C14, B14 }, SCUA8, 27 },
};
+static int aspeed_g4_sig_expr_set(const struct aspeed_pinmux_data *ctx,
+ const struct aspeed_sig_expr *expr,
+ bool enable)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < expr->ndescs; i++) {
+ const struct aspeed_sig_desc *desc = &expr->descs[i];
+ u32 pattern = enable ? desc->enable : desc->disable;
+ u32 val = (pattern << __ffs(desc->mask));
+
+ if (!ctx->maps[desc->ip])
+ return -ENODEV;
+
+ /*
+ * Strap registers are configured in hardware or by early-boot
+ * firmware. Treat them as read-only despite that we can write
+ * them. This may mean that certain functions cannot be
+ * deconfigured and is the reason we re-evaluate after writing
+ * all descriptor bits.
+ *
+ * Port D and port E GPIO loopback modes are the only exception
+ * as those are commonly used with front-panel buttons to allow
+ * normal operation of the host when the BMC is powered off or
+ * fails to boot. Once the BMC has booted, the loopback mode
+ * must be disabled for the BMC to control host power-on and
+ * reset.
+ */
+ if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
+ !(desc->mask & (BIT(21) | BIT(22))))
+ continue;
+
+ if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
+ continue;
+
+ ret = regmap_update_bits(ctx->maps[desc->ip], desc->reg,
+ desc->mask, val);
+
+ if (ret)
+ return ret;
+ }
+
+ ret = aspeed_sig_expr_eval(ctx, expr, enable);
+ if (ret < 0)
+ return ret;
+
+ if (!ret)
+ return -EPERM;
+
+ return 0;
+}
+
+static const struct aspeed_pinmux_ops aspeed_g4_ops = {
+ .set = aspeed_g4_sig_expr_set,
+};
+
static struct aspeed_pinctrl_data aspeed_g4_pinctrl_data = {
.pins = aspeed_g4_pins,
.npins = ARRAY_SIZE(aspeed_g4_pins),
- .groups = aspeed_g4_groups,
- .ngroups = ARRAY_SIZE(aspeed_g4_groups),
- .functions = aspeed_g4_functions,
- .nfunctions = ARRAY_SIZE(aspeed_g4_functions),
+ .pinmux = {
+ .ops = &aspeed_g4_ops,
+ .groups = aspeed_g4_groups,
+ .ngroups = ARRAY_SIZE(aspeed_g4_groups),
+ .functions = aspeed_g4_functions,
+ .nfunctions = ARRAY_SIZE(aspeed_g4_functions),
+ },
.configs = aspeed_g4_configs,
.nconfigs = ARRAY_SIZE(aspeed_g4_configs),
};
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
index aa7e148b38bb..e20e2a259141 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
@@ -21,6 +21,32 @@
#include "../pinctrl-utils.h"
#include "pinctrl-aspeed.h"
+/*
+ * The "Multi-function Pins Mapping and Control" table in the SoC datasheet
+ * references registers by the device/offset mnemonic. The register macros
+ * below are named the same way to ease transcription and verification (as
+ * opposed to naming them e.g. PINMUX_CTRL_[0-9]). Further, signal expressions
+ * reference registers beyond those dedicated to pinmux, such as the system
+ * reset control and MAC clock configuration registers. The AST2500 goes a step
+ * further and references registers in the graphics IP block.
+ */
+#define SCU2C 0x2C /* Misc. Control Register */
+#define SCU3C 0x3C /* System Reset Control/Status Register */
+#define SCU48 0x48 /* MAC Interface Clock Delay Setting */
+#define HW_STRAP1 0x70 /* AST2400 strapping is 33 bits, is split */
+#define HW_REVISION_ID 0x7C /* Silicon revision ID register */
+#define SCU80 0x80 /* Multi-function Pin Control #1 */
+#define SCU84 0x84 /* Multi-function Pin Control #2 */
+#define SCU88 0x88 /* Multi-function Pin Control #3 */
+#define SCU8C 0x8C /* Multi-function Pin Control #4 */
+#define SCU90 0x90 /* Multi-function Pin Control #5 */
+#define SCU94 0x94 /* Multi-function Pin Control #6 */
+#define SCUA0 0xA0 /* Multi-function Pin Control #7 */
+#define SCUA4 0xA4 /* Multi-function Pin Control #8 */
+#define SCUA8 0xA8 /* Multi-function Pin Control #9 */
+#define SCUAC 0xAC /* Multi-function Pin Control #10 */
+#define HW_STRAP2 0xD0 /* Strapping */
+
#define ASPEED_G5_NR_PINS 236
#define COND1 { ASPEED_IP_SCU, SCU90, BIT(6), 0, 0 }
@@ -2477,13 +2503,98 @@ static struct aspeed_pin_config aspeed_g5_configs[] = {
{ PIN_CONFIG_INPUT_DEBOUNCE, { A20, B19 }, SCUA8, 27 },
};
+/**
+ * Configure a pin's signal by applying an expression's descriptor state for
+ * all descriptors in the expression.
+ *
+ * @ctx: The pinmux context
+ * @expr: The expression associated with the function whose signal is to be
+ * configured
+ * @enable: true to enable an function's signal through a pin's signal
+ * expression, false to disable the function's signal
+ *
+ * Return: 0 if the expression is configured as requested and a negative error
+ * code otherwise
+ */
+static int aspeed_g5_sig_expr_set(const struct aspeed_pinmux_data *ctx,
+ const struct aspeed_sig_expr *expr,
+ bool enable)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < expr->ndescs; i++) {
+ const struct aspeed_sig_desc *desc = &expr->descs[i];
+ u32 pattern = enable ? desc->enable : desc->disable;
+ u32 val = (pattern << __ffs(desc->mask));
+
+ if (!ctx->maps[desc->ip])
+ return -ENODEV;
+
+ /*
+ * Strap registers are configured in hardware or by early-boot
+ * firmware. Treat them as read-only despite that we can write
+ * them. This may mean that certain functions cannot be
+ * deconfigured and is the reason we re-evaluate after writing
+ * all descriptor bits.
+ *
+ * Port D and port E GPIO loopback modes are the only exception
+ * as those are commonly used with front-panel buttons to allow
+ * normal operation of the host when the BMC is powered off or
+ * fails to boot. Once the BMC has booted, the loopback mode
+ * must be disabled for the BMC to control host power-on and
+ * reset.
+ */
+ if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
+ !(desc->mask & (BIT(21) | BIT(22))))
+ continue;
+
+ if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
+ continue;
+
+ /* On AST2500, Set bits in SCU70 are cleared from SCU7C */
+ if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1) {
+ u32 value = ~val & desc->mask;
+
+ if (value) {
+ ret = regmap_write(ctx->maps[desc->ip],
+ HW_REVISION_ID, value);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ ret = regmap_update_bits(ctx->maps[desc->ip], desc->reg,
+ desc->mask, val);
+
+ if (ret)
+ return ret;
+ }
+
+ ret = aspeed_sig_expr_eval(ctx, expr, enable);
+ if (ret < 0)
+ return ret;
+
+ if (!ret)
+ return -EPERM;
+
+ return 0;
+}
+
+static const struct aspeed_pinmux_ops aspeed_g5_ops = {
+ .set = aspeed_g5_sig_expr_set,
+};
+
static struct aspeed_pinctrl_data aspeed_g5_pinctrl_data = {
.pins = aspeed_g5_pins,
.npins = ARRAY_SIZE(aspeed_g5_pins),
- .groups = aspeed_g5_groups,
- .ngroups = ARRAY_SIZE(aspeed_g5_groups),
- .functions = aspeed_g5_functions,
- .nfunctions = ARRAY_SIZE(aspeed_g5_functions),
+ .pinmux = {
+ .ops = &aspeed_g5_ops,
+ .groups = aspeed_g5_groups,
+ .ngroups = ARRAY_SIZE(aspeed_g5_groups),
+ .functions = aspeed_g5_functions,
+ .nfunctions = ARRAY_SIZE(aspeed_g5_functions),
+ },
.configs = aspeed_g5_configs,
.nconfigs = ARRAY_SIZE(aspeed_g5_configs),
};
@@ -2539,7 +2650,7 @@ static int aspeed_g5_pinctrl_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "No GFX phandle found, some mux configurations may fail\n");
map = NULL;
}
- aspeed_g5_pinctrl_data.maps[ASPEED_IP_GFX] = map;
+ aspeed_g5_pinctrl_data.pinmux.maps[ASPEED_IP_GFX] = map;
node = of_parse_phandle(pdev->dev.of_node, "aspeed,external-nodes", 1);
if (node) {
@@ -2553,7 +2664,7 @@ static int aspeed_g5_pinctrl_probe(struct platform_device *pdev)
map = NULL;
}
of_node_put(node);
- aspeed_g5_pinctrl_data.maps[ASPEED_IP_LPC] = map;
+ aspeed_g5_pinctrl_data.pinmux.maps[ASPEED_IP_LPC] = map;
return aspeed_pinctrl_probe(pdev, &aspeed_g5_pinctrl_desc,
&aspeed_g5_pinctrl_data);
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index b510bb475851..18f46177b4af 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Copyright (C) 2016 IBM Corp.
- */
+/* Copyright (C) 2016 IBM Corp. */
#include <linux/mfd/syscon.h>
#include <linux/platform_device.h>
@@ -10,17 +8,11 @@
#include "../core.h"
#include "pinctrl-aspeed.h"
-static const char *const aspeed_pinmux_ips[] = {
- [ASPEED_IP_SCU] = "SCU",
- [ASPEED_IP_GFX] = "GFX",
- [ASPEED_IP_LPC] = "LPC",
-};
-
int aspeed_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
{
struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
- return pdata->ngroups;
+ return pdata->pinmux.ngroups;
}
const char *aspeed_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
@@ -28,7 +20,7 @@ const char *aspeed_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
{
struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
- return pdata->groups[group].name;
+ return pdata->pinmux.groups[group].name;
}
int aspeed_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
@@ -37,8 +29,8 @@ int aspeed_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
{
struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
- *pins = &pdata->groups[group].pins[0];
- *npins = pdata->groups[group].npins;
+ *pins = &pdata->pinmux.groups[group].pins[0];
+ *npins = pdata->pinmux.groups[group].npins;
return 0;
}
@@ -53,7 +45,7 @@ int aspeed_pinmux_get_fn_count(struct pinctrl_dev *pctldev)
{
struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
- return pdata->nfunctions;
+ return pdata->pinmux.nfunctions;
}
const char *aspeed_pinmux_get_fn_name(struct pinctrl_dev *pctldev,
@@ -61,7 +53,7 @@ const char *aspeed_pinmux_get_fn_name(struct pinctrl_dev *pctldev,
{
struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
- return pdata->functions[function].name;
+ return pdata->pinmux.functions[function].name;
}
int aspeed_pinmux_get_fn_groups(struct pinctrl_dev *pctldev,
@@ -71,208 +63,38 @@ int aspeed_pinmux_get_fn_groups(struct pinctrl_dev *pctldev,
{
struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
- *groups = pdata->functions[function].groups;
- *num_groups = pdata->functions[function].ngroups;
+ *groups = pdata->pinmux.functions[function].groups;
+ *num_groups = pdata->pinmux.functions[function].ngroups;
return 0;
}
-static inline void aspeed_sig_desc_print_val(
- const struct aspeed_sig_desc *desc, bool enable, u32 rv)
-{
- pr_debug("Want %s%X[0x%08X]=0x%X, got 0x%X from 0x%08X\n",
- aspeed_pinmux_ips[desc->ip], desc->reg,
- desc->mask, enable ? desc->enable : desc->disable,
- (rv & desc->mask) >> __ffs(desc->mask), rv);
-}
-
-/**
- * Query the enabled or disabled state of a signal descriptor
- *
- * @desc: The signal descriptor of interest
- * @enabled: True to query the enabled state, false to query disabled state
- * @map: The IP block's regmap instance
- *
- * Return: 1 if the descriptor's bitfield is configured to the state
- * selected by @enabled, 0 if not, and less than zero if an unrecoverable
- * failure occurred
- *
- * Evaluation of descriptor state is non-trivial in that it is not a binary
- * outcome: The bitfields can be greater than one bit in size and thus can take
- * a value that is neither the enabled nor disabled state recorded in the
- * descriptor (typically this means a different function to the one of interest
- * is enabled). Thus we must explicitly test for either condition as required.
- */
-static int aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc,
- bool enabled, struct regmap *map)
-{
- int ret;
- unsigned int raw;
- u32 want;
-
- if (!map)
- return -ENODEV;
-
- ret = regmap_read(map, desc->reg, &raw);
- if (ret)
- return ret;
-
- aspeed_sig_desc_print_val(desc, enabled, raw);
- want = enabled ? desc->enable : desc->disable;
-
- return ((raw & desc->mask) >> __ffs(desc->mask)) == want;
-}
-
-/**
- * Query the enabled or disabled state for a mux function's signal on a pin
- *
- * @expr: An expression controlling the signal for a mux function on a pin
- * @enabled: True to query the enabled state, false to query disabled state
- * @maps: The list of regmap instances
- *
- * Return: 1 if the expression composed by @enabled evaluates true, 0 if not,
- * and less than zero if an unrecoverable failure occurred.
- *
- * A mux function is enabled or disabled if the function's signal expression
- * for each pin in the function's pin group evaluates true for the desired
- * state. An signal expression evaluates true if all of its associated signal
- * descriptors evaluate true for the desired state.
- *
- * If an expression's state is described by more than one bit, either through
- * multi-bit bitfields in a single signal descriptor or through multiple signal
- * descriptors of a single bit then it is possible for the expression to be in
- * neither the enabled nor disabled state. Thus we must explicitly test for
- * either condition as required.
- */
-static int aspeed_sig_expr_eval(const struct aspeed_sig_expr *expr,
- bool enabled, struct regmap * const *maps)
-{
- int i;
- int ret;
-
- for (i = 0; i < expr->ndescs; i++) {
- const struct aspeed_sig_desc *desc = &expr->descs[i];
-
- ret = aspeed_sig_desc_eval(desc, enabled, maps[desc->ip]);
- if (ret <= 0)
- return ret;
- }
-
- return 1;
-}
-
-/**
- * Configure a pin's signal by applying an expression's descriptor state for
- * all descriptors in the expression.
- *
- * @expr: The expression associated with the function whose signal is to be
- * configured
- * @enable: true to enable an function's signal through a pin's signal
- * expression, false to disable the function's signal
- * @maps: The list of regmap instances for pinmux register access.
- *
- * Return: 0 if the expression is configured as requested and a negative error
- * code otherwise
- */
-static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
- bool enable, struct regmap * const *maps)
-{
- int ret;
- int i;
-
- for (i = 0; i < expr->ndescs; i++) {
- const struct aspeed_sig_desc *desc = &expr->descs[i];
- u32 pattern = enable ? desc->enable : desc->disable;
- u32 val = (pattern << __ffs(desc->mask));
-
- if (!maps[desc->ip])
- return -ENODEV;
-
- /*
- * Strap registers are configured in hardware or by early-boot
- * firmware. Treat them as read-only despite that we can write
- * them. This may mean that certain functions cannot be
- * deconfigured and is the reason we re-evaluate after writing
- * all descriptor bits.
- *
- * Port D and port E GPIO loopback modes are the only exception
- * as those are commonly used with front-panel buttons to allow
- * normal operation of the host when the BMC is powered off or
- * fails to boot. Once the BMC has booted, the loopback mode
- * must be disabled for the BMC to control host power-on and
- * reset.
- */
- if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
- !(desc->mask & (BIT(21) | BIT(22))))
- continue;
-
- if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
- continue;
-
- /* On AST2500, Set bits in SCU70 are cleared from SCU7C */
- if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1) {
- unsigned int rev_id;
-
- ret = regmap_read(maps[ASPEED_IP_SCU],
- HW_REVISION_ID, &rev_id);
- if (ret < 0)
- return ret;
-
- if (0x04 == (rev_id >> 24)) {
- u32 value = ~val & desc->mask;
-
- if (value) {
- ret = regmap_write(maps[desc->ip],
- HW_REVISION_ID, value);
- if (ret < 0)
- return ret;
- }
- }
- }
-
- ret = regmap_update_bits(maps[desc->ip], desc->reg,
- desc->mask, val);
-
- if (ret)
- return ret;
- }
-
- ret = aspeed_sig_expr_eval(expr, enable, maps);
- if (ret < 0)
- return ret;
-
- if (!ret)
- return -EPERM;
-
- return 0;
-}
-
-static int aspeed_sig_expr_enable(const struct aspeed_sig_expr *expr,
- struct regmap * const *maps)
+static int aspeed_sig_expr_enable(const struct aspeed_pinmux_data *ctx,
+ const struct aspeed_sig_expr *expr)
{
int ret;
- ret = aspeed_sig_expr_eval(expr, true, maps);
+ ret = aspeed_sig_expr_eval(ctx, expr, true);
if (ret < 0)
return ret;
if (!ret)
- return aspeed_sig_expr_set(expr, true, maps);
+ return aspeed_sig_expr_set(ctx, expr, true);
return 0;
}
-static int aspeed_sig_expr_disable(const struct aspeed_sig_expr *expr,
- struct regmap * const *maps)
+static int aspeed_sig_expr_disable(const struct aspeed_pinmux_data *ctx,
+ const struct aspeed_sig_expr *expr)
{
int ret;
- ret = aspeed_sig_expr_eval(expr, true, maps);
+ ret = aspeed_sig_expr_eval(ctx, expr, true);
if (ret < 0)
return ret;
if (ret)
- return aspeed_sig_expr_set(expr, false, maps);
+ return aspeed_sig_expr_set(ctx, expr, false);
return 0;
}
@@ -280,13 +102,13 @@ static int aspeed_sig_expr_disable(const struct aspeed_sig_expr *expr,
/**
* Disable a signal on a pin by disabling all provided signal expressions.
*
+ * @ctx: The pinmux context
* @exprs: The list of signal expressions (from a priority level on a pin)
- * @maps: The list of regmap instances for pinmux register access.
*
* Return: 0 if all expressions are disabled, otherwise a negative error code
*/
-static int aspeed_disable_sig(const struct aspeed_sig_expr **exprs,
- struct regmap * const *maps)
+static int aspeed_disable_sig(const struct aspeed_pinmux_data *ctx,
+ const struct aspeed_sig_expr **exprs)
{
int ret = 0;
@@ -294,7 +116,7 @@ static int aspeed_disable_sig(const struct aspeed_sig_expr **exprs,
return true;
while (*exprs && !ret) {
- ret = aspeed_sig_expr_disable(*exprs, maps);
+ ret = aspeed_sig_expr_disable(ctx, *exprs);
exprs++;
}
@@ -395,9 +217,9 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
int ret;
const struct aspeed_pinctrl_data *pdata =
pinctrl_dev_get_drvdata(pctldev);
- const struct aspeed_pin_group *pgroup = &pdata->groups[group];
+ const struct aspeed_pin_group *pgroup = &pdata->pinmux.groups[group];
const struct aspeed_pin_function *pfunc =
- &pdata->functions[function];
+ &pdata->pinmux.functions[function];
for (i = 0; i < pgroup->npins; i++) {
int pin = pgroup->pins[i];
@@ -423,7 +245,7 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
if (expr)
break;
- ret = aspeed_disable_sig(funcs, pdata->maps);
+ ret = aspeed_disable_sig(&pdata->pinmux, funcs);
if (ret)
return ret;
@@ -443,7 +265,7 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
return -ENXIO;
}
- ret = aspeed_sig_expr_enable(expr, pdata->maps);
+ ret = aspeed_sig_expr_enable(&pdata->pinmux, expr);
if (ret)
return ret;
}
@@ -500,7 +322,7 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
if (aspeed_gpio_in_exprs(funcs))
break;
- ret = aspeed_disable_sig(funcs, pdata->maps);
+ ret = aspeed_disable_sig(&pdata->pinmux, funcs);
if (ret)
return ret;
@@ -531,7 +353,7 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
* If GPIO is not the lowest priority signal type, assume there is only
* one expression defined to enable the GPIO function
*/
- return aspeed_sig_expr_enable(expr, pdata->maps);
+ return aspeed_sig_expr_enable(&pdata->pinmux, expr);
}
int aspeed_pinctrl_probe(struct platform_device *pdev,
@@ -547,12 +369,14 @@ int aspeed_pinctrl_probe(struct platform_device *pdev,
return -ENODEV;
}
- pdata->maps[ASPEED_IP_SCU] = syscon_node_to_regmap(parent->of_node);
- if (IS_ERR(pdata->maps[ASPEED_IP_SCU])) {
+ pdata->scu = syscon_node_to_regmap(parent->of_node);
+ if (IS_ERR(pdata->scu)) {
dev_err(&pdev->dev, "No regmap for syscon pincontroller parent\n");
- return PTR_ERR(pdata->maps[ASPEED_IP_SCU]);
+ return PTR_ERR(pdata->scu);
}
+ pdata->pinmux.maps[ASPEED_IP_SCU] = pdata->scu;
+
pctl = pinctrl_register(pdesc, &pdev->dev, pdata);
if (IS_ERR(pctl)) {
@@ -587,7 +411,9 @@ static inline const struct aspeed_pin_config *find_pinconf_config(
return NULL;
}
-/**
+/*
+ * Aspeed pin configuration description.
+ *
* @param: pinconf configuration parameter
* @arg: The supported argument for @param, or -1 if any value is supported
* @val: The register value to write to configure @arg for @param
@@ -661,7 +487,7 @@ int aspeed_pin_config_get(struct pinctrl_dev *pctldev, unsigned int offset,
if (!pconf)
return -ENOTSUPP;
- rc = regmap_read(pdata->maps[ASPEED_IP_SCU], pconf->reg, &val);
+ rc = regmap_read(pdata->scu, pconf->reg, &val);
if (rc < 0)
return rc;
@@ -716,8 +542,8 @@ int aspeed_pin_config_set(struct pinctrl_dev *pctldev, unsigned int offset,
val = pmap->val << pconf->bit;
- rc = regmap_update_bits(pdata->maps[ASPEED_IP_SCU], pconf->reg,
- BIT(pconf->bit), val);
+ rc = regmap_update_bits(pdata->scu, pconf->reg,
+ BIT(pconf->bit), val);
if (rc < 0)
return rc;
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
index c5918c4a087c..11cc0eb6666b 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
@@ -1,514 +1,15 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (C) 2016 IBM Corp.
- */
+/* Copyright (C) 2016,2019 IBM Corp. */
#ifndef PINCTRL_ASPEED
#define PINCTRL_ASPEED
+#include "pinmux-aspeed.h"
+
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinmux.h>
#include <linux/pinctrl/pinconf.h>
#include <linux/pinctrl/pinconf-generic.h>
-#include <linux/regmap.h>
-
-/*
- * The ASPEED SoCs provide typically more than 200 pins for GPIO and other
- * functions. The SoC function enabled on a pin is determined on a priority
- * basis where a given pin can provide a number of different signal types.
- *
- * The signal active on a pin is described by both a priority level and
- * compound logical expressions involving multiple operators, registers and
- * bits. Some difficulty arises as the pin's function bit masks for each
- * priority level are frequently not the same (i.e. cannot just flip a bit to
- * change from a high to low priority signal), or even in the same register.
- * Further, not all signals can be unmuxed, as some expressions depend on
- * values in the hardware strapping register (which is treated as read-only).
- *
- * SoC Multi-function Pin Expression Examples
- * ------------------------------------------
- *
- * Here are some sample mux configurations from the AST2400 and AST2500
- * datasheets to illustrate the corner cases, roughly in order of least to most
- * corner. The signal priorities are in decending order from P0 (highest).
- *
- * D6 is a pin with a single function (beside GPIO); a high priority signal
- * that participates in one function:
- *
- * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- * D6 GPIOA0 MAC1LINK SCU80[0]=1 GPIOA0
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- *
- * C5 is a multi-signal pin (high and low priority signals). Here we touch
- * different registers for the different functions that enable each signal:
- *
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- * C5 GPIOA4 SCL9 SCU90[22]=1 TIMER5 SCU80[4]=1 GPIOA4
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- *
- * E19 is a single-signal pin with two functions that influence the active
- * signal. In this case both bits have the same meaning - enable a dedicated
- * LPC reset pin. However it's not always the case that the bits in the
- * OR-relationship have the same meaning.
- *
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- * E19 GPIOB4 LPCRST# SCU80[12]=1 | Strap[14]=1 GPIOB4
- * -----+---------+-----------+-----------------------------+-----------+---------------+----------
- *
- * For example, pin B19 has a low-priority signal that's enabled by two
- * distinct SoC functions: A specific SIOPBI bit in register SCUA4, and an ACPI
- * bit in the STRAP register. The ACPI bit configures signals on pins in
- * addition to B19. Both of the low priority functions as well as the high
- * priority function must be disabled for GPIOF1 to be used.
- *
- * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
- * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
- * B19 GPIOF1 NDCD4 SCU80[25]=1 SIOPBI# SCUA4[12]=1 | Strap[19]=0 GPIOF1
- * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
- *
- * For pin E18, the SoC ANDs the expected state of three bits to determine the
- * pin's active signal:
- *
- * * SCU3C[3]: Enable external SOC reset function
- * * SCU80[15]: Enable SPICS1# or EXTRST# function pin
- * * SCU90[31]: Select SPI interface CS# output
- *
- * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
- * E18 GPIOB7 EXTRST# SCU3C[3]=1 & SCU80[15]=1 & SCU90[31]=0 SPICS1# SCU3C[3]=1 & SCU80[15]=1 & SCU90[31]=1 GPIOB7
- * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
- *
- * (Bits SCU3C[3] and SCU80[15] appear to only be used in the expressions for
- * selecting the signals on pin E18)
- *
- * Pin T5 is a multi-signal pin with a more complex configuration:
- *
- * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- * T5 GPIOL1 VPIDE SCU90[5:4]!=0 & SCU84[17]=1 NDCD1 SCU84[17]=1 GPIOL1
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- *
- * The high priority signal configuration is best thought of in terms of its
- * exploded form, with reference to the SCU90[5:4] bits:
- *
- * * SCU90[5:4]=00: disable
- * * SCU90[5:4]=01: 18 bits (R6/G6/B6) video mode.
- * * SCU90[5:4]=10: 24 bits (R8/G8/B8) video mode.
- * * SCU90[5:4]=11: 30 bits (R10/G10/B10) video mode.
- *
- * Re-writing:
- *
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- * T5 GPIOL1 VPIDE (SCU90[5:4]=1 & SCU84[17]=1) NDCD1 SCU84[17]=1 GPIOL1
- * | (SCU90[5:4]=2 & SCU84[17]=1)
- * | (SCU90[5:4]=3 & SCU84[17]=1)
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- *
- * For reference the SCU84[17] bit configure the "UART1 NDCD1 or Video VPIDE
- * function pin", where the signal itself is determined by whether SCU94[5:4]
- * is disabled or in one of the 18, 24 or 30bit video modes.
- *
- * Other video-input-related pins require an explicit state in SCU90[5:4], e.g.
- * W1 and U5:
- *
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- * W1 GPIOL6 VPIB0 SCU90[5:4]=3 & SCU84[22]=1 TXD1 SCU84[22]=1 GPIOL6
- * U5 GPIOL7 VPIB1 SCU90[5:4]=3 & SCU84[23]=1 RXD1 SCU84[23]=1 GPIOL7
- * -----+---------+-----------+------------------------------+-----------+---------------+----------
- *
- * The examples of T5 and W1 are particularly fertile, as they also demonstrate
- * that despite operating as part of the video input bus each signal needs to
- * be enabled individually via it's own SCU84 (in the cases of T5 and W1)
- * register bit. This is a little crazy if the bus doesn't have optional
- * signals, but is used to decent effect with some of the UARTs where not all
- * signals are required. However, this isn't done consistently - UART1 is
- * enabled on a per-pin basis, and by contrast, all signals for UART6 are
- * enabled by a single bit.
- *
- * Further, the high and low priority signals listed in the table above share
- * a configuration bit. The VPI signals should operate in concert in a single
- * function, but the UART signals should retain the ability to be configured
- * independently. This pushes the implementation down the path of tagging a
- * signal's expressions with the function they participate in, rather than
- * defining masks affecting multiple signals per function. The latter approach
- * fails in this instance where applying the configuration for the UART pin of
- * interest will stomp on the state of other UART signals when disabling the
- * VPI functions on the current pin.
- *
- * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
- * -----+------------+-----------+---------------------------+-----------+---------------+------------
- * A12 RGMII1TXCK GPIOT0 SCUA0[0]=1 RMII1TXEN Strap[6]=0 RGMII1TXCK
- * B12 RGMII1TXCTL GPIOT1 SCUA0[1]=1 ? Strap[6]=0 RGMII1TXCTL
- * -----+------------+-----------+---------------------------+-----------+---------------+------------
- *
- * A12 demonstrates that the "Other" signal isn't always GPIO - in this case
- * GPIOT0 is a high-priority signal and RGMII1TXCK is Other. Thus, GPIO
- * should be treated like any other signal type with full function expression
- * requirements, and not assumed to be the default case. Separately, GPIOT0 and
- * GPIOT1's signal descriptor bits are distinct, therefore we must iterate all
- * pins in the function's group to disable the higher-priority signals such
- * that the signal for the function of interest is correctly enabled.
- *
- * Finally, three priority levels aren't always enough; the AST2500 brings with
- * it 18 pins of five priority levels, however the 18 pins only use three of
- * the five priority levels.
- *
- * Ultimately the requirement to control pins in the examples above drive the
- * design:
- *
- * * Pins provide signals according to functions activated in the mux
- * configuration
- *
- * * Pins provide up to five signal types in a priority order
- *
- * * For priorities levels defined on a pin, each priority provides one signal
- *
- * * Enabling lower priority signals requires higher priority signals be
- * disabled
- *
- * * A function represents a set of signals; functions are distinct if their
- * sets of signals are not equal
- *
- * * Signals participate in one or more functions
- *
- * * A function is described by an expression of one or more signal
- * descriptors, which compare bit values in a register
- *
- * * A signal expression is the smallest set of signal descriptors whose
- * comparisons must evaluate 'true' for a signal to be enabled on a pin.
- *
- * * A function's signal is active on a pin if evaluating all signal
- * descriptors in the pin's signal expression for the function yields a 'true'
- * result
- *
- * * A signal at a given priority on a given pin is active if any of the
- * functions in which the signal participates are active, and no higher
- * priority signal on the pin is active
- *
- * * GPIO is configured per-pin
- *
- * And so:
- *
- * * To disable a signal, any function(s) activating the signal must be
- * disabled
- *
- * * Each pin must know the signal expressions of functions in which it
- * participates, for the purpose of enabling the Other function. This is done
- * by deactivating all functions that activate higher priority signals on the
- * pin.
- *
- * As a concrete example:
- *
- * * T5 provides three signals types: VPIDE, NDCD1 and GPIO
- *
- * * The VPIDE signal participates in 3 functions: VPI18, VPI24 and VPI30
- *
- * * The NDCD1 signal participates in just its own NDCD1 function
- *
- * * VPIDE is high priority, NDCD1 is low priority, and GPIOL1 is the least
- * prioritised
- *
- * * The prerequisit for activating the NDCD1 signal is that the VPI18, VPI24
- * and VPI30 functions all be disabled
- *
- * * Similarly, all of VPI18, VPI24, VPI30 and NDCD1 functions must be disabled
- * to provide GPIOL6
- *
- * Considerations
- * --------------
- *
- * If pinctrl allows us to allocate a pin we can configure a function without
- * concern for the function of already allocated pins, if pin groups are
- * created with respect to the SoC functions in which they participate. This is
- * intuitive, but it did not feel obvious from the bit/pin relationships.
- *
- * Conversely, failing to allocate all pins in a group indicates some bits (as
- * well as pins) required for the group's configuration will already be in use,
- * likely in a way that's inconsistent with the requirements of the failed
- * group.
- */
-
-#define ASPEED_IP_SCU 0
-#define ASPEED_IP_GFX 1
-#define ASPEED_IP_LPC 2
-#define ASPEED_NR_PINMUX_IPS 3
-
-/*
- * The "Multi-function Pins Mapping and Control" table in the SoC datasheet
- * references registers by the device/offset mnemonic. The register macros
- * below are named the same way to ease transcription and verification (as
- * opposed to naming them e.g. PINMUX_CTRL_[0-9]). Further, signal expressions
- * reference registers beyond those dedicated to pinmux, such as the system
- * reset control and MAC clock configuration registers. The AST2500 goes a step
- * further and references registers in the graphics IP block.
- */
-#define SCU2C 0x2C /* Misc. Control Register */
-#define SCU3C 0x3C /* System Reset Control/Status Register */
-#define SCU48 0x48 /* MAC Interface Clock Delay Setting */
-#define HW_STRAP1 0x70 /* AST2400 strapping is 33 bits, is split */
-#define HW_REVISION_ID 0x7C /* Silicon revision ID register */
-#define SCU80 0x80 /* Multi-function Pin Control #1 */
-#define SCU84 0x84 /* Multi-function Pin Control #2 */
-#define SCU88 0x88 /* Multi-function Pin Control #3 */
-#define SCU8C 0x8C /* Multi-function Pin Control #4 */
-#define SCU90 0x90 /* Multi-function Pin Control #5 */
-#define SCU94 0x94 /* Multi-function Pin Control #6 */
-#define SCUA0 0xA0 /* Multi-function Pin Control #7 */
-#define SCUA4 0xA4 /* Multi-function Pin Control #8 */
-#define SCUA8 0xA8 /* Multi-function Pin Control #9 */
-#define SCUAC 0xAC /* Multi-function Pin Control #10 */
-#define HW_STRAP2 0xD0 /* Strapping */
-
- /**
- * A signal descriptor, which describes the register, bits and the
- * enable/disable values that should be compared or written.
- *
- * @ip: The IP block identifier, used as an index into the regmap array in
- * struct aspeed_pinctrl_data
- * @reg: The register offset with respect to the base address of the IP block
- * @mask: The mask to apply to the register. The lowest set bit of the mask is
- * used to derive the shift value.
- * @enable: The value that enables the function. Value should be in the LSBs,
- * not at the position of the mask.
- * @disable: The value that disables the function. Value should be in the
- * LSBs, not at the position of the mask.
- */
-struct aspeed_sig_desc {
- unsigned int ip;
- unsigned int reg;
- u32 mask;
- u32 enable;
- u32 disable;
-};
-
-/**
- * Describes a signal expression. The expression is evaluated by ANDing the
- * evaluation of the descriptors.
- *
- * @signal: The signal name for the priority level on the pin. If the signal
- * type is GPIO, then the signal name must begin with the string
- * "GPIO", e.g. GPIOA0, GPIOT4 etc.
- * @function: The name of the function the signal participates in for the
- * associated expression
- * @ndescs: The number of signal descriptors in the expression
- * @descs: Pointer to an array of signal descriptors that comprise the
- * function expression
- */
-struct aspeed_sig_expr {
- const char *signal;
- const char *function;
- int ndescs;
- const struct aspeed_sig_desc *descs;
-};
-
-/**
- * A struct capturing the list of expressions enabling signals at each priority
- * for a given pin. The signal configuration for a priority level is evaluated
- * by ORing the evaluation of the signal expressions in the respective
- * priority's list.
- *
- * @name: A name for the pin
- * @prios: A pointer to an array of expression list pointers
- *
- */
-struct aspeed_pin_desc {
- const char *name;
- const struct aspeed_sig_expr ***prios;
-};
-
-/* Macro hell */
-
-#define SIG_DESC_IP_BIT(ip, reg, idx, val) \
- { ip, reg, BIT_MASK(idx), val, (((val) + 1) & 1) }
-
-/**
- * Short-hand macro for describing an SCU descriptor enabled by the state of
- * one bit. The disable value is derived.
- *
- * @reg: The signal's associated register, offset from base
- * @idx: The signal's bit index in the register
- * @val: The value (0 or 1) that enables the function
- */
-#define SIG_DESC_BIT(reg, idx, val) \
- SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, val)
-
-#define SIG_DESC_IP_SET(ip, reg, idx) SIG_DESC_IP_BIT(ip, reg, idx, 1)
-
-/**
- * A further short-hand macro expanding to an SCU descriptor enabled by a set
- * bit.
- *
- * @reg: The register, offset from base
- * @idx: The bit index in the register
- */
-#define SIG_DESC_SET(reg, idx) SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, 1)
-
-#define SIG_DESC_LIST_SYM(sig, func) sig_descs_ ## sig ## _ ## func
-#define SIG_DESC_LIST_DECL(sig, func, ...) \
- static const struct aspeed_sig_desc SIG_DESC_LIST_SYM(sig, func)[] = \
- { __VA_ARGS__ }
-
-#define SIG_EXPR_SYM(sig, func) sig_expr_ ## sig ## _ ## func
-#define SIG_EXPR_DECL_(sig, func) \
- static const struct aspeed_sig_expr SIG_EXPR_SYM(sig, func) = \
- { \
- .signal = #sig, \
- .function = #func, \
- .ndescs = ARRAY_SIZE(SIG_DESC_LIST_SYM(sig, func)), \
- .descs = &(SIG_DESC_LIST_SYM(sig, func))[0], \
- }
-
-/**
- * Declare a signal expression.
- *
- * @sig: A macro symbol name for the signal (is subjected to stringification
- * and token pasting)
- * @func: The function in which the signal is participating
- * @...: Signal descriptors that define the signal expression
- *
- * For example, the following declares the ROMD8 signal for the ROM16 function:
- *
- * SIG_EXPR_DECL(ROMD8, ROM16, SIG_DESC_SET(SCU90, 6));
- *
- * And with multiple signal descriptors:
- *
- * SIG_EXPR_DECL(ROMD8, ROM16S, SIG_DESC_SET(HW_STRAP1, 4),
- * { HW_STRAP1, GENMASK(1, 0), 0, 0 });
- */
-#define SIG_EXPR_DECL(sig, func, ...) \
- SIG_DESC_LIST_DECL(sig, func, __VA_ARGS__); \
- SIG_EXPR_DECL_(sig, func)
-
-/**
- * Declare a pointer to a signal expression
- *
- * @sig: The macro symbol name for the signal (subjected to token pasting)
- * @func: The macro symbol name for the function (subjected to token pasting)
- */
-#define SIG_EXPR_PTR(sig, func) (&SIG_EXPR_SYM(sig, func))
-
-#define SIG_EXPR_LIST_SYM(sig) sig_exprs_ ## sig
-
-/**
- * Declare a signal expression list for reference in a struct aspeed_pin_prio.
- *
- * @sig: A macro symbol name for the signal (is subjected to token pasting)
- * @...: Signal expression structure pointers (use SIG_EXPR_PTR())
- *
- * For example, the 16-bit ROM bus can be enabled by one of two possible signal
- * expressions:
- *
- * SIG_EXPR_DECL(ROMD8, ROM16, SIG_DESC_SET(SCU90, 6));
- * SIG_EXPR_DECL(ROMD8, ROM16S, SIG_DESC_SET(HW_STRAP1, 4),
- * { HW_STRAP1, GENMASK(1, 0), 0, 0 });
- * SIG_EXPR_LIST_DECL(ROMD8, SIG_EXPR_PTR(ROMD8, ROM16),
- * SIG_EXPR_PTR(ROMD8, ROM16S));
- */
-#define SIG_EXPR_LIST_DECL(sig, ...) \
- static const struct aspeed_sig_expr *SIG_EXPR_LIST_SYM(sig)[] = \
- { __VA_ARGS__, NULL }
-
-/**
- * A short-hand macro for declaring a function expression and an expression
- * list with a single function.
- *
- * @func: A macro symbol name for the function (is subjected to token pasting)
- * @...: Function descriptors that define the function expression
- *
- * For example, signal NCTS6 participates in its own function with one group:
- *
- * SIG_EXPR_LIST_DECL_SINGLE(NCTS6, NCTS6, SIG_DESC_SET(SCU90, 7));
- */
-#define SIG_EXPR_LIST_DECL_SINGLE(sig, func, ...) \
- SIG_DESC_LIST_DECL(sig, func, __VA_ARGS__); \
- SIG_EXPR_DECL_(sig, func); \
- SIG_EXPR_LIST_DECL(sig, SIG_EXPR_PTR(sig, func))
-
-#define SIG_EXPR_LIST_DECL_DUAL(sig, f0, f1) \
- SIG_EXPR_LIST_DECL(sig, SIG_EXPR_PTR(sig, f0), SIG_EXPR_PTR(sig, f1))
-
-#define SIG_EXPR_LIST_PTR(sig) (&SIG_EXPR_LIST_SYM(sig)[0])
-
-#define PIN_EXPRS_SYM(pin) pin_exprs_ ## pin
-#define PIN_EXPRS_PTR(pin) (&PIN_EXPRS_SYM(pin)[0])
-#define PIN_SYM(pin) pin_ ## pin
-
-#define MS_PIN_DECL_(pin, ...) \
- static const struct aspeed_sig_expr **PIN_EXPRS_SYM(pin)[] = \
- { __VA_ARGS__, NULL }; \
- static const struct aspeed_pin_desc PIN_SYM(pin) = \
- { #pin, PIN_EXPRS_PTR(pin) }
-
-/**
- * Declare a multi-signal pin
- *
- * @pin: The pin number
- * @other: Macro name for "other" functionality (subjected to stringification)
- * @high: Macro name for the highest priority signal functions
- * @low: Macro name for the low signal functions
- *
- * For example:
- *
- * #define A8 56
- * SIG_EXPR_DECL(ROMD8, ROM16, SIG_DESC_SET(SCU90, 6));
- * SIG_EXPR_DECL(ROMD8, ROM16S, SIG_DESC_SET(HW_STRAP1, 4),
- * { HW_STRAP1, GENMASK(1, 0), 0, 0 });
- * SIG_EXPR_LIST_DECL(ROMD8, SIG_EXPR_PTR(ROMD8, ROM16),
- * SIG_EXPR_PTR(ROMD8, ROM16S));
- * SIG_EXPR_LIST_DECL_SINGLE(NCTS6, NCTS6, SIG_DESC_SET(SCU90, 7));
- * MS_PIN_DECL(A8, GPIOH0, ROMD8, NCTS6);
- */
-#define MS_PIN_DECL(pin, other, high, low) \
- SIG_EXPR_LIST_DECL_SINGLE(other, other); \
- MS_PIN_DECL_(pin, \
- SIG_EXPR_LIST_PTR(high), \
- SIG_EXPR_LIST_PTR(low), \
- SIG_EXPR_LIST_PTR(other))
-
-#define PIN_GROUP_SYM(func) pins_ ## func
-#define FUNC_GROUP_SYM(func) groups_ ## func
-#define FUNC_GROUP_DECL(func, ...) \
- static const int PIN_GROUP_SYM(func)[] = { __VA_ARGS__ }; \
- static const char *FUNC_GROUP_SYM(func)[] = { #func }
-
-/**
- * Declare a single signal pin
- *
- * @pin: The pin number
- * @other: Macro name for "other" functionality (subjected to stringification)
- * @sig: Macro name for the signal (subjected to stringification)
- *
- * For example:
- *
- * #define E3 80
- * SIG_EXPR_LIST_DECL_SINGLE(SCL5, I2C5, I2C5_DESC);
- * SS_PIN_DECL(E3, GPIOK0, SCL5);
- */
-#define SS_PIN_DECL(pin, other, sig) \
- SIG_EXPR_LIST_DECL_SINGLE(other, other); \
- MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(sig), SIG_EXPR_LIST_PTR(other))
-
-/**
- * Single signal, single function pin declaration
- *
- * @pin: The pin number
- * @other: Macro name for "other" functionality (subjected to stringification)
- * @sig: Macro name for the signal (subjected to stringification)
- * @...: Signal descriptors that define the function expression
- *
- * For example:
- *
- * SSSF_PIN_DECL(A4, GPIOA2, TIMER3, SIG_DESC_SET(SCU80, 2));
- */
-#define SSSF_PIN_DECL(pin, other, sig, ...) \
- SIG_EXPR_LIST_DECL_SINGLE(sig, sig, __VA_ARGS__); \
- SIG_EXPR_LIST_DECL_SINGLE(other, other); \
- MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(sig), SIG_EXPR_LIST_PTR(other)); \
- FUNC_GROUP_DECL(sig, pin)
-
-#define GPIO_PIN_DECL(pin, gpio) \
- SIG_EXPR_LIST_DECL_SINGLE(gpio, gpio); \
- MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(gpio))
/**
* @param The pinconf parameter type
@@ -524,22 +25,6 @@ struct aspeed_pin_config {
u8 value;
};
-struct aspeed_pinctrl_data {
- struct regmap *maps[ASPEED_NR_PINMUX_IPS];
-
- const struct pinctrl_pin_desc *pins;
- const unsigned int npins;
-
- const struct aspeed_pin_group *groups;
- const unsigned int ngroups;
-
- const struct aspeed_pin_function *functions;
- const unsigned int nfunctions;
-
- const struct aspeed_pin_config *configs;
- const unsigned int nconfigs;
-};
-
#define ASPEED_PINCTRL_PIN(name_) \
[name_] = { \
.number = name_, \
@@ -547,30 +32,19 @@ struct aspeed_pinctrl_data {
.drv_data = (void *) &(PIN_SYM(name_)) \
}
-struct aspeed_pin_group {
- const char *name;
- const unsigned int *pins;
+struct aspeed_pinctrl_data {
+ struct regmap *scu;
+
+ const struct pinctrl_pin_desc *pins;
const unsigned int npins;
-};
-#define ASPEED_PINCTRL_GROUP(name_) { \
- .name = #name_, \
- .pins = &(PIN_GROUP_SYM(name_))[0], \
- .npins = ARRAY_SIZE(PIN_GROUP_SYM(name_)), \
-}
+ const struct aspeed_pin_config *configs;
+ const unsigned int nconfigs;
-struct aspeed_pin_function {
- const char *name;
- const char *const *groups;
- unsigned int ngroups;
+ struct aspeed_pinmux_data pinmux;
};
-#define ASPEED_PINCTRL_FUNC(name_, ...) { \
- .name = #name_, \
- .groups = &FUNC_GROUP_SYM(name_)[0], \
- .ngroups = ARRAY_SIZE(FUNC_GROUP_SYM(name_)), \
-}
-
+/* Aspeed pinctrl helpers */
int aspeed_pinctrl_get_groups_count(struct pinctrl_dev *pctldev);
const char *aspeed_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
unsigned int group);
diff --git a/drivers/pinctrl/aspeed/pinmux-aspeed.c b/drivers/pinctrl/aspeed/pinmux-aspeed.c
new file mode 100644
index 000000000000..5b0fe178ccf2
--- /dev/null
+++ b/drivers/pinctrl/aspeed/pinmux-aspeed.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (C) 2019 IBM Corp. */
+
+/* Pieces to enable drivers to implement the .set callback */
+
+#include "pinmux-aspeed.h"
+
+const char *const aspeed_pinmux_ips[] = {
+ [ASPEED_IP_SCU] = "SCU",
+ [ASPEED_IP_GFX] = "GFX",
+ [ASPEED_IP_LPC] = "LPC",
+};
+
+static inline void aspeed_sig_desc_print_val(
+ const struct aspeed_sig_desc *desc, bool enable, u32 rv)
+{
+ pr_debug("Want %s%X[0x%08X]=0x%X, got 0x%X from 0x%08X\n",
+ aspeed_pinmux_ips[desc->ip], desc->reg,
+ desc->mask, enable ? desc->enable : desc->disable,
+ (rv & desc->mask) >> __ffs(desc->mask), rv);
+}
+
+/**
+ * Query the enabled or disabled state of a signal descriptor
+ *
+ * @desc: The signal descriptor of interest
+ * @enabled: True to query the enabled state, false to query disabled state
+ * @map: The IP block's regmap instance
+ *
+ * Return: 1 if the descriptor's bitfield is configured to the state
+ * selected by @enabled, 0 if not, and less than zero if an unrecoverable
+ * failure occurred
+ *
+ * Evaluation of descriptor state is non-trivial in that it is not a binary
+ * outcome: The bitfields can be greater than one bit in size and thus can take
+ * a value that is neither the enabled nor disabled state recorded in the
+ * descriptor (typically this means a different function to the one of interest
+ * is enabled). Thus we must explicitly test for either condition as required.
+ */
+int aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc,
+ bool enabled, struct regmap *map)
+{
+ int ret;
+ unsigned int raw;
+ u32 want;
+
+ if (!map)
+ return -ENODEV;
+
+ ret = regmap_read(map, desc->reg, &raw);
+ if (ret)
+ return ret;
+
+ aspeed_sig_desc_print_val(desc, enabled, raw);
+ want = enabled ? desc->enable : desc->disable;
+
+ return ((raw & desc->mask) >> __ffs(desc->mask)) == want;
+}
+
+/**
+ * Query the enabled or disabled state for a mux function's signal on a pin
+ *
+ * @ctx: The driver context for the pinctrl IP
+ * @expr: An expression controlling the signal for a mux function on a pin
+ * @enabled: True to query the enabled state, false to query disabled state
+ *
+ * Return: 1 if the expression composed by @enabled evaluates true, 0 if not,
+ * and less than zero if an unrecoverable failure occurred.
+ *
+ * A mux function is enabled or disabled if the function's signal expression
+ * for each pin in the function's pin group evaluates true for the desired
+ * state. An signal expression evaluates true if all of its associated signal
+ * descriptors evaluate true for the desired state.
+ *
+ * If an expression's state is described by more than one bit, either through
+ * multi-bit bitfields in a single signal descriptor or through multiple signal
+ * descriptors of a single bit then it is possible for the expression to be in
+ * neither the enabled nor disabled state. Thus we must explicitly test for
+ * either condition as required.
+ */
+int aspeed_sig_expr_eval(const struct aspeed_pinmux_data *ctx,
+ const struct aspeed_sig_expr *expr, bool enabled)
+{
+ int i;
+ int ret;
+
+ for (i = 0; i < expr->ndescs; i++) {
+ const struct aspeed_sig_desc *desc = &expr->descs[i];
+
+ ret = aspeed_sig_desc_eval(desc, enabled, ctx->maps[desc->ip]);
+ if (ret <= 0)
+ return ret;
+ }
+
+ return 1;
+}
diff --git a/drivers/pinctrl/aspeed/pinmux-aspeed.h b/drivers/pinctrl/aspeed/pinmux-aspeed.h
new file mode 100644
index 000000000000..a036ce8f1571
--- /dev/null
+++ b/drivers/pinctrl/aspeed/pinmux-aspeed.h
@@ -0,0 +1,539 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (C) 2019 IBM Corp. */
+
+#ifndef ASPEED_PINMUX_H
+#define ASPEED_PINMUX_H
+
+#include <linux/regmap.h>
+#include <stdbool.h>
+
+/*
+ * The ASPEED SoCs provide typically more than 200 pins for GPIO and other
+ * functions. The SoC function enabled on a pin is determined on a priority
+ * basis where a given pin can provide a number of different signal types.
+ *
+ * The signal active on a pin is described by both a priority level and
+ * compound logical expressions involving multiple operators, registers and
+ * bits. Some difficulty arises as the pin's function bit masks for each
+ * priority level are frequently not the same (i.e. cannot just flip a bit to
+ * change from a high to low priority signal), or even in the same register.
+ * Further, not all signals can be unmuxed, as some expressions depend on
+ * values in the hardware strapping register (which is treated as read-only).
+ *
+ * SoC Multi-function Pin Expression Examples
+ * ------------------------------------------
+ *
+ * Here are some sample mux configurations from the AST2400 and AST2500
+ * datasheets to illustrate the corner cases, roughly in order of least to most
+ * corner. The signal priorities are in decending order from P0 (highest).
+ *
+ * D6 is a pin with a single function (beside GPIO); a high priority signal
+ * that participates in one function:
+ *
+ * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ * D6 GPIOA0 MAC1LINK SCU80[0]=1 GPIOA0
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ *
+ * C5 is a multi-signal pin (high and low priority signals). Here we touch
+ * different registers for the different functions that enable each signal:
+ *
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ * C5 GPIOA4 SCL9 SCU90[22]=1 TIMER5 SCU80[4]=1 GPIOA4
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ *
+ * E19 is a single-signal pin with two functions that influence the active
+ * signal. In this case both bits have the same meaning - enable a dedicated
+ * LPC reset pin. However it's not always the case that the bits in the
+ * OR-relationship have the same meaning.
+ *
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ * E19 GPIOB4 LPCRST# SCU80[12]=1 | Strap[14]=1 GPIOB4
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ *
+ * For example, pin B19 has a low-priority signal that's enabled by two
+ * distinct SoC functions: A specific SIOPBI bit in register SCUA4, and an ACPI
+ * bit in the STRAP register. The ACPI bit configures signals on pins in
+ * addition to B19. Both of the low priority functions as well as the high
+ * priority function must be disabled for GPIOF1 to be used.
+ *
+ * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
+ * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
+ * B19 GPIOF1 NDCD4 SCU80[25]=1 SIOPBI# SCUA4[12]=1 | Strap[19]=0 GPIOF1
+ * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
+ *
+ * For pin E18, the SoC ANDs the expected state of three bits to determine the
+ * pin's active signal:
+ *
+ * * SCU3C[3]: Enable external SOC reset function
+ * * SCU80[15]: Enable SPICS1# or EXTRST# function pin
+ * * SCU90[31]: Select SPI interface CS# output
+ *
+ * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
+ * E18 GPIOB7 EXTRST# SCU3C[3]=1 & SCU80[15]=1 & SCU90[31]=0 SPICS1# SCU3C[3]=1 & SCU80[15]=1 & SCU90[31]=1 GPIOB7
+ * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
+ *
+ * (Bits SCU3C[3] and SCU80[15] appear to only be used in the expressions for
+ * selecting the signals on pin E18)
+ *
+ * Pin T5 is a multi-signal pin with a more complex configuration:
+ *
+ * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
+ * -----+---------+-----------+------------------------------+-----------+---------------+----------
+ * T5 GPIOL1 VPIDE SCU90[5:4]!=0 & SCU84[17]=1 NDCD1 SCU84[17]=1 GPIOL1
+ * -----+---------+-----------+------------------------------+-----------+---------------+----------
+ *
+ * The high priority signal configuration is best thought of in terms of its
+ * exploded form, with reference to the SCU90[5:4] bits:
+ *
+ * * SCU90[5:4]=00: disable
+ * * SCU90[5:4]=01: 18 bits (R6/G6/B6) video mode.
+ * * SCU90[5:4]=10: 24 bits (R8/G8/B8) video mode.
+ * * SCU90[5:4]=11: 30 bits (R10/G10/B10) video mode.
+ *
+ * Re-writing:
+ *
+ * -----+---------+-----------+------------------------------+-----------+---------------+----------
+ * T5 GPIOL1 VPIDE (SCU90[5:4]=1 & SCU84[17]=1) NDCD1 SCU84[17]=1 GPIOL1
+ * | (SCU90[5:4]=2 & SCU84[17]=1)
+ * | (SCU90[5:4]=3 & SCU84[17]=1)
+ * -----+---------+-----------+------------------------------+-----------+---------------+----------
+ *
+ * For reference the SCU84[17] bit configure the "UART1 NDCD1 or Video VPIDE
+ * function pin", where the signal itself is determined by whether SCU94[5:4]
+ * is disabled or in one of the 18, 24 or 30bit video modes.
+ *
+ * Other video-input-related pins require an explicit state in SCU90[5:4], e.g.
+ * W1 and U5:
+ *
+ * -----+---------+-----------+------------------------------+-----------+---------------+----------
+ * W1 GPIOL6 VPIB0 SCU90[5:4]=3 & SCU84[22]=1 TXD1 SCU84[22]=1 GPIOL6
+ * U5 GPIOL7 VPIB1 SCU90[5:4]=3 & SCU84[23]=1 RXD1 SCU84[23]=1 GPIOL7
+ * -----+---------+-----------+------------------------------+-----------+---------------+----------
+ *
+ * The examples of T5 and W1 are particularly fertile, as they also demonstrate
+ * that despite operating as part of the video input bus each signal needs to
+ * be enabled individually via it's own SCU84 (in the cases of T5 and W1)
+ * register bit. This is a little crazy if the bus doesn't have optional
+ * signals, but is used to decent effect with some of the UARTs where not all
+ * signals are required. However, this isn't done consistently - UART1 is
+ * enabled on a per-pin basis, and by contrast, all signals for UART6 are
+ * enabled by a single bit.
+ *
+ * Further, the high and low priority signals listed in the table above share
+ * a configuration bit. The VPI signals should operate in concert in a single
+ * function, but the UART signals should retain the ability to be configured
+ * independently. This pushes the implementation down the path of tagging a
+ * signal's expressions with the function they participate in, rather than
+ * defining masks affecting multiple signals per function. The latter approach
+ * fails in this instance where applying the configuration for the UART pin of
+ * interest will stomp on the state of other UART signals when disabling the
+ * VPI functions on the current pin.
+ *
+ * Ball | Default | P0 Signal | P0 Expression | P1 Signal | P1 Expression | Other
+ * -----+------------+-----------+---------------------------+-----------+---------------+------------
+ * A12 RGMII1TXCK GPIOT0 SCUA0[0]=1 RMII1TXEN Strap[6]=0 RGMII1TXCK
+ * B12 RGMII1TXCTL GPIOT1 SCUA0[1]=1 ? Strap[6]=0 RGMII1TXCTL
+ * -----+------------+-----------+---------------------------+-----------+---------------+------------
+ *
+ * A12 demonstrates that the "Other" signal isn't always GPIO - in this case
+ * GPIOT0 is a high-priority signal and RGMII1TXCK is Other. Thus, GPIO
+ * should be treated like any other signal type with full function expression
+ * requirements, and not assumed to be the default case. Separately, GPIOT0 and
+ * GPIOT1's signal descriptor bits are distinct, therefore we must iterate all
+ * pins in the function's group to disable the higher-priority signals such
+ * that the signal for the function of interest is correctly enabled.
+ *
+ * Finally, three priority levels aren't always enough; the AST2500 brings with
+ * it 18 pins of five priority levels, however the 18 pins only use three of
+ * the five priority levels.
+ *
+ * Ultimately the requirement to control pins in the examples above drive the
+ * design:
+ *
+ * * Pins provide signals according to functions activated in the mux
+ * configuration
+ *
+ * * Pins provide up to five signal types in a priority order
+ *
+ * * For priorities levels defined on a pin, each priority provides one signal
+ *
+ * * Enabling lower priority signals requires higher priority signals be
+ * disabled
+ *
+ * * A function represents a set of signals; functions are distinct if their
+ * sets of signals are not equal
+ *
+ * * Signals participate in one or more functions
+ *
+ * * A function is described by an expression of one or more signal
+ * descriptors, which compare bit values in a register
+ *
+ * * A signal expression is the smallest set of signal descriptors whose
+ * comparisons must evaluate 'true' for a signal to be enabled on a pin.
+ *
+ * * A function's signal is active on a pin if evaluating all signal
+ * descriptors in the pin's signal expression for the function yields a 'true'
+ * result
+ *
+ * * A signal at a given priority on a given pin is active if any of the
+ * functions in which the signal participates are active, and no higher
+ * priority signal on the pin is active
+ *
+ * * GPIO is configured per-pin
+ *
+ * And so:
+ *
+ * * To disable a signal, any function(s) activating the signal must be
+ * disabled
+ *
+ * * Each pin must know the signal expressions of functions in which it
+ * participates, for the purpose of enabling the Other function. This is done
+ * by deactivating all functions that activate higher priority signals on the
+ * pin.
+ *
+ * As a concrete example:
+ *
+ * * T5 provides three signals types: VPIDE, NDCD1 and GPIO
+ *
+ * * The VPIDE signal participates in 3 functions: VPI18, VPI24 and VPI30
+ *
+ * * The NDCD1 signal participates in just its own NDCD1 function
+ *
+ * * VPIDE is high priority, NDCD1 is low priority, and GPIOL1 is the least
+ * prioritised
+ *
+ * * The prerequisit for activating the NDCD1 signal is that the VPI18, VPI24
+ * and VPI30 functions all be disabled
+ *
+ * * Similarly, all of VPI18, VPI24, VPI30 and NDCD1 functions must be disabled
+ * to provide GPIOL6
+ *
+ * Considerations
+ * --------------
+ *
+ * If pinctrl allows us to allocate a pin we can configure a function without
+ * concern for the function of already allocated pins, if pin groups are
+ * created with respect to the SoC functions in which they participate. This is
+ * intuitive, but it did not feel obvious from the bit/pin relationships.
+ *
+ * Conversely, failing to allocate all pins in a group indicates some bits (as
+ * well as pins) required for the group's configuration will already be in use,
+ * likely in a way that's inconsistent with the requirements of the failed
+ * group.
+ */
+
+#define ASPEED_IP_SCU 0
+#define ASPEED_IP_GFX 1
+#define ASPEED_IP_LPC 2
+#define ASPEED_NR_PINMUX_IPS 3
+
+ /**
+ * A signal descriptor, which describes the register, bits and the
+ * enable/disable values that should be compared or written.
+ *
+ * @ip: The IP block identifier, used as an index into the regmap array in
+ * struct aspeed_pinctrl_data
+ * @reg: The register offset with respect to the base address of the IP block
+ * @mask: The mask to apply to the register. The lowest set bit of the mask is
+ * used to derive the shift value.
+ * @enable: The value that enables the function. Value should be in the LSBs,
+ * not at the position of the mask.
+ * @disable: The value that disables the function. Value should be in the
+ * LSBs, not at the position of the mask.
+ */
+struct aspeed_sig_desc {
+ unsigned int ip;
+ unsigned int reg;
+ u32 mask;
+ u32 enable;
+ u32 disable;
+};
+
+/**
+ * Describes a signal expression. The expression is evaluated by ANDing the
+ * evaluation of the descriptors.
+ *
+ * @signal: The signal name for the priority level on the pin. If the signal
+ * type is GPIO, then the signal name must begin with the string
+ * "GPIO", e.g. GPIOA0, GPIOT4 etc.
+ * @function: The name of the function the signal participates in for the
+ * associated expression
+ * @ndescs: The number of signal descriptors in the expression
+ * @descs: Pointer to an array of signal descriptors that comprise the
+ * function expression
+ */
+struct aspeed_sig_expr {
+ const char *signal;
+ const char *function;
+ int ndescs;
+ const struct aspeed_sig_desc *descs;
+};
+
+/**
+ * A struct capturing the list of expressions enabling signals at each priority
+ * for a given pin. The signal configuration for a priority level is evaluated
+ * by ORing the evaluation of the signal expressions in the respective
+ * priority's list.
+ *
+ * @name: A name for the pin
+ * @prios: A pointer to an array of expression list pointers
+ *
+ */
+struct aspeed_pin_desc {
+ const char *name;
+ const struct aspeed_sig_expr ***prios;
+};
+
+/* Macro hell */
+
+#define SIG_DESC_IP_BIT(ip, reg, idx, val) \
+ { ip, reg, BIT_MASK(idx), val, (((val) + 1) & 1) }
+
+/**
+ * Short-hand macro for describing an SCU descriptor enabled by the state of
+ * one bit. The disable value is derived.
+ *
+ * @reg: The signal's associated register, offset from base
+ * @idx: The signal's bit index in the register
+ * @val: The value (0 or 1) that enables the function
+ */
+#define SIG_DESC_BIT(reg, idx, val) \
+ SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, val)
+
+#define SIG_DESC_IP_SET(ip, reg, idx) SIG_DESC_IP_BIT(ip, reg, idx, 1)
+
+/**
+ * A further short-hand macro expanding to an SCU descriptor enabled by a set
+ * bit.
+ *
+ * @reg: The register, offset from base
+ * @idx: The bit index in the register
+ */
+#define SIG_DESC_SET(reg, idx) SIG_DESC_IP_BIT(ASPEED_IP_SCU, reg, idx, 1)
+
+#define SIG_DESC_LIST_SYM(sig, func) sig_descs_ ## sig ## _ ## func
+#define SIG_DESC_LIST_DECL(sig, func, ...) \
+ static const struct aspeed_sig_desc SIG_DESC_LIST_SYM(sig, func)[] = \
+ { __VA_ARGS__ }
+
+#define SIG_EXPR_SYM(sig, func) sig_expr_ ## sig ## _ ## func
+#define SIG_EXPR_DECL_(sig, func) \
+ static const struct aspeed_sig_expr SIG_EXPR_SYM(sig, func) = \
+ { \
+ .signal = #sig, \
+ .function = #func, \
+ .ndescs = ARRAY_SIZE(SIG_DESC_LIST_SYM(sig, func)), \
+ .descs = &(SIG_DESC_LIST_SYM(sig, func))[0], \
+ }
+
+/**
+ * Declare a signal expression.
+ *
+ * @sig: A macro symbol name for the signal (is subjected to stringification
+ * and token pasting)
+ * @func: The function in which the signal is participating
+ * @...: Signal descriptors that define the signal expression
+ *
+ * For example, the following declares the ROMD8 signal for the ROM16 function:
+ *
+ * SIG_EXPR_DECL(ROMD8, ROM16, SIG_DESC_SET(SCU90, 6));
+ *
+ * And with multiple signal descriptors:
+ *
+ * SIG_EXPR_DECL(ROMD8, ROM16S, SIG_DESC_SET(HW_STRAP1, 4),
+ * { HW_STRAP1, GENMASK(1, 0), 0, 0 });
+ */
+#define SIG_EXPR_DECL(sig, func, ...) \
+ SIG_DESC_LIST_DECL(sig, func, __VA_ARGS__); \
+ SIG_EXPR_DECL_(sig, func)
+
+/**
+ * Declare a pointer to a signal expression
+ *
+ * @sig: The macro symbol name for the signal (subjected to token pasting)
+ * @func: The macro symbol name for the function (subjected to token pasting)
+ */
+#define SIG_EXPR_PTR(sig, func) (&SIG_EXPR_SYM(sig, func))
+
+#define SIG_EXPR_LIST_SYM(sig) sig_exprs_ ## sig
+
+/**
+ * Declare a signal expression list for reference in a struct aspeed_pin_prio.
+ *
+ * @sig: A macro symbol name for the signal (is subjected to token pasting)
+ * @...: Signal expression structure pointers (use SIG_EXPR_PTR())
+ *
+ * For example, the 16-bit ROM bus can be enabled by one of two possible signal
+ * expressions:
+ *
+ * SIG_EXPR_DECL(ROMD8, ROM16, SIG_DESC_SET(SCU90, 6));
+ * SIG_EXPR_DECL(ROMD8, ROM16S, SIG_DESC_SET(HW_STRAP1, 4),
+ * { HW_STRAP1, GENMASK(1, 0), 0, 0 });
+ * SIG_EXPR_LIST_DECL(ROMD8, SIG_EXPR_PTR(ROMD8, ROM16),
+ * SIG_EXPR_PTR(ROMD8, ROM16S));
+ */
+#define SIG_EXPR_LIST_DECL(sig, ...) \
+ static const struct aspeed_sig_expr *SIG_EXPR_LIST_SYM(sig)[] = \
+ { __VA_ARGS__, NULL }
+
+/**
+ * A short-hand macro for declaring a function expression and an expression
+ * list with a single function.
+ *
+ * @func: A macro symbol name for the function (is subjected to token pasting)
+ * @...: Function descriptors that define the function expression
+ *
+ * For example, signal NCTS6 participates in its own function with one group:
+ *
+ * SIG_EXPR_LIST_DECL_SINGLE(NCTS6, NCTS6, SIG_DESC_SET(SCU90, 7));
+ */
+#define SIG_EXPR_LIST_DECL_SINGLE(sig, func, ...) \
+ SIG_DESC_LIST_DECL(sig, func, __VA_ARGS__); \
+ SIG_EXPR_DECL_(sig, func); \
+ SIG_EXPR_LIST_DECL(sig, SIG_EXPR_PTR(sig, func))
+
+#define SIG_EXPR_LIST_DECL_DUAL(sig, f0, f1) \
+ SIG_EXPR_LIST_DECL(sig, SIG_EXPR_PTR(sig, f0), SIG_EXPR_PTR(sig, f1))
+
+#define SIG_EXPR_LIST_PTR(sig) (&SIG_EXPR_LIST_SYM(sig)[0])
+
+#define PIN_EXPRS_SYM(pin) pin_exprs_ ## pin
+#define PIN_EXPRS_PTR(pin) (&PIN_EXPRS_SYM(pin)[0])
+#define PIN_SYM(pin) pin_ ## pin
+
+#define MS_PIN_DECL_(pin, ...) \
+ static const struct aspeed_sig_expr **PIN_EXPRS_SYM(pin)[] = \
+ { __VA_ARGS__, NULL }; \
+ static const struct aspeed_pin_desc PIN_SYM(pin) = \
+ { #pin, PIN_EXPRS_PTR(pin) }
+
+/**
+ * Declare a multi-signal pin
+ *
+ * @pin: The pin number
+ * @other: Macro name for "other" functionality (subjected to stringification)
+ * @high: Macro name for the highest priority signal functions
+ * @low: Macro name for the low signal functions
+ *
+ * For example:
+ *
+ * #define A8 56
+ * SIG_EXPR_DECL(ROMD8, ROM16, SIG_DESC_SET(SCU90, 6));
+ * SIG_EXPR_DECL(ROMD8, ROM16S, SIG_DESC_SET(HW_STRAP1, 4),
+ * { HW_STRAP1, GENMASK(1, 0), 0, 0 });
+ * SIG_EXPR_LIST_DECL(ROMD8, SIG_EXPR_PTR(ROMD8, ROM16),
+ * SIG_EXPR_PTR(ROMD8, ROM16S));
+ * SIG_EXPR_LIST_DECL_SINGLE(NCTS6, NCTS6, SIG_DESC_SET(SCU90, 7));
+ * MS_PIN_DECL(A8, GPIOH0, ROMD8, NCTS6);
+ */
+#define MS_PIN_DECL(pin, other, high, low) \
+ SIG_EXPR_LIST_DECL_SINGLE(other, other); \
+ MS_PIN_DECL_(pin, \
+ SIG_EXPR_LIST_PTR(high), \
+ SIG_EXPR_LIST_PTR(low), \
+ SIG_EXPR_LIST_PTR(other))
+
+#define PIN_GROUP_SYM(func) pins_ ## func
+#define FUNC_GROUP_SYM(func) groups_ ## func
+#define FUNC_GROUP_DECL(func, ...) \
+ static const int PIN_GROUP_SYM(func)[] = { __VA_ARGS__ }; \
+ static const char *FUNC_GROUP_SYM(func)[] = { #func }
+
+/**
+ * Declare a single signal pin
+ *
+ * @pin: The pin number
+ * @other: Macro name for "other" functionality (subjected to stringification)
+ * @sig: Macro name for the signal (subjected to stringification)
+ *
+ * For example:
+ *
+ * #define E3 80
+ * SIG_EXPR_LIST_DECL_SINGLE(SCL5, I2C5, I2C5_DESC);
+ * SS_PIN_DECL(E3, GPIOK0, SCL5);
+ */
+#define SS_PIN_DECL(pin, other, sig) \
+ SIG_EXPR_LIST_DECL_SINGLE(other, other); \
+ MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(sig), SIG_EXPR_LIST_PTR(other))
+
+/**
+ * Single signal, single function pin declaration
+ *
+ * @pin: The pin number
+ * @other: Macro name for "other" functionality (subjected to stringification)
+ * @sig: Macro name for the signal (subjected to stringification)
+ * @...: Signal descriptors that define the function expression
+ *
+ * For example:
+ *
+ * SSSF_PIN_DECL(A4, GPIOA2, TIMER3, SIG_DESC_SET(SCU80, 2));
+ */
+#define SSSF_PIN_DECL(pin, other, sig, ...) \
+ SIG_EXPR_LIST_DECL_SINGLE(sig, sig, __VA_ARGS__); \
+ SIG_EXPR_LIST_DECL_SINGLE(other, other); \
+ MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(sig), SIG_EXPR_LIST_PTR(other)); \
+ FUNC_GROUP_DECL(sig, pin)
+
+#define GPIO_PIN_DECL(pin, gpio) \
+ SIG_EXPR_LIST_DECL_SINGLE(gpio, gpio); \
+ MS_PIN_DECL_(pin, SIG_EXPR_LIST_PTR(gpio))
+
+struct aspeed_pin_group {
+ const char *name;
+ const unsigned int *pins;
+ const unsigned int npins;
+};
+
+#define ASPEED_PINCTRL_GROUP(name_) { \
+ .name = #name_, \
+ .pins = &(PIN_GROUP_SYM(name_))[0], \
+ .npins = ARRAY_SIZE(PIN_GROUP_SYM(name_)), \
+}
+
+struct aspeed_pin_function {
+ const char *name;
+ const char *const *groups;
+ unsigned int ngroups;
+};
+
+#define ASPEED_PINCTRL_FUNC(name_, ...) { \
+ .name = #name_, \
+ .groups = &FUNC_GROUP_SYM(name_)[0], \
+ .ngroups = ARRAY_SIZE(FUNC_GROUP_SYM(name_)), \
+}
+
+struct aspeed_pinmux_data;
+
+struct aspeed_pinmux_ops {
+ int (*set)(const struct aspeed_pinmux_data *ctx,
+ const struct aspeed_sig_expr *expr, bool enabled);
+};
+
+struct aspeed_pinmux_data {
+ struct regmap *maps[ASPEED_NR_PINMUX_IPS];
+
+ const struct aspeed_pinmux_ops *ops;
+
+ const struct aspeed_pin_group *groups;
+ const unsigned int ngroups;
+
+ const struct aspeed_pin_function *functions;
+ const unsigned int nfunctions;
+};
+
+int aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc, bool enabled,
+ struct regmap *map);
+
+int aspeed_sig_expr_eval(const struct aspeed_pinmux_data *ctx,
+ const struct aspeed_sig_expr *expr,
+ bool enabled);
+
+static inline int aspeed_sig_expr_set(const struct aspeed_pinmux_data *ctx,
+ const struct aspeed_sig_expr *expr,
+ bool enabled)
+{
+ return ctx->ops->set(ctx, expr, enabled);
+}
+
+#endif /* ASPEED_PINMUX_H */
--
2.20.1
^ permalink raw reply related
* [PATCH v2 6/8] pinctrl: aspeed: Clarify comment about strapping W1C
From: Andrew Jeffery @ 2019-06-28 2:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-1-andrew@aj.id.au>
Writes of 1 to SCU7C clear set bits in SCU70, the hardware strapping
register. The information was correct if you squinted while reading, but
hopefully switching the order of the registers as listed conveys it
better.
Cc: Johnny Huang <johnny_huang@aspeedtech.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Acked-by: Joel Stanley <joel@jms.id.au>
---
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index 4c775b8ffdc4..b510bb475851 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -209,7 +209,7 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
continue;
- /* On AST2500, Set bits in SCU7C are cleared from SCU70 */
+ /* On AST2500, Set bits in SCU70 are cleared from SCU7C */
if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1) {
unsigned int rev_id;
--
2.20.1
^ permalink raw reply related
* [PATCH v2 5/8] pinctrl: aspeed: Correct comment that is no longer true
From: Andrew Jeffery @ 2019-06-28 2:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-1-andrew@aj.id.au>
We have handled the GFX register case for quite some time now.
Cc: Johnny Huang <johnny_huang@aspeedtech.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Acked-by: Joel Stanley <joel@jms.id.au>
---
drivers/pinctrl/aspeed/pinctrl-aspeed.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
index 4b06ddbc6aec..c5918c4a087c 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
@@ -240,8 +240,7 @@
* opposed to naming them e.g. PINMUX_CTRL_[0-9]). Further, signal expressions
* reference registers beyond those dedicated to pinmux, such as the system
* reset control and MAC clock configuration registers. The AST2500 goes a step
- * further and references registers in the graphics IP block, but that isn't
- * handled yet.
+ * further and references registers in the graphics IP block.
*/
#define SCU2C 0x2C /* Misc. Control Register */
#define SCU3C 0x3C /* System Reset Control/Status Register */
--
2.20.1
^ permalink raw reply related
* [PATCH v2 4/8] MAINTAINERS: Add entry for ASPEED pinctrl drivers
From: Andrew Jeffery @ 2019-06-28 2:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-1-andrew@aj.id.au>
Add myself as maintainer to avoid burdening others with the madness.
Cc: Johnny Huang <johnny_huang@aspeedtech.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d0ed735994a5..e70fcaa56094 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2586,6 +2586,15 @@ S: Maintained
F: Documentation/hwmon/asc7621.rst
F: drivers/hwmon/asc7621.c
+ASPEED PINCTRL DRIVERS
+M: Andrew Jeffery <andrew@aj.id.au>
+L: linux-aspeed at lists.ozlabs.org (moderated for non-subscribers)
+L: openbmc at lists.ozlabs.org (moderated for non-subscribers)
+L: linux-gpio at vger.kernel.org
+S: Maintained
+F: drivers/pinctrl/aspeed/
+F: Documentation/devicetree/bindings/pinctrl/aspeed,*
+
ASPEED VIDEO ENGINE DRIVER
M: Eddie James <eajames@linux.ibm.com>
L: linux-media at vger.kernel.org
--
2.20.1
^ permalink raw reply related
* [PATCH v2 3/8] dt-bindings: pinctrl: aspeed: Convert AST2500 bindings to json-schema
From: Andrew Jeffery @ 2019-06-28 2:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-1-andrew@aj.id.au>
Convert ASPEED pinctrl bindings to DT schema format using json-schema.
Cc: Johnny Huang <johnny_huang@aspeedtech.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
In v2:
* Enforce function/group names in bindings
* Move description above properties
* Simplify specification of compatible
* Cleanup license specification
.../pinctrl/aspeed,ast2500-pinctrl.txt | 119 ----------------
.../pinctrl/aspeed,ast2500-pinctrl.yaml | 134 ++++++++++++++++++
2 files changed, 134 insertions(+), 119 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.txt
create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.txt
deleted file mode 100644
index 2f16e401338a..000000000000
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.txt
+++ /dev/null
@@ -1,119 +0,0 @@
-=============================
-Aspeed AST2500 Pin Controller
-=============================
-
-Required properties for g5:
-- compatible : Should be one of the following:
- "aspeed,ast2500-pinctrl"
- "aspeed,g5-pinctrl"
-
-- aspeed,external-nodes: A cell of phandles to external controller nodes:
- 0: compatible with "aspeed,ast2500-gfx", "syscon"
- 1: compatible with "aspeed,ast2500-lhc", "syscon"
-
-The pin controller node should be the child of a syscon node with the required
-property:
-
-- compatible : Should be one of the following:
- "aspeed,ast2500-scu", "syscon", "simple-mfd"
- "aspeed,g5-scu", "syscon", "simple-mfd"
-
-Refer to the the bindings described in
-Documentation/devicetree/bindings/mfd/syscon.txt
-
-Subnode Format
-==============
-
-The required properties of pinmux child nodes are:
-- function: the mux function to select
-- groups : the list of groups to select with this function
-
-Required properties of pinconf child nodes are:
-- groups: A list of groups to select (either this or "pins" must be
- specified)
-- pins : A list of ball names as strings, eg "D14" (either this or "groups"
- must be specified)
-
-Optional properties of pinconf child nodes are:
-- bias-disable : disable any pin bias
-- bias-pull-down: pull down the pin
-- drive-strength: sink or source at most X mA
-
-Definitions are as specified in
-Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt, with any
-further limitations as described above.
-
-For pinmux, each mux function has only one associated pin group. Each group is
-named by its function. The following values for the function and groups
-properties are supported:
-
-ACPI ADC0 ADC1 ADC10 ADC11 ADC12 ADC13 ADC14 ADC15 ADC2 ADC3 ADC4 ADC5 ADC6
-ADC7 ADC8 ADC9 BMCINT DDCCLK DDCDAT ESPI FWSPICS1 FWSPICS2 GPID0 GPID2 GPID4
-GPID6 GPIE0 GPIE2 GPIE4 GPIE6 I2C10 I2C11 I2C12 I2C13 I2C14 I2C3 I2C4 I2C5 I2C6
-I2C7 I2C8 I2C9 LAD0 LAD1 LAD2 LAD3 LCLK LFRAME LPCHC LPCPD LPCPLUS LPCPME
-LPCRST LPCSMI LSIRQ MAC1LINK MAC2LINK MDIO1 MDIO2 NCTS1 NCTS2 NCTS3 NCTS4 NDCD1
-NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4 NDTR1 NDTR2 NDTR3 NDTR4 NRI1 NRI2
-NRI3 NRI4 NRTS1 NRTS2 NRTS3 NRTS4 OSCCLK PEWAKE PNOR PWM0 PWM1 PWM2 PWM3 PWM4
-PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 RXD1 RXD2 RXD3 RXD4 SALT1 SALT10
-SALT11 SALT12 SALT13 SALT14 SALT2 SALT3 SALT4 SALT5 SALT6 SALT7 SALT8 SALT9
-SCL1 SCL2 SD1 SD2 SDA1 SDA2 SGPS1 SGPS2 SIOONCTRL SIOPBI SIOPBO SIOPWREQ
-SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1CS1 SPI1DEBUG SPI1PASSTHRU SPI2CK SPI2CS0
-SPI2CS1 SPI2MISO SPI2MOSI TIMER3 TIMER4 TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2
-TXD3 TXD4 UART6 USB11BHID USB2AD USB2AH USB2BD USB2BH USBCKI VGABIOSROM VGAHS
-VGAVS VPI24 VPO WDTRST1 WDTRST2
-
-Example
-=======
-
-ahb {
- apb {
- syscon: scu at 1e6e2000 {
- compatible = "aspeed,ast2500-scu", "syscon", "simple-mfd";
- reg = <0x1e6e2000 0x1a8>;
-
- pinctrl: pinctrl {
- compatible = "aspeed,g5-pinctrl";
- aspeed,external-nodes = <&gfx &lhc>;
-
- pinctrl_i2c3_default: i2c3_default {
- function = "I2C3";
- groups = "I2C3";
- };
-
- pinctrl_gpioh0_unbiased_default: gpioh0 {
- pins = "A18";
- bias-disable;
- };
- };
- };
-
- gfx: display at 1e6e6000 {
- compatible = "aspeed,ast2500-gfx", "syscon";
- reg = <0x1e6e6000 0x1000>;
- };
- };
-
- lpc: lpc at 1e789000 {
- compatible = "aspeed,ast2500-lpc", "simple-mfd";
- reg = <0x1e789000 0x1000>;
-
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0x0 0x1e789000 0x1000>;
-
- lpc_host: lpc-host at 80 {
- compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon";
- reg = <0x80 0x1e0>;
- reg-io-width = <4>;
-
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0x0 0x80 0x1e0>;
-
- lhc: lhc at 20 {
- compatible = "aspeed,ast2500-lhc";
- reg = <0x20 0x24 0x48 0x8>;
- };
- };
- };
-};
diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
new file mode 100644
index 000000000000..cf561bd55128
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
@@ -0,0 +1,134 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/aspeed,ast2500-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED AST2500 Pin Controller
+
+maintainers:
+ - Andrew Jeffery <andrew@aj.id.au>
+
+description: |+
+ The pin controller node should be the child of a syscon node with the
+ required property:
+
+ - compatible: Should be one of the following:
+ "aspeed,ast2500-scu", "syscon", "simple-mfd"
+ "aspeed,g5-scu", "syscon", "simple-mfd"
+
+ Refer to the the bindings described in
+ Documentation/devicetree/bindings/mfd/syscon.txt
+
+properties:
+ compatible:
+ enum: [ aspeed,ast2500-pinctrl, aspeed,g5-pinctrl ]
+ aspeed,external-nodes:
+ minItems: 2
+ maxItems: 2
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: |
+ A cell of phandles to external controller nodes:
+ 0: compatible with "aspeed,ast2500-gfx", "syscon"
+ 1: compatible with "aspeed,ast2500-lhc", "syscon"
+
+patternProperties:
+ '^.*$':
+ if:
+ type: object
+ then:
+ patternProperties:
+ "^function|groups$":
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/string"
+ - enum: [ "ACPI", "ADC0", "ADC1", "ADC10", "ADC11", "ADC12", "ADC13",
+ "ADC14", "ADC15", "ADC2", "ADC3", "ADC4", "ADC5", "ADC6", "ADC7",
+ "ADC8", "ADC9", "BMCINT", "DDCCLK", "DDCDAT", "ESPI", "FWSPICS1",
+ "FWSPICS2", "GPID0", "GPID2", "GPID4", "GPID6", "GPIE0", "GPIE2",
+ "GPIE4", "GPIE6", "I2C10", "I2C11", "I2C12", "I2C13", "I2C14",
+ "I2C3", "I2C4", "I2C5", "I2C6", "I2C7", "I2C8", "I2C9", "LAD0",
+ "LAD1", "LAD2", "LAD3", "LCLK", "LFRAME", "LPCHC", "LPCPD",
+ "LPCPLUS", "LPCPME", "LPCRST", "LPCSMI", "LSIRQ", "MAC1LINK",
+ "MAC2LINK", "MDIO1", "MDIO2", "NCTS1", "NCTS2", "NCTS3", "NCTS4",
+ "NDCD1", "NDCD2", "NDCD3", "NDCD4", "NDSR1", "NDSR2", "NDSR3",
+ "NDSR4", "NDTR1", "NDTR2", "NDTR3", "NDTR4", "NRI1", "NRI2",
+ "NRI3", "NRI4", "NRTS1", "NRTS2", "NRTS3", "NRTS4", "OSCCLK",
+ "PEWAKE", "PNOR", "PWM0", "PWM1", "PWM2", "PWM3", "PWM4", "PWM5",
+ "PWM6", "PWM7", "RGMII1", "RGMII2", "RMII1", "RMII2", "RXD1",
+ "RXD2", "RXD3", "RXD4", "SALT1", "SALT10", "SALT11", "SALT12",
+ "SALT13", "SALT14", "SALT2", "SALT3", "SALT4", "SALT5", "SALT6",
+ "SALT7", "SALT8", "SALT9", "SCL1", "SCL2", "SD1", "SD2", "SDA1",
+ "SDA2", "SGPS1", "SGPS2", "SIOONCTRL", "SIOPBI", "SIOPBO",
+ "SIOPWREQ", "SIOPWRGD", "SIOS3", "SIOS5", "SIOSCI", "SPI1",
+ "SPI1CS1", "SPI1DEBUG", "SPI1PASSTHRU", "SPI2CK", "SPI2CS0",
+ "SPI2CS1", "SPI2MISO", "SPI2MOSI", "TIMER3", "TIMER4", "TIMER5",
+ "TIMER6", "TIMER7", "TIMER8", "TXD1", "TXD2", "TXD3", "TXD4",
+ "UART6", "USB11BHID", "USB2AD", "USB2AH", "USB2BD", "USB2BH",
+ "USBCKI", "VGABIOSROM", "VGAHS", "VGAVS", "VPI24", "VPO",
+ "WDTRST1", "WDTRST2", ]
+
+required:
+ - compatible
+ - aspeed,external-nodes
+
+examples:
+ - |
+ compatible = "simple-bus";
+ ranges;
+
+ apb {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ syscon: scu at 1e6e2000 {
+ compatible = "aspeed,ast2500-scu", "syscon", "simple-mfd";
+ reg = <0x1e6e2000 0x1a8>;
+
+ pinctrl: pinctrl {
+ compatible = "aspeed,g5-pinctrl";
+ aspeed,external-nodes = <&gfx &lhc>;
+
+ pinctrl_i2c3_default: i2c3_default {
+ function = "I2C3";
+ groups = "I2C3";
+ };
+
+ pinctrl_gpioh0_unbiased_default: gpioh0 {
+ pins = "A18";
+ bias-disable;
+ };
+ };
+ };
+
+ gfx: display at 1e6e6000 {
+ compatible = "aspeed,ast2500-gfx", "syscon";
+ reg = <0x1e6e6000 0x1000>;
+ };
+ };
+
+ lpc: lpc at 1e789000 {
+ compatible = "aspeed,ast2500-lpc", "simple-mfd";
+ reg = <0x1e789000 0x1000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x1e789000 0x1000>;
+
+ lpc_host: lpc-host at 80 {
+ compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon";
+ reg = <0x80 0x1e0>;
+ reg-io-width = <4>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x80 0x1e0>;
+
+ lhc: lhc at 20 {
+ compatible = "aspeed,ast2500-lhc";
+ reg = <0x20 0x24 0x48 0x8>;
+ };
+ };
+ };
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/8] dt-bindings: pinctrl: aspeed: Convert AST2400 bindings to json-schema
From: Andrew Jeffery @ 2019-06-28 2:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-1-andrew@aj.id.au>
Convert ASPEED pinctrl bindings to DT schema format using json-schema
Cc: Johnny Huang <johnny_huang@aspeedtech.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
In v2:
* Enforce function/group names in bindings
* Move description above properties
* Simplify specification of compatible
* Cleanup license specification
.../pinctrl/aspeed,ast2400-pinctrl.txt | 80 ------------------
.../pinctrl/aspeed,ast2400-pinctrl.yaml | 81 +++++++++++++++++++
2 files changed, 81 insertions(+), 80 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
deleted file mode 100644
index 67e0325ccf2e..000000000000
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
+++ /dev/null
@@ -1,80 +0,0 @@
-=============================
-Aspeed AST2400 Pin Controller
-=============================
-
-Required properties for the AST2400:
-- compatible : Should be one of the following:
- "aspeed,ast2400-pinctrl"
- "aspeed,g4-pinctrl"
-
-The pin controller node should be the child of a syscon node with the required
-property:
-
-- compatible : Should be one of the following:
- "aspeed,ast2400-scu", "syscon", "simple-mfd"
- "aspeed,g4-scu", "syscon", "simple-mfd"
-
-Refer to the the bindings described in
-Documentation/devicetree/bindings/mfd/syscon.txt
-
-Subnode Format
-==============
-
-The required properties of pinmux child nodes are:
-- function: the mux function to select
-- groups : the list of groups to select with this function
-
-Required properties of pinconf child nodes are:
-- groups: A list of groups to select (either this or "pins" must be
- specified)
-- pins : A list of ball names as strings, eg "D14" (either this or "groups"
- must be specified)
-
-Optional properties of pinconf child nodes are:
-- bias-disable : disable any pin bias
-- bias-pull-down: pull down the pin
-- drive-strength: sink or source at most X mA
-
-Definitions are as specified in
-Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt, with any
-further limitations as described above.
-
-For pinmux, each mux function has only one associated pin group. Each group is
-named by its function. The following values for the function and groups
-properties are supported:
-
-ACPI ADC0 ADC1 ADC10 ADC11 ADC12 ADC13 ADC14 ADC15 ADC2 ADC3 ADC4 ADC5 ADC6
-ADC7 ADC8 ADC9 BMCINT DDCCLK DDCDAT EXTRST FLACK FLBUSY FLWP GPID GPID0 GPID2
-GPID4 GPID6 GPIE0 GPIE2 GPIE4 GPIE6 I2C10 I2C11 I2C12 I2C13 I2C14 I2C3 I2C4
-I2C5 I2C6 I2C7 I2C8 I2C9 LPCPD LPCPME LPCRST LPCSMI MAC1LINK MAC2LINK MDIO1
-MDIO2 NCTS1 NCTS2 NCTS3 NCTS4 NDCD1 NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4
-NDTR1 NDTR2 NDTR3 NDTR4 NDTS4 NRI1 NRI2 NRI3 NRI4 NRTS1 NRTS2 NRTS3 OSCCLK PWM0
-PWM1 PWM2 PWM3 PWM4 PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 ROM16 ROM8 ROMCS1
-ROMCS2 ROMCS3 ROMCS4 RXD1 RXD2 RXD3 RXD4 SALT1 SALT2 SALT3 SALT4 SD1 SD2 SGPMCK
-SGPMI SGPMLD SGPMO SGPSCK SGPSI0 SGPSI1 SGPSLD SIOONCTRL SIOPBI SIOPBO SIOPWREQ
-SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1DEBUG SPI1PASSTHRU SPICS1 TIMER3 TIMER4
-TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2 TXD3 TXD4 UART6 USB11D1 USB11H2 USB2D1
-USB2H1 USBCKI VGABIOS_ROM VGAHS VGAVS VPI18 VPI24 VPI30 VPO12 VPO24 WDTRST1
-WDTRST2
-
-Example
-=======
-
-syscon: scu at 1e6e2000 {
- compatible = "aspeed,ast2400-scu", "syscon", "simple-mfd";
- reg = <0x1e6e2000 0x1a8>;
-
- pinctrl: pinctrl {
- compatible = "aspeed,g4-pinctrl";
-
- pinctrl_i2c3_default: i2c3_default {
- function = "I2C3";
- groups = "I2C3";
- };
-
- pinctrl_gpioh0_unbiased_default: gpioh0 {
- pins = "A8";
- bias-disable;
- };
- };
-};
diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
new file mode 100644
index 000000000000..61a110a7db8a
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/aspeed,ast2400-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED AST2400 Pin Controller
+
+maintainers:
+ - Andrew Jeffery <andrew@aj.id.au>
+
+description: |+
+ The pin controller node should be the child of a syscon node with the
+ required property:
+
+ - compatible: Should be one of the following:
+ "aspeed,ast2400-scu", "syscon", "simple-mfd"
+ "aspeed,g4-scu", "syscon", "simple-mfd"
+
+ Refer to the the bindings described in
+ Documentation/devicetree/bindings/mfd/syscon.txt
+
+properties:
+ compatible:
+ enum: [ aspeed,ast2400-pinctrl, aspeed,g4-pinctrl ]
+
+patternProperties:
+ '^.*$':
+ if:
+ type: object
+ then:
+ patternProperties:
+ "^function|groups$":
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/string"
+ - enum: [ "ACPI", "ADC0", "ADC1", "ADC10", "ADC11", "ADC12", "ADC13",
+ "ADC14", "ADC15", "ADC2", "ADC3", "ADC4", "ADC5", "ADC6", "ADC7",
+ "ADC8", "ADC9", "BMCINT", "DDCCLK", "DDCDAT", "EXTRST", "FLACK",
+ "FLBUSY", "FLWP", "GPID", "GPID0", "GPID2", "GPID4", "GPID6",
+ "GPIE0", "GPIE2", "GPIE4", "GPIE6", "I2C10", "I2C11", "I2C12",
+ "I2C13", "I2C14", "I2C3", "I2C4", "I2C5", "I2C6", "I2C7", "I2C8",
+ "I2C9", "LPCPD", "LPCPME", "LPCRST", "LPCSMI", "MAC1LINK",
+ "MAC2LINK", "MDIO1", "MDIO2", "NCTS1", "NCTS2", "NCTS3", "NCTS4",
+ "NDCD1", "NDCD2", "NDCD3", "NDCD4", "NDSR1", "NDSR2", "NDSR3",
+ "NDSR4", "NDTR1", "NDTR2", "NDTR3", "NDTR4", "NDTS4", "NRI1",
+ "NRI2", "NRI3", "NRI4", "NRTS1", "NRTS2", "NRTS3", "OSCCLK",
+ "PWM0", "PWM1", "PWM2", "PWM3", "PWM4", "PWM5", "PWM6", "PWM7",
+ "RGMII1", "RGMII2", "RMII1", "RMII2", "ROM16", "ROM8", "ROMCS1",
+ "ROMCS2", "ROMCS3", "ROMCS4", "RXD1", "RXD2", "RXD3", "RXD4",
+ "SALT1", "SALT2", "SALT3", "SALT4", "SD1", "SD2", "SGPMCK",
+ "SGPMI", "SGPMLD", "SGPMO", "SGPSCK", "SGPSI0", "SGPSI1", "SGPSLD",
+ "SIOONCTRL", "SIOPBI", "SIOPBO", "SIOPWREQ", "SIOPWRGD", "SIOS3",
+ "SIOS5", "SIOSCI", "SPI1", "SPI1DEBUG", "SPI1PASSTHRU", "SPICS1",
+ "TIMER3", "TIMER4", "TIMER5", "TIMER6", "TIMER7", "TIMER8", "TXD1",
+ "TXD2", "TXD3", "TXD4", "UART6", "USB11D1", "USB11H2", "USB2D1",
+ "USB2H1", "USBCKI", "VGABIOS_ROM", "VGAHS", "VGAVS", "VPI18",
+ "VPI24", "VPI30", "VPO12", "VPO24", "WDTRST1", "WDTRST2" ]
+
+required:
+ - compatible
+
+examples:
+ - |
+ syscon: scu at 1e6e2000 {
+ compatible = "aspeed,ast2400-scu", "syscon", "simple-mfd";
+ reg = <0x1e6e2000 0x1a8>;
+
+ pinctrl: pinctrl {
+ compatible = "aspeed,g4-pinctrl";
+
+ pinctrl_i2c3_default: i2c3_default {
+ function = "I2C3";
+ groups = "I2C3";
+ };
+
+ pinctrl_gpioh0_unbiased_default: gpioh0 {
+ pins = "A8";
+ bias-disable;
+ };
+ };
+ };
--
2.20.1
^ permalink raw reply related
* [PATCH v2 1/8] dt-bindings: pinctrl: aspeed: Split bindings document in two
From: Andrew Jeffery @ 2019-06-28 2:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190628023838.15426-1-andrew@aj.id.au>
Have one for each of the AST2400 and AST2500. The only thing that was
common was the fact that both support ASPEED BMC SoCs.
Cc: Johnny Huang <johnny_huang@aspeedtech.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Acked-by: Joel Stanley <joel@jms.id.au>
---
.../pinctrl/aspeed,ast2400-pinctrl.txt | 80 +++++++++++++++++++
...-aspeed.txt => aspeed,ast2500-pinctrl.txt} | 63 ++-------------
2 files changed, 85 insertions(+), 58 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
rename Documentation/devicetree/bindings/pinctrl/{pinctrl-aspeed.txt => aspeed,ast2500-pinctrl.txt} (66%)
diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
new file mode 100644
index 000000000000..67e0325ccf2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
@@ -0,0 +1,80 @@
+=============================
+Aspeed AST2400 Pin Controller
+=============================
+
+Required properties for the AST2400:
+- compatible : Should be one of the following:
+ "aspeed,ast2400-pinctrl"
+ "aspeed,g4-pinctrl"
+
+The pin controller node should be the child of a syscon node with the required
+property:
+
+- compatible : Should be one of the following:
+ "aspeed,ast2400-scu", "syscon", "simple-mfd"
+ "aspeed,g4-scu", "syscon", "simple-mfd"
+
+Refer to the the bindings described in
+Documentation/devicetree/bindings/mfd/syscon.txt
+
+Subnode Format
+==============
+
+The required properties of pinmux child nodes are:
+- function: the mux function to select
+- groups : the list of groups to select with this function
+
+Required properties of pinconf child nodes are:
+- groups: A list of groups to select (either this or "pins" must be
+ specified)
+- pins : A list of ball names as strings, eg "D14" (either this or "groups"
+ must be specified)
+
+Optional properties of pinconf child nodes are:
+- bias-disable : disable any pin bias
+- bias-pull-down: pull down the pin
+- drive-strength: sink or source at most X mA
+
+Definitions are as specified in
+Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt, with any
+further limitations as described above.
+
+For pinmux, each mux function has only one associated pin group. Each group is
+named by its function. The following values for the function and groups
+properties are supported:
+
+ACPI ADC0 ADC1 ADC10 ADC11 ADC12 ADC13 ADC14 ADC15 ADC2 ADC3 ADC4 ADC5 ADC6
+ADC7 ADC8 ADC9 BMCINT DDCCLK DDCDAT EXTRST FLACK FLBUSY FLWP GPID GPID0 GPID2
+GPID4 GPID6 GPIE0 GPIE2 GPIE4 GPIE6 I2C10 I2C11 I2C12 I2C13 I2C14 I2C3 I2C4
+I2C5 I2C6 I2C7 I2C8 I2C9 LPCPD LPCPME LPCRST LPCSMI MAC1LINK MAC2LINK MDIO1
+MDIO2 NCTS1 NCTS2 NCTS3 NCTS4 NDCD1 NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4
+NDTR1 NDTR2 NDTR3 NDTR4 NDTS4 NRI1 NRI2 NRI3 NRI4 NRTS1 NRTS2 NRTS3 OSCCLK PWM0
+PWM1 PWM2 PWM3 PWM4 PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 ROM16 ROM8 ROMCS1
+ROMCS2 ROMCS3 ROMCS4 RXD1 RXD2 RXD3 RXD4 SALT1 SALT2 SALT3 SALT4 SD1 SD2 SGPMCK
+SGPMI SGPMLD SGPMO SGPSCK SGPSI0 SGPSI1 SGPSLD SIOONCTRL SIOPBI SIOPBO SIOPWREQ
+SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1DEBUG SPI1PASSTHRU SPICS1 TIMER3 TIMER4
+TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2 TXD3 TXD4 UART6 USB11D1 USB11H2 USB2D1
+USB2H1 USBCKI VGABIOS_ROM VGAHS VGAVS VPI18 VPI24 VPI30 VPO12 VPO24 WDTRST1
+WDTRST2
+
+Example
+=======
+
+syscon: scu at 1e6e2000 {
+ compatible = "aspeed,ast2400-scu", "syscon", "simple-mfd";
+ reg = <0x1e6e2000 0x1a8>;
+
+ pinctrl: pinctrl {
+ compatible = "aspeed,g4-pinctrl";
+
+ pinctrl_i2c3_default: i2c3_default {
+ function = "I2C3";
+ groups = "I2C3";
+ };
+
+ pinctrl_gpioh0_unbiased_default: gpioh0 {
+ pins = "A8";
+ bias-disable;
+ };
+ };
+};
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.txt
similarity index 66%
rename from Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
rename to Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.txt
index 3b7266c7c438..2f16e401338a 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.txt
@@ -1,14 +1,6 @@
-======================
-Aspeed Pin Controllers
-======================
-
-The Aspeed SoCs vary in functionality inside a generation but have a common mux
-device register layout.
-
-Required properties for g4:
-- compatible : Should be one of the following:
- "aspeed,ast2400-pinctrl"
- "aspeed,g4-pinctrl"
+=============================
+Aspeed AST2500 Pin Controller
+=============================
Required properties for g5:
- compatible : Should be one of the following:
@@ -23,8 +15,6 @@ The pin controller node should be the child of a syscon node with the required
property:
- compatible : Should be one of the following:
- "aspeed,ast2400-scu", "syscon", "simple-mfd"
- "aspeed,g4-scu", "syscon", "simple-mfd"
"aspeed,ast2500-scu", "syscon", "simple-mfd"
"aspeed,g5-scu", "syscon", "simple-mfd"
@@ -57,24 +47,6 @@ For pinmux, each mux function has only one associated pin group. Each group is
named by its function. The following values for the function and groups
properties are supported:
-aspeed,ast2400-pinctrl, aspeed,g4-pinctrl:
-
-ACPI ADC0 ADC1 ADC10 ADC11 ADC12 ADC13 ADC14 ADC15 ADC2 ADC3 ADC4 ADC5 ADC6
-ADC7 ADC8 ADC9 BMCINT DDCCLK DDCDAT EXTRST FLACK FLBUSY FLWP GPID GPID0 GPID2
-GPID4 GPID6 GPIE0 GPIE2 GPIE4 GPIE6 I2C10 I2C11 I2C12 I2C13 I2C14 I2C3 I2C4
-I2C5 I2C6 I2C7 I2C8 I2C9 LPCPD LPCPME LPCRST LPCSMI MAC1LINK MAC2LINK MDIO1
-MDIO2 NCTS1 NCTS2 NCTS3 NCTS4 NDCD1 NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4
-NDTR1 NDTR2 NDTR3 NDTR4 NDTS4 NRI1 NRI2 NRI3 NRI4 NRTS1 NRTS2 NRTS3 OSCCLK PWM0
-PWM1 PWM2 PWM3 PWM4 PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 ROM16 ROM8 ROMCS1
-ROMCS2 ROMCS3 ROMCS4 RXD1 RXD2 RXD3 RXD4 SALT1 SALT2 SALT3 SALT4 SD1 SD2 SGPMCK
-SGPMI SGPMLD SGPMO SGPSCK SGPSI0 SGPSI1 SGPSLD SIOONCTRL SIOPBI SIOPBO SIOPWREQ
-SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1DEBUG SPI1PASSTHRU SPICS1 TIMER3 TIMER4
-TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2 TXD3 TXD4 UART6 USB11D1 USB11H2 USB2D1
-USB2H1 USBCKI VGABIOS_ROM VGAHS VGAVS VPI18 VPI24 VPI30 VPO12 VPO24 WDTRST1
-WDTRST2
-
-aspeed,ast2500-pinctrl, aspeed,g5-pinctrl:
-
ACPI ADC0 ADC1 ADC10 ADC11 ADC12 ADC13 ADC14 ADC15 ADC2 ADC3 ADC4 ADC5 ADC6
ADC7 ADC8 ADC9 BMCINT DDCCLK DDCDAT ESPI FWSPICS1 FWSPICS2 GPID0 GPID2 GPID4
GPID6 GPIE0 GPIE2 GPIE4 GPIE6 I2C10 I2C11 I2C12 I2C13 I2C14 I2C3 I2C4 I2C5 I2C6
@@ -90,33 +62,8 @@ SPI2CS1 SPI2MISO SPI2MOSI TIMER3 TIMER4 TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2
TXD3 TXD4 UART6 USB11BHID USB2AD USB2AH USB2BD USB2BH USBCKI VGABIOSROM VGAHS
VGAVS VPI24 VPO WDTRST1 WDTRST2
-Examples
-========
-
-g4 Example
-----------
-
-syscon: scu at 1e6e2000 {
- compatible = "aspeed,ast2400-scu", "syscon", "simple-mfd";
- reg = <0x1e6e2000 0x1a8>;
-
- pinctrl: pinctrl {
- compatible = "aspeed,g4-pinctrl";
-
- pinctrl_i2c3_default: i2c3_default {
- function = "I2C3";
- groups = "I2C3";
- };
-
- pinctrl_gpioh0_unbiased_default: gpioh0 {
- pins = "A8";
- bias-disable;
- };
- };
-};
-
-g5 Example
-----------
+Example
+=======
ahb {
apb {
--
2.20.1
^ permalink raw reply related
* [PATCH v2 0/8] pinctrl: aspeed: Preparation for AST2600
From: Andrew Jeffery @ 2019-06-28 2:38 UTC (permalink / raw)
To: linux-aspeed
Hello!
The ASPEED AST2600 is in the pipeline, and we have enough information to start
preparing to upstream support for it. This series lays some ground work;
splitting the bindings and dicing the implementation up a little further to
facilitate differences between the 2600 and previous SoC generations.
v2 addresses Rob's comments on the bindings conversion patches. v1 can be found
here:
https://www.spinics.net/lists/linux-gpio/msg40157.html
Please review!
Andrew
Andrew Jeffery (8):
dt-bindings: pinctrl: aspeed: Split bindings document in two
dt-bindings: pinctrl: aspeed: Convert AST2400 bindings to json-schema
dt-bindings: pinctrl: aspeed: Convert AST2500 bindings to json-schema
MAINTAINERS: Add entry for ASPEED pinctrl drivers
pinctrl: aspeed: Correct comment that is no longer true
pinctrl: aspeed: Clarify comment about strapping W1C
pinctrl: aspeed: Split out pinmux from general pinctrl
pinctrl: aspeed: Add implementation-related documentation
.../pinctrl/aspeed,ast2400-pinctrl.yaml | 81 ++
.../pinctrl/aspeed,ast2500-pinctrl.yaml | 134 ++++
.../bindings/pinctrl/pinctrl-aspeed.txt | 172 ----
MAINTAINERS | 9 +
drivers/pinctrl/aspeed/Makefile | 2 +-
drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c | 94 ++-
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 123 ++-
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 250 +-----
drivers/pinctrl/aspeed/pinctrl-aspeed.h | 549 +------------
drivers/pinctrl/aspeed/pinmux-aspeed.c | 96 +++
drivers/pinctrl/aspeed/pinmux-aspeed.h | 735 ++++++++++++++++++
11 files changed, 1312 insertions(+), 933 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
delete mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
create mode 100644 drivers/pinctrl/aspeed/pinmux-aspeed.c
create mode 100644 drivers/pinctrl/aspeed/pinmux-aspeed.h
--
2.20.1
^ permalink raw reply
* Re: [PATCH 1/8] dt-bindings: pinctrl: aspeed: Split bindings document in two
From: Andrew Jeffery @ 2019-06-28 1:01 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACRpkdZtTy-HHu2O4aOaqV5ZdxcYYPFRuxK2jjnw+_O1xcF1rg@mail.gmail.com>
On Thu, 27 Jun 2019, at 20:56, Linus Walleij wrote:
> On Thu, Jun 27, 2019 at 4:32 AM Joel Stanley <joel@jms.id.au> wrote:
>
> > I think we can use this as an opportunity to drop the unused g4-scu
> > compatible from the bindings. Similarly for the g5.
> >
> > Acked-by: Joel Stanley <joel@jms.id.au>
>
> I assume I should wait for a new version of the patches that does
> this?
I'll take a look at the gX compatibles more broadly in a separate series.
I'm cleaning up the current series wrt Rob's comments and I hope to
send it out shortly.
Andrew
^ permalink raw reply
* Re: [PATCH 2/8] dt-bindings: pinctrl: aspeed: Convert AST2400 bindings to json-schema
From: Andrew Jeffery @ 2019-06-28 0:56 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAL_JsqJo37LQV9WKx_Zqy8KZ52=37TiGcNbFah6MsJmMYP23XA@mail.gmail.com>
On Fri, 28 Jun 2019, at 00:02, Rob Herring wrote:
> On Wed, Jun 26, 2019 at 9:55 PM Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> >
> >
> > On Wed, 26 Jun 2019, at 23:17, Rob Herring wrote:
> > > On Wed, Jun 26, 2019 at 1:21 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> > > > + The pin controller node should be the child of a syscon node with the
> > > > + required property:
> > > > +
> > > > + - compatible: Should be one of the following:
> > > > + "aspeed,ast2400-scu", "syscon", "simple-mfd"
> > > > + "aspeed,g4-scu", "syscon", "simple-mfd"
> > > > +
> > > > + Refer to the the bindings described in
> > > > + Documentation/devicetree/bindings/mfd/syscon.txt
> > > > +
> > > > + For the AST2400 pinmux, each mux function has only one associated pin group.
> > > > + Each group is named by its function. The following values for the function
> > > > + and groups properties are supported:
> > > > +
> > > > + ACPI ADC0 ADC1 ADC10 ADC11 ADC12 ADC13 ADC14 ADC15 ADC2 ADC3 ADC4 ADC5 ADC6
> > > > + ADC7 ADC8 ADC9 BMCINT DDCCLK DDCDAT EXTRST FLACK FLBUSY FLWP GPID GPID0 GPID2
> > > > + GPID4 GPID6 GPIE0 GPIE2 GPIE4 GPIE6 I2C10 I2C11 I2C12 I2C13 I2C14 I2C3 I2C4
> > > > + I2C5 I2C6 I2C7 I2C8 I2C9 LPCPD LPCPME LPCRST LPCSMI MAC1LINK MAC2LINK MDIO1
> > > > + MDIO2 NCTS1 NCTS2 NCTS3 NCTS4 NDCD1 NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4
> > > > + NDTR1 NDTR2 NDTR3 NDTR4 NDTS4 NRI1 NRI2 NRI3 NRI4 NRTS1 NRTS2 NRTS3 OSCCLK
> > > > + PWM0 PWM1 PWM2 PWM3 PWM4 PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 ROM16 ROM8
> > > > + ROMCS1 ROMCS2 ROMCS3 ROMCS4 RXD1 RXD2 RXD3 RXD4 SALT1 SALT2 SALT3 SALT4 SD1
> > > > + SD2 SGPMCK SGPMI SGPMLD SGPMO SGPSCK SGPSI0 SGPSI1 SGPSLD SIOONCTRL SIOPBI
> > > > + SIOPBO SIOPWREQ SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1DEBUG SPI1PASSTHRU
> > > > + SPICS1 TIMER3 TIMER4 TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2 TXD3 TXD4 UART6
> > > > + USB11D1 USB11H2 USB2D1 USB2H1 USBCKI VGABIOS_ROM VGAHS VGAVS VPI18 VPI24
> > > > + VPI30 VPO12 VPO24 WDTRST1 WDTRST2
> > >
> > > This should be a schema. You need to define child nodes and list these
> > > as values for 'function' and 'group'. Ideally, the child nodes would
> > > have some sort of pattern, but if not, you can just match on '^.*$'
> > > under patternProperties.
> >
> > The children don't have any pattern in their node name, which drives
> > me towards the '^.*$' pattern match, however, what I've found is that
> > I get the following errors for some of the relevant dts files:
> >
> > ```
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: compatible: ['aspeed,g4-pinctrl'] is not of type 'object'
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: pinctrl-names: ['default'] is not of type 'object'
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: pinctrl-0: [[7, 8, 9, 10, 11, 12]] is not of type 'object'
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: phandle: [[13]] is not of type 'object'
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: $nodename: ['pinctrl'] is not of type 'object'
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: compatible: ['aspeed,g4-pinctrl'] is not of type 'object'
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: pinctrl-names: ['default'] is not of type 'object'
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: pinctrl-0: [[9, 10, 11, 12]] is not of type 'object'
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: phandle: [[13]] is not of type 'object'
> > /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: $nodename: ['pinctrl'] is not of type 'object'
> > ```
> >
>
> The problem is "^.*$" matches both properties and child nodes.
>
> > We shouldn't be expecting these properties in the child nodes, so
> > something is busted. Looking at processed-schema.yaml, we have:
> >
> > ```
> > - $filename: /home/andrew/src/linux/aspeed/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> > $id: http://devicetree.org/schemas/pinctrl/aspeed,ast2400-pinctrl.yaml#
> > $schema: http://devicetree.org/meta-schemas/core.yaml#
> > patternProperties:
> > ^.*$:
> > patternProperties:
> > ^function|groups$:
> > allOf:
> > - {$ref: /schemas/types.yaml#/definitions/string}
> > - additionalItems: false
> > items:
> > enum: [ACPI, ADC0, ADC1, ADC10, ADC11, ADC12, ADC13, ADC14, ADC15, ADC2,
> > ADC3, ADC4, ADC5, ADC6, ADC7, ADC8, ADC9, BMCINT, DDCCLK, DDCDAT,
> > EXTRST, FLACK, FLBUSY, FLWP, GPID, GPID0, GPID2, GPID4, GPID6, GPIE0,
> > GPIE2, GPIE4, GPIE6, I2C10, I2C11, I2C12, I2C13, I2C14, I2C3, I2C4,
> > I2C5, I2C6, I2C7, I2C8, I2C9, LPCPD, LPCPME, LPCRST, LPCSMI, MAC1LINK,
> > MAC2LINK, MDIO1, MDIO2, NCTS1, NCTS2, NCTS3, NCTS4, NDCD1, NDCD2,
> > NDCD3, NDCD4, NDSR1, NDSR2, NDSR3, NDSR4, NDTR1, NDTR2, NDTR3, NDTR4,
> > NDTS4, NRI1, NRI2, NRI3, NRI4, NRTS1, NRTS2, NRTS3, OSCCLK, PWM0,
> > PWM1, PWM2, PWM3, PWM4, PWM5, PWM6, PWM7, RGMII1, RGMII2, RMII1, RMII2,
> > ROM16, ROM8, ROMCS1, ROMCS2, ROMCS3, ROMCS4, RXD1, RXD2, RXD3, RXD4,
> > SALT1, SALT2, SALT3, SALT4, SD1, SD2, SGPMCK, SGPMI, SGPMLD, SGPMO,
> > SGPSCK, SGPSI0, SGPSI1, SGPSLD, SIOONCTRL, SIOPBI, SIOPBO, SIOPWREQ,
> > SIOPWRGD, SIOS3, SIOS5, SIOSCI, SPI1, SPI1DEBUG, SPI1PASSTHRU, SPICS1,
> > TIMER3, TIMER4, TIMER5, TIMER6, TIMER7, TIMER8, TXD1, TXD2, TXD3,
> > TXD4, UART6, USB11D1, USB11H2, USB2D1, USB2H1, USBCKI, VGABIOS_ROM,
> > VGAHS, VGAVS, VPI18, VPI24, VPI30, VPO12, VPO24, WDTRST1, WDTRST2]
> > maxItems: 1
> > minItems: 1
> > type: array
> > pinctrl-[0-9]+: true
> > properties: {phandle: true, pinctrl-names: true, status: true}
> > type: object
> > pinctrl-[0-9]+: true
> > properties:
> > $nodename: true
> > compatible:
> > additionalItems: false
> > items:
> > - enum: ['aspeed,ast2400-pinctrl', 'aspeed,g4-pinctrl']
> > maxItems: 1
> > minItems: 1
> > type: array
> > phandle: true
> > pinctrl-names: true
> > status: true
> > required: [compatible]
> > select:
> > properties:
> > compatible:
> > contains:
> > enum: ['aspeed,ast2400-pinctrl', 'aspeed,g4-pinctrl']
> > required: [compatible]
> > title: ASPEED AST2400 Pin Controller
> > ```
> >
> > `properties: {phandle: true, pinctrl-names: true, status: true}` has been
> > merged into my '^.*$' patternProperty, presumably partly from
> > pinctrl-consumer.yaml, and this seems to be the source of the bad
> > output. If as a hack I change my pattern to '^.*_default$' the problem
> > goes away as we no longer try to enforce the constraints on properties
> > provided by other bindings, but the problem is the node names are
> > largely freeform[1] (unless I enforce a naming constraint as part of my
> > bindings?).
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt?h=v5.2-rc6#n112
> >
> > >
> > > BTW, You can put the names under a 'definitions' key and then use
> > > '$ref' to reference them from function and group to avoid duplicating
> > > the names. Or use patternProperties with '^(function|group)$'.
> >
> > I've used the patternProperties approach above as I couldn't get the
> > definitions/$ref approach to work. I did the following:
>
> The problem is we'd need to process the schema under definitions. The
> YAML encoding we validate against always encodes strings as arrays as
> dtc has no way of knowing if a given property is a string array or
> single string. So to avoid a bunch of boilerplate in every binding, we
> process the schema to transform single strings into arrays of length
> 1.
>
> It's probably best to stick with the patternProperties approach. I
> think you can do something like this:
>
> "^.*$":
> if:
> type: object
> then:
> patternProperties:
> '^(function|group)$':
> ...
>
> I'm not completely certain this works though, so if you can send me an
> updated binding with what you have so far I can test it out.
This works, thanks. I'll send an updated series.
Andrew
^ permalink raw reply
* Re: [PATCH 2/8] dt-bindings: pinctrl: aspeed: Convert AST2400 bindings to json-schema
From: Andrew Jeffery @ 2019-06-28 0:47 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAL_JsqLiZzkJZ+CeaMDer=Arm9vFdG1Y_6F0M=AZV=82EqORFg@mail.gmail.com>
On Thu, 27 Jun 2019, at 23:40, Rob Herring wrote:
> On Wed, Jun 26, 2019 at 6:44 PM Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> >
> >
> > On Wed, 26 Jun 2019, at 23:17, Rob Herring wrote:
> > > On Wed, Jun 26, 2019 at 1:21 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> > > >
> > > > Convert ASPEED pinctrl bindings to DT schema format using json-schema
> > >
> > > BTW, ASPEED is one of the remaining platforms needing the top-level
> > > board bindings converted.
> >
> > Okay, I'll put together patches to fix that.
> >
> > >
> > > >
> > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > > ---
> > > > .../pinctrl/aspeed,ast2400-pinctrl.txt | 80 -------------------
> > > > .../pinctrl/aspeed,ast2400-pinctrl.yaml | 73 +++++++++++++++++
> > > > 2 files changed, 73 insertions(+), 80 deletions(-)
> > > > delete mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
> > > > create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> > >
> > > > diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> > > > new file mode 100644
> > > > index 000000000000..3b8cf3e51506
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> > > > @@ -0,0 +1,73 @@
> > > > +# SPDX-License-Identifier: GPL-2.0+
> > >
> > > Do you have rights to change the license?
> >
> > Where are you coming from with this question? The bindings previously didn't list a
> > license, is there some implicit license for them? I would have thought it was GPL-2.0?
>
> Yes, it is implicitly GPL-2.0 since it is in the kernel tree and has
> no other license text.
>
> > IBM's (my employer's) preferred contribution license is GPL 2.0-or-later, so I was just
> > adding the SPDX marker to clarify.
>
> Adding 'or-later' is a licensing change. If IBM is the copyright
> holder on all this file, then that is fine.
I authored the file for IBM and they hold the copyright, so the change is permitted.
>
> > > If so, the preference is to
> > > dual license with (GPL-2.0 OR BSD-2-Clause).
> >
> > You're asking if I have the power to relicense so I can dual license it this way?
>
> It would probably be up to your company. If that's an issue, then not
> dual licensing is fine. I don't want to hold things up on that.
Okay. I've asked and the query is being resolved internally. I'm not sure when
that will occur though, so I'll relicense it in a future patch if the request gets
the go ahead. Just for the record, what's the motivation for the dual license?
Understanding why will likely help resolve the request.
>
> [...]
>
> > > > +required:
> > > > + - compatible
> > > > +
> > > > +description: |+
> > >
> > > description goes before properties.
> >
> > Okay. I wouldn't have thought the ordering mattered. Is this just a preference?
>
> Yes, just a preference.
>
> > The tools seemed to run fine as is.
> >
> > I'll re-order it regardless.
> >
> > >
> > > > + The pin controller node should be the child of a syscon node with the
> > > > + required property:
> > > > +
> > > > + - compatible: Should be one of the following:
> > > > + "aspeed,ast2400-scu", "syscon", "simple-mfd"
> > > > + "aspeed,g4-scu", "syscon", "simple-mfd"
> > > > +
> > > > + Refer to the the bindings described in
> > > > + Documentation/devicetree/bindings/mfd/syscon.txt
> > > > +
> > > > + For the AST2400 pinmux, each mux function has only one associated pin group.
> > > > + Each group is named by its function. The following values for the function
> > > > + and groups properties are supported:
> > > > +
> > > > + ACPI ADC0 ADC1 ADC10 ADC11 ADC12 ADC13 ADC14 ADC15 ADC2 ADC3 ADC4 ADC5 ADC6
> > > > + ADC7 ADC8 ADC9 BMCINT DDCCLK DDCDAT EXTRST FLACK FLBUSY FLWP GPID GPID0 GPID2
> > > > + GPID4 GPID6 GPIE0 GPIE2 GPIE4 GPIE6 I2C10 I2C11 I2C12 I2C13 I2C14 I2C3 I2C4
> > > > + I2C5 I2C6 I2C7 I2C8 I2C9 LPCPD LPCPME LPCRST LPCSMI MAC1LINK MAC2LINK MDIO1
> > > > + MDIO2 NCTS1 NCTS2 NCTS3 NCTS4 NDCD1 NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4
> > > > + NDTR1 NDTR2 NDTR3 NDTR4 NDTS4 NRI1 NRI2 NRI3 NRI4 NRTS1 NRTS2 NRTS3 OSCCLK
> > > > + PWM0 PWM1 PWM2 PWM3 PWM4 PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 ROM16 ROM8
> > > > + ROMCS1 ROMCS2 ROMCS3 ROMCS4 RXD1 RXD2 RXD3 RXD4 SALT1 SALT2 SALT3 SALT4 SD1
> > > > + SD2 SGPMCK SGPMI SGPMLD SGPMO SGPSCK SGPSI0 SGPSI1 SGPSLD SIOONCTRL SIOPBI
> > > > + SIOPBO SIOPWREQ SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1DEBUG SPI1PASSTHRU
> > > > + SPICS1 TIMER3 TIMER4 TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2 TXD3 TXD4 UART6
> > > > + USB11D1 USB11H2 USB2D1 USB2H1 USBCKI VGABIOS_ROM VGAHS VGAVS VPI18 VPI24
> > > > + VPI30 VPO12 VPO24 WDTRST1 WDTRST2
> > >
> > > This should be a schema.
> >
> > Yeah, I covered this in my cover letter. I was hoping to get away without
> > that for the moment as this seems like the first pinctrl binding to be
> > converted, however if you insist...
>
> That generally doesn't matter. You can assume common properties will
> have a schema and you don't need to define common constraints (like
> 'function' is a string array). You only need what is specific to this
> binding which is possible values.
Right, it just wasn't clear to me how much effort was involved. Having
hacked around a bit now I've found it's not so much.
Thanks for your feedback.
Andrew
^ permalink raw reply
* [PATCH v3 1/8] dt-bindings: soc: Add Aspeed XDMA engine binding documentation
From: Eddie James @ 2019-06-27 19:19 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <58b74556-cbf0-4da2-9392-4c4ac40ad760@www.fastmail.com>
On 5/30/19 12:30 AM, Andrew Jeffery wrote:
>
> On Thu, 30 May 2019, at 03:40, Eddie James wrote:
>> Document the bindings.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>> .../devicetree/bindings/soc/aspeed/xdma.txt | 23 ++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/soc/aspeed/xdma.txt
>>
>> diff --git a/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
>> b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
>> new file mode 100644
>> index 0000000..85e82ea
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
>> @@ -0,0 +1,23 @@
>> +* Device tree bindings for the Aspeed XDMA Engine
>> +
>> +The XDMA Engine embedded in the AST2500 SOC can perform automatic DMA
>> +operations over PCI between the AST2500 (acting as a BMC) and a host
>> processor.
>> +
>> +Required properties:
>> +
>> + - compatible "aspeed,ast2500-xdma"
>> + - reg contains the offset and length of the memory region
>> + assigned to the XDMA registers
>> + - resets reset specifier for the syscon reset associated with
>> + the XDMA engine
>> + - interrupts the interrupt associated with the XDMA engine on this
>> + platform
> The indentation is quite distracting. If you rev the series can you fix it?
I think the diff is throwing it off; it all lines up when applied.
Thanks,
Eddie
>
> Otherwise,
>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
>
>> +
>> +Example:
>> +
>> + xdma at 1e6e7000 {
>> + compatible = "aspeed,ast2500-xdma";
>> + reg = <0x1e6e7000 0x100>;
>> + resets = <&syscon ASPEED_RESET_XDMA>;
>> + interrupts = <6>;
>> + };
>> --
>> 1.8.3.1
>>
>>
^ permalink raw reply
* [PATCH 2/8] dt-bindings: pinctrl: aspeed: Convert AST2400 bindings to json-schema
From: Rob Herring @ 2019-06-27 14:32 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <30d5585b-7591-4149-87c4-816e4c18fb9d@www.fastmail.com>
On Wed, Jun 26, 2019 at 9:55 PM Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Wed, 26 Jun 2019, at 23:17, Rob Herring wrote:
> > On Wed, Jun 26, 2019 at 1:21 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> > > + The pin controller node should be the child of a syscon node with the
> > > + required property:
> > > +
> > > + - compatible: Should be one of the following:
> > > + "aspeed,ast2400-scu", "syscon", "simple-mfd"
> > > + "aspeed,g4-scu", "syscon", "simple-mfd"
> > > +
> > > + Refer to the the bindings described in
> > > + Documentation/devicetree/bindings/mfd/syscon.txt
> > > +
> > > + For the AST2400 pinmux, each mux function has only one associated pin group.
> > > + Each group is named by its function. The following values for the function
> > > + and groups properties are supported:
> > > +
> > > + ACPI ADC0 ADC1 ADC10 ADC11 ADC12 ADC13 ADC14 ADC15 ADC2 ADC3 ADC4 ADC5 ADC6
> > > + ADC7 ADC8 ADC9 BMCINT DDCCLK DDCDAT EXTRST FLACK FLBUSY FLWP GPID GPID0 GPID2
> > > + GPID4 GPID6 GPIE0 GPIE2 GPIE4 GPIE6 I2C10 I2C11 I2C12 I2C13 I2C14 I2C3 I2C4
> > > + I2C5 I2C6 I2C7 I2C8 I2C9 LPCPD LPCPME LPCRST LPCSMI MAC1LINK MAC2LINK MDIO1
> > > + MDIO2 NCTS1 NCTS2 NCTS3 NCTS4 NDCD1 NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4
> > > + NDTR1 NDTR2 NDTR3 NDTR4 NDTS4 NRI1 NRI2 NRI3 NRI4 NRTS1 NRTS2 NRTS3 OSCCLK
> > > + PWM0 PWM1 PWM2 PWM3 PWM4 PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 ROM16 ROM8
> > > + ROMCS1 ROMCS2 ROMCS3 ROMCS4 RXD1 RXD2 RXD3 RXD4 SALT1 SALT2 SALT3 SALT4 SD1
> > > + SD2 SGPMCK SGPMI SGPMLD SGPMO SGPSCK SGPSI0 SGPSI1 SGPSLD SIOONCTRL SIOPBI
> > > + SIOPBO SIOPWREQ SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1DEBUG SPI1PASSTHRU
> > > + SPICS1 TIMER3 TIMER4 TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2 TXD3 TXD4 UART6
> > > + USB11D1 USB11H2 USB2D1 USB2H1 USBCKI VGABIOS_ROM VGAHS VGAVS VPI18 VPI24
> > > + VPI30 VPO12 VPO24 WDTRST1 WDTRST2
> >
> > This should be a schema. You need to define child nodes and list these
> > as values for 'function' and 'group'. Ideally, the child nodes would
> > have some sort of pattern, but if not, you can just match on '^.*$'
> > under patternProperties.
>
> The children don't have any pattern in their node name, which drives
> me towards the '^.*$' pattern match, however, what I've found is that
> I get the following errors for some of the relevant dts files:
>
> ```
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: compatible: ['aspeed,g4-pinctrl'] is not of type 'object'
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: pinctrl-names: ['default'] is not of type 'object'
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: pinctrl-0: [[7, 8, 9, 10, 11, 12]] is not of type 'object'
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: phandle: [[13]] is not of type 'object'
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dt.yaml: $nodename: ['pinctrl'] is not of type 'object'
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: compatible: ['aspeed,g4-pinctrl'] is not of type 'object'
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: pinctrl-names: ['default'] is not of type 'object'
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: pinctrl-0: [[9, 10, 11, 12]] is not of type 'object'
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: phandle: [[13]] is not of type 'object'
> /home/andrew/src/linux/aspeed/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dt.yaml: $nodename: ['pinctrl'] is not of type 'object'
> ```
>
The problem is "^.*$" matches both properties and child nodes.
> We shouldn't be expecting these properties in the child nodes, so
> something is busted. Looking at processed-schema.yaml, we have:
>
> ```
> - $filename: /home/andrew/src/linux/aspeed/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> $id: http://devicetree.org/schemas/pinctrl/aspeed,ast2400-pinctrl.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
> patternProperties:
> ^.*$:
> patternProperties:
> ^function|groups$:
> allOf:
> - {$ref: /schemas/types.yaml#/definitions/string}
> - additionalItems: false
> items:
> enum: [ACPI, ADC0, ADC1, ADC10, ADC11, ADC12, ADC13, ADC14, ADC15, ADC2,
> ADC3, ADC4, ADC5, ADC6, ADC7, ADC8, ADC9, BMCINT, DDCCLK, DDCDAT,
> EXTRST, FLACK, FLBUSY, FLWP, GPID, GPID0, GPID2, GPID4, GPID6, GPIE0,
> GPIE2, GPIE4, GPIE6, I2C10, I2C11, I2C12, I2C13, I2C14, I2C3, I2C4,
> I2C5, I2C6, I2C7, I2C8, I2C9, LPCPD, LPCPME, LPCRST, LPCSMI, MAC1LINK,
> MAC2LINK, MDIO1, MDIO2, NCTS1, NCTS2, NCTS3, NCTS4, NDCD1, NDCD2,
> NDCD3, NDCD4, NDSR1, NDSR2, NDSR3, NDSR4, NDTR1, NDTR2, NDTR3, NDTR4,
> NDTS4, NRI1, NRI2, NRI3, NRI4, NRTS1, NRTS2, NRTS3, OSCCLK, PWM0,
> PWM1, PWM2, PWM3, PWM4, PWM5, PWM6, PWM7, RGMII1, RGMII2, RMII1, RMII2,
> ROM16, ROM8, ROMCS1, ROMCS2, ROMCS3, ROMCS4, RXD1, RXD2, RXD3, RXD4,
> SALT1, SALT2, SALT3, SALT4, SD1, SD2, SGPMCK, SGPMI, SGPMLD, SGPMO,
> SGPSCK, SGPSI0, SGPSI1, SGPSLD, SIOONCTRL, SIOPBI, SIOPBO, SIOPWREQ,
> SIOPWRGD, SIOS3, SIOS5, SIOSCI, SPI1, SPI1DEBUG, SPI1PASSTHRU, SPICS1,
> TIMER3, TIMER4, TIMER5, TIMER6, TIMER7, TIMER8, TXD1, TXD2, TXD3,
> TXD4, UART6, USB11D1, USB11H2, USB2D1, USB2H1, USBCKI, VGABIOS_ROM,
> VGAHS, VGAVS, VPI18, VPI24, VPI30, VPO12, VPO24, WDTRST1, WDTRST2]
> maxItems: 1
> minItems: 1
> type: array
> pinctrl-[0-9]+: true
> properties: {phandle: true, pinctrl-names: true, status: true}
> type: object
> pinctrl-[0-9]+: true
> properties:
> $nodename: true
> compatible:
> additionalItems: false
> items:
> - enum: ['aspeed,ast2400-pinctrl', 'aspeed,g4-pinctrl']
> maxItems: 1
> minItems: 1
> type: array
> phandle: true
> pinctrl-names: true
> status: true
> required: [compatible]
> select:
> properties:
> compatible:
> contains:
> enum: ['aspeed,ast2400-pinctrl', 'aspeed,g4-pinctrl']
> required: [compatible]
> title: ASPEED AST2400 Pin Controller
> ```
>
> `properties: {phandle: true, pinctrl-names: true, status: true}` has been
> merged into my '^.*$' patternProperty, presumably partly from
> pinctrl-consumer.yaml, and this seems to be the source of the bad
> output. If as a hack I change my pattern to '^.*_default$' the problem
> goes away as we no longer try to enforce the constraints on properties
> provided by other bindings, but the problem is the node names are
> largely freeform[1] (unless I enforce a naming constraint as part of my
> bindings?).
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt?h=v5.2-rc6#n112
>
> >
> > BTW, You can put the names under a 'definitions' key and then use
> > '$ref' to reference them from function and group to avoid duplicating
> > the names. Or use patternProperties with '^(function|group)$'.
>
> I've used the patternProperties approach above as I couldn't get the
> definitions/$ref approach to work. I did the following:
The problem is we'd need to process the schema under definitions. The
YAML encoding we validate against always encodes strings as arrays as
dtc has no way of knowing if a given property is a string array or
single string. So to avoid a bunch of boilerplate in every binding, we
process the schema to transform single strings into arrays of length
1.
It's probably best to stick with the patternProperties approach. I
think you can do something like this:
"^.*$":
if:
type: object
then:
patternProperties:
'^(function|group)$':
...
I'm not completely certain this works though, so if you can send me an
updated binding with what you have so far I can test it out.
Rob
^ permalink raw reply
* [PATCH 2/8] dt-bindings: pinctrl: aspeed: Convert AST2400 bindings to json-schema
From: Rob Herring @ 2019-06-27 14:09 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <ee0cac9e-4b39-4900-87a8-3dabb58ed883@www.fastmail.com>
On Wed, Jun 26, 2019 at 6:44 PM Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Wed, 26 Jun 2019, at 23:17, Rob Herring wrote:
> > On Wed, Jun 26, 2019 at 1:21 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> > >
> > > Convert ASPEED pinctrl bindings to DT schema format using json-schema
> >
> > BTW, ASPEED is one of the remaining platforms needing the top-level
> > board bindings converted.
>
> Okay, I'll put together patches to fix that.
>
> >
> > >
> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > ---
> > > .../pinctrl/aspeed,ast2400-pinctrl.txt | 80 -------------------
> > > .../pinctrl/aspeed,ast2400-pinctrl.yaml | 73 +++++++++++++++++
> > > 2 files changed, 73 insertions(+), 80 deletions(-)
> > > delete mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> >
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> > > new file mode 100644
> > > index 000000000000..3b8cf3e51506
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> > > @@ -0,0 +1,73 @@
> > > +# SPDX-License-Identifier: GPL-2.0+
> >
> > Do you have rights to change the license?
>
> Where are you coming from with this question? The bindings previously didn't list a
> license, is there some implicit license for them? I would have thought it was GPL-2.0?
Yes, it is implicitly GPL-2.0 since it is in the kernel tree and has
no other license text.
> IBM's (my employer's) preferred contribution license is GPL 2.0-or-later, so I was just
> adding the SPDX marker to clarify.
Adding 'or-later' is a licensing change. If IBM is the copyright
holder on all this file, then that is fine.
> > If so, the preference is to
> > dual license with (GPL-2.0 OR BSD-2-Clause).
>
> You're asking if I have the power to relicense so I can dual license it this way?
It would probably be up to your company. If that's an issue, then not
dual licensing is fine. I don't want to hold things up on that.
[...]
> > > +required:
> > > + - compatible
> > > +
> > > +description: |+
> >
> > description goes before properties.
>
> Okay. I wouldn't have thought the ordering mattered. Is this just a preference?
Yes, just a preference.
> The tools seemed to run fine as is.
>
> I'll re-order it regardless.
>
> >
> > > + The pin controller node should be the child of a syscon node with the
> > > + required property:
> > > +
> > > + - compatible: Should be one of the following:
> > > + "aspeed,ast2400-scu", "syscon", "simple-mfd"
> > > + "aspeed,g4-scu", "syscon", "simple-mfd"
> > > +
> > > + Refer to the the bindings described in
> > > + Documentation/devicetree/bindings/mfd/syscon.txt
> > > +
> > > + For the AST2400 pinmux, each mux function has only one associated pin group.
> > > + Each group is named by its function. The following values for the function
> > > + and groups properties are supported:
> > > +
> > > + ACPI ADC0 ADC1 ADC10 ADC11 ADC12 ADC13 ADC14 ADC15 ADC2 ADC3 ADC4 ADC5 ADC6
> > > + ADC7 ADC8 ADC9 BMCINT DDCCLK DDCDAT EXTRST FLACK FLBUSY FLWP GPID GPID0 GPID2
> > > + GPID4 GPID6 GPIE0 GPIE2 GPIE4 GPIE6 I2C10 I2C11 I2C12 I2C13 I2C14 I2C3 I2C4
> > > + I2C5 I2C6 I2C7 I2C8 I2C9 LPCPD LPCPME LPCRST LPCSMI MAC1LINK MAC2LINK MDIO1
> > > + MDIO2 NCTS1 NCTS2 NCTS3 NCTS4 NDCD1 NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4
> > > + NDTR1 NDTR2 NDTR3 NDTR4 NDTS4 NRI1 NRI2 NRI3 NRI4 NRTS1 NRTS2 NRTS3 OSCCLK
> > > + PWM0 PWM1 PWM2 PWM3 PWM4 PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 ROM16 ROM8
> > > + ROMCS1 ROMCS2 ROMCS3 ROMCS4 RXD1 RXD2 RXD3 RXD4 SALT1 SALT2 SALT3 SALT4 SD1
> > > + SD2 SGPMCK SGPMI SGPMLD SGPMO SGPSCK SGPSI0 SGPSI1 SGPSLD SIOONCTRL SIOPBI
> > > + SIOPBO SIOPWREQ SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1DEBUG SPI1PASSTHRU
> > > + SPICS1 TIMER3 TIMER4 TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2 TXD3 TXD4 UART6
> > > + USB11D1 USB11H2 USB2D1 USB2H1 USBCKI VGABIOS_ROM VGAHS VGAVS VPI18 VPI24
> > > + VPI30 VPO12 VPO24 WDTRST1 WDTRST2
> >
> > This should be a schema.
>
> Yeah, I covered this in my cover letter. I was hoping to get away without
> that for the moment as this seems like the first pinctrl binding to be
> converted, however if you insist...
That generally doesn't matter. You can assume common properties will
have a schema and you don't need to define common constraints (like
'function' is a string array). You only need what is specific to this
binding which is possible values.
Rob
^ permalink raw reply
* [PATCH 1/8] dt-bindings: pinctrl: aspeed: Split bindings document in two
From: Linus Walleij @ 2019-06-27 11:26 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACPK8Xfdd1ReAHr9f6zRbZ-WJRquDJsTdUQeT_JuEBhOzS8tig@mail.gmail.com>
On Thu, Jun 27, 2019 at 4:32 AM Joel Stanley <joel@jms.id.au> wrote:
> I think we can use this as an opportunity to drop the unused g4-scu
> compatible from the bindings. Similarly for the g5.
>
> Acked-by: Joel Stanley <joel@jms.id.au>
I assume I should wait for a new version of the patches that does
this?
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 1/8] dt-bindings: pinctrl: aspeed: Split bindings document in two
From: Joel Stanley @ 2019-06-27 4:07 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <226afe63-cc86-4920-abc1-025bdda32063@www.fastmail.com>
On Thu, 27 Jun 2019 at 04:02, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Thu, 27 Jun 2019, at 13:02, Joel Stanley wrote:
> > On Wed, 26 Jun 2019 at 07:15, Andrew Jeffery <andrew@aj.id.au> wrote:
> > >
> > > Have one for each of the AST2400 and AST2500. The only thing that was
> > > common was the fact that both support ASPEED BMC SoCs.
> > >
> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > ---
> > > .../pinctrl/aspeed,ast2400-pinctrl.txt | 80 +++++++++++++++++++
> > > ...-aspeed.txt => aspeed,ast2500-pinctrl.txt} | 63 ++-------------
> > > 2 files changed, 85 insertions(+), 58 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
> > > rename Documentation/devicetree/bindings/pinctrl/{pinctrl-aspeed.txt => aspeed,ast2500-pinctrl.txt} (66%)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
> > > new file mode 100644
> > > index 000000000000..67e0325ccf2e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
> > > @@ -0,0 +1,80 @@
> > > +=============================
> > > +Aspeed AST2400 Pin Controller
> > > +=============================
> > > +
> > > +Required properties for the AST2400:
> > > +- compatible : Should be one of the following:
> > > + "aspeed,ast2400-pinctrl"
> > > + "aspeed,g4-pinctrl"
> > > +
> > > +The pin controller node should be the child of a syscon node with the required
> > > +property:
> > > +
> > > +- compatible : Should be one of the following:
> > > + "aspeed,ast2400-scu", "syscon", "simple-mfd"
> > > + "aspeed,g4-scu", "syscon", "simple-mfd"
> >
> > I think we can use this as an opportunity to drop the unused g4-scu
> > compatible from the bindings. Similarly for the g5.
>
> I Wonder if we should eradicate that pattern for all the aspeed compatibles?
Yes. We've settled on ast2x00,aspeed-<foo> for most of them. If you're
aware of others we should remove them from the bindings.
I think we've stopped any new users of the gx style from getting merged.
^ permalink raw reply
* Re: [PATCH 1/8] dt-bindings: pinctrl: aspeed: Split bindings document in two
From: Andrew Jeffery @ 2019-06-27 4:02 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACPK8Xfdd1ReAHr9f6zRbZ-WJRquDJsTdUQeT_JuEBhOzS8tig@mail.gmail.com>
On Thu, 27 Jun 2019, at 13:02, Joel Stanley wrote:
> On Wed, 26 Jun 2019 at 07:15, Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> > Have one for each of the AST2400 and AST2500. The only thing that was
> > common was the fact that both support ASPEED BMC SoCs.
> >
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > .../pinctrl/aspeed,ast2400-pinctrl.txt | 80 +++++++++++++++++++
> > ...-aspeed.txt => aspeed,ast2500-pinctrl.txt} | 63 ++-------------
> > 2 files changed, 85 insertions(+), 58 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
> > rename Documentation/devicetree/bindings/pinctrl/{pinctrl-aspeed.txt => aspeed,ast2500-pinctrl.txt} (66%)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
> > new file mode 100644
> > index 000000000000..67e0325ccf2e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.txt
> > @@ -0,0 +1,80 @@
> > +=============================
> > +Aspeed AST2400 Pin Controller
> > +=============================
> > +
> > +Required properties for the AST2400:
> > +- compatible : Should be one of the following:
> > + "aspeed,ast2400-pinctrl"
> > + "aspeed,g4-pinctrl"
> > +
> > +The pin controller node should be the child of a syscon node with the required
> > +property:
> > +
> > +- compatible : Should be one of the following:
> > + "aspeed,ast2400-scu", "syscon", "simple-mfd"
> > + "aspeed,g4-scu", "syscon", "simple-mfd"
>
> I think we can use this as an opportunity to drop the unused g4-scu
> compatible from the bindings. Similarly for the g5.
I Wonder if we should eradicate that pattern for all the aspeed compatibles?
>
> Acked-by: Joel Stanley <joel@jms.id.au>
Cheers,
Andrew
^ permalink raw reply
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