* [PATCH 0/2] arm: aspeed: Add LPC uart routing support @ 2021-09-01 6:22 ` Chia-Wei Wang 0 siblings, 0 replies; 18+ messages in thread From: Chia-Wei Wang @ 2021-09-01 6:22 UTC (permalink / raw) To: robh+dt, joel, andrew, osk, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, openbmc Add LPC uart routing driver and the device tree nodes. Chia-Wei Wang (2): soc: aspeed: Add LPC UART routing support ARM: dts: aspeed: Add uart routing to device tree arch/arm/boot/dts/aspeed-g5.dtsi | 6 + arch/arm/boot/dts/aspeed-g6.dtsi | 6 + drivers/soc/aspeed/Kconfig | 11 + drivers/soc/aspeed/Makefile | 9 +- drivers/soc/aspeed/aspeed-lpc-uart-routing.c | 621 +++++++++++++++++++ 5 files changed, 649 insertions(+), 4 deletions(-) create mode 100644 drivers/soc/aspeed/aspeed-lpc-uart-routing.c -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/2] arm: aspeed: Add LPC uart routing support @ 2021-09-01 6:22 ` Chia-Wei Wang 0 siblings, 0 replies; 18+ messages in thread From: Chia-Wei Wang @ 2021-09-01 6:22 UTC (permalink / raw) To: robh+dt, joel, andrew, osk, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, openbmc Add LPC uart routing driver and the device tree nodes. Chia-Wei Wang (2): soc: aspeed: Add LPC UART routing support ARM: dts: aspeed: Add uart routing to device tree arch/arm/boot/dts/aspeed-g5.dtsi | 6 + arch/arm/boot/dts/aspeed-g6.dtsi | 6 + drivers/soc/aspeed/Kconfig | 11 + drivers/soc/aspeed/Makefile | 9 +- drivers/soc/aspeed/aspeed-lpc-uart-routing.c | 621 +++++++++++++++++++ 5 files changed, 649 insertions(+), 4 deletions(-) create mode 100644 drivers/soc/aspeed/aspeed-lpc-uart-routing.c -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] soc: aspeed: Add LPC UART routing support 2021-09-01 6:22 ` Chia-Wei Wang @ 2021-09-01 6:22 ` Chia-Wei Wang -1 siblings, 0 replies; 18+ messages in thread From: Chia-Wei Wang @ 2021-09-01 6:22 UTC (permalink / raw) To: robh+dt, joel, andrew, osk, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, openbmc Add driver support for the LPC UART routing control. Users can perform runtime configuration of the RX muxes among the UART controllers and the UART TXD/RXD IO pins. This is achieved through the exported sysfs interface. Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> --- drivers/soc/aspeed/Kconfig | 11 + drivers/soc/aspeed/Makefile | 9 +- drivers/soc/aspeed/aspeed-lpc-uart-routing.c | 621 +++++++++++++++++++ 3 files changed, 637 insertions(+), 4 deletions(-) create mode 100644 drivers/soc/aspeed/aspeed-lpc-uart-routing.c diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig index 243ca196e6ad..6866757e6be2 100644 --- a/drivers/soc/aspeed/Kconfig +++ b/drivers/soc/aspeed/Kconfig @@ -24,6 +24,17 @@ config ASPEED_LPC_SNOOP allows the BMC to listen on and save the data written by the host to an arbitrary LPC I/O port. +config ASPEED_LPC_UART_ROUTING + tristate "ASPEED LPC uart routing control" + select REGMAP + select MFD_SYSCON + default ARCH_ASPEED + help + Provides a driver to control the LPC UART routing paths. + An sysfs interface is exported allowing users to perform + runtime configuration of the RX muxes between the UART + controllers and I/O pins. + config ASPEED_P2A_CTRL tristate "ASPEED P2A (VGA MMIO to BMC) bridge control" select REGMAP diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile index fcab7192e1a4..87ad6578ddbb 100644 --- a/drivers/soc/aspeed/Makefile +++ b/drivers/soc/aspeed/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only -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_SOCINFO) += aspeed-socinfo.o +obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o +obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o +obj-$(CONFIG_ASPEED_LPC_UART_ROUTING) += aspeed-lpc-uart-routing.o +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o +obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o diff --git a/drivers/soc/aspeed/aspeed-lpc-uart-routing.c b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c new file mode 100644 index 000000000000..141d0d08b4ff --- /dev/null +++ b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c @@ -0,0 +1,621 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2018 Google LLC + * Copyright (c) 2021 Aspeed Technology Inc. + */ +#include <linux/device.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> +#include <linux/platform_device.h> + +/* register offsets */ +#define HICR9 0x98 +#define HICRA 0x9c + +/* attributes options */ +#define UART_ROUTING_IO1 "io1" +#define UART_ROUTING_IO2 "io2" +#define UART_ROUTING_IO3 "io3" +#define UART_ROUTING_IO4 "io4" +#define UART_ROUTING_IO5 "io5" +#define UART_ROUTING_IO6 "io6" +#define UART_ROUTING_IO10 "io10" +#define UART_ROUTING_UART1 "uart1" +#define UART_ROUTING_UART2 "uart2" +#define UART_ROUTING_UART3 "uart3" +#define UART_ROUTING_UART4 "uart4" +#define UART_ROUTING_UART5 "uart5" +#define UART_ROUTING_UART6 "uart6" +#define UART_ROUTING_UART10 "uart10" +#define UART_ROUTING_RES "reserved" + +struct aspeed_uart_routing { + struct regmap *map; + spinlock_t lock; + struct attribute_group const *attr_grp; +}; + +struct aspeed_uart_routing_selector { + struct device_attribute dev_attr; + uint32_t reg; + uint32_t mask; + uint32_t shift; + const char *const options[]; +}; + +#define to_routing_selector(_dev_attr) \ + container_of(_dev_attr, struct aspeed_uart_routing_selector, dev_attr) + +static ssize_t aspeed_uart_routing_show(struct device *dev, + struct device_attribute *attr, + char *buf); + +static ssize_t aspeed_uart_routing_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count); + +#define ROUTING_ATTR(_name) { \ + .attr = {.name = _name, \ + .mode = VERIFY_OCTAL_PERMISSIONS(0644) }, \ + .show = aspeed_uart_routing_show, \ + .store = aspeed_uart_routing_store, \ +} + +/* routing selector for AST25xx */ +static struct aspeed_uart_routing_selector ast2500_io6_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO6), + .reg = HICR9, + .shift = 8, + .mask = 0xf, + .options = { + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO5, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart5_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART5), + .reg = HICRA, + .shift = 28, + .mask = 0xf, + .options = { + UART_ROUTING_IO5, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart4_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART4), + .reg = HICRA, + .shift = 25, + .mask = 0x7, + .options = { + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart3_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART3), + .reg = HICRA, + .shift = 22, + .mask = 0x7, + .options = { + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_UART4, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart2_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART2), + .reg = HICRA, + .shift = 19, + .mask = 0x7, + .options = { + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART1, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart1_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART1), + .reg = HICRA, + .shift = 16, + .mask = 0x7, + .options = { + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io5_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO5), + .reg = HICRA, + .shift = 12, + .mask = 0x7, + .options = { + UART_ROUTING_UART5, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_IO1, + UART_ROUTING_IO3, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io4_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO4), + .reg = HICRA, + .shift = 9, + .mask = 0x7, + .options = { + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io3_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO3), + .reg = HICRA, + .shift = 6, + .mask = 0x7, + .options = { + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io2_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO2), + .reg = HICRA, + .shift = 3, + .mask = 0x7, + .options = { + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_UART1, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io1_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO1), + .reg = HICRA, + .shift = 0, + .mask = 0x7, + .options = { + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct attribute *ast2500_uart_routing_attrs[] = { + &ast2500_io6_sel.dev_attr.attr, + &ast2500_uart5_sel.dev_attr.attr, + &ast2500_uart4_sel.dev_attr.attr, + &ast2500_uart3_sel.dev_attr.attr, + &ast2500_uart2_sel.dev_attr.attr, + &ast2500_uart1_sel.dev_attr.attr, + &ast2500_io5_sel.dev_attr.attr, + &ast2500_io4_sel.dev_attr.attr, + &ast2500_io3_sel.dev_attr.attr, + &ast2500_io2_sel.dev_attr.attr, + &ast2500_io1_sel.dev_attr.attr, + NULL, +}; + +static const struct attribute_group ast2500_uart_routing_attr_group = { + .attrs = ast2500_uart_routing_attrs, +}; + +/* routing selector for AST26xx */ +static struct aspeed_uart_routing_selector ast2600_uart10_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART10), + .reg = HICR9, + .shift = 12, + .mask = 0xf, + .options = { + UART_ROUTING_IO10, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_RES, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io10_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO10), + .reg = HICR9, + .shift = 8, + .mask = 0xf, + .options = { + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_RES, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_RES, + UART_ROUTING_UART10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_uart4_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART4), + .reg = HICRA, + .shift = 25, + .mask = 0x7, + .options = { + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_uart3_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART3), + .reg = HICRA, + .shift = 22, + .mask = 0x7, + .options = { + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_UART4, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_uart2_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART2), + .reg = HICRA, + .shift = 19, + .mask = 0x7, + .options = { + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART1, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_uart1_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART1), + .reg = HICRA, + .shift = 16, + .mask = 0x7, + .options = { + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io4_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO4), + .reg = HICRA, + .shift = 9, + .mask = 0x7, + .options = { + UART_ROUTING_UART4, + UART_ROUTING_UART10, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io3_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO3), + .reg = HICRA, + .shift = 6, + .mask = 0x7, + .options = { + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART10, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io2_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO2), + .reg = HICRA, + .shift = 3, + .mask = 0x7, + .options = { + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART10, + UART_ROUTING_UART1, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io1_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO1), + .reg = HICRA, + .shift = 0, + .mask = 0x7, + .options = { + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART10, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct attribute *ast2600_uart_routing_attrs[] = { + &ast2600_uart10_sel.dev_attr.attr, + &ast2600_io10_sel.dev_attr.attr, + &ast2600_uart4_sel.dev_attr.attr, + &ast2600_uart3_sel.dev_attr.attr, + &ast2600_uart2_sel.dev_attr.attr, + &ast2600_uart1_sel.dev_attr.attr, + &ast2600_io4_sel.dev_attr.attr, + &ast2600_io3_sel.dev_attr.attr, + &ast2600_io2_sel.dev_attr.attr, + &ast2600_io1_sel.dev_attr.attr, + NULL, +}; + +static const struct attribute_group ast2600_uart_routing_attr_group = { + .attrs = ast2600_uart_routing_attrs, +}; + +static ssize_t aspeed_uart_routing_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); + int val, pos, len; + + regmap_read(uart_routing->map, sel->reg, &val); + val = (val >> sel->shift) & sel->mask; + + len = 0; + for (pos = 0; sel->options[pos] != NULL; ++pos) { + if (pos == val) { + len += snprintf(buf + len, PAGE_SIZE - 1 - len, + "[%s] ", sel->options[pos]); + } else { + len += snprintf(buf + len, PAGE_SIZE - 1 - len, + "%s ", sel->options[pos]); + } + } + + if (val >= pos) { + len += snprintf(buf + len, PAGE_SIZE - 1 - len, + "[unknown(%d)]", val); + } + + len += snprintf(buf + len, PAGE_SIZE - 1 - len, "\n"); + + return len; +} + +static ssize_t aspeed_uart_routing_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + unsigned long flags; + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); + int val; + + val = match_string(sel->options, -1, buf); + if (val < 0) { + dev_err(dev, "invalid value \"%s\"\n", buf); + return -EINVAL; + } + + spin_lock_irqsave(&uart_routing->lock, flags); + + regmap_update_bits(uart_routing->map, sel->reg, + (sel->mask << sel->shift), + (val & sel->mask) << sel->shift); + + spin_unlock_irqrestore(&uart_routing->lock, flags); + + return count; +} + +static int aspeed_uart_routing_probe(struct platform_device *pdev) +{ + int rc; + struct device *dev = &pdev->dev; + struct aspeed_uart_routing *uart_routing; + + uart_routing = devm_kzalloc(&pdev->dev, + sizeof(*uart_routing), + GFP_KERNEL); + if (!uart_routing) { + dev_err(dev, "cannot allocate memory\n"); + return -ENOMEM; + } + + uart_routing->map = syscon_node_to_regmap(dev->parent->of_node); + if (IS_ERR(uart_routing->map)) { + dev_err(dev, "cannot get regmap\n"); + return PTR_ERR(uart_routing->map); + } + + uart_routing->attr_grp = of_device_get_match_data(dev); + + spin_lock_init(&uart_routing->lock); + + rc = sysfs_create_group(&dev->kobj, + uart_routing->attr_grp); + if (rc < 0) + return rc; + + dev_set_drvdata(dev, uart_routing); + + dev_info(dev, "module loaded\n"); + + return 0; +} + +static int aspeed_uart_routing_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct aspeed_uart_routing *uart_routing = platform_get_drvdata(pdev); + + sysfs_remove_group(&dev->kobj, + uart_routing->attr_grp); + + return 0; +} + +static const struct of_device_id aspeed_uart_routing_table[] = { + { .compatible = "aspeed,ast2500-lpc-uart-routing", + .data = &ast2500_uart_routing_attr_group }, + { .compatible = "aspeed,ast2600-lpc-uart-routing", + .data = &ast2600_uart_routing_attr_group }, + { }, +}; + +static struct platform_driver aspeed_uart_routing_driver = { + .driver = { + .name = "aspeed-lpc-uart-routing", + .of_match_table = aspeed_uart_routing_table, + }, + .probe = aspeed_uart_routing_probe, + .remove = aspeed_uart_routing_remove, +}; + +module_platform_driver(aspeed_uart_routing_driver); + +MODULE_AUTHOR("Oskar Senft <osk@google.com>"); +MODULE_AUTHOR("Chia-Wei Wang <chiawei_wang@aspeedtech.com>"); +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Driver to configure Aspeed UART routing"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1/2] soc: aspeed: Add LPC UART routing support @ 2021-09-01 6:22 ` Chia-Wei Wang 0 siblings, 0 replies; 18+ messages in thread From: Chia-Wei Wang @ 2021-09-01 6:22 UTC (permalink / raw) To: robh+dt, joel, andrew, osk, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, openbmc Add driver support for the LPC UART routing control. Users can perform runtime configuration of the RX muxes among the UART controllers and the UART TXD/RXD IO pins. This is achieved through the exported sysfs interface. Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> --- drivers/soc/aspeed/Kconfig | 11 + drivers/soc/aspeed/Makefile | 9 +- drivers/soc/aspeed/aspeed-lpc-uart-routing.c | 621 +++++++++++++++++++ 3 files changed, 637 insertions(+), 4 deletions(-) create mode 100644 drivers/soc/aspeed/aspeed-lpc-uart-routing.c diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig index 243ca196e6ad..6866757e6be2 100644 --- a/drivers/soc/aspeed/Kconfig +++ b/drivers/soc/aspeed/Kconfig @@ -24,6 +24,17 @@ config ASPEED_LPC_SNOOP allows the BMC to listen on and save the data written by the host to an arbitrary LPC I/O port. +config ASPEED_LPC_UART_ROUTING + tristate "ASPEED LPC uart routing control" + select REGMAP + select MFD_SYSCON + default ARCH_ASPEED + help + Provides a driver to control the LPC UART routing paths. + An sysfs interface is exported allowing users to perform + runtime configuration of the RX muxes between the UART + controllers and I/O pins. + config ASPEED_P2A_CTRL tristate "ASPEED P2A (VGA MMIO to BMC) bridge control" select REGMAP diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile index fcab7192e1a4..87ad6578ddbb 100644 --- a/drivers/soc/aspeed/Makefile +++ b/drivers/soc/aspeed/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only -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_SOCINFO) += aspeed-socinfo.o +obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o +obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o +obj-$(CONFIG_ASPEED_LPC_UART_ROUTING) += aspeed-lpc-uart-routing.o +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o +obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o diff --git a/drivers/soc/aspeed/aspeed-lpc-uart-routing.c b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c new file mode 100644 index 000000000000..141d0d08b4ff --- /dev/null +++ b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c @@ -0,0 +1,621 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2018 Google LLC + * Copyright (c) 2021 Aspeed Technology Inc. + */ +#include <linux/device.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> +#include <linux/platform_device.h> + +/* register offsets */ +#define HICR9 0x98 +#define HICRA 0x9c + +/* attributes options */ +#define UART_ROUTING_IO1 "io1" +#define UART_ROUTING_IO2 "io2" +#define UART_ROUTING_IO3 "io3" +#define UART_ROUTING_IO4 "io4" +#define UART_ROUTING_IO5 "io5" +#define UART_ROUTING_IO6 "io6" +#define UART_ROUTING_IO10 "io10" +#define UART_ROUTING_UART1 "uart1" +#define UART_ROUTING_UART2 "uart2" +#define UART_ROUTING_UART3 "uart3" +#define UART_ROUTING_UART4 "uart4" +#define UART_ROUTING_UART5 "uart5" +#define UART_ROUTING_UART6 "uart6" +#define UART_ROUTING_UART10 "uart10" +#define UART_ROUTING_RES "reserved" + +struct aspeed_uart_routing { + struct regmap *map; + spinlock_t lock; + struct attribute_group const *attr_grp; +}; + +struct aspeed_uart_routing_selector { + struct device_attribute dev_attr; + uint32_t reg; + uint32_t mask; + uint32_t shift; + const char *const options[]; +}; + +#define to_routing_selector(_dev_attr) \ + container_of(_dev_attr, struct aspeed_uart_routing_selector, dev_attr) + +static ssize_t aspeed_uart_routing_show(struct device *dev, + struct device_attribute *attr, + char *buf); + +static ssize_t aspeed_uart_routing_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count); + +#define ROUTING_ATTR(_name) { \ + .attr = {.name = _name, \ + .mode = VERIFY_OCTAL_PERMISSIONS(0644) }, \ + .show = aspeed_uart_routing_show, \ + .store = aspeed_uart_routing_store, \ +} + +/* routing selector for AST25xx */ +static struct aspeed_uart_routing_selector ast2500_io6_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO6), + .reg = HICR9, + .shift = 8, + .mask = 0xf, + .options = { + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO5, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart5_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART5), + .reg = HICRA, + .shift = 28, + .mask = 0xf, + .options = { + UART_ROUTING_IO5, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart4_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART4), + .reg = HICRA, + .shift = 25, + .mask = 0x7, + .options = { + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart3_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART3), + .reg = HICRA, + .shift = 22, + .mask = 0x7, + .options = { + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_UART4, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart2_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART2), + .reg = HICRA, + .shift = 19, + .mask = 0x7, + .options = { + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART1, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_uart1_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART1), + .reg = HICRA, + .shift = 16, + .mask = 0x7, + .options = { + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io5_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO5), + .reg = HICRA, + .shift = 12, + .mask = 0x7, + .options = { + UART_ROUTING_UART5, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_IO1, + UART_ROUTING_IO3, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io4_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO4), + .reg = HICRA, + .shift = 9, + .mask = 0x7, + .options = { + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io3_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO3), + .reg = HICRA, + .shift = 6, + .mask = 0x7, + .options = { + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io2_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO2), + .reg = HICRA, + .shift = 3, + .mask = 0x7, + .options = { + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_UART1, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2500_io1_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO1), + .reg = HICRA, + .shift = 0, + .mask = 0x7, + .options = { + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART5, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO6, + NULL, + }, +}; + +static struct attribute *ast2500_uart_routing_attrs[] = { + &ast2500_io6_sel.dev_attr.attr, + &ast2500_uart5_sel.dev_attr.attr, + &ast2500_uart4_sel.dev_attr.attr, + &ast2500_uart3_sel.dev_attr.attr, + &ast2500_uart2_sel.dev_attr.attr, + &ast2500_uart1_sel.dev_attr.attr, + &ast2500_io5_sel.dev_attr.attr, + &ast2500_io4_sel.dev_attr.attr, + &ast2500_io3_sel.dev_attr.attr, + &ast2500_io2_sel.dev_attr.attr, + &ast2500_io1_sel.dev_attr.attr, + NULL, +}; + +static const struct attribute_group ast2500_uart_routing_attr_group = { + .attrs = ast2500_uart_routing_attrs, +}; + +/* routing selector for AST26xx */ +static struct aspeed_uart_routing_selector ast2600_uart10_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART10), + .reg = HICR9, + .shift = 12, + .mask = 0xf, + .options = { + UART_ROUTING_IO10, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_RES, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io10_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO10), + .reg = HICR9, + .shift = 8, + .mask = 0xf, + .options = { + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_RES, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_RES, + UART_ROUTING_UART10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_uart4_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART4), + .reg = HICRA, + .shift = 25, + .mask = 0x7, + .options = { + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_uart3_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART3), + .reg = HICRA, + .shift = 22, + .mask = 0x7, + .options = { + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_UART4, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_uart2_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART2), + .reg = HICRA, + .shift = 19, + .mask = 0x7, + .options = { + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO1, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART1, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_uart1_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_UART1), + .reg = HICRA, + .shift = 16, + .mask = 0x7, + .options = { + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io4_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO4), + .reg = HICRA, + .shift = 9, + .mask = 0x7, + .options = { + UART_ROUTING_UART4, + UART_ROUTING_UART10, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io3_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO3), + .reg = HICRA, + .shift = 6, + .mask = 0x7, + .options = { + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART10, + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_IO1, + UART_ROUTING_IO2, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io2_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO2), + .reg = HICRA, + .shift = 3, + .mask = 0x7, + .options = { + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART10, + UART_ROUTING_UART1, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct aspeed_uart_routing_selector ast2600_io1_sel = { + .dev_attr = ROUTING_ATTR(UART_ROUTING_IO1), + .reg = HICRA, + .shift = 0, + .mask = 0x7, + .options = { + UART_ROUTING_UART1, + UART_ROUTING_UART2, + UART_ROUTING_UART3, + UART_ROUTING_UART4, + UART_ROUTING_UART10, + UART_ROUTING_IO3, + UART_ROUTING_IO4, + UART_ROUTING_IO10, + NULL, + }, +}; + +static struct attribute *ast2600_uart_routing_attrs[] = { + &ast2600_uart10_sel.dev_attr.attr, + &ast2600_io10_sel.dev_attr.attr, + &ast2600_uart4_sel.dev_attr.attr, + &ast2600_uart3_sel.dev_attr.attr, + &ast2600_uart2_sel.dev_attr.attr, + &ast2600_uart1_sel.dev_attr.attr, + &ast2600_io4_sel.dev_attr.attr, + &ast2600_io3_sel.dev_attr.attr, + &ast2600_io2_sel.dev_attr.attr, + &ast2600_io1_sel.dev_attr.attr, + NULL, +}; + +static const struct attribute_group ast2600_uart_routing_attr_group = { + .attrs = ast2600_uart_routing_attrs, +}; + +static ssize_t aspeed_uart_routing_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); + int val, pos, len; + + regmap_read(uart_routing->map, sel->reg, &val); + val = (val >> sel->shift) & sel->mask; + + len = 0; + for (pos = 0; sel->options[pos] != NULL; ++pos) { + if (pos == val) { + len += snprintf(buf + len, PAGE_SIZE - 1 - len, + "[%s] ", sel->options[pos]); + } else { + len += snprintf(buf + len, PAGE_SIZE - 1 - len, + "%s ", sel->options[pos]); + } + } + + if (val >= pos) { + len += snprintf(buf + len, PAGE_SIZE - 1 - len, + "[unknown(%d)]", val); + } + + len += snprintf(buf + len, PAGE_SIZE - 1 - len, "\n"); + + return len; +} + +static ssize_t aspeed_uart_routing_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + unsigned long flags; + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); + int val; + + val = match_string(sel->options, -1, buf); + if (val < 0) { + dev_err(dev, "invalid value \"%s\"\n", buf); + return -EINVAL; + } + + spin_lock_irqsave(&uart_routing->lock, flags); + + regmap_update_bits(uart_routing->map, sel->reg, + (sel->mask << sel->shift), + (val & sel->mask) << sel->shift); + + spin_unlock_irqrestore(&uart_routing->lock, flags); + + return count; +} + +static int aspeed_uart_routing_probe(struct platform_device *pdev) +{ + int rc; + struct device *dev = &pdev->dev; + struct aspeed_uart_routing *uart_routing; + + uart_routing = devm_kzalloc(&pdev->dev, + sizeof(*uart_routing), + GFP_KERNEL); + if (!uart_routing) { + dev_err(dev, "cannot allocate memory\n"); + return -ENOMEM; + } + + uart_routing->map = syscon_node_to_regmap(dev->parent->of_node); + if (IS_ERR(uart_routing->map)) { + dev_err(dev, "cannot get regmap\n"); + return PTR_ERR(uart_routing->map); + } + + uart_routing->attr_grp = of_device_get_match_data(dev); + + spin_lock_init(&uart_routing->lock); + + rc = sysfs_create_group(&dev->kobj, + uart_routing->attr_grp); + if (rc < 0) + return rc; + + dev_set_drvdata(dev, uart_routing); + + dev_info(dev, "module loaded\n"); + + return 0; +} + +static int aspeed_uart_routing_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct aspeed_uart_routing *uart_routing = platform_get_drvdata(pdev); + + sysfs_remove_group(&dev->kobj, + uart_routing->attr_grp); + + return 0; +} + +static const struct of_device_id aspeed_uart_routing_table[] = { + { .compatible = "aspeed,ast2500-lpc-uart-routing", + .data = &ast2500_uart_routing_attr_group }, + { .compatible = "aspeed,ast2600-lpc-uart-routing", + .data = &ast2600_uart_routing_attr_group }, + { }, +}; + +static struct platform_driver aspeed_uart_routing_driver = { + .driver = { + .name = "aspeed-lpc-uart-routing", + .of_match_table = aspeed_uart_routing_table, + }, + .probe = aspeed_uart_routing_probe, + .remove = aspeed_uart_routing_remove, +}; + +module_platform_driver(aspeed_uart_routing_driver); + +MODULE_AUTHOR("Oskar Senft <osk@google.com>"); +MODULE_AUTHOR("Chia-Wei Wang <chiawei_wang@aspeedtech.com>"); +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Driver to configure Aspeed UART routing"); -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] soc: aspeed: Add LPC UART routing support 2021-09-01 6:22 ` Chia-Wei Wang (?) @ 2021-09-01 7:36 ` Joel Stanley -1 siblings, 0 replies; 18+ messages in thread From: Joel Stanley @ 2021-09-01 7:36 UTC (permalink / raw) To: Chia-Wei Wang Cc: Rob Herring, Andrew Jeffery, Oskar Senft, devicetree, Linux ARM, linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote: > > Add driver support for the LPC UART routing control. Users can perform As we discussed, remove the "LPC" part of the name. > runtime configuration of the RX muxes among the UART controllers and the > UART TXD/RXD IO pins. This is achieved through the exported sysfs interface. > > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> It would be good to have some example of how to use it, and the output from sysfs. You should also add a patch to document the sysfs files in Documentation/ABI. > +++ b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c > @@ -0,0 +1,621 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2018 Google LLC > + * Copyright (c) 2021 Aspeed Technology Inc. > + */ > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/of_address.h> You can drop this one. > +#include <linux/of_platform.h> > +#include <linux/mfd/syscon.h> > +#include <linux/regmap.h> > +#include <linux/platform_device.h> > + > +/* register offsets */ > +#define HICR9 0x98 > +#define HICRA 0x9c > + > +/* attributes options */ > +#define UART_ROUTING_IO1 "io1" > +#define UART_ROUTING_IO2 "io2" > +#define UART_ROUTING_IO3 "io3" > +#define UART_ROUTING_IO4 "io4" > +#define UART_ROUTING_IO5 "io5" > +#define UART_ROUTING_IO6 "io6" > +#define UART_ROUTING_IO10 "io10" > +#define UART_ROUTING_UART1 "uart1" > +#define UART_ROUTING_UART2 "uart2" > +#define UART_ROUTING_UART3 "uart3" > +#define UART_ROUTING_UART4 "uart4" > +#define UART_ROUTING_UART5 "uart5" > +#define UART_ROUTING_UART6 "uart6" > +#define UART_ROUTING_UART10 "uart10" > +#define UART_ROUTING_RES "reserved" > + > +struct aspeed_uart_routing { > + struct regmap *map; > + spinlock_t lock; > + struct attribute_group const *attr_grp; > +}; > + > +struct aspeed_uart_routing_selector { > + struct device_attribute dev_attr; > + uint32_t reg; > + uint32_t mask; > + uint32_t shift; These can be u8. > +static ssize_t aspeed_uart_routing_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); > + int val, pos, len; > + > + regmap_read(uart_routing->map, sel->reg, &val); > + val = (val >> sel->shift) & sel->mask; > + > + len = 0; > + for (pos = 0; sel->options[pos] != NULL; ++pos) { > + if (pos == val) { > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > + "[%s] ", sel->options[pos]); > + } else { > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > + "%s ", sel->options[pos]); The kernel prefers sysfs_emit and sysfs_emit_at insteading of using snprintf directly. > + } > + } > + > + if (val >= pos) { > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > + "[unknown(%d)]", val); > + } > + > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, "\n"); > + > + return len; > +} > + > +static ssize_t aspeed_uart_routing_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + unsigned long flags; > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); > + int val; > + > + val = match_string(sel->options, -1, buf); > + if (val < 0) { > + dev_err(dev, "invalid value \"%s\"\n", buf); > + return -EINVAL; > + } > + > + spin_lock_irqsave(&uart_routing->lock, flags); I can't see why you would need a spinlock here. The regmap has it's own locking so it will protect against concurrent updates to the registers. > + > + regmap_update_bits(uart_routing->map, sel->reg, > + (sel->mask << sel->shift), > + (val & sel->mask) << sel->shift); > + > + spin_unlock_irqrestore(&uart_routing->lock, flags); > + > + return count; > +} > + > +static int aspeed_uart_routing_probe(struct platform_device *pdev) > +{ > + int rc; > + struct device *dev = &pdev->dev; > + struct aspeed_uart_routing *uart_routing; > + > + uart_routing = devm_kzalloc(&pdev->dev, > + sizeof(*uart_routing), > + GFP_KERNEL); You can reformat this file to have longer lines; the kernel is ok with up to 100 columsn these days. > + if (!uart_routing) { > + dev_err(dev, "cannot allocate memory\n"); I'd drop this error message. > + return -ENOMEM; > + } > + > + uart_routing->map = syscon_node_to_regmap(dev->parent->of_node); > + if (IS_ERR(uart_routing->map)) { > + dev_err(dev, "cannot get regmap\n"); > + return PTR_ERR(uart_routing->map); > + } > + > + uart_routing->attr_grp = of_device_get_match_data(dev); > + > + spin_lock_init(&uart_routing->lock); I don't think you need the lock at all. Cheers, Joel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] soc: aspeed: Add LPC UART routing support @ 2021-09-01 7:36 ` Joel Stanley 0 siblings, 0 replies; 18+ messages in thread From: Joel Stanley @ 2021-09-01 7:36 UTC (permalink / raw) To: Chia-Wei Wang Cc: Rob Herring, Andrew Jeffery, Oskar Senft, devicetree, Linux ARM, linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote: > > Add driver support for the LPC UART routing control. Users can perform As we discussed, remove the "LPC" part of the name. > runtime configuration of the RX muxes among the UART controllers and the > UART TXD/RXD IO pins. This is achieved through the exported sysfs interface. > > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> It would be good to have some example of how to use it, and the output from sysfs. You should also add a patch to document the sysfs files in Documentation/ABI. > +++ b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c > @@ -0,0 +1,621 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2018 Google LLC > + * Copyright (c) 2021 Aspeed Technology Inc. > + */ > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/of_address.h> You can drop this one. > +#include <linux/of_platform.h> > +#include <linux/mfd/syscon.h> > +#include <linux/regmap.h> > +#include <linux/platform_device.h> > + > +/* register offsets */ > +#define HICR9 0x98 > +#define HICRA 0x9c > + > +/* attributes options */ > +#define UART_ROUTING_IO1 "io1" > +#define UART_ROUTING_IO2 "io2" > +#define UART_ROUTING_IO3 "io3" > +#define UART_ROUTING_IO4 "io4" > +#define UART_ROUTING_IO5 "io5" > +#define UART_ROUTING_IO6 "io6" > +#define UART_ROUTING_IO10 "io10" > +#define UART_ROUTING_UART1 "uart1" > +#define UART_ROUTING_UART2 "uart2" > +#define UART_ROUTING_UART3 "uart3" > +#define UART_ROUTING_UART4 "uart4" > +#define UART_ROUTING_UART5 "uart5" > +#define UART_ROUTING_UART6 "uart6" > +#define UART_ROUTING_UART10 "uart10" > +#define UART_ROUTING_RES "reserved" > + > +struct aspeed_uart_routing { > + struct regmap *map; > + spinlock_t lock; > + struct attribute_group const *attr_grp; > +}; > + > +struct aspeed_uart_routing_selector { > + struct device_attribute dev_attr; > + uint32_t reg; > + uint32_t mask; > + uint32_t shift; These can be u8. > +static ssize_t aspeed_uart_routing_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); > + int val, pos, len; > + > + regmap_read(uart_routing->map, sel->reg, &val); > + val = (val >> sel->shift) & sel->mask; > + > + len = 0; > + for (pos = 0; sel->options[pos] != NULL; ++pos) { > + if (pos == val) { > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > + "[%s] ", sel->options[pos]); > + } else { > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > + "%s ", sel->options[pos]); The kernel prefers sysfs_emit and sysfs_emit_at insteading of using snprintf directly. > + } > + } > + > + if (val >= pos) { > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > + "[unknown(%d)]", val); > + } > + > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, "\n"); > + > + return len; > +} > + > +static ssize_t aspeed_uart_routing_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + unsigned long flags; > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); > + int val; > + > + val = match_string(sel->options, -1, buf); > + if (val < 0) { > + dev_err(dev, "invalid value \"%s\"\n", buf); > + return -EINVAL; > + } > + > + spin_lock_irqsave(&uart_routing->lock, flags); I can't see why you would need a spinlock here. The regmap has it's own locking so it will protect against concurrent updates to the registers. > + > + regmap_update_bits(uart_routing->map, sel->reg, > + (sel->mask << sel->shift), > + (val & sel->mask) << sel->shift); > + > + spin_unlock_irqrestore(&uart_routing->lock, flags); > + > + return count; > +} > + > +static int aspeed_uart_routing_probe(struct platform_device *pdev) > +{ > + int rc; > + struct device *dev = &pdev->dev; > + struct aspeed_uart_routing *uart_routing; > + > + uart_routing = devm_kzalloc(&pdev->dev, > + sizeof(*uart_routing), > + GFP_KERNEL); You can reformat this file to have longer lines; the kernel is ok with up to 100 columsn these days. > + if (!uart_routing) { > + dev_err(dev, "cannot allocate memory\n"); I'd drop this error message. > + return -ENOMEM; > + } > + > + uart_routing->map = syscon_node_to_regmap(dev->parent->of_node); > + if (IS_ERR(uart_routing->map)) { > + dev_err(dev, "cannot get regmap\n"); > + return PTR_ERR(uart_routing->map); > + } > + > + uart_routing->attr_grp = of_device_get_match_data(dev); > + > + spin_lock_init(&uart_routing->lock); I don't think you need the lock at all. Cheers, Joel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] soc: aspeed: Add LPC UART routing support @ 2021-09-01 7:36 ` Joel Stanley 0 siblings, 0 replies; 18+ messages in thread From: Joel Stanley @ 2021-09-01 7:36 UTC (permalink / raw) To: Chia-Wei Wang Cc: devicetree, linux-aspeed, Andrew Jeffery, OpenBMC Maillist, Linux Kernel Mailing List, Rob Herring, Oskar Senft, Linux ARM On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote: > > Add driver support for the LPC UART routing control. Users can perform As we discussed, remove the "LPC" part of the name. > runtime configuration of the RX muxes among the UART controllers and the > UART TXD/RXD IO pins. This is achieved through the exported sysfs interface. > > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> It would be good to have some example of how to use it, and the output from sysfs. You should also add a patch to document the sysfs files in Documentation/ABI. > +++ b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c > @@ -0,0 +1,621 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2018 Google LLC > + * Copyright (c) 2021 Aspeed Technology Inc. > + */ > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/of_address.h> You can drop this one. > +#include <linux/of_platform.h> > +#include <linux/mfd/syscon.h> > +#include <linux/regmap.h> > +#include <linux/platform_device.h> > + > +/* register offsets */ > +#define HICR9 0x98 > +#define HICRA 0x9c > + > +/* attributes options */ > +#define UART_ROUTING_IO1 "io1" > +#define UART_ROUTING_IO2 "io2" > +#define UART_ROUTING_IO3 "io3" > +#define UART_ROUTING_IO4 "io4" > +#define UART_ROUTING_IO5 "io5" > +#define UART_ROUTING_IO6 "io6" > +#define UART_ROUTING_IO10 "io10" > +#define UART_ROUTING_UART1 "uart1" > +#define UART_ROUTING_UART2 "uart2" > +#define UART_ROUTING_UART3 "uart3" > +#define UART_ROUTING_UART4 "uart4" > +#define UART_ROUTING_UART5 "uart5" > +#define UART_ROUTING_UART6 "uart6" > +#define UART_ROUTING_UART10 "uart10" > +#define UART_ROUTING_RES "reserved" > + > +struct aspeed_uart_routing { > + struct regmap *map; > + spinlock_t lock; > + struct attribute_group const *attr_grp; > +}; > + > +struct aspeed_uart_routing_selector { > + struct device_attribute dev_attr; > + uint32_t reg; > + uint32_t mask; > + uint32_t shift; These can be u8. > +static ssize_t aspeed_uart_routing_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); > + int val, pos, len; > + > + regmap_read(uart_routing->map, sel->reg, &val); > + val = (val >> sel->shift) & sel->mask; > + > + len = 0; > + for (pos = 0; sel->options[pos] != NULL; ++pos) { > + if (pos == val) { > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > + "[%s] ", sel->options[pos]); > + } else { > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > + "%s ", sel->options[pos]); The kernel prefers sysfs_emit and sysfs_emit_at insteading of using snprintf directly. > + } > + } > + > + if (val >= pos) { > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > + "[unknown(%d)]", val); > + } > + > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, "\n"); > + > + return len; > +} > + > +static ssize_t aspeed_uart_routing_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + unsigned long flags; > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > + struct aspeed_uart_routing_selector *sel = to_routing_selector(attr); > + int val; > + > + val = match_string(sel->options, -1, buf); > + if (val < 0) { > + dev_err(dev, "invalid value \"%s\"\n", buf); > + return -EINVAL; > + } > + > + spin_lock_irqsave(&uart_routing->lock, flags); I can't see why you would need a spinlock here. The regmap has it's own locking so it will protect against concurrent updates to the registers. > + > + regmap_update_bits(uart_routing->map, sel->reg, > + (sel->mask << sel->shift), > + (val & sel->mask) << sel->shift); > + > + spin_unlock_irqrestore(&uart_routing->lock, flags); > + > + return count; > +} > + > +static int aspeed_uart_routing_probe(struct platform_device *pdev) > +{ > + int rc; > + struct device *dev = &pdev->dev; > + struct aspeed_uart_routing *uart_routing; > + > + uart_routing = devm_kzalloc(&pdev->dev, > + sizeof(*uart_routing), > + GFP_KERNEL); You can reformat this file to have longer lines; the kernel is ok with up to 100 columsn these days. > + if (!uart_routing) { > + dev_err(dev, "cannot allocate memory\n"); I'd drop this error message. > + return -ENOMEM; > + } > + > + uart_routing->map = syscon_node_to_regmap(dev->parent->of_node); > + if (IS_ERR(uart_routing->map)) { > + dev_err(dev, "cannot get regmap\n"); > + return PTR_ERR(uart_routing->map); > + } > + > + uart_routing->attr_grp = of_device_get_match_data(dev); > + > + spin_lock_init(&uart_routing->lock); I don't think you need the lock at all. Cheers, Joel ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 1/2] soc: aspeed: Add LPC UART routing support 2021-09-01 7:36 ` Joel Stanley (?) @ 2021-09-01 9:43 ` ChiaWei Wang -1 siblings, 0 replies; 18+ messages in thread From: ChiaWei Wang @ 2021-09-01 9:43 UTC (permalink / raw) To: Joel Stanley Cc: Rob Herring, Andrew Jeffery, Oskar Senft, devicetree, Linux ARM, linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist > -----Original Message----- > From: Joel Stanley <joel@jms.id.au> > Sent: Wednesday, September 1, 2021 3:37 PM > > On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang > <chiawei_wang@aspeedtech.com> wrote: > > > > Add driver support for the LPC UART routing control. Users can perform > > As we discussed, remove the "LPC" part of the name. > > > runtime configuration of the RX muxes among the UART controllers and > > the UART TXD/RXD IO pins. This is achieved through the exported sysfs > interface. > > > > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> > > It would be good to have some example of how to use it, and the output from > sysfs. > > You should also add a patch to document the sysfs files in Documentation/ABI. > Will add a new commit for the sysfs description. > > +++ b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c > > @@ -0,0 +1,621 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2018 Google LLC > > + * Copyright (c) 2021 Aspeed Technology Inc. > > + */ > > +#include <linux/device.h> > > +#include <linux/module.h> > > +#include <linux/of_device.h> > > +#include <linux/of_address.h> > > You can drop this one. Revised as suggested. > > > +#include <linux/of_platform.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/regmap.h> > > +#include <linux/platform_device.h> > > + > > +/* register offsets */ > > +#define HICR9 0x98 > > +#define HICRA 0x9c > > + > > +/* attributes options */ > > +#define UART_ROUTING_IO1 "io1" > > +#define UART_ROUTING_IO2 "io2" > > +#define UART_ROUTING_IO3 "io3" > > +#define UART_ROUTING_IO4 "io4" > > +#define UART_ROUTING_IO5 "io5" > > +#define UART_ROUTING_IO6 "io6" > > +#define UART_ROUTING_IO10 "io10" > > +#define UART_ROUTING_UART1 "uart1" > > +#define UART_ROUTING_UART2 "uart2" > > +#define UART_ROUTING_UART3 "uart3" > > +#define UART_ROUTING_UART4 "uart4" > > +#define UART_ROUTING_UART5 "uart5" > > +#define UART_ROUTING_UART6 "uart6" > > +#define UART_ROUTING_UART10 "uart10" > > +#define UART_ROUTING_RES "reserved" > > + > > +struct aspeed_uart_routing { > > + struct regmap *map; > > + spinlock_t lock; > > + struct attribute_group const *attr_grp; }; > > + > > +struct aspeed_uart_routing_selector { > > + struct device_attribute dev_attr; > > + uint32_t reg; > > + uint32_t mask; > > + uint32_t shift; > > These can be u8. Revised as suggested. > > > +static ssize_t aspeed_uart_routing_show(struct device *dev, > > + struct device_attribute > *attr, > > + char *buf) { > > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > > + struct aspeed_uart_routing_selector *sel = > to_routing_selector(attr); > > + int val, pos, len; > > + > > + regmap_read(uart_routing->map, sel->reg, &val); > > + val = (val >> sel->shift) & sel->mask; > > + > > + len = 0; > > + for (pos = 0; sel->options[pos] != NULL; ++pos) { > > + if (pos == val) { > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > > + "[%s] ", sel->options[pos]); > > + } else { > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > > + "%s ", sel->options[pos]); > > The kernel prefers sysfs_emit and sysfs_emit_at insteading of using snprintf > directly. Revised as suggested. > > > + } > > + } > > + > > + if (val >= pos) { > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > > + "[unknown(%d)]", val); > > + } > > + > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, "\n"); > > + > > + return len; > > +} > > + > > +static ssize_t aspeed_uart_routing_store(struct device *dev, > > + struct device_attribute > *attr, > > + const char *buf, size_t > > +count) { > > + unsigned long flags; > > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > > + struct aspeed_uart_routing_selector *sel = > to_routing_selector(attr); > > + int val; > > + > > + val = match_string(sel->options, -1, buf); > > + if (val < 0) { > > + dev_err(dev, "invalid value \"%s\"\n", buf); > > + return -EINVAL; > > + } > > + > > + spin_lock_irqsave(&uart_routing->lock, flags); > > I can't see why you would need a spinlock here. The regmap has it's own > locking so it will protect against concurrent updates to the registers. You are right. Lock is not needed here. Will removed it as suggested. > > > + > > + regmap_update_bits(uart_routing->map, sel->reg, > > + (sel->mask << sel->shift), > > + (val & sel->mask) << sel->shift); > > + > > + spin_unlock_irqrestore(&uart_routing->lock, flags); > > + > > + return count; > > +} > > + > > +static int aspeed_uart_routing_probe(struct platform_device *pdev) { > > + int rc; > > + struct device *dev = &pdev->dev; > > + struct aspeed_uart_routing *uart_routing; > > + > > + uart_routing = devm_kzalloc(&pdev->dev, > > + sizeof(*uart_routing), > > + GFP_KERNEL); > > You can reformat this file to have longer lines; the kernel is ok with up to 100 > columsn these days. > > > + if (!uart_routing) { > > + dev_err(dev, "cannot allocate memory\n"); > > I'd drop this error message. Revised as suggested > > > + return -ENOMEM; > > + } > > + > > + uart_routing->map = > syscon_node_to_regmap(dev->parent->of_node); > > + if (IS_ERR(uart_routing->map)) { > > + dev_err(dev, "cannot get regmap\n"); > > + return PTR_ERR(uart_routing->map); > > + } > > + > > + uart_routing->attr_grp = of_device_get_match_data(dev); > > + > > + spin_lock_init(&uart_routing->lock); > > I don't think you need the lock at all. Same as above. Thanks for reviewing this. The v2 patch will include the driver refactoring and additional documentation. Chiawei ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 1/2] soc: aspeed: Add LPC UART routing support @ 2021-09-01 9:43 ` ChiaWei Wang 0 siblings, 0 replies; 18+ messages in thread From: ChiaWei Wang @ 2021-09-01 9:43 UTC (permalink / raw) To: Joel Stanley Cc: Rob Herring, Andrew Jeffery, Oskar Senft, devicetree, Linux ARM, linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist > -----Original Message----- > From: Joel Stanley <joel@jms.id.au> > Sent: Wednesday, September 1, 2021 3:37 PM > > On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang > <chiawei_wang@aspeedtech.com> wrote: > > > > Add driver support for the LPC UART routing control. Users can perform > > As we discussed, remove the "LPC" part of the name. > > > runtime configuration of the RX muxes among the UART controllers and > > the UART TXD/RXD IO pins. This is achieved through the exported sysfs > interface. > > > > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> > > It would be good to have some example of how to use it, and the output from > sysfs. > > You should also add a patch to document the sysfs files in Documentation/ABI. > Will add a new commit for the sysfs description. > > +++ b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c > > @@ -0,0 +1,621 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2018 Google LLC > > + * Copyright (c) 2021 Aspeed Technology Inc. > > + */ > > +#include <linux/device.h> > > +#include <linux/module.h> > > +#include <linux/of_device.h> > > +#include <linux/of_address.h> > > You can drop this one. Revised as suggested. > > > +#include <linux/of_platform.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/regmap.h> > > +#include <linux/platform_device.h> > > + > > +/* register offsets */ > > +#define HICR9 0x98 > > +#define HICRA 0x9c > > + > > +/* attributes options */ > > +#define UART_ROUTING_IO1 "io1" > > +#define UART_ROUTING_IO2 "io2" > > +#define UART_ROUTING_IO3 "io3" > > +#define UART_ROUTING_IO4 "io4" > > +#define UART_ROUTING_IO5 "io5" > > +#define UART_ROUTING_IO6 "io6" > > +#define UART_ROUTING_IO10 "io10" > > +#define UART_ROUTING_UART1 "uart1" > > +#define UART_ROUTING_UART2 "uart2" > > +#define UART_ROUTING_UART3 "uart3" > > +#define UART_ROUTING_UART4 "uart4" > > +#define UART_ROUTING_UART5 "uart5" > > +#define UART_ROUTING_UART6 "uart6" > > +#define UART_ROUTING_UART10 "uart10" > > +#define UART_ROUTING_RES "reserved" > > + > > +struct aspeed_uart_routing { > > + struct regmap *map; > > + spinlock_t lock; > > + struct attribute_group const *attr_grp; }; > > + > > +struct aspeed_uart_routing_selector { > > + struct device_attribute dev_attr; > > + uint32_t reg; > > + uint32_t mask; > > + uint32_t shift; > > These can be u8. Revised as suggested. > > > +static ssize_t aspeed_uart_routing_show(struct device *dev, > > + struct device_attribute > *attr, > > + char *buf) { > > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > > + struct aspeed_uart_routing_selector *sel = > to_routing_selector(attr); > > + int val, pos, len; > > + > > + regmap_read(uart_routing->map, sel->reg, &val); > > + val = (val >> sel->shift) & sel->mask; > > + > > + len = 0; > > + for (pos = 0; sel->options[pos] != NULL; ++pos) { > > + if (pos == val) { > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > > + "[%s] ", sel->options[pos]); > > + } else { > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > > + "%s ", sel->options[pos]); > > The kernel prefers sysfs_emit and sysfs_emit_at insteading of using snprintf > directly. Revised as suggested. > > > + } > > + } > > + > > + if (val >= pos) { > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > > + "[unknown(%d)]", val); > > + } > > + > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, "\n"); > > + > > + return len; > > +} > > + > > +static ssize_t aspeed_uart_routing_store(struct device *dev, > > + struct device_attribute > *attr, > > + const char *buf, size_t > > +count) { > > + unsigned long flags; > > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > > + struct aspeed_uart_routing_selector *sel = > to_routing_selector(attr); > > + int val; > > + > > + val = match_string(sel->options, -1, buf); > > + if (val < 0) { > > + dev_err(dev, "invalid value \"%s\"\n", buf); > > + return -EINVAL; > > + } > > + > > + spin_lock_irqsave(&uart_routing->lock, flags); > > I can't see why you would need a spinlock here. The regmap has it's own > locking so it will protect against concurrent updates to the registers. You are right. Lock is not needed here. Will removed it as suggested. > > > + > > + regmap_update_bits(uart_routing->map, sel->reg, > > + (sel->mask << sel->shift), > > + (val & sel->mask) << sel->shift); > > + > > + spin_unlock_irqrestore(&uart_routing->lock, flags); > > + > > + return count; > > +} > > + > > +static int aspeed_uart_routing_probe(struct platform_device *pdev) { > > + int rc; > > + struct device *dev = &pdev->dev; > > + struct aspeed_uart_routing *uart_routing; > > + > > + uart_routing = devm_kzalloc(&pdev->dev, > > + sizeof(*uart_routing), > > + GFP_KERNEL); > > You can reformat this file to have longer lines; the kernel is ok with up to 100 > columsn these days. > > > + if (!uart_routing) { > > + dev_err(dev, "cannot allocate memory\n"); > > I'd drop this error message. Revised as suggested > > > + return -ENOMEM; > > + } > > + > > + uart_routing->map = > syscon_node_to_regmap(dev->parent->of_node); > > + if (IS_ERR(uart_routing->map)) { > > + dev_err(dev, "cannot get regmap\n"); > > + return PTR_ERR(uart_routing->map); > > + } > > + > > + uart_routing->attr_grp = of_device_get_match_data(dev); > > + > > + spin_lock_init(&uart_routing->lock); > > I don't think you need the lock at all. Same as above. Thanks for reviewing this. The v2 patch will include the driver refactoring and additional documentation. Chiawei _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 1/2] soc: aspeed: Add LPC UART routing support @ 2021-09-01 9:43 ` ChiaWei Wang 0 siblings, 0 replies; 18+ messages in thread From: ChiaWei Wang @ 2021-09-01 9:43 UTC (permalink / raw) To: Joel Stanley Cc: devicetree, linux-aspeed, Andrew Jeffery, OpenBMC Maillist, Linux Kernel Mailing List, Rob Herring, Oskar Senft, Linux ARM > -----Original Message----- > From: Joel Stanley <joel@jms.id.au> > Sent: Wednesday, September 1, 2021 3:37 PM > > On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang > <chiawei_wang@aspeedtech.com> wrote: > > > > Add driver support for the LPC UART routing control. Users can perform > > As we discussed, remove the "LPC" part of the name. > > > runtime configuration of the RX muxes among the UART controllers and > > the UART TXD/RXD IO pins. This is achieved through the exported sysfs > interface. > > > > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> > > It would be good to have some example of how to use it, and the output from > sysfs. > > You should also add a patch to document the sysfs files in Documentation/ABI. > Will add a new commit for the sysfs description. > > +++ b/drivers/soc/aspeed/aspeed-lpc-uart-routing.c > > @@ -0,0 +1,621 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2018 Google LLC > > + * Copyright (c) 2021 Aspeed Technology Inc. > > + */ > > +#include <linux/device.h> > > +#include <linux/module.h> > > +#include <linux/of_device.h> > > +#include <linux/of_address.h> > > You can drop this one. Revised as suggested. > > > +#include <linux/of_platform.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/regmap.h> > > +#include <linux/platform_device.h> > > + > > +/* register offsets */ > > +#define HICR9 0x98 > > +#define HICRA 0x9c > > + > > +/* attributes options */ > > +#define UART_ROUTING_IO1 "io1" > > +#define UART_ROUTING_IO2 "io2" > > +#define UART_ROUTING_IO3 "io3" > > +#define UART_ROUTING_IO4 "io4" > > +#define UART_ROUTING_IO5 "io5" > > +#define UART_ROUTING_IO6 "io6" > > +#define UART_ROUTING_IO10 "io10" > > +#define UART_ROUTING_UART1 "uart1" > > +#define UART_ROUTING_UART2 "uart2" > > +#define UART_ROUTING_UART3 "uart3" > > +#define UART_ROUTING_UART4 "uart4" > > +#define UART_ROUTING_UART5 "uart5" > > +#define UART_ROUTING_UART6 "uart6" > > +#define UART_ROUTING_UART10 "uart10" > > +#define UART_ROUTING_RES "reserved" > > + > > +struct aspeed_uart_routing { > > + struct regmap *map; > > + spinlock_t lock; > > + struct attribute_group const *attr_grp; }; > > + > > +struct aspeed_uart_routing_selector { > > + struct device_attribute dev_attr; > > + uint32_t reg; > > + uint32_t mask; > > + uint32_t shift; > > These can be u8. Revised as suggested. > > > +static ssize_t aspeed_uart_routing_show(struct device *dev, > > + struct device_attribute > *attr, > > + char *buf) { > > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > > + struct aspeed_uart_routing_selector *sel = > to_routing_selector(attr); > > + int val, pos, len; > > + > > + regmap_read(uart_routing->map, sel->reg, &val); > > + val = (val >> sel->shift) & sel->mask; > > + > > + len = 0; > > + for (pos = 0; sel->options[pos] != NULL; ++pos) { > > + if (pos == val) { > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > > + "[%s] ", sel->options[pos]); > > + } else { > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > > + "%s ", sel->options[pos]); > > The kernel prefers sysfs_emit and sysfs_emit_at insteading of using snprintf > directly. Revised as suggested. > > > + } > > + } > > + > > + if (val >= pos) { > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, > > + "[unknown(%d)]", val); > > + } > > + > > + len += snprintf(buf + len, PAGE_SIZE - 1 - len, "\n"); > > + > > + return len; > > +} > > + > > +static ssize_t aspeed_uart_routing_store(struct device *dev, > > + struct device_attribute > *attr, > > + const char *buf, size_t > > +count) { > > + unsigned long flags; > > + struct aspeed_uart_routing *uart_routing = dev_get_drvdata(dev); > > + struct aspeed_uart_routing_selector *sel = > to_routing_selector(attr); > > + int val; > > + > > + val = match_string(sel->options, -1, buf); > > + if (val < 0) { > > + dev_err(dev, "invalid value \"%s\"\n", buf); > > + return -EINVAL; > > + } > > + > > + spin_lock_irqsave(&uart_routing->lock, flags); > > I can't see why you would need a spinlock here. The regmap has it's own > locking so it will protect against concurrent updates to the registers. You are right. Lock is not needed here. Will removed it as suggested. > > > + > > + regmap_update_bits(uart_routing->map, sel->reg, > > + (sel->mask << sel->shift), > > + (val & sel->mask) << sel->shift); > > + > > + spin_unlock_irqrestore(&uart_routing->lock, flags); > > + > > + return count; > > +} > > + > > +static int aspeed_uart_routing_probe(struct platform_device *pdev) { > > + int rc; > > + struct device *dev = &pdev->dev; > > + struct aspeed_uart_routing *uart_routing; > > + > > + uart_routing = devm_kzalloc(&pdev->dev, > > + sizeof(*uart_routing), > > + GFP_KERNEL); > > You can reformat this file to have longer lines; the kernel is ok with up to 100 > columsn these days. > > > + if (!uart_routing) { > > + dev_err(dev, "cannot allocate memory\n"); > > I'd drop this error message. Revised as suggested > > > + return -ENOMEM; > > + } > > + > > + uart_routing->map = > syscon_node_to_regmap(dev->parent->of_node); > > + if (IS_ERR(uart_routing->map)) { > > + dev_err(dev, "cannot get regmap\n"); > > + return PTR_ERR(uart_routing->map); > > + } > > + > > + uart_routing->attr_grp = of_device_get_match_data(dev); > > + > > + spin_lock_init(&uart_routing->lock); > > I don't think you need the lock at all. Same as above. Thanks for reviewing this. The v2 patch will include the driver refactoring and additional documentation. Chiawei ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] ARM: dts: aspeed: Add uart routing to device tree 2021-09-01 6:22 ` Chia-Wei Wang @ 2021-09-01 6:22 ` Chia-Wei Wang -1 siblings, 0 replies; 18+ messages in thread From: Chia-Wei Wang @ 2021-09-01 6:22 UTC (permalink / raw) To: robh+dt, joel, andrew, osk, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, openbmc Add LPC uart routing to the device tree for Aspeed AST25xx/AST26xx SoCs. Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> --- arch/arm/boot/dts/aspeed-g5.dtsi | 6 ++++++ arch/arm/boot/dts/aspeed-g6.dtsi | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi index 329eaeef66fb..ba7744cb0842 100644 --- a/arch/arm/boot/dts/aspeed-g5.dtsi +++ b/arch/arm/boot/dts/aspeed-g5.dtsi @@ -492,6 +492,12 @@ #reset-cells = <1>; }; + lpc_uart_routing: lpc-uart-routing@98 { + compatible = "aspeed,ast2500-lpc-uart-routing"; + reg = <0x98 0x8>; + status = "disabled"; + }; + lhc: lhc@a0 { compatible = "aspeed,ast2500-lhc"; reg = <0xa0 0x24 0xc8 0x8>; diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi index f96607b7b4e2..3699c50a2945 100644 --- a/arch/arm/boot/dts/aspeed-g6.dtsi +++ b/arch/arm/boot/dts/aspeed-g6.dtsi @@ -523,6 +523,12 @@ #reset-cells = <1>; }; + lpc_uart_routing: lpc-uart-routing@98 { + compatible = "aspeed,ast2600-lpc-uart-routing"; + reg = <0x98 0x8>; + status = "disabled"; + }; + ibt: ibt@140 { compatible = "aspeed,ast2600-ibt-bmc"; reg = <0x140 0x18>; -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] ARM: dts: aspeed: Add uart routing to device tree @ 2021-09-01 6:22 ` Chia-Wei Wang 0 siblings, 0 replies; 18+ messages in thread From: Chia-Wei Wang @ 2021-09-01 6:22 UTC (permalink / raw) To: robh+dt, joel, andrew, osk, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, openbmc Add LPC uart routing to the device tree for Aspeed AST25xx/AST26xx SoCs. Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> --- arch/arm/boot/dts/aspeed-g5.dtsi | 6 ++++++ arch/arm/boot/dts/aspeed-g6.dtsi | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi index 329eaeef66fb..ba7744cb0842 100644 --- a/arch/arm/boot/dts/aspeed-g5.dtsi +++ b/arch/arm/boot/dts/aspeed-g5.dtsi @@ -492,6 +492,12 @@ #reset-cells = <1>; }; + lpc_uart_routing: lpc-uart-routing@98 { + compatible = "aspeed,ast2500-lpc-uart-routing"; + reg = <0x98 0x8>; + status = "disabled"; + }; + lhc: lhc@a0 { compatible = "aspeed,ast2500-lhc"; reg = <0xa0 0x24 0xc8 0x8>; diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi index f96607b7b4e2..3699c50a2945 100644 --- a/arch/arm/boot/dts/aspeed-g6.dtsi +++ b/arch/arm/boot/dts/aspeed-g6.dtsi @@ -523,6 +523,12 @@ #reset-cells = <1>; }; + lpc_uart_routing: lpc-uart-routing@98 { + compatible = "aspeed,ast2600-lpc-uart-routing"; + reg = <0x98 0x8>; + status = "disabled"; + }; + ibt: ibt@140 { compatible = "aspeed,ast2600-ibt-bmc"; reg = <0x140 0x18>; -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] arm: aspeed: Add LPC uart routing support 2021-09-01 6:22 ` Chia-Wei Wang (?) @ 2021-09-01 7:03 ` Joel Stanley -1 siblings, 0 replies; 18+ messages in thread From: Joel Stanley @ 2021-09-01 7:03 UTC (permalink / raw) To: Chia-Wei Wang, John Wang, Lei YU Cc: Rob Herring, Andrew Jeffery, Oskar Senft, devicetree, Linux ARM, linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote: > > Add LPC uart routing driver and the device tree nodes. Thanks for submitting this driver. There are many parties who are interested in this, so hopefully they can step forward and review. Is this LPC UART routing, or UART routing in general? I know the register to control the routing is hidden in the LPC space, but I thought it was just general routing. I would drop the LPC part of the name. You also need to update the bindings document with the new compatible string. Send a patch for Documentation/devicetree/bindings/mfd/aspeed-lpc.txt. Cheers, Joel > > Chia-Wei Wang (2): > soc: aspeed: Add LPC UART routing support > ARM: dts: aspeed: Add uart routing to device tree > > arch/arm/boot/dts/aspeed-g5.dtsi | 6 + > arch/arm/boot/dts/aspeed-g6.dtsi | 6 + > drivers/soc/aspeed/Kconfig | 11 + > drivers/soc/aspeed/Makefile | 9 +- > drivers/soc/aspeed/aspeed-lpc-uart-routing.c | 621 +++++++++++++++++++ > 5 files changed, 649 insertions(+), 4 deletions(-) > create mode 100644 drivers/soc/aspeed/aspeed-lpc-uart-routing.c > > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] arm: aspeed: Add LPC uart routing support @ 2021-09-01 7:03 ` Joel Stanley 0 siblings, 0 replies; 18+ messages in thread From: Joel Stanley @ 2021-09-01 7:03 UTC (permalink / raw) To: Chia-Wei Wang, John Wang, Lei YU Cc: Rob Herring, Andrew Jeffery, Oskar Senft, devicetree, Linux ARM, linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote: > > Add LPC uart routing driver and the device tree nodes. Thanks for submitting this driver. There are many parties who are interested in this, so hopefully they can step forward and review. Is this LPC UART routing, or UART routing in general? I know the register to control the routing is hidden in the LPC space, but I thought it was just general routing. I would drop the LPC part of the name. You also need to update the bindings document with the new compatible string. Send a patch for Documentation/devicetree/bindings/mfd/aspeed-lpc.txt. Cheers, Joel > > Chia-Wei Wang (2): > soc: aspeed: Add LPC UART routing support > ARM: dts: aspeed: Add uart routing to device tree > > arch/arm/boot/dts/aspeed-g5.dtsi | 6 + > arch/arm/boot/dts/aspeed-g6.dtsi | 6 + > drivers/soc/aspeed/Kconfig | 11 + > drivers/soc/aspeed/Makefile | 9 +- > drivers/soc/aspeed/aspeed-lpc-uart-routing.c | 621 +++++++++++++++++++ > 5 files changed, 649 insertions(+), 4 deletions(-) > create mode 100644 drivers/soc/aspeed/aspeed-lpc-uart-routing.c > > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] arm: aspeed: Add LPC uart routing support @ 2021-09-01 7:03 ` Joel Stanley 0 siblings, 0 replies; 18+ messages in thread From: Joel Stanley @ 2021-09-01 7:03 UTC (permalink / raw) To: Chia-Wei Wang, John Wang, Lei YU Cc: devicetree, linux-aspeed, Andrew Jeffery, OpenBMC Maillist, Linux Kernel Mailing List, Rob Herring, Oskar Senft, Linux ARM On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote: > > Add LPC uart routing driver and the device tree nodes. Thanks for submitting this driver. There are many parties who are interested in this, so hopefully they can step forward and review. Is this LPC UART routing, or UART routing in general? I know the register to control the routing is hidden in the LPC space, but I thought it was just general routing. I would drop the LPC part of the name. You also need to update the bindings document with the new compatible string. Send a patch for Documentation/devicetree/bindings/mfd/aspeed-lpc.txt. Cheers, Joel > > Chia-Wei Wang (2): > soc: aspeed: Add LPC UART routing support > ARM: dts: aspeed: Add uart routing to device tree > > arch/arm/boot/dts/aspeed-g5.dtsi | 6 + > arch/arm/boot/dts/aspeed-g6.dtsi | 6 + > drivers/soc/aspeed/Kconfig | 11 + > drivers/soc/aspeed/Makefile | 9 +- > drivers/soc/aspeed/aspeed-lpc-uart-routing.c | 621 +++++++++++++++++++ > 5 files changed, 649 insertions(+), 4 deletions(-) > create mode 100644 drivers/soc/aspeed/aspeed-lpc-uart-routing.c > > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 0/2] arm: aspeed: Add LPC uart routing support 2021-09-01 7:03 ` Joel Stanley (?) @ 2021-09-01 7:09 ` ChiaWei Wang -1 siblings, 0 replies; 18+ messages in thread From: ChiaWei Wang @ 2021-09-01 7:09 UTC (permalink / raw) To: Joel Stanley, John Wang, Lei YU Cc: Rob Herring, Andrew Jeffery, Oskar Senft, devicetree, Linux ARM, linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist Hi Joel, > From: Joel Stanley <joel@jms.id.au> > Sent: Wednesday, September 1, 2021 3:04 PM > > On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang > <chiawei_wang@aspeedtech.com> wrote: > > > > Add LPC uart routing driver and the device tree nodes. > > Thanks for submitting this driver. There are many parties who are interested in > this, so hopefully they can step forward and review. > > Is this LPC UART routing, or UART routing in general? I know the register to > control the routing is hidden in the LPC space, but I thought it was just general > routing. I would drop the LPC part of the name. > > You also need to update the bindings document with the new compatible string. > Send a patch for Documentation/devicetree/bindings/mfd/aspeed-lpc.txt. Thanks for the feedback. The UART routing is general. I will send a v2 patch which drops the LPC part of name. And the compatible string will also be updated to the dt-bindings. Chiawei ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 0/2] arm: aspeed: Add LPC uart routing support @ 2021-09-01 7:09 ` ChiaWei Wang 0 siblings, 0 replies; 18+ messages in thread From: ChiaWei Wang @ 2021-09-01 7:09 UTC (permalink / raw) To: Joel Stanley, John Wang, Lei YU Cc: Rob Herring, Andrew Jeffery, Oskar Senft, devicetree, Linux ARM, linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist Hi Joel, > From: Joel Stanley <joel@jms.id.au> > Sent: Wednesday, September 1, 2021 3:04 PM > > On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang > <chiawei_wang@aspeedtech.com> wrote: > > > > Add LPC uart routing driver and the device tree nodes. > > Thanks for submitting this driver. There are many parties who are interested in > this, so hopefully they can step forward and review. > > Is this LPC UART routing, or UART routing in general? I know the register to > control the routing is hidden in the LPC space, but I thought it was just general > routing. I would drop the LPC part of the name. > > You also need to update the bindings document with the new compatible string. > Send a patch for Documentation/devicetree/bindings/mfd/aspeed-lpc.txt. Thanks for the feedback. The UART routing is general. I will send a v2 patch which drops the LPC part of name. And the compatible string will also be updated to the dt-bindings. Chiawei _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 0/2] arm: aspeed: Add LPC uart routing support @ 2021-09-01 7:09 ` ChiaWei Wang 0 siblings, 0 replies; 18+ messages in thread From: ChiaWei Wang @ 2021-09-01 7:09 UTC (permalink / raw) To: Joel Stanley, John Wang, Lei YU Cc: devicetree, linux-aspeed, Andrew Jeffery, OpenBMC Maillist, Linux Kernel Mailing List, Rob Herring, Oskar Senft, Linux ARM Hi Joel, > From: Joel Stanley <joel@jms.id.au> > Sent: Wednesday, September 1, 2021 3:04 PM > > On Wed, 1 Sept 2021 at 06:22, Chia-Wei Wang > <chiawei_wang@aspeedtech.com> wrote: > > > > Add LPC uart routing driver and the device tree nodes. > > Thanks for submitting this driver. There are many parties who are interested in > this, so hopefully they can step forward and review. > > Is this LPC UART routing, or UART routing in general? I know the register to > control the routing is hidden in the LPC space, but I thought it was just general > routing. I would drop the LPC part of the name. > > You also need to update the bindings document with the new compatible string. > Send a patch for Documentation/devicetree/bindings/mfd/aspeed-lpc.txt. Thanks for the feedback. The UART routing is general. I will send a v2 patch which drops the LPC part of name. And the compatible string will also be updated to the dt-bindings. Chiawei ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-09-01 13:43 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-01 6:22 [PATCH 0/2] arm: aspeed: Add LPC uart routing support Chia-Wei Wang 2021-09-01 6:22 ` Chia-Wei Wang 2021-09-01 6:22 ` [PATCH 1/2] soc: aspeed: Add LPC UART " Chia-Wei Wang 2021-09-01 6:22 ` Chia-Wei Wang 2021-09-01 7:36 ` Joel Stanley 2021-09-01 7:36 ` Joel Stanley 2021-09-01 7:36 ` Joel Stanley 2021-09-01 9:43 ` ChiaWei Wang 2021-09-01 9:43 ` ChiaWei Wang 2021-09-01 9:43 ` ChiaWei Wang 2021-09-01 6:22 ` [PATCH 2/2] ARM: dts: aspeed: Add uart routing to device tree Chia-Wei Wang 2021-09-01 6:22 ` Chia-Wei Wang 2021-09-01 7:03 ` [PATCH 0/2] arm: aspeed: Add LPC uart routing support Joel Stanley 2021-09-01 7:03 ` Joel Stanley 2021-09-01 7:03 ` Joel Stanley 2021-09-01 7:09 ` ChiaWei Wang 2021-09-01 7:09 ` ChiaWei Wang 2021-09-01 7:09 ` ChiaWei Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.