diff mbox

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

Message ID 1452344854-2576-2-git-send-email-kernel@martin.sperl.org (mailing list archive)
State New, archived
Headers show

Commit Message

Martin Sperl Jan. 9, 2016, 1:07 p.m. UTC
From: Martin Sperl <kernel@martin.sperl.org>

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

Signed-off-by: Martin Sperl <kernel@martin.sperl.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

Comments

Arnd Bergmann Jan. 9, 2016, 8:53 p.m. UTC | #1
On Saturday 09 January 2016 13:07:32 kernel@martin.sperl.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
Martin Sperl Jan. 10, 2016, 10:49 a.m. UTC | #2
> On 09.01.2016, at 21:53, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> On Saturday 09 January 2016 13:07:32 kernel@martin.sperl.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
Arnd Bergmann Jan. 11, 2016, 12:21 p.m. UTC | #3
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
Martin Sperl Jan. 11, 2016, 1:57 p.m. UTC | #4
> On 11.01.2016, at 13:21, Arnd Bergmann <arnd@arndb.de> 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?
Arnd Bergmann Jan. 11, 2016, 2:40 p.m. UTC | #5
On Monday 11 January 2016 14:57:46 Martin Sperl wrote:
> > On 11.01.2016, at 13:21, Arnd Bergmann <arnd@arndb.de> 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
Eric Anholt Jan. 28, 2016, 6:39 a.m. UTC | #6
Arnd Bergmann <arnd@arndb.de> writes:

> On Monday 11 January 2016 14:57:46 Martin Sperl wrote:
>> > On 11.01.2016, at 13:21, Arnd Bergmann <arnd@arndb.de> 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.
diff mbox

Patch

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>;
+	};