[PATCH V2 0/2] serial: bcm2835: add bcm2835 auxiliar uart driver

[PATCH V2 0/2] serial: bcm2835: add bcm2835 auxiliar uart driver

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

This patch series adds the bcm2835aux uart driver, which is created
as a wrapper arround the 8250 driver (based on recommendations by
Stephen Warren)

The device tree patch itself is not included yet because this
patch requires the clk-bcm2835aux driver, which is right now only
in clk-next, but should become available with 4.5.

Martin Sperl (2):
  dt/bindings: serial: bcm2835: add binding documentation for
    bcm2835-aux-uart
  serial: bcm2835: add driver for bcm2835-aux-uart

 .../bindings/serial/brcm,bcm2835-aux-uart.txt      |   18 +++
 drivers/tty/serial/8250/8250_bcm2835aux.c          |  143 ++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig                    |    8 ++
 drivers/tty/serial/8250/Makefile                   |    1 +
 4 files changed, 170 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/brcm,bcm2835-aux-uart.txt
 create mode 100644 drivers/tty/serial/8250/8250_bcm2835aux.c

-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 1/2] dt/bindings: serial: bcm2835: add binding documentation for bcm2835-aux-uart

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Add binding documentation for the bcm2835-aux-uart driver.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 .../bindings/serial/brcm,bcm2835-aux-uart.txt        |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/brcm,bcm2835-aux-uart.txt

diff --git a/Documentation/devicetree/bindings/serial/brcm,bcm2835-aux-uart.txt b/Documentation/devicetree/bindings/serial/brcm,bcm2835-aux-uart.txt
new file mode 100644
index 0000000..2defe5c
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/brcm,bcm2835-aux-uart.txt
@@ -0,0 +1,18 @@
+* BCM2835 AUXILIAR UART
+
+Required properties:
+
+- compatible: "brcm,bcm2835-aux-uart"
+- reg: The base address of the UART register bank.
+- interrupts: A single interrupt specifier.
+- clocks: Clock driving the hardware; used to figure out the baud rate
+  divisor.
+
+Example:
+
+	uart1: uart@7e215040 {
+		compatible = "brcm,bcm2835-aux-uart";
+		reg = <0x7e215040 0x40>;
+		interrupts = <1 29>;
+		clocks = <&aux BCM2835_AUX_CLOCK_UART>;
+	};
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/2] dt/bindings: serial: bcm2835: add binding documentation for bcm2835-aux-uart

[Arnd Bergmann]

On Saturday 09 January 2016 13:07:32 kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> +
> +       uart1: uart@7e215040 {
> +               compatible = "brcm,bcm2835-aux-uart";
> +               reg = <0x7e215040 0x40>;
> +               interrupts = <1 29>;
> +               clocks = <&aux BCM2835_AUX_CLOCK_UART>;
> +       };
> 

The node name should always be "serial@...", not "uart@..."

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/2] dt/bindings: serial: bcm2835: add binding documentation for bcm2835-aux-uart

[Martin Sperl]

> On 09.01.2016, at 21:53, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> 
> On Saturday 09 January 2016 13:07:32 kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>> +
>> +       uart1: uart@7e215040 {
>> +               compatible = "brcm,bcm2835-aux-uart";
>> +               reg = <0x7e215040 0x40>;
>> +               interrupts = <1 29>;
>> +               clocks = <&aux BCM2835_AUX_CLOCK_UART>;
>> +       };
>> 
> 
> The node name should always be "serial@...", not "uart@..."
> 
> 	Arnd

The existing dts arch/arm/boot/dts/bcm2835.dtsi already contains:
                uart0: uart@7e201000 {
                        compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell";
                        reg = <0x7e201000 0x1000>;
                        interrupts = <2 25>;
                        clocks = <&clocks BCM2835_CLOCK_UART>,
                                 <&clocks BCM2835_CLOCK_VPU>;
                        clock-names = "uartclk", "apb_pclk";
                        arm,primecell-periphid = <0x00241011>;
                };
for the primary uart.

So I was just following that example and would hesitate to
use a different nomenclature in the same file (even though I
saw serial examples elsewhere).

I guess this is a more general thing that needs to get resolved.

If you tell me to re-spin the patch because of this then I can do that,
but that would not solve the issue with uart0.

Thanks,
	Martin


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/2] dt/bindings: serial: bcm2835: add binding documentation for bcm2835-aux-uart

[Arnd Bergmann]

On Sunday 10 January 2016 11:49:19 Martin Sperl wrote:
> The existing dts arch/arm/boot/dts/bcm2835.dtsi already contains:
>                 uart0: uart@7e201000 {
>                         compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell";
>                         reg = <0x7e201000 0x1000>;
>                         interrupts = <2 25>;
>                         clocks = <&clocks BCM2835_CLOCK_UART>,
>                                  <&clocks BCM2835_CLOCK_VPU>;
>                         clock-names = "uartclk", "apb_pclk";
>                         arm,primecell-periphid = <0x00241011>;
>                 };
> for the primary uart.
> 
> So I was just following that example and would hesitate to
> use a different nomenclature in the same file (even though I
> saw serial examples elsewhere).
> 
> I guess this is a more general thing that needs to get resolved.
> 
> If you tell me to re-spin the patch because of this then I can do that,
> but that would not solve the issue with uart0.

Please just add another patch to rename the existing uarts appropriately
first, then add this one on top with the correct name.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/2] dt/bindings: serial: bcm2835: add binding documentation for bcm2835-aux-uart

[Martin Sperl]

> On 11.01.2016, at 13:21, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> 
> Please just add another patch to rename the existing uarts appropriately
> first, then add this one on top with the correct name.

I can do that with the next incarnation of the patch.

But would that not break dt-source files, that refer to uart0?
Is this not considered an API change as well or does this only apply
to the binary files?



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/2] dt/bindings: serial: bcm2835: add binding documentation for bcm2835-aux-uart

[Arnd Bergmann]

On Monday 11 January 2016 14:57:46 Martin Sperl wrote:
> > On 11.01.2016, at 13:21, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > 
> > Please just add another patch to rename the existing uarts appropriately
> > first, then add this one on top with the correct name.
> 
> I can do that with the next incarnation of the patch.
> 
> But would that not break dt-source files, that refer to uart0?
> Is this not considered an API change as well or does this only apply
> to the binary files?

To clarify: I meant you should change the node name, not the label.

The label is not part of the dtb data that gets passed to the kernel
and can have any value, including "uart0".

The node name is part of the dtb and should follow common naming
conventions (per ePAPR), but the kernel doesn't actually care, so
it won't break anything.

There are cases where a bootloader relies on a particular node name
in order to find a device node and patch its properties, but I don't
think this is the case here.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/2] dt/bindings: serial: bcm2835: add binding documentation for bcm2835-aux-uart

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:

> On Monday 11 January 2016 14:57:46 Martin Sperl wrote:
>> > On 11.01.2016, at 13:21, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> > 
>> > Please just add another patch to rename the existing uarts appropriately
>> > first, then add this one on top with the correct name.
>> 
>> I can do that with the next incarnation of the patch.
>> 
>> But would that not break dt-source files, that refer to uart0?
>> Is this not considered an API change as well or does this only apply
>> to the binary files?
>
> To clarify: I meant you should change the node name, not the label.
>
> The label is not part of the dtb data that gets passed to the kernel
> and can have any value, including "uart0".
>
> The node name is part of the dtb and should follow common naming
> conventions (per ePAPR), but the kernel doesn't actually care, so
> it won't break anything.
>
> There are cases where a bootloader relies on a particular node name
> in order to find a device node and patch its properties, but I don't
> think this is the case here.

Confirmed, the booloader source I have doesn't do anything with uart
nodes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH V2 2/2] serial: bcm2835: add driver for bcm2835-aux-uart

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

The bcm2835 SOC contains an auxiliary uart, which is very close
to the ns16550 with some differences.

The big difference is that the uart HW is not using an internal divider
of 16 but 8, which results in an effictive baud-rate being twice
the requested baud-rate.

This driver handles this device correctly and handles the difference in
the HW divider by scaling up the clock ba a factor of 2.

The approach to write a separate (wrapper) driver instead of using a
multiplying clock and "ns16550" as compatibility in the device-tree
has been recommended by Stephen Warren.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/tty/serial/8250/8250_bcm2835aux.c |  143 +++++++++++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig           |    8 ++
 drivers/tty/serial/8250/Makefile          |    1 +
 3 files changed, 152 insertions(+)
 create mode 100644 drivers/tty/serial/8250/8250_bcm2835aux.c

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
new file mode 100644
index 0000000..718c357
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -0,0 +1,143 @@
+/*
+ * Serial port driver for BCM2835AUX UART
+ *
+ * Copyright (C) 2016 Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
+ *
+ * Based on 8250_lpc18xx.c:
+ * Copyright (C) 2015 Joachim Eastwood <manabian-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "8250.h"
+
+struct bcm2835aux_data {
+	struct uart_8250_port uart;
+	struct clk *clk;
+};
+
+static int bcm2835aux_serial_probe(struct platform_device *pdev)
+{
+	struct bcm2835aux_data *data;
+	struct resource *res;
+	int ret;
+
+	/* allocate the custom structure */
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	/* initialize data */
+	spin_lock_init(&data->uart.port.lock);
+	data->uart.port.dev = &pdev->dev;
+	data->uart.port.regshift = 2;
+	data->uart.port.type = PORT_16550;
+	data->uart.port.iotype = UPIO_MEM;
+	data->uart.port.fifosize = 8;
+	data->uart.port.flags = UPF_SHARE_IRQ |
+				UPF_FIXED_PORT |
+				UPF_FIXED_TYPE |
+				UPF_SKIP_TEST;
+
+	/* get the clock - this also enables the HW */
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	ret = PTR_ERR_OR_ZERO(data->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "could not get clk: %d\n", ret);
+		return ret;
+	}
+
+	/* get the interrupt */
+	data->uart.port.irq = platform_get_irq(pdev, 0);
+	if (data->uart.port.irq < 0) {
+		dev_err(&pdev->dev, "irq not found - %i",
+			data->uart.port.irq);
+		return data->uart.port.irq;
+	}
+
+	/* map the main registers */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "memory resource not found");
+		return -EINVAL;
+	}
+	data->uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
+	ret = PTR_ERR_OR_ZERO(data->uart.port.membase);
+	if (ret)
+		return ret;
+
+	/* Check for a fixed line number */
+	ret = of_alias_get_id(pdev->dev.of_node, "serial");
+	if (ret >= 0)
+		data->uart.port.line = ret;
+
+	/* enable the clock as a last step */
+	ret = clk_prepare_enable(data->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "unable to enable uart clock\n");
+		return ret;
+	}
+
+	/* the HW-clock divider for bcm2835aux is 8,
+	 * but 8250 expects a divider of 16,
+	 * so we have to multiply the actual clock by 2
+	 * to get identical baudrates.
+	 */
+	data->uart.port.uartclk = clk_get_rate(data->clk) * 2;
+
+	/* register the port */
+	ret = serial8250_register_8250_port(&data->uart);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"unable to register 8250 port - %d\n", ret);
+		goto dis_clk;
+	}
+	data->uart.port.line = ret;
+
+	platform_set_drvdata(pdev, data);
+
+	return 0;
+
+dis_clk:
+	clk_disable_unprepare(data->clk);
+	return ret;
+}
+
+static int bcm2835aux_serial_remove(struct platform_device *pdev)
+{
+	struct bcm2835aux_data *data = platform_get_drvdata(pdev);
+
+	serial8250_unregister_port(data->uart.port.line);
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+static const struct of_device_id bcm2835aux_serial_match[] = {
+	{ .compatible = "brcm,bcm2835-aux-uart" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bcm2835aux_serial_match);
+
+static struct platform_driver bcm2835aux_serial_driver = {
+	.driver = {
+		.name = "bcm2835-aux-uart",
+		.of_match_table = bcm2835aux_serial_match,
+	},
+	.probe  = bcm2835aux_serial_probe,
+	.remove = bcm2835aux_serial_remove,
+};
+module_platform_driver(bcm2835aux_serial_driver);
+
+MODULE_DESCRIPTION("BCM2835 auxiliar UART driver");
+MODULE_AUTHOR("Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 6412f14..105aa19 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -272,6 +272,14 @@ config SERIAL_8250_ACORN
 	  system, say Y to this option.  The driver can handle 1, 2, or 3 port
 	  cards.  If unsure, say N.
 
+config SERIAL_8250_BCM2835AUX
+	tristate "BCM2835 auxiliar uart support"
+	depends on ARCH_BCM2835 || COMPILE_TEST
+	depends on SERIAL_8250 && SERIAL_8250_SHARE_IRQ
+	help
+	  support for the BCM2835 auxiliar uart.
+	  If unsure, say N.
+
 config SERIAL_8250_FSL
 	bool
 	depends on SERIAL_8250_CONSOLE
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index e177f86..6327d0a 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SERIAL_8250_PCI)		+= 8250_pci.o
 obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
 obj-$(CONFIG_SERIAL_8250_CS)		+= serial_cs.o
 obj-$(CONFIG_SERIAL_8250_ACORN)		+= 8250_acorn.o
+obj-$(CONFIG_SERIAL_8250_BCM2835AUX)	+= 8250_bcm2835aux.o
 obj-$(CONFIG_SERIAL_8250_CONSOLE)	+= 8250_early.o
 obj-$(CONFIG_SERIAL_8250_FOURPORT)	+= 8250_fourport.o
 obj-$(CONFIG_SERIAL_8250_ACCENT)	+= 8250_accent.o
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/2] serial: bcm2835: add driver for bcm2835-aux-uart

[Stefan Wahren]

Hi Martin,

Am 09.01.2016 um 14:07 schrieb kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>
> The bcm2835 SOC contains an auxiliary uart, which is very close
> to the ns16550 with some differences.
>
> The big difference is that the uart HW is not using an internal divider
> of 16 but 8, which results in an effictive baud-rate being twice
> the requested baud-rate.
>
> This driver handles this device correctly and handles the difference in
> the HW divider by scaling up the clock ba a factor of 2.
>
> The approach to write a separate (wrapper) driver instead of using a
> multiplying clock and "ns16550" as compatibility in the device-tree
> has been recommended by Stephen Warren.
>
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> ---
>  drivers/tty/serial/8250/8250_bcm2835aux.c |  143 +++++++++++++++++++++++++++++
>  drivers/tty/serial/8250/Kconfig           |    8 ++
>  drivers/tty/serial/8250/Makefile          |    1 +
>  3 files changed, 152 insertions(+)
>  create mode 100644 drivers/tty/serial/8250/8250_bcm2835aux.c
>
> diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
> new file mode 100644
> index 0000000..718c357
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
> @@ -0,0 +1,143 @@
> +/*
> + * Serial port driver for BCM2835AUX UART
> + *
> + * Copyright (C) 2016 Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> + *
> + * Based on 8250_lpc18xx.c:
> + * Copyright (C) 2015 Joachim Eastwood <manabian-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include "8250.h"
> +
> +struct bcm2835aux_data {
> +	struct uart_8250_port uart;
> +	struct clk *clk;
> +};
> +
> +static int bcm2835aux_serial_probe(struct platform_device *pdev)
> +{
> +	struct bcm2835aux_data *data;
> +	struct resource *res;
> +	int ret;
> +
> +	/* allocate the custom structure */
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	/* initialize data */
> +	spin_lock_init(&data->uart.port.lock);
> +	data->uart.port.dev = &pdev->dev;
> +	data->uart.port.regshift = 2;
> +	data->uart.port.type = PORT_16550;
> +	data->uart.port.iotype = UPIO_MEM;
> +	data->uart.port.fifosize = 8;

data->uart.capabilities = UART_CAP_FIFO; ?

> +	data->uart.port.flags = UPF_SHARE_IRQ |
> +				UPF_FIXED_PORT |
> +				UPF_FIXED_TYPE |
> +				UPF_SKIP_TEST;
> +
> +	/* get the clock - this also enables the HW */
> +	data->clk = devm_clk_get(&pdev->dev, NULL);
> +	ret = PTR_ERR_OR_ZERO(data->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "could not get clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* get the interrupt */
> +	data->uart.port.irq = platform_get_irq(pdev, 0);
> +	if (data->uart.port.irq < 0) {
> +		dev_err(&pdev->dev, "irq not found - %i",
> +			data->uart.port.irq);
> +		return data->uart.port.irq;
> +	}
> +
> +	/* map the main registers */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "memory resource not found");
> +		return -EINVAL;
> +	}
> +	data->uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
> +	ret = PTR_ERR_OR_ZERO(data->uart.port.membase);
> +	if (ret)
> +		return ret;
> +
> +	/* Check for a fixed line number */
> +	ret = of_alias_get_id(pdev->dev.of_node, "serial");
> +	if (ret >= 0)
> +		data->uart.port.line = ret;
> +
> +	/* enable the clock as a last step */
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "unable to enable uart clock\n");
> +		return ret;
> +	}
> +
> +	/* the HW-clock divider for bcm2835aux is 8,
> +	 * but 8250 expects a divider of 16,
> +	 * so we have to multiply the actual clock by 2
> +	 * to get identical baudrates.
> +	 */
> +	data->uart.port.uartclk = clk_get_rate(data->clk) * 2;
> +
> +	/* register the port */
> +	ret = serial8250_register_8250_port(&data->uart);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"unable to register 8250 port - %d\n", ret);

This should fit in one line.

> +		goto dis_clk;
> +	}
> +	data->uart.port.line = ret;

This looks wrong because it overwrites the result of of_alias_get_id.
Either
you store it in a new member of bcm2835aux_data or drop the assignment.

> +
> +	platform_set_drvdata(pdev, data);
> +
> +	return 0;
> +
> +dis_clk:
> +	clk_disable_unprepare(data->clk);
> +	return ret;
> +}
> +
> +static int bcm2835aux_serial_remove(struct platform_device *pdev)
> +{
> +	struct bcm2835aux_data *data = platform_get_drvdata(pdev);
> +
> +	serial8250_unregister_port(data->uart.port.line);
> +	clk_disable_unprepare(data->clk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id bcm2835aux_serial_match[] = {
> +	{ .compatible = "brcm,bcm2835-aux-uart" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, bcm2835aux_serial_match);
> +
> +static struct platform_driver bcm2835aux_serial_driver = {
> +	.driver = {
> +		.name = "bcm2835-aux-uart",
> +		.of_match_table = bcm2835aux_serial_match,
> +	},
> +	.probe  = bcm2835aux_serial_probe,
> +	.remove = bcm2835aux_serial_remove,
> +};
> +module_platform_driver(bcm2835aux_serial_driver);
> +
> +MODULE_DESCRIPTION("BCM2835 auxiliar UART driver");
> +MODULE_AUTHOR("Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 6412f14..105aa19 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -272,6 +272,14 @@ config SERIAL_8250_ACORN
>  	  system, say Y to this option.  The driver can handle 1, 2, or 3 port
>  	  cards.  If unsure, say N.
>  
> +config SERIAL_8250_BCM2835AUX
> +	tristate "BCM2835 auxiliar uart support"

BCM2835 auxiliary mini UART support

> +	depends on ARCH_BCM2835 || COMPILE_TEST
> +	depends on SERIAL_8250 && SERIAL_8250_SHARE_IRQ
> +	help
> +	  support for the BCM2835 auxiliar uart.

auxiliary UART

Since it's a mini UART it would be helpful to write down the
limitiations here.

> +	  If unsure, say N.
> +
>  config SERIAL_8250_FSL
>  	bool
>  	depends on SERIAL_8250_CONSOLE
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/2] serial: bcm2835: add driver for bcm2835-aux-uart

[Peter Korsgaard]

>>>>> "kernel" == kernel  <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> writes:

 > From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
 > The bcm2835 SOC contains an auxiliary uart, which is very close
 > to the ns16550 with some differences.

 > The big difference is that the uart HW is not using an internal divider
 > of 16 but 8, which results in an effictive baud-rate being twice
 > the requested baud-rate.

 > This driver handles this device correctly and handles the difference in
 > the HW divider by scaling up the clock ba a factor of 2.

s/ba/by/

-- 
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 0/2] serial: bcm2835: add bcm2835 auxiliar uart driver

[Stefan Wahren]

Hi Martin,

> kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org hat am 9. Januar 2016 um 14:07 geschrieben:
>
>
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>
> This patch series adds the bcm2835aux uart driver, which is created
> as a wrapper arround the 8250 driver (based on recommendations by
> Stephen Warren)
>
> The device tree patch itself is not included yet because this
> patch requires the clk-bcm2835aux driver, which is right now only
> in clk-next, but should become available with 4.5.

it doesn't make sense without the device tree patch. IMHO it's okay as long as
you point out the dependency and set the node status to disable.

Best regards
Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 0/2] serial: bcm2835: add bcm2835 auxiliar uart driver

[Martin Sperl]

> On 10.01.2016, at 16:05, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
> 
> Hi Martin,
> 
>> kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org hat am 9. Januar 2016 um 14:07 geschrieben:
>> 
>> 
>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>> 
>> This patch series adds the bcm2835aux uart driver, which is created
>> as a wrapper arround the 8250 driver (based on recommendations by
>> Stephen Warren)
>> 
>> The device tree patch itself is not included yet because this
>> patch requires the clk-bcm2835aux driver, which is right now only
>> in clk-next, but should become available with 4.5.
> 
> it doesn't make sense without the device tree patch. IMHO it's okay as long as
> you point out the dependency and set the node status to disable.

I agree in principle, but it is the same as with the spi-bcm2835aux driver,
where I had provided a patch to the dt, that still has not been applied
yet, but with the driver itself already in the kernel.

Also the reorganisation of the dt to support the bcm2836 makes this
another moving target.

The other concern is that we would need to use defines included
from "dt-bindings/clock/bcm2835aux.h” and without those the dt will not
work.

For the record: this is what would be required:
	uart1: uart@7e215040 {
		compatible = "brcm,bcm2835-aux-uart";
		reg = <0x7e215040 0x40>;
		interrupts = <1 29>;
                        clocks = <&aux BCM2835_AUX_CLOCK_UART>;
                        status = "disabled";
                };

I can respin the whole patchset to include this as well if you tell me to.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 0/2] serial: bcm2835: add bcm2835 auxiliar uart driver

[Stefan Wahren]

> Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> hat am 10. Januar 2016 um 16:45
> geschrieben:
>
>
>
> > On 10.01.2016, at 16:05, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
> >
> > Hi Martin,
> >
> >> kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org hat am 9. Januar 2016 um 14:07 geschrieben:
> >>
> >>
> >> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> >>
> >> This patch series adds the bcm2835aux uart driver, which is created
> >> as a wrapper arround the 8250 driver (based on recommendations by
> >> Stephen Warren)
> >>
> >> The device tree patch itself is not included yet because this
> >> patch requires the clk-bcm2835aux driver, which is right now only
> >> in clk-next, but should become available with 4.5.
> >
> > it doesn't make sense without the device tree patch. IMHO it's okay as long
> > as
> > you point out the dependency and set the node status to disable.
>
> I agree in principle, but it is the same as with the spi-bcm2835aux driver,
> where I had provided a patch to the dt, that still has not been applied
> yet, but with the driver itself already in the kernel.

Please provide a new version, because it won't apply anymore. It increase the
chance of more testing.

>
> Also the reorganisation of the dt to support the bcm2836 makes this
> another moving target.
>
> The other concern is that we would need to use defines included
> from "dt-bindings/clock/bcm2835aux.h” and without those the dt will not
> work.

AFAIK there has been a discussion about such a problem.

>
> For the record: this is what would be required:
> uart1: uart@7e215040 {
> compatible = "brcm,bcm2835-aux-uart";
> reg = <0x7e215040 0x40>;
> interrupts = <1 29>;
> clocks = <&aux BCM2835_AUX_CLOCK_UART>;
> status = "disabled";
> };
>
> I can respin the whole patchset to include this as well if you tell me to.

I think we should wait for a rpi maintainer's response about that. No need to
hurry.

>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html