*** BLURB HERE ***
Tobias Klauser (4):
tty: serial: altera_uart: Handle pdev->id == -1 in altera_uart_remove
tty: serial: altera_uart: Use port->regshift to store bus shift
tty: serial: altera_uart: Add devicetree support
MAINTAINERS: Add myself as a maintainer for
altera_uart/altera_jtaguart
.../devicetree/bindings/serial/altera_uart.txt | 7 ++
MAINTAINERS | 10 +++
drivers/tty/serial/altera_uart.c | 69 ++++++++++++++++----
3 files changed, 74 insertions(+), 12 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
Commit 6b5756f176568a710d008d3b478128fafb6707f0 introduced the
possibility for pdev->id being -1 but the change was not done equally in
altera_uart_remove. This patch fixes this.
Cc: Anton Vorontsov <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
---
drivers/tty/serial/altera_uart.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 7212162..dee7a0e 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -561,9 +561,15 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
static int __devexit altera_uart_remove(struct platform_device *pdev)
{
- struct uart_port *port = &altera_uart_ports[pdev->id].port;
+ struct uart_port *port;
+ int i = pdev->id;
+ if (i == -1)
+ i = 0;
+
+ port = &altera_uart_ports[i].port;
uart_remove_one_port(&altera_uart_driver, port);
+
return 0;
}
--
1.7.0.4
On 2011-02-09 at 10:51:10 +0100, Tobias Klauser <[email protected]> wrote:
> *** BLURB HERE ***
Sorry for this mess. I've accidentialy overwritten my original cover
letter. Will send again with the correct subject and text (and better
test my submissions in the future).
>
> Tobias Klauser (4):
> tty: serial: altera_uart: Handle pdev->id == -1 in altera_uart_remove
> tty: serial: altera_uart: Use port->regshift to store bus shift
> tty: serial: altera_uart: Add devicetree support
> MAINTAINERS: Add myself as a maintainer for
> altera_uart/altera_jtaguart
>
> .../devicetree/bindings/serial/altera_uart.txt | 7 ++
> MAINTAINERS | 10 +++
> drivers/tty/serial/altera_uart.c | 69 ++++++++++++++++----
> 3 files changed, 74 insertions(+), 12 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
>
This patchset adds some updates to the altera_uart taken from the Nios2
Linux port (currently out-of-tree) repository, the most important one
probably being the added devicetree support.
Tobias Klauser (4):
tty: serial: altera_uart: Handle pdev->id == -1 in altera_uart_remove
tty: serial: altera_uart: Use port->regshift to store bus shift
tty: serial: altera_uart: Add devicetree support
MAINTAINERS: Add myself as a maintainer for
altera_uart/altera_jtaguart
.../devicetree/bindings/serial/altera_uart.txt | 7 ++
MAINTAINERS | 10 +++
drivers/tty/serial/altera_uart.c | 69 ++++++++++++++++----
3 files changed, 74 insertions(+), 12 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
On 2011-02-09 at 10:51:34 +0100, Tobias Klauser <[email protected]> wrote:
> Commit 6b5756f176568a710d008d3b478128fafb6707f0 introduced the
> possibility for pdev->id being -1 but the change was not done equally in
> altera_uart_remove. This patch fixes this.
Please disregard this patch. I'll send it again as a follow up to the
correct cover letter.
Sorry for the mess
Tobias
>
> Cc: Anton Vorontsov <[email protected]>
> Signed-off-by: Tobias Klauser <[email protected]>
> ---
> drivers/tty/serial/altera_uart.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
> index 7212162..dee7a0e 100644
> --- a/drivers/tty/serial/altera_uart.c
> +++ b/drivers/tty/serial/altera_uart.c
> @@ -561,9 +561,15 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
>
> static int __devexit altera_uart_remove(struct platform_device *pdev)
> {
> - struct uart_port *port = &altera_uart_ports[pdev->id].port;
> + struct uart_port *port;
> + int i = pdev->id;
>
> + if (i == -1)
> + i = 0;
> +
> + port = &altera_uart_ports[i].port;
> uart_remove_one_port(&altera_uart_driver, port);
> +
> return 0;
> }
>
> --
> 1.7.0.4
>
Commit 6b5756f176568a710d008d3b478128fafb6707f0 introduced the
possibility for pdev->id being -1 but the change was not done equally in
altera_uart_remove. This patch fixes this.
Cc: Anton Vorontsov <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
---
drivers/tty/serial/altera_uart.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 7212162..dee7a0e 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -561,9 +561,15 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
static int __devexit altera_uart_remove(struct platform_device *pdev)
{
- struct uart_port *port = &altera_uart_ports[pdev->id].port;
+ struct uart_port *port;
+ int i = pdev->id;
+ if (i == -1)
+ i = 0;
+
+ port = &altera_uart_ports[i].port;
uart_remove_one_port(&altera_uart_driver, port);
+
return 0;
}
--
1.7.0.4
Use the regshift member of struct uart_port to store the address stride
from platform data. This way we can save one dereference per call of
altera_uart_readl and altera_uart_writel.
This also allows us to use the driver without platform data, which is
needed for device tree support in the Nios2 port.
Cc: Anton Vorontsov <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
---
drivers/tty/serial/altera_uart.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index dee7a0e..3a57352 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -86,16 +86,12 @@ struct altera_uart {
static u32 altera_uart_readl(struct uart_port *port, int reg)
{
- struct altera_uart_platform_uart *platp = port->private_data;
-
- return readl(port->membase + (reg << platp->bus_shift));
+ return readl(port->membase + (reg << port->regshift));
}
static void altera_uart_writel(struct uart_port *port, u32 dat, int reg)
{
- struct altera_uart_platform_uart *platp = port->private_data;
-
- writel(dat, port->membase + (reg << platp->bus_shift));
+ writel(dat, port->membase + (reg << port->regshift));
}
static unsigned int altera_uart_tx_empty(struct uart_port *port)
@@ -546,13 +542,17 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
if (!port->membase)
return -ENOMEM;
+ if (platp)
+ port->regshift = platp->bus_shift;
+ else
+ port->regshift = 0;
+
port->line = i;
port->type = PORT_ALTERA_UART;
port->iotype = SERIAL_IO_MEM;
port->uartclk = platp->uartclk;
port->ops = &altera_uart_ops;
port->flags = UPF_BOOT_AUTOCONF;
- port->private_data = platp;
uart_add_one_port(&altera_uart_driver, port);
--
1.7.0.4
With the recent switch of the (currently still out-of-tree) Nios2 Linux
port to devicetree we want to be able to retreive the resources and
properties from dts.
The old method to retreive resources and properties from platform data
is still supported.
Signed-off-by: Tobias Klauser <[email protected]>
---
.../devicetree/bindings/serial/altera_uart.txt | 7 +++
drivers/tty/serial/altera_uart.c | 47 ++++++++++++++++++--
2 files changed, 50 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
diff --git a/Documentation/devicetree/bindings/serial/altera_uart.txt b/Documentation/devicetree/bindings/serial/altera_uart.txt
new file mode 100644
index 0000000..2ab151c
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/altera_uart.txt
@@ -0,0 +1,7 @@
+Altera UART
+
+Required properties:
+- compatible : should be "ALTR,uart-1.0"
+
+Optional properties:
+- clock-frequency : frequency of the clock input to the UART
diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 3a57352..5b46ca7 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -24,6 +24,7 @@
#include <linux/serial.h>
#include <linux/serial_core.h>
#include <linux/platform_device.h>
+#include <linux/of.h>
#include <linux/io.h>
#include <linux/altera_uart.h>
@@ -507,6 +508,34 @@ static struct uart_driver altera_uart_driver = {
.cons = ALTERA_UART_CONSOLE,
};
+#ifdef CONFIG_OF
+static int altera_uart_get_uartclk(struct platform_device *pdev,
+ struct uart_port *port)
+{
+ const __be32 *clk = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
+
+ if (!clk)
+ return -ENODEV;
+
+ port->uartclk = be32_to_cpup(clk);
+
+ return 0;
+}
+#else
+static int altera_uart_get_uartclk(struct platform_device *pdev,
+ struct uart_port *port)
+{
+ struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
+
+ if (!platp)
+ return -ENODEV;
+
+ port->uartclk = platp->uartclk;
+
+ return 0;
+}
+#endif /* CONFIG_OF */
+
static int __devinit altera_uart_probe(struct platform_device *pdev)
{
struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
@@ -514,6 +543,7 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
struct resource *res_mem;
struct resource *res_irq;
int i = pdev->id;
+ int ret;
/* -1 emphasizes that the platform must have one port, no .N suffix */
if (i == -1)
@@ -538,6 +568,10 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
else if (platp->irq)
port->irq = platp->irq;
+ ret = altera_uart_get_uartclk(pdev, port);
+ if (ret)
+ return ret;
+
port->membase = ioremap(port->mapbase, ALTERA_UART_SIZE);
if (!port->membase)
return -ENOMEM;
@@ -550,7 +584,6 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
port->line = i;
port->type = PORT_ALTERA_UART;
port->iotype = SERIAL_IO_MEM;
- port->uartclk = platp->uartclk;
port->ops = &altera_uart_ops;
port->flags = UPF_BOOT_AUTOCONF;
@@ -573,13 +606,19 @@ static int __devexit altera_uart_remove(struct platform_device *pdev)
return 0;
}
+static struct of_device_id altera_uart_match[] = {
+ { .compatible = "altr,uart-1.0", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, altera_uart_match);
+
static struct platform_driver altera_uart_platform_driver = {
.probe = altera_uart_probe,
.remove = __devexit_p(altera_uart_remove),
.driver = {
- .name = DRV_NAME,
- .owner = THIS_MODULE,
- .pm = NULL,
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = altera_uart_match,
},
};
--
1.7.0.4
Signed-off-by: Tobias Klauser <[email protected]>
---
MAINTAINERS | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index ecef0f1..4e86a11 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -465,6 +465,16 @@ M: Matt Turner <[email protected]>
L: [email protected]
F: arch/alpha/
+ALTERA UART/JTAG UART SERIAL DRIVERS
+M: Tobias Klauser <[email protected]>
+L: [email protected]
+L: [email protected] (moderated for non-subscribers)
+S: Maintained
+F: drivers/tty/serial/altera_uart.c
+F: drivers/tty/serial/altera_jtaguart.c
+F: include/linux/altera_uart.h
+F: include/linux/altera_jtaguart.h
+
AMD GEODE CS5536 USB DEVICE CONTROLLER DRIVER
M: Thomas Dahlmann <[email protected]>
L: [email protected] (moderated for non-subscribers)
--
1.7.0.4
On Wed, Feb 09, 2011 at 10:56:52AM +0100, Tobias Klauser wrote:
> Commit 6b5756f176568a710d008d3b478128fafb6707f0 introduced the
> possibility for pdev->id being -1 but the change was not done equally in
> altera_uart_remove. This patch fixes this.
>
> Cc: Anton Vorontsov <[email protected]>
> Signed-off-by: Tobias Klauser <[email protected]>
Acked-by: Anton Vorontsov <[email protected]>
Thanks!
--
Anton Vorontsov
Email: [email protected]
On Wed, Feb 09, 2011 at 10:57:04AM +0100, Tobias Klauser wrote:
> Use the regshift member of struct uart_port to store the address stride
> from platform data. This way we can save one dereference per call of
> altera_uart_readl and altera_uart_writel.
>
> This also allows us to use the driver without platform data, which is
> needed for device tree support in the Nios2 port.
>
> Cc: Anton Vorontsov <[email protected]>
> Signed-off-by: Tobias Klauser <[email protected]>
Acked-by: Anton Vorontsov <[email protected]>
Thanks!
--
Anton Vorontsov
Email: [email protected]
On Wed, Feb 09, 2011 at 10:58:13AM +0100, Tobias Klauser wrote:
> With the recent switch of the (currently still out-of-tree) Nios2 Linux
> port to devicetree we want to be able to retreive the resources and
> properties from dts.
>
> The old method to retreive resources and properties from platform data
> is still supported.
>
> Signed-off-by: Tobias Klauser <[email protected]>
> ---
> .../devicetree/bindings/serial/altera_uart.txt | 7 +++
> drivers/tty/serial/altera_uart.c | 47 ++++++++++++++++++--
> 2 files changed, 50 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
>
> diff --git a/Documentation/devicetree/bindings/serial/altera_uart.txt b/Documentation/devicetree/bindings/serial/altera_uart.txt
> new file mode 100644
> index 0000000..2ab151c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/altera_uart.txt
> @@ -0,0 +1,7 @@
> +Altera UART
> +
> +Required properties:
> +- compatible : should be "ALTR,uart-1.0"
> +
> +Optional properties:
> +- clock-frequency : frequency of the clock input to the UART
> diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
> index 3a57352..5b46ca7 100644
> --- a/drivers/tty/serial/altera_uart.c
> +++ b/drivers/tty/serial/altera_uart.c
> @@ -24,6 +24,7 @@
> #include <linux/serial.h>
> #include <linux/serial_core.h>
> #include <linux/platform_device.h>
> +#include <linux/of.h>
> #include <linux/io.h>
> #include <linux/altera_uart.h>
>
> @@ -507,6 +508,34 @@ static struct uart_driver altera_uart_driver = {
> .cons = ALTERA_UART_CONSOLE,
> };
>
> +#ifdef CONFIG_OF
> +static int altera_uart_get_uartclk(struct platform_device *pdev,
> + struct uart_port *port)
> +{
> + const __be32 *clk = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
> +
> + if (!clk)
> + return -ENODEV;
> +
> + port->uartclk = be32_to_cpup(clk);
> +
> + return 0;
> +}
> +#else
> +static int altera_uart_get_uartclk(struct platform_device *pdev,
> + struct uart_port *port)
> +{
> + struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
> +
> + if (!platp)
> + return -ENODEV;
> +
> + port->uartclk = platp->uartclk;
> +
> + return 0;
> +}
> +#endif /* CONFIG_OF */
This either/or situation isn't really desirable. The driver should be
written so that platform_data works regardless of whether or not
CONFIG_OF is set. So, if the of_node pointer is set, use it to get
the configuration, but fall back to platform data if it is NULL.
> +
> static int __devinit altera_uart_probe(struct platform_device *pdev)
> {
> struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
> @@ -514,6 +543,7 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
> struct resource *res_mem;
> struct resource *res_irq;
> int i = pdev->id;
> + int ret;
>
> /* -1 emphasizes that the platform must have one port, no .N suffix */
> if (i == -1)
> @@ -538,6 +568,10 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
> else if (platp->irq)
> port->irq = platp->irq;
>
> + ret = altera_uart_get_uartclk(pdev, port);
> + if (ret)
> + return ret;
> +
> port->membase = ioremap(port->mapbase, ALTERA_UART_SIZE);
> if (!port->membase)
> return -ENOMEM;
> @@ -550,7 +584,6 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
> port->line = i;
> port->type = PORT_ALTERA_UART;
> port->iotype = SERIAL_IO_MEM;
> - port->uartclk = platp->uartclk;
> port->ops = &altera_uart_ops;
> port->flags = UPF_BOOT_AUTOCONF;
>
> @@ -573,13 +606,19 @@ static int __devexit altera_uart_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static struct of_device_id altera_uart_match[] = {
> + { .compatible = "altr,uart-1.0", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, altera_uart_match);
Need to protect the MODULE_DEVICE_TABLE with #ifdef/#endif. You don't
want to advertise device tree support when CONFIG_OF isn't selected.
> +
> static struct platform_driver altera_uart_platform_driver = {
> .probe = altera_uart_probe,
> .remove = __devexit_p(altera_uart_remove),
> .driver = {
> - .name = DRV_NAME,
> - .owner = THIS_MODULE,
> - .pm = NULL,
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = altera_uart_match,
> },
> };
>
> --
> 1.7.0.4
>
On 2011-02-16 at 05:32:01 +0100, Grant Likely <[email protected]> wrote:
> On Wed, Feb 09, 2011 at 10:58:13AM +0100, Tobias Klauser wrote:
> > With the recent switch of the (currently still out-of-tree) Nios2 Linux
> > port to devicetree we want to be able to retreive the resources and
> > properties from dts.
> >
> > The old method to retreive resources and properties from platform data
> > is still supported.
> >
> > Signed-off-by: Tobias Klauser <[email protected]>
> > ---
> > .../devicetree/bindings/serial/altera_uart.txt | 7 +++
> > drivers/tty/serial/altera_uart.c | 47 ++++++++++++++++++--
> > 2 files changed, 50 insertions(+), 4 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
> >
> > diff --git a/Documentation/devicetree/bindings/serial/altera_uart.txt b/Documentation/devicetree/bindings/serial/altera_uart.txt
> > new file mode 100644
> > index 0000000..2ab151c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/altera_uart.txt
> > @@ -0,0 +1,7 @@
> > +Altera UART
> > +
> > +Required properties:
> > +- compatible : should be "ALTR,uart-1.0"
> > +
> > +Optional properties:
> > +- clock-frequency : frequency of the clock input to the UART
> > diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
> > index 3a57352..5b46ca7 100644
> > --- a/drivers/tty/serial/altera_uart.c
> > +++ b/drivers/tty/serial/altera_uart.c
> > @@ -24,6 +24,7 @@
> > #include <linux/serial.h>
> > #include <linux/serial_core.h>
> > #include <linux/platform_device.h>
> > +#include <linux/of.h>
> > #include <linux/io.h>
> > #include <linux/altera_uart.h>
> >
> > @@ -507,6 +508,34 @@ static struct uart_driver altera_uart_driver = {
> > .cons = ALTERA_UART_CONSOLE,
> > };
> >
> > +#ifdef CONFIG_OF
> > +static int altera_uart_get_uartclk(struct platform_device *pdev,
> > + struct uart_port *port)
> > +{
> > + const __be32 *clk = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
> > +
> > + if (!clk)
> > + return -ENODEV;
> > +
> > + port->uartclk = be32_to_cpup(clk);
> > +
> > + return 0;
> > +}
> > +#else
> > +static int altera_uart_get_uartclk(struct platform_device *pdev,
> > + struct uart_port *port)
> > +{
> > + struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
> > +
> > + if (!platp)
> > + return -ENODEV;
> > +
> > + port->uartclk = platp->uartclk;
> > +
> > + return 0;
> > +}
> > +#endif /* CONFIG_OF */
>
> This either/or situation isn't really desirable. The driver should be
> written so that platform_data works regardless of whether or not
> CONFIG_OF is set. So, if the of_node pointer is set, use it to get
> the configuration, but fall back to platform data if it is NULL.
Agreed, I'll fix this. Though that case will most probably never happen,
we either use device tree data or platform data.
> > +
> > static int __devinit altera_uart_probe(struct platform_device *pdev)
> > {
> > struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
> > @@ -514,6 +543,7 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
> > struct resource *res_mem;
> > struct resource *res_irq;
> > int i = pdev->id;
> > + int ret;
> >
> > /* -1 emphasizes that the platform must have one port, no .N suffix */
> > if (i == -1)
> > @@ -538,6 +568,10 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
> > else if (platp->irq)
> > port->irq = platp->irq;
> >
> > + ret = altera_uart_get_uartclk(pdev, port);
> > + if (ret)
> > + return ret;
> > +
> > port->membase = ioremap(port->mapbase, ALTERA_UART_SIZE);
> > if (!port->membase)
> > return -ENOMEM;
> > @@ -550,7 +584,6 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
> > port->line = i;
> > port->type = PORT_ALTERA_UART;
> > port->iotype = SERIAL_IO_MEM;
> > - port->uartclk = platp->uartclk;
> > port->ops = &altera_uart_ops;
> > port->flags = UPF_BOOT_AUTOCONF;
> >
> > @@ -573,13 +606,19 @@ static int __devexit altera_uart_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +static struct of_device_id altera_uart_match[] = {
> > + { .compatible = "altr,uart-1.0", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, altera_uart_match);
>
> Need to protect the MODULE_DEVICE_TABLE with #ifdef/#endif. You don't
> want to advertise device tree support when CONFIG_OF isn't selected.
Shall I put the #ifdef around the whole table and define it as NULL if
CONFIG_OF is not defined - like this:
#ifdef CONFIG_OF
static struct of_device_id altera_uart_match[] = {
{ .compatible = "altr,uart-1.0", },
{},
};
MODULE_DEVICE_TABLE(of, altera_uart_match);
#else
#define altera_uart_match NULL
#endif /* CONFIG_OF */
or will it be sufficient to just #ifdef the MODULE_DEVICE_TABLE:
static struct of_device_id altera_uart_match[] = {
{ .compatible = "altr,uart-1.0", },
{},
};
#ifdef CONFIG_OF
MODULE_DEVICE_TABLE(of, altera_uart_match);
#endif
> > +
> > static struct platform_driver altera_uart_platform_driver = {
> > .probe = altera_uart_probe,
> > .remove = __devexit_p(altera_uart_remove),
> > .driver = {
> > - .name = DRV_NAME,
> > - .owner = THIS_MODULE,
> > - .pm = NULL,
> > + .name = DRV_NAME,
> > + .owner = THIS_MODULE,
> > + .of_match_table = altera_uart_match,
> > },
> > };
> >
> > --
> > 1.7.0.4
> >
>
On Wed, Feb 16, 2011 at 08:43:21AM +0100, Tobias Klauser wrote:
> On 2011-02-16 at 05:32:01 +0100, Grant Likely <[email protected]> wrote:
> > On Wed, Feb 09, 2011 at 10:58:13AM +0100, Tobias Klauser wrote:
> > > With the recent switch of the (currently still out-of-tree) Nios2 Linux
> > > port to devicetree we want to be able to retreive the resources and
> > > properties from dts.
> > >
> > > The old method to retreive resources and properties from platform data
> > > is still supported.
> > >
> > > Signed-off-by: Tobias Klauser <[email protected]>
> > > ---
[...]
> > > +static struct of_device_id altera_uart_match[] = {
> > > + { .compatible = "altr,uart-1.0", },
> > > + {},
> > > +};
> > > +MODULE_DEVICE_TABLE(of, altera_uart_match);
> >
> > Need to protect the MODULE_DEVICE_TABLE with #ifdef/#endif. You don't
> > want to advertise device tree support when CONFIG_OF isn't selected.
>
> Shall I put the #ifdef around the whole table and define it as NULL if
> CONFIG_OF is not defined - like this:
>
> #ifdef CONFIG_OF
> static struct of_device_id altera_uart_match[] = {
> { .compatible = "altr,uart-1.0", },
> {},
> };
> MODULE_DEVICE_TABLE(of, altera_uart_match);
> #else
> #define altera_uart_match NULL
> #endif /* CONFIG_OF */
>
> or will it be sufficient to just #ifdef the MODULE_DEVICE_TABLE:
>
> static struct of_device_id altera_uart_match[] = {
> { .compatible = "altr,uart-1.0", },
> {},
> };
> #ifdef CONFIG_OF
> MODULE_DEVICE_TABLE(of, altera_uart_match);
> #endif
Either is fine, but the first will have a smaller memory footprint
when CONFIG_OF is deselected.
g.
With the recent switch of the (currently still out-of-tree) Nios2 Linux
port to devicetree we want to be able to retreive the resources and
properties from dts.
The old method to retreive resources and properties from platform data
is still supported.
Cc: Grant Likely <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
---
Thanks to Grant Likely for the review.
changes in v2:
- fall back to uartclk from platform data if the device tree property
is not available.
- Only include MODULE_DEVICE_TABLE if CONFIG_OF is set so we don't
advertise device tree support if CONFIG_OF isn't active.
- change vendor prefix in match table to be uppercase for consistency
with documentation
.../devicetree/bindings/serial/altera_uart.txt | 7 +++
drivers/tty/serial/altera_uart.c | 50 ++++++++++++++++++--
2 files changed, 53 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
diff --git a/Documentation/devicetree/bindings/serial/altera_uart.txt b/Documentation/devicetree/bindings/serial/altera_uart.txt
new file mode 100644
index 0000000..71cae3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/altera_uart.txt
@@ -0,0 +1,7 @@
+Altera UART
+
+Required properties:
+- compatible : should be "ALTR,uart-1.0"
+
+Optional properties:
+- clock-frequency : frequency of the clock input to the UART
diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 5e80977..4f0898c 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -24,6 +24,7 @@
#include <linux/serial.h>
#include <linux/serial_core.h>
#include <linux/platform_device.h>
+#include <linux/of.h>
#include <linux/io.h>
#include <linux/altera_uart.h>
@@ -484,6 +485,29 @@ static struct uart_driver altera_uart_driver = {
.cons = ALTERA_UART_CONSOLE,
};
+#ifdef CONFIG_OF
+static int altera_uart_get_of_uartclk(struct platform_device *pdev,
+ struct uart_port *port)
+{
+ int len;
+ const __be32 *clk;
+
+ clk = of_get_property(pdev->dev.of_node, "clock-frequency", &len);
+ if (!clk || len < sizeof(__be32))
+ return -ENODEV;
+
+ port->uartclk = be32_to_cpup(clk);
+
+ return 0;
+}
+#else
+static int altera_uart_get_of_uartclk(struct platform_device *pdev,
+ struct uart_port *port)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
static int __devinit altera_uart_probe(struct platform_device *pdev)
{
struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
@@ -491,6 +515,7 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
struct resource *res_mem;
struct resource *res_irq;
int i = pdev->id;
+ int ret;
/* -1 emphasizes that the platform must have one port, no .N suffix */
if (i == -1)
@@ -515,6 +540,14 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
else if (platp->irq)
port->irq = platp->irq;
+ /* Try to get the uartclk from devicetree, fall back to platform data
+ * otherwise */
+ ret = altera_uart_get_of_uartclk(pdev, port);
+ if (ret && platp)
+ port->uartclk = platp->uartclk;
+ else if (ret)
+ return ret;
+
port->membase = ioremap(port->mapbase, ALTERA_UART_SIZE);
if (!port->membase)
return -ENOMEM;
@@ -527,7 +560,6 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
port->line = i;
port->type = PORT_ALTERA_UART;
port->iotype = SERIAL_IO_MEM;
- port->uartclk = platp->uartclk;
port->ops = &altera_uart_ops;
port->flags = UPF_BOOT_AUTOCONF;
@@ -550,13 +582,23 @@ static int __devexit altera_uart_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_OF
+static struct of_device_id altera_uart_match[] = {
+ { .compatible = "ALTR,uart-1.0", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, altera_uart_match);
+#else
+#define altera_uart_match NULL
+#endif /* CONFIG_OF */
+
static struct platform_driver altera_uart_platform_driver = {
.probe = altera_uart_probe,
.remove = __devexit_p(altera_uart_remove),
.driver = {
- .name = DRV_NAME,
- .owner = THIS_MODULE,
- .pm = NULL,
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = altera_uart_match,
},
};
--
1.7.0.4
Hi Tobias,
On 02/17/2011 12:12 AM, Tobias Klauser wrote:
> With the recent switch of the (currently still out-of-tree) Nios2 Linux
> port to devicetree we want to be able to retreive the resources and
> properties from dts.
>
> The old method to retreive resources and properties from platform data
> is still supported.
>
> Cc: Grant Likely<[email protected]>
> Signed-off-by: Tobias Klauser<[email protected]>
> ---
> Thanks to Grant Likely for the review.
>
> changes in v2:
> - fall back to uartclk from platform data if the device tree property
> is not available.
> - Only include MODULE_DEVICE_TABLE if CONFIG_OF is set so we don't
> advertise device tree support if CONFIG_OF isn't active.
> - change vendor prefix in match table to be uppercase for consistency
> with documentation
>
> .../devicetree/bindings/serial/altera_uart.txt | 7 +++
> drivers/tty/serial/altera_uart.c | 50 ++++++++++++++++++--
> 2 files changed, 53 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
>
> diff --git a/Documentation/devicetree/bindings/serial/altera_uart.txt b/Documentation/devicetree/bindings/serial/altera_uart.txt
> new file mode 100644
> index 0000000..71cae3f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/altera_uart.txt
> @@ -0,0 +1,7 @@
> +Altera UART
> +
> +Required properties:
> +- compatible : should be "ALTR,uart-1.0"
> +
> +Optional properties:
> +- clock-frequency : frequency of the clock input to the UART
> diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
> index 5e80977..4f0898c 100644
> --- a/drivers/tty/serial/altera_uart.c
> +++ b/drivers/tty/serial/altera_uart.c
> @@ -24,6 +24,7 @@
> #include<linux/serial.h>
> #include<linux/serial_core.h>
> #include<linux/platform_device.h>
> +#include<linux/of.h>
> #include<linux/io.h>
> #include<linux/altera_uart.h>
>
> @@ -484,6 +485,29 @@ static struct uart_driver altera_uart_driver = {
> .cons = ALTERA_UART_CONSOLE,
> };
>
> +#ifdef CONFIG_OF
> +static int altera_uart_get_of_uartclk(struct platform_device *pdev,
> + struct uart_port *port)
> +{
> + int len;
> + const __be32 *clk;
> +
> + clk = of_get_property(pdev->dev.of_node, "clock-frequency",&len);
> + if (!clk || len< sizeof(__be32))
> + return -ENODEV;
> +
> + port->uartclk = be32_to_cpup(clk);
> +
> + return 0;
> +}
> +#else
> +static int altera_uart_get_of_uartclk(struct platform_device *pdev,
> + struct uart_port *port)
> +{
> + return -ENODEV;
> +}
> +#endif /* CONFIG_OF */
> +
> static int __devinit altera_uart_probe(struct platform_device *pdev)
> {
> struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
> @@ -491,6 +515,7 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
> struct resource *res_mem;
> struct resource *res_irq;
> int i = pdev->id;
> + int ret;
>
> /* -1 emphasizes that the platform must have one port, no .N suffix */
> if (i == -1)
> @@ -515,6 +540,14 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
> else if (platp->irq)
> port->irq = platp->irq;
>
> + /* Try to get the uartclk from devicetree, fall back to platform data
> + * otherwise */
Please follow multi-line comment style.
/*
* xxxx
* xxxx
*/
> + ret = altera_uart_get_of_uartclk(pdev, port);
> + if (ret&& platp)
> + port->uartclk = platp->uartclk;
> + else if (ret)
> + return ret;
> +
Better reverse the priority, with platform data checked first.
if (platp)
port->uartclk = platp->uartclk;
else {
ret = altera_uart_get_of_uartclk(pdev, port);
if (ret)
return ret;
}
> port->membase = ioremap(port->mapbase, ALTERA_UART_SIZE);
> if (!port->membase)
> return -ENOMEM;
> @@ -527,7 +560,6 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
> port->line = i;
> port->type = PORT_ALTERA_UART;
> port->iotype = SERIAL_IO_MEM;
> - port->uartclk = platp->uartclk;
> port->ops =&altera_uart_ops;
> port->flags = UPF_BOOT_AUTOCONF;
>
> @@ -550,13 +582,23 @@ static int __devexit altera_uart_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static struct of_device_id altera_uart_match[] = {
> + { .compatible = "ALTR,uart-1.0", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, altera_uart_match);
> +#else
> +#define altera_uart_match NULL
> +#endif /* CONFIG_OF */
> +
> static struct platform_driver altera_uart_platform_driver = {
> .probe = altera_uart_probe,
> .remove = __devexit_p(altera_uart_remove),
> .driver = {
> - .name = DRV_NAME,
> - .owner = THIS_MODULE,
> - .pm = NULL,
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = altera_uart_match,
> },
> };
>
Regards,
Thomas
On 02/09/2011 05:58 PM, Tobias Klauser wrote:
> Signed-off-by: Tobias Klauser<[email protected]>
> ---
> MAINTAINERS | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ecef0f1..4e86a11 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -465,6 +465,16 @@ M: Matt Turner<[email protected]>
> L: [email protected]
> F: arch/alpha/
>
> +ALTERA UART/JTAG UART SERIAL DRIVERS
> +M: Tobias Klauser<[email protected]>
> +L: [email protected]
> +L: [email protected] (moderated for non-subscribers)
> +S: Maintained
> +F: drivers/tty/serial/altera_uart.c
> +F: drivers/tty/serial/altera_jtaguart.c
> +F: include/linux/altera_uart.h
> +F: include/linux/altera_jtaguart.h
> +
> AMD GEODE CS5536 USB DEVICE CONTROLLER DRIVER
> M: Thomas Dahlmann<[email protected]>
> L: [email protected] (moderated for non-subscribers)
Hi Tobias,
As we get more drivers merged into mainline, we might consider adding a
general entry for all altera related drivers to our mailing list. We
could add "F: arch/nios2" to this entry in the near future.
1) subject: MAINTAINERS: add Altera related drivers entry
send this to Andrew Morton.
ALTERA EMBEDDED ARCH AND PERIPHERALS DRIVERS
L: [email protected] (moderated for non-subscribers)
S: Maintained
K: altera
2) subject: MAINTAINERS: add Altera UART/JTAG UART maintainer
ALTERA UART/JTAG UART SERIAL DRIVERS
M: Tobias Klauser <[email protected]>
L: [email protected]
S: Maintained
F: drivers/tty/serial/altera_uart.c
F: drivers/tty/serial/altera_jtaguart.c
F: include/linux/altera_uart.h
F: include/linux/altera_jtaguart.h
Regards,
Thomas
Hi Thomas,
Thanks a lot for your comments.
On 2011-02-17 at 03:24:01 +0100, Thomas Chou <[email protected]> wrote:
> On 02/17/2011 12:12 AM, Tobias Klauser wrote:
>> With the recent switch of the (currently still out-of-tree) Nios2 Linux
>> port to devicetree we want to be able to retreive the resources and
>> properties from dts.
>>
>> The old method to retreive resources and properties from platform data
>> is still supported.
>>
>> Cc: Grant Likely<[email protected]>
>> Signed-off-by: Tobias Klauser<[email protected]>
>> ---
>> Thanks to Grant Likely for the review.
>>
>> changes in v2:
>> - fall back to uartclk from platform data if the device tree property
>> is not available.
>> - Only include MODULE_DEVICE_TABLE if CONFIG_OF is set so we don't
>> advertise device tree support if CONFIG_OF isn't active.
>> - change vendor prefix in match table to be uppercase for consistency
>> with documentation
>>
>> .../devicetree/bindings/serial/altera_uart.txt | 7 +++
>> drivers/tty/serial/altera_uart.c | 50 ++++++++++++++++++--
>> 2 files changed, 53 insertions(+), 4 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
>>
>> diff --git a/Documentation/devicetree/bindings/serial/altera_uart.txt b/Documentation/devicetree/bindings/serial/altera_uart.txt
>> new file mode 100644
>> index 0000000..71cae3f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/altera_uart.txt
>> @@ -0,0 +1,7 @@
>> +Altera UART
>> +
>> +Required properties:
>> +- compatible : should be "ALTR,uart-1.0"
>> +
>> +Optional properties:
>> +- clock-frequency : frequency of the clock input to the UART
>> diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
>> index 5e80977..4f0898c 100644
>> --- a/drivers/tty/serial/altera_uart.c
>> +++ b/drivers/tty/serial/altera_uart.c
>> @@ -24,6 +24,7 @@
>> #include<linux/serial.h>
>> #include<linux/serial_core.h>
>> #include<linux/platform_device.h>
>> +#include<linux/of.h>
>> #include<linux/io.h>
>> #include<linux/altera_uart.h>
>>
>> @@ -484,6 +485,29 @@ static struct uart_driver altera_uart_driver = {
>> .cons = ALTERA_UART_CONSOLE,
>> };
>>
>> +#ifdef CONFIG_OF
>> +static int altera_uart_get_of_uartclk(struct platform_device *pdev,
>> + struct uart_port *port)
>> +{
>> + int len;
>> + const __be32 *clk;
>> +
>> + clk = of_get_property(pdev->dev.of_node, "clock-frequency",&len);
>> + if (!clk || len< sizeof(__be32))
>> + return -ENODEV;
>> +
>> + port->uartclk = be32_to_cpup(clk);
>> +
>> + return 0;
>> +}
>> +#else
>> +static int altera_uart_get_of_uartclk(struct platform_device *pdev,
>> + struct uart_port *port)
>> +{
>> + return -ENODEV;
>> +}
>> +#endif /* CONFIG_OF */
>> +
>> static int __devinit altera_uart_probe(struct platform_device *pdev)
>> {
>> struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
>> @@ -491,6 +515,7 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
>> struct resource *res_mem;
>> struct resource *res_irq;
>> int i = pdev->id;
>> + int ret;
>>
>> /* -1 emphasizes that the platform must have one port, no .N suffix */
>> if (i == -1)
>> @@ -515,6 +540,14 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
>> else if (platp->irq)
>> port->irq = platp->irq;
>>
>> + /* Try to get the uartclk from devicetree, fall back to platform data
>> + * otherwise */
>
> Please follow multi-line comment style.
>
> /*
> * xxxx
> * xxxx
> */
I will fix this.
>> + ret = altera_uart_get_of_uartclk(pdev, port);
>> + if (ret&& platp)
>> + port->uartclk = platp->uartclk;
>> + else if (ret)
>> + return ret;
>> +
>
> Better reverse the priority, with platform data checked first.
>
> if (platp)
> port->uartclk = platp->uartclk;
> else {
> ret = altera_uart_get_of_uartclk(pdev, port);
> if (ret)
> return ret;
> }
Do you have a specific reasoning for this? I thought it might make sense
to do it in the same order as with the resources above, but I have no
problem changing it to the way you suggest.
>
>> port->membase = ioremap(port->mapbase, ALTERA_UART_SIZE);
>> if (!port->membase)
>> return -ENOMEM;
>> @@ -527,7 +560,6 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
>> port->line = i;
>> port->type = PORT_ALTERA_UART;
>> port->iotype = SERIAL_IO_MEM;
>> - port->uartclk = platp->uartclk;
>> port->ops =&altera_uart_ops;
>> port->flags = UPF_BOOT_AUTOCONF;
>>
>> @@ -550,13 +582,23 @@ static int __devexit altera_uart_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_OF
>> +static struct of_device_id altera_uart_match[] = {
>> + { .compatible = "ALTR,uart-1.0", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, altera_uart_match);
>> +#else
>> +#define altera_uart_match NULL
>> +#endif /* CONFIG_OF */
>> +
>> static struct platform_driver altera_uart_platform_driver = {
>> .probe = altera_uart_probe,
>> .remove = __devexit_p(altera_uart_remove),
>> .driver = {
>> - .name = DRV_NAME,
>> - .owner = THIS_MODULE,
>> - .pm = NULL,
>> + .name = DRV_NAME,
>> + .owner = THIS_MODULE,
>> + .of_match_table = altera_uart_match,
>> },
>> };
Cheers
Tobias
Hi Thomas
On 2011-02-17 at 04:16:36 +0100, Thomas Chou <[email protected]> wrote:
> On 02/09/2011 05:58 PM, Tobias Klauser wrote:
>> Signed-off-by: Tobias Klauser<[email protected]>
>> ---
>> MAINTAINERS | 10 ++++++++++
>> 1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ecef0f1..4e86a11 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -465,6 +465,16 @@ M: Matt Turner<[email protected]>
>> L: [email protected]
>> F: arch/alpha/
>>
>> +ALTERA UART/JTAG UART SERIAL DRIVERS
>> +M: Tobias Klauser<[email protected]>
>> +L: [email protected]
>> +L: [email protected] (moderated for non-subscribers)
>> +S: Maintained
>> +F: drivers/tty/serial/altera_uart.c
>> +F: drivers/tty/serial/altera_jtaguart.c
>> +F: include/linux/altera_uart.h
>> +F: include/linux/altera_jtaguart.h
>> +
>> AMD GEODE CS5536 USB DEVICE CONTROLLER DRIVER
>> M: Thomas Dahlmann<[email protected]>
>> L: [email protected] (moderated for non-subscribers)
>
> Hi Tobias,
>
> As we get more drivers merged into mainline, we might consider adding a
> general entry for all altera related drivers to our mailing list. We
> could add "F: arch/nios2" to this entry in the near future.
That's great. I didn't know about the K: section entry.
I will send a patch as below.
> 1) subject: MAINTAINERS: add Altera related drivers entry
> send this to Andrew Morton.
>
> ALTERA EMBEDDED ARCH AND PERIPHERALS DRIVERS
> L: [email protected] (moderated for non-subscribers)
> S: Maintained
> K: altera
>
> 2) subject: MAINTAINERS: add Altera UART/JTAG UART maintainer
>
> ALTERA UART/JTAG UART SERIAL DRIVERS
> M: Tobias Klauser <[email protected]>
> L: [email protected]
> S: Maintained
> F: drivers/tty/serial/altera_uart.c
> F: drivers/tty/serial/altera_jtaguart.c
> F: include/linux/altera_uart.h
> F: include/linux/altera_jtaguart.h
Thanks
Tobias
On Thursday 17 February 2011, Thomas Chou wrote:
> As we get more drivers merged into mainline, we might consider adding a
> general entry for all altera related drivers to our mailing list. We
> could add "F: arch/nios2" to this entry in the near future.
On a related topic, are there plans to submit the nios2 arch code
for review any time soon?
I'm usually reviewing all architectures that come in, and unicore32
is almost ready to go in, so there is room to start reviewing the next
one.
I expect nios2, lm32 and c64x to be submitted this year, but I'll
probably only be able to review one at time. First come, first
serve.
Arnd
Hi Arnd,
On 2011-02-17 at 19:02:41 +0100, Arnd Bergmann <[email protected]> wrote:
> On Thursday 17 February 2011, Thomas Chou wrote:
> > As we get more drivers merged into mainline, we might consider adding a
> > general entry for all altera related drivers to our mailing list. We
> > could add "F: arch/nios2" to this entry in the near future.
>
> On a related topic, are there plans to submit the nios2 arch code
> for review any time soon?
I'm currently in contact with Altera on exactly this matter. We're
planning to submit the nios2 architecture code for review really soon
now.
> I'm usually reviewing all architectures that come in, and unicore32
> is almost ready to go in, so there is room to start reviewing the next
> one.
That's great to hear. I followed the submission and review of unicore32
a bit and read some of your replies on how to proceed. I already learned
a lot from that and will try to apply it for the nios2 submission.
> I expect nios2, lm32 and c64x to be submitted this year, but I'll
> probably only be able to review one at time. First come, first
> serve.
So I hope we'll be the first ones then :-)
Thanks a lot for your notice
Tobias
On Wed, Feb 09, 2011 at 10:55:28AM +0100, Tobias Klauser wrote:
> This patchset adds some updates to the altera_uart taken from the Nios2
> Linux port (currently out-of-tree) repository, the most important one
> probably being the added devicetree support.
>
> Tobias Klauser (4):
> tty: serial: altera_uart: Handle pdev->id == -1 in altera_uart_remove
> tty: serial: altera_uart: Use port->regshift to store bus shift
> tty: serial: altera_uart: Add devicetree support
> MAINTAINERS: Add myself as a maintainer for
> altera_uart/altera_jtaguart
I've applied 3 of these patches, skipping the device tree patch, as it
looks like you are going to rework it.
thanks,
greg k-h
On 2011-02-17 at 20:32:55 +0100, Greg KH <[email protected]> wrote:
> On Wed, Feb 09, 2011 at 10:55:28AM +0100, Tobias Klauser wrote:
> > This patchset adds some updates to the altera_uart taken from the Nios2
> > Linux port (currently out-of-tree) repository, the most important one
> > probably being the added devicetree support.
> >
> > Tobias Klauser (4):
> > tty: serial: altera_uart: Handle pdev->id == -1 in altera_uart_remove
> > tty: serial: altera_uart: Use port->regshift to store bus shift
> > tty: serial: altera_uart: Add devicetree support
> > MAINTAINERS: Add myself as a maintainer for
> > altera_uart/altera_jtaguart
>
> I've applied 3 of these patches, skipping the device tree patch, as it
> looks like you are going to rework it.
Yup, I'm currently waiting for a reply from Thomas and will update it
accordingly then. I'll send the updated patch later on (hopefully with
correct subject line this time).
Thanks a lot
Tobias
On 02/17/2011 03:48 PM, Tobias Klauser wrote:
>>> + ret = altera_uart_get_of_uartclk(pdev, port);
>>> + if (ret&& platp)
>>> + port->uartclk = platp->uartclk;
>>> + else if (ret)
>>> + return ret;
>>> +
>>
>> Better reverse the priority, with platform data checked first.
>>
>> if (platp)
>> port->uartclk = platp->uartclk;
>> else {
>> ret = altera_uart_get_of_uartclk(pdev, port);
>> if (ret)
>> return ret;
>> }
>
> Do you have a specific reasoning for this? I thought it might make sense
> to do it in the same order as with the resources above, but I have no
> problem changing it to the way you suggest.
Not quite sure. But I see some drivers follow this order, and I just
followed, too.
- Thomas
On Thu, Feb 17, 2011 at 6:28 PM, Thomas Chou <[email protected]> wrote:
> On 02/17/2011 03:48 PM, Tobias Klauser wrote:
>>>>
>>>> + ret = altera_uart_get_of_uartclk(pdev, port);
>>>> + if (ret&& platp)
>>>> + port->uartclk = platp->uartclk;
>>>> + else if (ret)
>>>> + return ret;
>>>> +
>>>
>>> Better reverse the priority, with platform data checked first.
>>>
>>> if (platp)
>>> port->uartclk = platp->uartclk;
>>> else {
>>> ret = altera_uart_get_of_uartclk(pdev, port);
>>> if (ret)
>>> return ret;
>>> }
>>
>> Do you have a specific reasoning for this? I thought it might make sense
>> to do it in the same order as with the resources above, but I have no
>> problem changing it to the way you suggest.
>
> Not quite sure. But I see some drivers follow this order, and I just
> followed, too.
The reason to check for platform_data first is that if a device has
*both* platform data and a device node pointer, then more than likely
the platform_data is indented to override the device node data.
g.
On 2011-02-18 at 09:08:53 +0100, Grant Likely <[email protected]> wrote:
> On Thu, Feb 17, 2011 at 6:28 PM, Thomas Chou <[email protected]> wrote:
> > On 02/17/2011 03:48 PM, Tobias Klauser wrote:
> >>>>
> >>>> + ret = altera_uart_get_of_uartclk(pdev, port);
> >>>> + if (ret&& platp)
> >>>> + port->uartclk = platp->uartclk;
> >>>> + else if (ret)
> >>>> + return ret;
> >>>> +
> >>>
> >>> Better reverse the priority, with platform data checked first.
> >>>
> >>> if (platp)
> >>> port->uartclk = platp->uartclk;
> >>> else {
> >>> ret = altera_uart_get_of_uartclk(pdev, port);
> >>> if (ret)
> >>> return ret;
> >>> }
> >>
> >> Do you have a specific reasoning for this? I thought it might make sense
> >> to do it in the same order as with the resources above, but I have no
> >> problem changing it to the way you suggest.
> >
> > Not quite sure. But I see some drivers follow this order, and I just
> > followed, too.
>
> The reason to check for platform_data first is that if a device has
> *both* platform data and a device node pointer, then more than likely
> the platform_data is indented to override the device node data.
Thanks Grant and Thomas for the explanation. I was just wondering. I'll
send an updated patch including the changes suggested by Thomas.
Tobias
With the recent switch of the (currently still out-of-tree) Nios2 Linux
port to devicetree we want to be able to retreive the resources and
properties from dts.
The old method to retreive resources and properties from platform data
is still supported.
Cc: Grant Likely <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
---
Greg: This patch will have the same compile problem in your tree as the
altera_jtaguart patch has. As this patch depends on the previous ones of
this series it might be best to apply it once commit c9e358dfc
"driver-core: remove conditionals around devicetree pointers" from
Grant's tree is merged.
Thanks to Grant Likely and Thomas Chou for their comments.
v3:
- Change order for getting uartclk.
v2:
- fall back to uartclk from platform data if the device tree property
is not available.
- Only include MODULE_DEVICE_TABLE if CONFIG_OF is set so we don't
advertise device tree support if CONFIG_OF isn't active.
- change vendor prefix in match table to be uppercase for consistency
with documentation
.../devicetree/bindings/serial/altera_uart.txt | 7 +++
drivers/tty/serial/altera_uart.c | 51 ++++++++++++++++++--
2 files changed, 54 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/altera_uart.txt
diff --git a/Documentation/devicetree/bindings/serial/altera_uart.txt b/Documentation/devicetree/bindings/serial/altera_uart.txt
new file mode 100644
index 0000000..71cae3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/altera_uart.txt
@@ -0,0 +1,7 @@
+Altera UART
+
+Required properties:
+- compatible : should be "ALTR,uart-1.0"
+
+Optional properties:
+- clock-frequency : frequency of the clock input to the UART
diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 5e80977..6a1ebd9 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -24,6 +24,7 @@
#include <linux/serial.h>
#include <linux/serial_core.h>
#include <linux/platform_device.h>
+#include <linux/of.h>
#include <linux/io.h>
#include <linux/altera_uart.h>
@@ -484,6 +485,29 @@ static struct uart_driver altera_uart_driver = {
.cons = ALTERA_UART_CONSOLE,
};
+#ifdef CONFIG_OF
+static int altera_uart_get_of_uartclk(struct platform_device *pdev,
+ struct uart_port *port)
+{
+ int len;
+ const __be32 *clk;
+
+ clk = of_get_property(pdev->dev.of_node, "clock-frequency", &len);
+ if (!clk || len < sizeof(__be32))
+ return -ENODEV;
+
+ port->uartclk = be32_to_cpup(clk);
+
+ return 0;
+}
+#else
+static int altera_uart_get_of_uartclk(struct platform_device *pdev,
+ struct uart_port *port)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
static int __devinit altera_uart_probe(struct platform_device *pdev)
{
struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
@@ -491,6 +515,7 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
struct resource *res_mem;
struct resource *res_irq;
int i = pdev->id;
+ int ret;
/* -1 emphasizes that the platform must have one port, no .N suffix */
if (i == -1)
@@ -515,6 +540,15 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
else if (platp->irq)
port->irq = platp->irq;
+ /* Check platform data first so we can override device node data */
+ if (platp)
+ port->uartclk = platp->uartclk;
+ else {
+ ret = altera_uart_get_of_uartclk(pdev, port);
+ if (ret)
+ return ret;
+ }
+
port->membase = ioremap(port->mapbase, ALTERA_UART_SIZE);
if (!port->membase)
return -ENOMEM;
@@ -527,7 +561,6 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
port->line = i;
port->type = PORT_ALTERA_UART;
port->iotype = SERIAL_IO_MEM;
- port->uartclk = platp->uartclk;
port->ops = &altera_uart_ops;
port->flags = UPF_BOOT_AUTOCONF;
@@ -550,13 +583,23 @@ static int __devexit altera_uart_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_OF
+static struct of_device_id altera_uart_match[] = {
+ { .compatible = "ALTR,uart-1.0", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, altera_uart_match);
+#else
+#define altera_uart_match NULL
+#endif /* CONFIG_OF */
+
static struct platform_driver altera_uart_platform_driver = {
.probe = altera_uart_probe,
.remove = __devexit_p(altera_uart_remove),
.driver = {
- .name = DRV_NAME,
- .owner = THIS_MODULE,
- .pm = NULL,
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = altera_uart_match,
},
};
--
1.7.0.4
On Fri, Feb 18, 2011 at 11:35:32AM +0100, Tobias Klauser wrote:
> With the recent switch of the (currently still out-of-tree) Nios2 Linux
> port to devicetree we want to be able to retreive the resources and
> properties from dts.
>
> The old method to retreive resources and properties from platform data
> is still supported.
>
> Cc: Grant Likely <[email protected]>
> Signed-off-by: Tobias Klauser <[email protected]>
> ---
>
> Greg: This patch will have the same compile problem in your tree as the
> altera_jtaguart patch has. As this patch depends on the previous ones of
> this series it might be best to apply it once commit c9e358dfc
> "driver-core: remove conditionals around devicetree pointers" from
> Grant's tree is merged.
Ok, I can't take this then, Grant, can you?
Please feel free to add:
Acked-by: Greg Kroah-Hartman <[email protected]>
to it.
thanks,
greg k-h
On Fri, Feb 25, 2011 at 09:58:56AM -0800, Greg KH wrote:
> On Fri, Feb 18, 2011 at 11:35:32AM +0100, Tobias Klauser wrote:
> > With the recent switch of the (currently still out-of-tree) Nios2 Linux
> > port to devicetree we want to be able to retreive the resources and
> > properties from dts.
> >
> > The old method to retreive resources and properties from platform data
> > is still supported.
> >
> > Cc: Grant Likely <[email protected]>
> > Signed-off-by: Tobias Klauser <[email protected]>
> > ---
> >
> > Greg: This patch will have the same compile problem in your tree as the
> > altera_jtaguart patch has. As this patch depends on the previous ones of
> > this series it might be best to apply it once commit c9e358dfc
> > "driver-core: remove conditionals around devicetree pointers" from
> > Grant's tree is merged.
>
> Ok, I can't take this then, Grant, can you?
>
> Please feel free to add:
> Acked-by: Greg Kroah-Hartman <[email protected]>
> to it.
Merged, thanks,
g.