[PATCH 0/4] bcm2835: enable auxiliary uart1

[PATCH 0/4] bcm2835: enable auxiliary uart1

[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.

The bcm2835-aux-uart is also special in such that it is enabled/disabled
by a gate in the clock, which is managed by the clk-bcm2835-aux
clock driver.

there are 2 options:
* defining the clock-frequency property in the device tree to 500k
  instead of 250k, but this keeps the HW block disabled making the
  uart not work.
* defining a clock in the device tree, but this results in a baud rate
  that is twice the requested baud-rate.

To address this this patch-set introduce a new property in the device tree
to define a clock divider other than 16.

This currently just scales the clock by a factor of 16/divider.

Note that the use of fixed-factor-clock has also been proposed as a
workarround, but this does not really describe the hw in the device tree
so another solution was needed that allows a correct representation of
the HW in the device tree.

Martin Sperl (4):
  serial: of-serial: allow for a custom clock divider different from 16
  dt/bindings: serial: of-serial: add description for clock-divider
    property
  serial: bcm2835: add auxiliary uart1 to device tree of bcm2835
  ARM: bcm2835: add of-serial and 8250 to bcm2835_defconfig

 Documentation/devicetree/bindings/serial/8250.txt |    1 +
 arch/arm/boot/dts/bcm2835.dtsi                    |   11 +++++++++++
 arch/arm/configs/bcm2835_defconfig                |    7 +++++++
 drivers/tty/serial/of_serial.c                    |   14 ++++++++++++++
 4 files changed, 33 insertions(+)

--
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 1/4] serial: of-serial: allow for a custom clock divider different from 16

[kernel-TqfNSX0MhmxHKSADF0wUEw]

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

The bcm2835 SOC has an auxiliary uart that is compatible to ns16550.
the only difference is that it has an internal clock divider of 8
instead of 16.

The net result is that the baud-rate is twice the baud-rate requested.

This patch allows to configure an alternate clock-divider in the
device-tree, which then scales the clock coming from the clock
framework to get scaled by 16/divider.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/tty/serial/of_serial.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index afae58a..2fcbfee 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -65,6 +65,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
 	struct resource resource;
 	struct device_node *np = ofdev->dev.of_node;
 	u32 clk, spd, prop;
+	u64 clk_scaled;
 	int ret;
 
 	memset(port, 0, sizeof *port);
@@ -84,6 +85,19 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
 
 		clk = clk_get_rate(info->clk);
 	}
+	/* a custom clock divider instead of the default 16
+	 * this (as of now) does not change the low level behaviour
+	 * but only scales the clock accordingly
+	 */
+	if (of_property_read_u32(np, "clock-divider", &spd) == 0) {
+		clk_scaled = (u64) clk * 16;
+		do_div(clk_scaled, spd);
+		dev_info(&ofdev->dev,
+			"scaling clock %uHz to %uHz to allow a clock divider of %u\n",
+			clk, (u32)clk_scaled, spd);
+		clk = (u32) clk_scaled;
+	}
+
 	/* If current-speed was set, then try not to change it. */
 	if (of_property_read_u32(np, "current-speed", &spd) == 0)
 		port->custom_divisor = clk / (16 * spd);
-- 
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 2/4] dt/bindings: serial: of-serial: add description for clock-divider property

[kernel-TqfNSX0MhmxHKSADF0wUEw]

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

Add description for clock-divider to the binding documentation.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 Documentation/devicetree/bindings/serial/8250.txt |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 91d5ab0..2c6eaec 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -43,6 +43,7 @@ Optional properties:
 - auto-flow-control: one way to enable automatic flow control support. The
   driver is allowed to detect support for the capability even without this
   property.
+- clock-divider: set an alternative clock divider instead of the default 16
 
 Note:
 * fsl,ns16550:
-- 
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 3/4] serial: bcm2835: add auxiliary uart1 to device tree of bcm2835

[kernel-TqfNSX0MhmxHKSADF0wUEw]

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

Add the auxiliary uart1 device to the device tree of the bcm2835 SOC.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 arch/arm/boot/dts/bcm2835.dtsi |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 3f5a7f1..d68e30c 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -185,6 +185,17 @@
 			compatible = "arm,arm1176-pmu";
 		};
 
+		uart1: uart@7e215040 {
+			compatible = "ns16550";
+			reg = <0x7e215040 0x40>;
+			reg-shift = <2>;
+			interrupts = <1 29>;
+			no-loopback-test;
+			status = "disabled";
+			clock-divider = <8>;
+			clocks = <&aux_clocks BCM2835_AUX_CLOCK_UART>;
+		};
+
 		spi1: spi@7e215080 {
 			compatible = "brcm,bcm2835-aux-spi";
 			reg = <0x7e215080 0x40>;
-- 
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 3/4] serial: bcm2835: add auxiliary uart1 to device tree of bcm2835

[Stephen Warren]

On 09/11/2015 05:20 AM, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> 
> Add the auxiliary uart1 device to the device tree of the bcm2835 SOC.

> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi

> +		uart1: uart@7e215040 {
> +			compatible = "ns16550";

compatible should always include a precise HW-specific value; something
like brcm,bcm2835-aux-uart. That way, if we find some other issue that
needs working around on this HW in the future, all DTs will already
contain the compatible value that SW needs in order to trigger that
workaround. That's a generally true statement; i.e. irrespective of
anything else in this series.

I don't believe "ns16550" should be in the compatible value for this
device, since the device cannot be successfully driven by SW that only
knows about a standard 16650 UART. SW must know about the different
divider, and there is no possibility of SW knowing about that before
this series.
--
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 3/4] serial: bcm2835: add auxiliary uart1 to device tree of bcm2835

[Martin Sperl]

On 22.09.2015 04:42, Stephen Warren wrote:
> On 09/11/2015 05:20 AM, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>
>> Add the auxiliary uart1 device to the device tree of the bcm2835 SOC.
>
>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
>
>> +		uart1: uart@7e215040 {
>> +			compatible = "ns16550";
>
> compatible should always include a precise HW-specific value; something
> like brcm,bcm2835-aux-uart. That way, if we find some other issue that
> needs working around on this HW in the future, all DTs will already
> contain the compatible value that SW needs in order to trigger that
> workaround. That's a generally true statement; i.e. irrespective of
> anything else in this series.
>
> I don't believe "ns16550" should be in the compatible value for this
> device, since the device cannot be successfully driven by SW that only
> knows about a standard 16650 UART. SW must know about the different
> divider, and there is no possibility of SW knowing about that before
> this series.
>

OK - I will look into creating a separate driver then that uses the 8250
implementation as a basis...
--
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 3/4] serial: bcm2835: add auxiliary uart1 to device tree of bcm2835

[Stephen Warren]

On 09/23/2015 04:01 AM, Martin Sperl wrote:
> On 22.09.2015 04:42, Stephen Warren wrote:
>> On 09/11/2015 05:20 AM, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>>
>>> Add the auxiliary uart1 device to the device tree of the bcm2835 SOC.
>>
>>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi
>>> b/arch/arm/boot/dts/bcm2835.dtsi
>>
>>> +        uart1: uart@7e215040 {
>>> +            compatible = "ns16550";
>>
>> compatible should always include a precise HW-specific value; something
>> like brcm,bcm2835-aux-uart. That way, if we find some other issue that
>> needs working around on this HW in the future, all DTs will already
>> contain the compatible value that SW needs in order to trigger that
>> workaround. That's a generally true statement; i.e. irrespective of
>> anything else in this series.
>>
>> I don't believe "ns16550" should be in the compatible value for this
>> device, since the device cannot be successfully driven by SW that only
>> knows about a standard 16650 UART. SW must know about the different
>> divider, and there is no possibility of SW knowing about that before
>> this series.
>>
>
> OK - I will look into creating a separate driver then that uses the 8250
> implementation as a basis...

I expect all you need to do is add a new PORT_* value to the existing 
driver (which then drives some new quirk setting re: the clock rate), 
and add new entry for the compatible value in of_platform_serial_table[] 
in drivers/tty/serial/of_serial.c to select the correct TYPE_*.
--
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 4/4] ARM: bcm2835: add of-serial and 8250 to bcm2835_defconfig

[kernel-TqfNSX0MhmxHKSADF0wUEw]

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

Add 8250 amd of-serial to get compiled in when using bcm2835_defconfig.

This driver supports the bcm2835 auxiliary uart.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 arch/arm/configs/bcm2835_defconfig |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig
index a205c8b..e74db5f 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -67,8 +67,15 @@ CONFIG_ZD1211RW=y
 CONFIG_INPUT_EVDEV=y
 # CONFIG_LEGACY_PTYS is not set
 # CONFIG_DEVKMEM is not set
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_DEPRECATED_OPTIONS=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_8250_DMA=n
+CONFIG_SERIAL_8250_EXTENDED=y
+CONFIG_SERIAL_8250_SHARE_IRQ=y
 CONFIG_SERIAL_AMBA_PL011=y
 CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
+CONFIG_SERIAL_OF_PLATFORM=y
 CONFIG_TTY_PRINTK=y
 CONFIG_I2C=y
 CONFIG_I2C_CHARDEV=y
--
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 0/4] bcm2835: enable auxiliary uart1

[Eric Anholt]

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

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.
>
> The bcm2835-aux-uart is also special in such that it is enabled/disabled
> by a gate in the clock, which is managed by the clk-bcm2835-aux
> clock driver.
>
> there are 2 options:
> * defining the clock-frequency property in the device tree to 500k
>   instead of 250k, but this keeps the HW block disabled making the
>   uart not work.
> * defining a clock in the device tree, but this results in a baud rate
>   that is twice the requested baud-rate.
>
> To address this this patch-set introduce a new property in the device tree
> to define a clock divider other than 16.
>
> This currently just scales the clock by a factor of 16/divider.
>
> Note that the use of fixed-factor-clock has also been proposed as a
> workarround, but this does not really describe the hw in the device tree
> so another solution was needed that allows a correct representation of
> the HW in the device tree.

I personally lean toward the fixed-factor-clock solution, but could go
either way.  Serial maintainers, what do you think?

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

Re: [PATCH 0/4] bcm2835: enable auxiliary uart1

[Stephen Warren]

On 09/14/2015 07:53 AM, Eric Anholt wrote:
> 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.
>> 
>> The bcm2835-aux-uart is also special in such that it is
>> enabled/disabled by a gate in the clock, which is managed by the
>> clk-bcm2835-aux clock driver.
>> 
>> there are 2 options: * defining the clock-frequency property in
>> the device tree to 500k instead of 250k, but this keeps the HW
>> block disabled making the uart not work.

Why does this keep the HW block disabled?

>> * defining a clock in the device tree, but this results in a baud
>> rate that is twice the requested baud-rate.
>> 
>> To address this this patch-set introduce a new property in the
>> device tree to define a clock divider other than 16.
>> 
>> This currently just scales the clock by a factor of 16/divider.
>> 
>> Note that the use of fixed-factor-clock has also been proposed as
>> a workarround, but this does not really describe the hw in the
>> device tree so another solution was needed that allows a correct
>> representation of the HW in the device tree.
> 
> I personally lean toward the fixed-factor-clock solution, but could
> go either way.  Serial maintainers, what do you think?

The external fixed-factor-clock solution sounds more like a workaround
than a real fix. It means that the UART driver isn't aware of what's
going on and only "accidentally" works due to some manipulation of the
clock values that it requests. That kind of thing almost always come
back to bite you later.

Rather than adding a DT property to configure the internal clock
divider, it seems best to add a new compatible value to the list it
supports, and have that compatible value set up internal data that
indicates divide-by-16 vs. divide-by-8. After all, the HW isn't 100%
compatible with ns16550, so the DT should not say that it is. While
the clock-divider property this series adds to the DT does solve the
issue, it does not prevent an older piece of SW that predates this
series, and hence which does not implement clock-divider, attempting
to bind to this DT but fail to operate correctly since it doesn't know
about the different divider.
--
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