* [PATCH v2] serial: 8250_fsl: Add ACPI support
@ 2020-08-25 13:31 kuldip dwivedi
2020-08-28 8:13 ` Greg Kroah-Hartman
0 siblings, 1 reply; 3+ messages in thread
From: kuldip dwivedi @ 2020-08-25 13:31 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Dmitry Safonov, linux-serial
Cc: Vabhav Sharma, Pankaj Bansal, Varun Sethi, kuldip dwivedi
This adds support for ACPI enumerated FSL 16550 UARTs.
For supporting ACPI, I added a wrapper so that this driver
can be used if firmware has exposed the HID "NXP0018" in
DSDT table.
This will be built as object file if config "SERIAL_8250_FSL"
is enabled which depends on config "SERIAL_8250_CONSOLE".
Signed-off-by: kuldip dwivedi <kuldip.dwivedi@puresoftware.com>
---
Notes:
1. revert the license change from v1
2. revert the file header format change from v1
3. remove extra headers from v1
4. this is tested on layerscape platform LS1046A
with the firmware having ACPI support.
This will be built as object file if config " SERIAL_8250_FSL"
is enabled which depends on config " SERIAL_8250_CONSOLE ".
this driver will be probed if firmware has exposed HID " NXP0018" in DSDT table
drivers/tty/serial/8250/8250_fsl.c | 98 ++++++++++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
index 0d0c80905c58..62e5e2d33f69 100644
--- a/drivers/tty/serial/8250/8250_fsl.c
+++ b/drivers/tty/serial/8250/8250_fsl.c
@@ -1,11 +1,16 @@
// SPDX-License-Identifier: GPL-2.0
+#include <linux/acpi.h>
#include <linux/serial_reg.h>
#include <linux/serial_8250.h>
#include "8250.h"
+#define DRIVER_NAME "fsl-ns16550-uart"
+
/*
* Freescale 16550 UART "driver", Copyright (C) 2011 Paul Gortmaker.
+ * Copyright 2020 NXP
+ * Copyright 2020 Puresoftware Ltd.
*
* This isn't a full driver; it just provides an alternate IRQ
* handler to deal with an errata. Everything else is just
@@ -20,6 +25,10 @@
* IRQ event to the next one.
*/
+struct fsl8250_data {
+ int line;
+};
+
int fsl8250_handle_irq(struct uart_port *port)
{
unsigned char lsr, orig_lsr;
@@ -79,3 +88,92 @@ int fsl8250_handle_irq(struct uart_port *port)
return 1;
}
EXPORT_SYMBOL_GPL(fsl8250_handle_irq);
+
+static int fsl8250_acpi_probe(struct platform_device *pdev)
+{
+ struct fsl8250_data *data;
+ struct uart_8250_port port8250;
+ struct device *dev = &pdev->dev;
+ struct resource *regs;
+
+ int ret, irq;
+
+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!regs) {
+ dev_err(dev, "no registers defined\n");
+ return -EINVAL;
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ if (irq != -EPROBE_DEFER)
+ dev_err(dev, "cannot get irq\n");
+ return irq;
+ }
+
+ memset(&port8250, 0, sizeof(port8250));
+
+ ret = device_property_read_u32(dev, "clock-frequency",
+ &port8250.port.uartclk);
+ if (ret)
+ return ret;
+
+ spin_lock_init(&port8250.port.lock);
+
+ port8250.port.mapbase = regs->start;
+ port8250.port.irq = irq;
+ port8250.port.handle_irq = fsl8250_handle_irq;
+ port8250.port.type = PORT_16550A;
+ port8250.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
+ | UPF_FIXED_PORT | UPF_IOREMAP
+ | UPF_FIXED_TYPE;
+ port8250.port.dev = dev;
+ port8250.port.mapsize = resource_size(regs);
+ port8250.port.iotype = UPIO_MEM;
+ port8250.port.irqflags = IRQF_SHARED;
+
+ port8250.port.membase = devm_ioremap(dev, port8250.port.mapbase,
+ port8250.port.mapsize);
+ if (!port8250.port.membase)
+ return -ENOMEM;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->line = serial8250_register_8250_port(&port8250);
+ if (data->line < 0)
+ ret = data->line;
+
+ platform_set_drvdata(pdev, data);
+ return 0;
+}
+
+static int fsl8250_acpi_remove(struct platform_device *pdev)
+{
+ struct fsl8250_data *data = platform_get_drvdata(pdev);
+
+ serial8250_unregister_port(data->line);
+ return 0;
+}
+
+static const struct acpi_device_id fsl_8250_acpi_id[] = {
+ { "NXP0018", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, fsl_8250_acpi_id);
+
+static struct platform_driver fsl8250_platform_driver = {
+ .driver = {
+ .name = "fsl-16550-uart",
+ .acpi_match_table = ACPI_PTR(fsl_8250_acpi_id),
+ },
+ .probe = fsl8250_acpi_probe,
+ .remove = fsl8250_acpi_remove,
+};
+
+module_platform_driver(fsl8250_platform_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("FSL 8250 serial port driver");
+MODULE_ALIAS("platform:" DRIVER_NAME);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] serial: 8250_fsl: Add ACPI support
2020-08-25 13:31 [PATCH v2] serial: 8250_fsl: Add ACPI support kuldip dwivedi
@ 2020-08-28 8:13 ` Greg Kroah-Hartman
2020-08-29 12:18 ` Kuldip Dwivedi
0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-28 8:13 UTC (permalink / raw)
To: kuldip dwivedi
Cc: Jiri Slaby, Dmitry Safonov, linux-serial, Vabhav Sharma,
Pankaj Bansal, Varun Sethi
On Tue, Aug 25, 2020 at 07:01:35PM +0530, kuldip dwivedi wrote:
> This adds support for ACPI enumerated FSL 16550 UARTs.
> For supporting ACPI, I added a wrapper so that this driver
> can be used if firmware has exposed the HID "NXP0018" in
> DSDT table.
>
> This will be built as object file if config "SERIAL_8250_FSL"
> is enabled which depends on config "SERIAL_8250_CONSOLE".
>
> Signed-off-by: kuldip dwivedi <kuldip.dwivedi@puresoftware.com>
> ---
>
> Notes:
> 1. revert the license change from v1
> 2. revert the file header format change from v1
> 3. remove extra headers from v1
> 4. this is tested on layerscape platform LS1046A
> with the firmware having ACPI support.
>
> This will be built as object file if config " SERIAL_8250_FSL"
> is enabled which depends on config " SERIAL_8250_CONSOLE ".
> this driver will be probed if firmware has exposed HID " NXP0018" in DSDT table
>
> drivers/tty/serial/8250/8250_fsl.c | 98 ++++++++++++++++++++++++++++++
> 1 file changed, 98 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
> index 0d0c80905c58..62e5e2d33f69 100644
> --- a/drivers/tty/serial/8250/8250_fsl.c
> +++ b/drivers/tty/serial/8250/8250_fsl.c
> @@ -1,11 +1,16 @@
> // SPDX-License-Identifier: GPL-2.0
> +#include <linux/acpi.h>
> #include <linux/serial_reg.h>
> #include <linux/serial_8250.h>
>
> #include "8250.h"
>
> +#define DRIVER_NAME "fsl-ns16550-uart"
Why is this needed?
> +
> /*
> * Freescale 16550 UART "driver", Copyright (C) 2011 Paul Gortmaker.
> + * Copyright 2020 NXP
> + * Copyright 2020 Puresoftware Ltd.
Copyright info goes at the top of the file, not down here.
> *
> * This isn't a full driver; it just provides an alternate IRQ
> * handler to deal with an errata. Everything else is just
> @@ -20,6 +25,10 @@
> * IRQ event to the next one.
> */
>
> +struct fsl8250_data {
> + int line;
> +};
> +
> int fsl8250_handle_irq(struct uart_port *port)
> {
> unsigned char lsr, orig_lsr;
> @@ -79,3 +88,92 @@ int fsl8250_handle_irq(struct uart_port *port)
> return 1;
> }
> EXPORT_SYMBOL_GPL(fsl8250_handle_irq);
> +
> +static int fsl8250_acpi_probe(struct platform_device *pdev)
> +{
> + struct fsl8250_data *data;
> + struct uart_8250_port port8250;
> + struct device *dev = &pdev->dev;
> + struct resource *regs;
> +
> + int ret, irq;
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs) {
> + dev_err(dev, "no registers defined\n");
> + return -EINVAL;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + if (irq != -EPROBE_DEFER)
> + dev_err(dev, "cannot get irq\n");
> + return irq;
> + }
> +
> + memset(&port8250, 0, sizeof(port8250));
> +
> + ret = device_property_read_u32(dev, "clock-frequency",
> + &port8250.port.uartclk);
> + if (ret)
> + return ret;
> +
> + spin_lock_init(&port8250.port.lock);
> +
> + port8250.port.mapbase = regs->start;
> + port8250.port.irq = irq;
> + port8250.port.handle_irq = fsl8250_handle_irq;
> + port8250.port.type = PORT_16550A;
> + port8250.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
> + | UPF_FIXED_PORT | UPF_IOREMAP
> + | UPF_FIXED_TYPE;
> + port8250.port.dev = dev;
> + port8250.port.mapsize = resource_size(regs);
> + port8250.port.iotype = UPIO_MEM;
> + port8250.port.irqflags = IRQF_SHARED;
> +
> + port8250.port.membase = devm_ioremap(dev, port8250.port.mapbase,
> + port8250.port.mapsize);
> + if (!port8250.port.membase)
> + return -ENOMEM;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->line = serial8250_register_8250_port(&port8250);
> + if (data->line < 0)
> + ret = data->line;
> +
> + platform_set_drvdata(pdev, data);
> + return 0;
> +}
> +
> +static int fsl8250_acpi_remove(struct platform_device *pdev)
> +{
> + struct fsl8250_data *data = platform_get_drvdata(pdev);
> +
> + serial8250_unregister_port(data->line);
> + return 0;
> +}
> +
> +static const struct acpi_device_id fsl_8250_acpi_id[] = {
> + { "NXP0018", 0 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(acpi, fsl_8250_acpi_id);
Will this now break the build on non-acpi systems, which the original
driver first was written for?
Don't you need some sort of dependancy on ACPI now?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: [PATCH v2] serial: 8250_fsl: Add ACPI support
2020-08-28 8:13 ` Greg Kroah-Hartman
@ 2020-08-29 12:18 ` Kuldip Dwivedi
0 siblings, 0 replies; 3+ messages in thread
From: Kuldip Dwivedi @ 2020-08-29 12:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Dmitry Safonov, linux-serial, Vabhav Sharma,
Pankaj Bansal, Varun Sethi
> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Friday, August 28, 2020 1:44 PM
> To: kuldip dwivedi <kuldip.dwivedi@puresoftware.com>
> Cc: Jiri Slaby <jirislaby@kernel.org>; Dmitry Safonov
<0x7f454c46@gmail.com>;
> linux-serial@vger.kernel.org; Vabhav Sharma <vabhav.sharma@nxp.com>;
Pankaj
> Bansal <pankaj.bansal@nxp.com>; Varun Sethi <V.Sethi@nxp.com>
> Subject: Re: [PATCH v2] serial: 8250_fsl: Add ACPI support
>
> On Tue, Aug 25, 2020 at 07:01:35PM +0530, kuldip dwivedi wrote:
> > This adds support for ACPI enumerated FSL 16550 UARTs.
> > For supporting ACPI, I added a wrapper so that this driver can be used
> > if firmware has exposed the HID "NXP0018" in DSDT table.
> >
> > This will be built as object file if config "SERIAL_8250_FSL"
> > is enabled which depends on config "SERIAL_8250_CONSOLE".
> >
> > Signed-off-by: kuldip dwivedi <kuldip.dwivedi@puresoftware.com>
> > ---
> >
> > Notes:
> > 1. revert the license change from v1
> > 2. revert the file header format change from v1
> > 3. remove extra headers from v1
> > 4. this is tested on layerscape platform LS1046A
> > with the firmware having ACPI support.
> >
> > This will be built as object file if config " SERIAL_8250_FSL"
> > is enabled which depends on config " SERIAL_8250_CONSOLE ".
> > this driver will be probed if firmware has exposed HID "
> > NXP0018" in DSDT table
> >
> > drivers/tty/serial/8250/8250_fsl.c | 98
> > ++++++++++++++++++++++++++++++
> > 1 file changed, 98 insertions(+)
> >
> > diff --git a/drivers/tty/serial/8250/8250_fsl.c
> > b/drivers/tty/serial/8250/8250_fsl.c
> > index 0d0c80905c58..62e5e2d33f69 100644
> > --- a/drivers/tty/serial/8250/8250_fsl.c
> > +++ b/drivers/tty/serial/8250/8250_fsl.c
> > @@ -1,11 +1,16 @@
> > // SPDX-License-Identifier: GPL-2.0
> > +#include <linux/acpi.h>
> > #include <linux/serial_reg.h>
> > #include <linux/serial_8250.h>
> >
> > #include "8250.h"
> >
> > +#define DRIVER_NAME "fsl-ns16550-uart"
>
> Why is this needed?
As this is not standalone module so not required. I added MODULE_ALIAS
but I don't think it is required so I will remove this in v3 PATCH.
>
>
> > +
> > /*
> > * Freescale 16550 UART "driver", Copyright (C) 2011 Paul Gortmaker.
> > + * Copyright 2020 NXP
> > + * Copyright 2020 Puresoftware Ltd.
>
> Copyright info goes at the top of the file, not down here.
Old Copyright was already there so I just added our copyright but I will
move
Copyright and file description at top in v3
>
> > *
> > * This isn't a full driver; it just provides an alternate IRQ
> > * handler to deal with an errata. Everything else is just @@ -20,6
> > +25,10 @@
> > * IRQ event to the next one.
> > */
> >
> > +struct fsl8250_data {
> > + int line;
> > +};
> > +
> > int fsl8250_handle_irq(struct uart_port *port) {
> > unsigned char lsr, orig_lsr;
> > @@ -79,3 +88,92 @@ int fsl8250_handle_irq(struct uart_port *port)
> > return 1;
> > }
> > EXPORT_SYMBOL_GPL(fsl8250_handle_irq);
> > +
> > +static int fsl8250_acpi_probe(struct platform_device *pdev) {
> > + struct fsl8250_data *data;
> > + struct uart_8250_port port8250;
> > + struct device *dev = &pdev->dev;
> > + struct resource *regs;
> > +
> > + int ret, irq;
> > +
> > + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!regs) {
> > + dev_err(dev, "no registers defined\n");
> > + return -EINVAL;
> > + }
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + if (irq != -EPROBE_DEFER)
> > + dev_err(dev, "cannot get irq\n");
> > + return irq;
> > + }
> > +
> > + memset(&port8250, 0, sizeof(port8250));
> > +
> > + ret = device_property_read_u32(dev, "clock-frequency",
> > + &port8250.port.uartclk);
> > + if (ret)
> > + return ret;
> > +
> > + spin_lock_init(&port8250.port.lock);
> > +
> > + port8250.port.mapbase = regs->start;
> > + port8250.port.irq = irq;
> > + port8250.port.handle_irq = fsl8250_handle_irq;
> > + port8250.port.type = PORT_16550A;
> > + port8250.port.flags = UPF_SHARE_IRQ |
UPF_BOOT_AUTOCONF
> > + | UPF_FIXED_PORT |
> UPF_IOREMAP
> > + | UPF_FIXED_TYPE;
> > + port8250.port.dev = dev;
> > + port8250.port.mapsize = resource_size(regs);
> > + port8250.port.iotype = UPIO_MEM;
> > + port8250.port.irqflags = IRQF_SHARED;
> > +
> > + port8250.port.membase = devm_ioremap(dev, port8250.port.mapbase,
> > +
port8250.port.mapsize);
> > + if (!port8250.port.membase)
> > + return -ENOMEM;
> > +
> > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + data->line = serial8250_register_8250_port(&port8250);
> > + if (data->line < 0)
> > + ret = data->line;
> > +
> > + platform_set_drvdata(pdev, data);
> > + return 0;
> > +}
> > +
> > +static int fsl8250_acpi_remove(struct platform_device *pdev) {
> > + struct fsl8250_data *data = platform_get_drvdata(pdev);
> > +
> > + serial8250_unregister_port(data->line);
> > + return 0;
> > +}
> > +
> > +static const struct acpi_device_id fsl_8250_acpi_id[] = {
> > + { "NXP0018", 0 },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(acpi, fsl_8250_acpi_id);
>
> Will this now break the build on non-acpi systems, which the original
driver first
> was written for?
No this will not break on non-acpi systems as irq handler is already
exported
As "fsl8250_handle_irq" and I have tested this change with ACPI as well
as DT boot.
Safe side I can put acpi specific changes in acpi config macro
"CONFIG_ACPI".
>
> Don't you need some sort of dependancy on ACPI now?
Can you please explain more ?
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-29 12:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-25 13:31 [PATCH v2] serial: 8250_fsl: Add ACPI support kuldip dwivedi
2020-08-28 8:13 ` Greg Kroah-Hartman
2020-08-29 12:18 ` Kuldip Dwivedi
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).