On Thu, 16 Oct 2014 16:55:56 +0200
, David Woodhouse <dw...@infradead.org>
wrote:
> On Wed, 2014-10-15 at 17:43 +0200, Darren Hart wrote:
> >
> > So my objection here is that by keeping the of_* terms in the driver we
> > are required to include of, although it does safely convert to returning
> > NULL if !CONFIG_OF I suppose.
>
> New version removes everything but the of_match_id bits which we need to
> match ACPI devices too. Perhaps they ought to be renamed, but I'm not
> sure it's worth it.
>
> This also removes the call to platform_get_resource(IORESOURCE_MEM) and
> fall back to platform_get_resource(IORESOURCE_IO) as discussed IRL with
> Rafael. I'm not sure it's much of an improvement, mind you :)
>
> Still untested. I think it's OK to switch to platform_get_irq() and then
> drop the irq_dispose_mapping() call, right? The platform_device takes
> care of all of that for us?
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 26cec64..be95a4c 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1094,14 +1094,14 @@ config SERIAL_NETX_CONSOLE
> you can make it the console by answering Y to this option.
>
> config SERIAL_OF_PLATFORM
> - tristate "Serial port on Open Firmware platform bus"
> - depends on OF
> + tristate "Serial port on firmware platform bus"
> + depends on OF || ACPI
> depends on SERIAL_8250 || SERIAL_OF_PLATFORM_NWPSERIAL
> help
> - If you have a PowerPC based system that has serial ports
> - on a platform specific bus, you should enable this option.
> - Currently, only 8250 compatible ports are supported, but
> - others can easily be added.
> + If you have a system which advertises its serial ports through
> + devicetree or ACPI, you should enable this option. Currently
> + only 8250 compatible and NWP ports and are supported, but others
> + can easily be added.
>
> config SERIAL_OMAP
> tristate "OMAP serial port support"
> diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
> index 68d4455..cc6c99b 100644
> --- a/drivers/tty/serial/of_serial.c
> +++ b/drivers/tty/serial/of_serial.c
> @@ -14,8 +14,7 @@
> #include <linux/delay.h>
> #include <linux/serial_core.h>
> #include <linux/serial_reg.h>
> -#include <linux/of_address.h>
> -#include <linux/of_irq.h>
> +#include <linux/property.h>
> #include <linux/of_platform.h>
> #include <linux/nwpserial.h>
> #include <linux/clk.h>
> @@ -53,22 +52,22 @@ static inline void tegra_serial_handle_break(struct
> uart_port *port)
> /*
> * Fill a struct uart_port for a given device node
> */
> -static int of_platform_serial_setup(struct platform_device *ofdev,
> +static int of_platform_serial_setup(struct platform_device *pdev,
> int type, struct uart_port *port,
> struct of_serial_info *info)
> {
> - struct resource resource;
> - struct device_node *np = ofdev->dev.of_node;
> u32 clk, spd, prop;
> - int ret;
> + int iotype = -1;
> + u32 res_start;
> + int ret, i;
>
> memset(port, 0, sizeof *port);
> - if (of_property_read_u32(np, "clock-frequency", &clk)) {
> + if (device_property_read_u32(&pdev->dev, "clock-frequency", &clk)) {
>
> /* Get clk rate through clk driver if present */
> - info->clk = clk_get(&ofdev->dev, NULL);
> + info->clk = clk_get(&pdev->dev, NULL);
> if (IS_ERR(info->clk)) {
> - dev_warn(&ofdev->dev,
> + dev_warn(&pdev->dev,
> "clk or clock-frequency not defined\n");
> return PTR_ERR(info->clk);
> }
> @@ -77,57 +76,63 @@ static int of_platform_serial_setup(struct
> platform_device *ofdev,
> clk = clk_get_rate(info->clk);
> }
> /* If current-speed was set, then try not to change it. */
> - if (of_property_read_u32(np, "current-speed", &spd) == 0)
> + if (device_property_read_u32(&pdev->dev, "current-speed", &spd) == 0)
> port->custom_divisor = clk / (16 * spd);
>
> - ret = of_address_to_resource(np, 0, &resource);
> - if (ret) {
> - dev_warn(&ofdev->dev, "invalid address\n");
> + /* Check for shifted address mapping */
> + if (device_property_read_u32(&pdev->dev, "reg-offset", &prop) != 0)
> + prop = 0;
> +
> + for (i = 0; iotype == -1 && i < pdev->num_resources; i++) {
> + struct resource *resource = &pdev->resource[i];
> + if (resource_type(resource) == IORESOURCE_MEM) {
> + iotype = UPIO_MEM;
> + port->mapbase = res_start + prop;
> + } else if (resource_type(resource) == IORESOURCE_IO) {
> + iotype = UPIO_PORT;
> + port->iobase = res_start + prop;
> + }
> +
> + res_start = resource->start;
> + }
> + if (iotype == -1) {
> + dev_warn(&pdev->dev, "invalid address\n");
> goto out;
> }
>
> spin_lock_init(&port->lock);
> - port->mapbase = resource.start;
> -
> - /* Check for shifted address mapping */
> - if (of_property_read_u32(np, "reg-offset", &prop) == 0)
> - port->mapbase += prop;
>
> /* Check for registers offset within the devices address range */
> - if (of_property_read_u32(np, "reg-shift", &prop) == 0)
> + if (device_property_read_u32(&pdev->dev, "reg-shift", &prop) == 0)
> port->regshift = prop;
>
> /* Check for fifo size */
> - if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> + if (device_property_read_u32(&pdev->dev, "fifo-size", &prop) == 0)
> port->fifosize = prop;
>
> - port->irq = irq_of_parse_and_map(np, 0);
> - port->iotype = UPIO_MEM;
> - if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
> - switch (prop) {
> - case 1:
> - port->iotype = UPIO_MEM;
> - break;
> - case 4:
> - port->iotype = UPIO_MEM32;
> - break;
> - default:
> - dev_warn(&ofdev->dev, "unsupported reg-io-width (%d)\n",
> + port->irq = platform_get_irq(pdev, 0);
> +
> + if (device_property_read_u32(&pdev->dev, "reg-io-width", &prop) == 0) {
> + if (prop == 4 && iotype == UPIO_MEM) {
> + iotype = UPIO_MEM32;
> + } else {
> + dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
> prop);
> ret = -EINVAL;
> goto out;
> }
> }
>
> + port->iotype = iotype;
> port->type = type;
> port->uartclk = clk;
> port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP
> | UPF_FIXED_PORT | UPF_FIXED_TYPE;
>
> - if (of_find_property(np, "no-loopback-test", NULL))
> + if (!device_get_property(&pdev->dev, "no-loopback-test", NULL))
> port->flags |= UPF_SKIP_TEST;
>
> - port->dev = &ofdev->dev;
> + port->dev = &pdev->dev;
>
> if (type == PORT_TEGRA)
> port->handle_break = tegra_serial_handle_break;
> @@ -143,7 +148,7 @@ out:
> * Try to register a serial port
> */
> static struct of_device_id of_platform_serial_table[];
> -static int of_platform_serial_probe(struct platform_device *ofdev)
> +static int of_platform_serial_probe(struct platform_device *pdev)
Nit: Separate patch please. The rename of ofdev to pdev is nothing more
than cleanup. It has no functional impact and just makes the patch
larger.
> {
> const struct of_device_id *match;
> struct of_serial_info *info;
> @@ -151,11 +156,11 @@ static int of_platform_serial_probe(struct
> platform_device *ofdev)
> int port_type;
> int ret;
>
> - match = of_match_device(of_platform_serial_table, &ofdev->dev);
> + match = of_match_device(of_platform_serial_table, &pdev->dev);
> if (!match)
> return -EINVAL;
>
> - if (of_find_property(ofdev->dev.of_node, "used-by-rtas", NULL))
> + if (!device_get_property(&pdev->dev, "used-by-rtas", NULL))
> return -EBUSY;
>
> info = kmalloc(sizeof(*info), GFP_KERNEL);
> @@ -163,7 +168,7 @@ static int of_platform_serial_probe(struct
> platform_device *ofdev)
> return -ENOMEM;
>
> port_type = (unsigned long)match->data;
> - ret = of_platform_serial_setup(ofdev, port_type, &port, info);
> + ret = of_platform_serial_setup(pdev, port_type, &port, info);
> if (ret)
> goto out;
>
> @@ -179,12 +184,10 @@ static int of_platform_serial_probe(struct
> platform_device *ofdev)
> if (port.fifosize)
> port8250.capabilities = UART_CAP_FIFO;
>
> - if (of_property_read_bool(ofdev->dev.of_node,
> - "auto-flow-control"))
> + if (!device_get_property(&pdev->dev, "auto-flow-control", NULL))
> port8250.capabilities |= UART_CAP_AFE;
>
> - if (of_property_read_bool(ofdev->dev.of_node,
> - "has-hw-flow-control"))
> + if (!device_get_property(&pdev->dev, "has-hw-flow-control",
> NULL))
> port8250.port.flags |= UPF_HARD_FLOW;
>
> ret = serial8250_register_8250_port(&port8250);
> @@ -199,7 +202,7 @@ static int of_platform_serial_probe(struct
> platform_device *ofdev)
> default:
> /* need to add code for these */
> case PORT_UNKNOWN:
> - dev_info(&ofdev->dev, "Unknown serial port found, ignored\n");
> + dev_info(&pdev->dev, "Unknown serial port found, ignored\n");
> ret = -ENODEV;
> break;
> }
> @@ -208,20 +211,19 @@ static int of_platform_serial_probe(struct
> platform_device *ofdev)
>
> info->type = port_type;
> info->line = ret;
> - platform_set_drvdata(ofdev, info);
> + platform_set_drvdata(pdev, info);
> return 0;
> out:
> kfree(info);
> - irq_dispose_mapping(port.irq);
> return ret;
> }
>
> /*
> * Release a line
> */
> -static int of_platform_serial_remove(struct platform_device *ofdev)
> +static int of_platform_serial_remove(struct platform_device *pdev)
> {
> - struct of_serial_info *info = platform_get_drvdata(ofdev);
> + struct of_serial_info *info = platform_get_drvdata(pdev);
> switch (info->type) {
> #ifdef CONFIG_SERIAL_8250
> case PORT_8250 ... PORT_MAX_8250:
>
>
> --
> David Woodhouse Open Source Technology Centre
> david.woodho...@intel.com Intel Corporation
Attachment: smime.p7s (application/x-pkcs7-signature)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
