* [PATCH] serial: 8250_platform: Enable generic 16550A platform devices
@ 2024-07-30 5:12 Sunil V L
2024-09-17 13:10 ` Palmer Dabbelt
0 siblings, 1 reply; 3+ messages in thread
From: Sunil V L @ 2024-07-30 5:12 UTC (permalink / raw)
To: linux-kernel, linux-serial, linux-riscv
Cc: Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, Andrei Warkentin,
Anup Patel, Andrew Jones, Drew Fustini, Sunil V L
Currently, 8250_platform driver is used only for devices with fixed
serial ports (plat_serial8250_port). Extend this driver for any generic
16550A platform devices which can be probed using standard hardware
discovery mechanisms like ACPI.
This is required in particular for RISC-V which has non-PNP generic
16550A compatible UART that needs to be enumerated as ACPI platform
device.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
drivers/tty/serial/8250/8250_platform.c | 77 +++++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_platform.c b/drivers/tty/serial/8250/8250_platform.c
index d5c8d851348d..bdfb16bed4f2 100644
--- a/drivers/tty/serial/8250/8250_platform.c
+++ b/drivers/tty/serial/8250/8250_platform.c
@@ -6,7 +6,9 @@
* PNP 8250/16550 ports
* "serial8250" platform devices
*/
+#include <linux/acpi.h>
#include <linux/array_size.h>
+#include <linux/io.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/once.h>
@@ -100,6 +102,65 @@ void __init serial8250_isa_init_ports(void)
DO_ONCE(__serial8250_isa_init_ports);
}
+/*
+ * Generic 16550A platform devices
+ */
+static int serial8250_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct uart_8250_port uart = { 0 };
+ struct resource *regs;
+ unsigned char iotype;
+ int ret, line;
+
+ regs = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ if (regs) {
+ uart.port.iobase = regs->start;
+ iotype = UPIO_PORT;
+ } else {
+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!regs) {
+ dev_err(dev, "no registers defined\n");
+ return -EINVAL;
+ }
+
+ uart.port.mapbase = regs->start;
+ uart.port.mapsize = resource_size(regs);
+ uart.port.flags = UPF_IOREMAP;
+ iotype = UPIO_MEM;
+ }
+
+ /* Default clock frequency*/
+ uart.port.uartclk = 1843200;
+ uart.port.type = PORT_16550A;
+ uart.port.dev = &pdev->dev;
+ uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
+ ret = uart_read_and_validate_port_properties(&uart.port);
+ /* no interrupt -> fall back to polling */
+ if (ret == -ENXIO)
+ ret = 0;
+ if (ret)
+ return ret;
+
+ if (uart.port.mapbase) {
+ uart.port.membase = devm_ioremap(dev, uart.port.mapbase, uart.port.mapsize);
+ if (!uart.port.membase)
+ return -ENOMEM;
+ }
+
+ /*
+ * The previous call may not set iotype correctly when reg-io-width
+ * property is absent and it doesn't support IO port resource.
+ */
+ uart.port.iotype = iotype;
+
+ line = serial8250_register_8250_port(&uart);
+ if (line < 0)
+ return -ENODEV;
+
+ return 0;
+}
+
/*
* Register a set of serial devices attached to a platform device. The
* list is terminated with a zero flags entry, which means we expect
@@ -110,6 +171,15 @@ static int serial8250_probe(struct platform_device *dev)
struct plat_serial8250_port *p = dev_get_platdata(&dev->dev);
struct uart_8250_port uart;
int ret, i, irqflag = 0;
+ struct fwnode_handle *fwnode = dev_fwnode(&dev->dev);
+
+ /*
+ * Probe platform UART devices defined using standard hardware
+ * discovery mechanism like ACPI or DT. Support only ACPI based
+ * serial device for now.
+ */
+ if (!p && is_acpi_node(fwnode))
+ return serial8250_platform_probe(dev);
memset(&uart, 0, sizeof(uart));
@@ -198,6 +268,12 @@ static int serial8250_resume(struct platform_device *dev)
return 0;
}
+static const struct acpi_device_id acpi_platform_serial_table[] = {
+ { "RSCV0003", 0 }, // RISC-V Generic 16550A UART
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, acpi_platform_serial_table);
+
static struct platform_driver serial8250_isa_driver = {
.probe = serial8250_probe,
.remove_new = serial8250_remove,
@@ -205,6 +281,7 @@ static struct platform_driver serial8250_isa_driver = {
.resume = serial8250_resume,
.driver = {
.name = "serial8250",
+ .acpi_match_table = ACPI_PTR(acpi_platform_serial_table),
},
};
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] serial: 8250_platform: Enable generic 16550A platform devices
2024-07-30 5:12 [PATCH] serial: 8250_platform: Enable generic 16550A platform devices Sunil V L
@ 2024-09-17 13:10 ` Palmer Dabbelt
2024-09-17 13:58 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Palmer Dabbelt @ 2024-09-17 13:10 UTC (permalink / raw)
To: Sunil V L
Cc: linux-kernel, linux-serial, linux-riscv, Greg KH, jirislaby,
andriy.shevchenko, andrei.warkentin, apatel, ajones, dfustini,
Sunil V L
On Mon, 29 Jul 2024 22:12:18 PDT (-0700), Sunil V L wrote:
> Currently, 8250_platform driver is used only for devices with fixed
> serial ports (plat_serial8250_port). Extend this driver for any generic
> 16550A platform devices which can be probed using standard hardware
> discovery mechanisms like ACPI.
>
> This is required in particular for RISC-V which has non-PNP generic
> 16550A compatible UART that needs to be enumerated as ACPI platform
> device.
>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
> drivers/tty/serial/8250/8250_platform.c | 77 +++++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
It seems a bit awkward that we need something RISC-V specific here. I
guess the idea is that we don't want to depend on PNP because we'd end
up with a bunch of PCI dependencies as well? That seems like kind of an
odd way to do things, but I'm frequently suprised by ACPI stuff so
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
if someone was waiting for me...
> diff --git a/drivers/tty/serial/8250/8250_platform.c b/drivers/tty/serial/8250/8250_platform.c
> index d5c8d851348d..bdfb16bed4f2 100644
> --- a/drivers/tty/serial/8250/8250_platform.c
> +++ b/drivers/tty/serial/8250/8250_platform.c
> @@ -6,7 +6,9 @@
> * PNP 8250/16550 ports
> * "serial8250" platform devices
> */
> +#include <linux/acpi.h>
> #include <linux/array_size.h>
> +#include <linux/io.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/once.h>
> @@ -100,6 +102,65 @@ void __init serial8250_isa_init_ports(void)
> DO_ONCE(__serial8250_isa_init_ports);
> }
>
> +/*
> + * Generic 16550A platform devices
> + */
> +static int serial8250_platform_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct uart_8250_port uart = { 0 };
> + struct resource *regs;
> + unsigned char iotype;
> + int ret, line;
> +
> + regs = platform_get_resource(pdev, IORESOURCE_IO, 0);
> + if (regs) {
> + uart.port.iobase = regs->start;
> + iotype = UPIO_PORT;
> + } else {
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs) {
> + dev_err(dev, "no registers defined\n");
> + return -EINVAL;
> + }
> +
> + uart.port.mapbase = regs->start;
> + uart.port.mapsize = resource_size(regs);
> + uart.port.flags = UPF_IOREMAP;
> + iotype = UPIO_MEM;
> + }
> +
> + /* Default clock frequency*/
> + uart.port.uartclk = 1843200;
> + uart.port.type = PORT_16550A;
> + uart.port.dev = &pdev->dev;
> + uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
> + ret = uart_read_and_validate_port_properties(&uart.port);
> + /* no interrupt -> fall back to polling */
> + if (ret == -ENXIO)
> + ret = 0;
> + if (ret)
> + return ret;
> +
> + if (uart.port.mapbase) {
> + uart.port.membase = devm_ioremap(dev, uart.port.mapbase, uart.port.mapsize);
> + if (!uart.port.membase)
> + return -ENOMEM;
> + }
> +
> + /*
> + * The previous call may not set iotype correctly when reg-io-width
> + * property is absent and it doesn't support IO port resource.
> + */
> + uart.port.iotype = iotype;
> +
> + line = serial8250_register_8250_port(&uart);
> + if (line < 0)
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> /*
> * Register a set of serial devices attached to a platform device. The
> * list is terminated with a zero flags entry, which means we expect
> @@ -110,6 +171,15 @@ static int serial8250_probe(struct platform_device *dev)
> struct plat_serial8250_port *p = dev_get_platdata(&dev->dev);
> struct uart_8250_port uart;
> int ret, i, irqflag = 0;
> + struct fwnode_handle *fwnode = dev_fwnode(&dev->dev);
> +
> + /*
> + * Probe platform UART devices defined using standard hardware
> + * discovery mechanism like ACPI or DT. Support only ACPI based
> + * serial device for now.
> + */
> + if (!p && is_acpi_node(fwnode))
> + return serial8250_platform_probe(dev);
>
> memset(&uart, 0, sizeof(uart));
>
> @@ -198,6 +268,12 @@ static int serial8250_resume(struct platform_device *dev)
> return 0;
> }
>
> +static const struct acpi_device_id acpi_platform_serial_table[] = {
> + { "RSCV0003", 0 }, // RISC-V Generic 16550A UART
> + { },
> +};
> +MODULE_DEVICE_TABLE(acpi, acpi_platform_serial_table);
> +
> static struct platform_driver serial8250_isa_driver = {
> .probe = serial8250_probe,
> .remove_new = serial8250_remove,
> @@ -205,6 +281,7 @@ static struct platform_driver serial8250_isa_driver = {
> .resume = serial8250_resume,
> .driver = {
> .name = "serial8250",
> + .acpi_match_table = ACPI_PTR(acpi_platform_serial_table),
> },
> };
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] serial: 8250_platform: Enable generic 16550A platform devices
2024-09-17 13:10 ` Palmer Dabbelt
@ 2024-09-17 13:58 ` Andy Shevchenko
0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2024-09-17 13:58 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: Sunil V L, linux-kernel, linux-serial, linux-riscv, Greg KH,
jirislaby, andrei.warkentin, apatel, ajones, dfustini
On Tue, Sep 17, 2024 at 06:10:25AM -0700, Palmer Dabbelt wrote:
> On Mon, 29 Jul 2024 22:12:18 PDT (-0700), Sunil V L wrote:
> > Currently, 8250_platform driver is used only for devices with fixed
> > serial ports (plat_serial8250_port). Extend this driver for any generic
> > 16550A platform devices which can be probed using standard hardware
> > discovery mechanisms like ACPI.
> >
> > This is required in particular for RISC-V which has non-PNP generic
> > 16550A compatible UART that needs to be enumerated as ACPI platform
> > device.
> It seems a bit awkward that we need something RISC-V specific here. I
> guess the idea is that we don't want to depend on PNP because we'd end
> up with a bunch of PCI dependencies as well? That seems like kind of an
> odd way to do things, but I'm frequently suprised by ACPI stuff so
We have the same for HSUART on Intel (see 8250_dw.c). The ACPI devices use
different approach than PNP. PNP has the exact requirements for compatibility
with the legacy UARTs (to some extend). It puts some limitations. I believe
having specifics for RISC-V is fine. But I haven't checked the real IP used
in the SoC. If it's one that we have the existing driver for (like again
mentioned 8250_dw.c), then it should be added there.
TL;DR: so, what is the IP is in hardware for UART? Custom?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-17 13:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-30 5:12 [PATCH] serial: 8250_platform: Enable generic 16550A platform devices Sunil V L
2024-09-17 13:10 ` Palmer Dabbelt
2024-09-17 13:58 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).