Re: [PATCH v2] tty: ar933x_uart: add device tree support and binding documentation

Re: [PATCH v2] tty: ar933x_uart: add device tree support and binding documentation

[Gabor Juhos]

2013.09.04. 0:16 keltezéssel, Kumar Gala írta:
> 
> On Aug 29, 2013, at 4:48 AM, Gabor Juhos wrote:
> 
>> Modify the probe routine to get the port line number
>> from device tree if the 'of_node' is populated in the
>> platform device. The driver can be built as module,
>> thus add an OF specific module device table as well
>> to support module auto loading.
>>
>> This makes it possible to use the driver for AR9330
>> UART devices specified in device tree.
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
>> ---
>> Changes since v1:
>>  - move the doc from bindings/tty/serial to bindings/serial
>>
>> The patch is agains the 'tty-next' branch of the
>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tree
>> ---
>> .../devicetree/bindings/serial/qca,ar9330-uart.txt |   34 ++++++++++++++++++++
>> drivers/tty/serial/ar933x_uart.c                   |   28 ++++++++++++++--
>> 2 files changed, 59 insertions(+), 3 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
>>
>> diff --git a/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
>> new file mode 100644
>> index 0000000..c5e032c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
>> @@ -0,0 +1,34 @@
>> +* Qualcomm Atheros AR9330 High-Speed UART
>> +
>> +Required properties:
>> +
>> +- compatible: Must be "qca,ar9330-uart"
> 
> If the driver is called ar933x, is there a reason not to do so with the compatible and binding spec?

Yes, there is a reason, the compatible string should not contain wildcards. At
least the devicetree.org wiki [1] says the following:

  Warning: Don't use wildcard compatible values, like "fsl,mpc83xx-uart" or
  similar. Silicon vendors will invariably make a change that breaks your wildcard
  assumptions the moment it is too late to change it. Instead, choose a specific
  silicon implementations and make all subsequent silicon compatible with it.

Additionally, Grant Likely wrote the same thing here basically:

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-January/082265.html

-Gabor

1. http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property

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

[PATCH v2] tty: ar933x_uart: add device tree support and binding documentation

[Gabor Juhos]

Modify the probe routine to get the port line number
from device tree if the 'of_node' is populated in the
platform device. The driver can be built as module,
thus add an OF specific module device table as well
to support module auto loading.

This makes it possible to use the driver for AR9330
UART devices specified in device tree.

Cc: devicetree@vger.kernel.org
Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
Changes since v1:
  - move the doc from bindings/tty/serial to bindings/serial

The patch is agains the 'tty-next' branch of the
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tree
---
 .../devicetree/bindings/serial/qca,ar9330-uart.txt |   34 ++++++++++++++++++++
 drivers/tty/serial/ar933x_uart.c                   |   28 ++++++++++++++--
 2 files changed, 59 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt

diff --git a/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
new file mode 100644
index 0000000..c5e032c
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
@@ -0,0 +1,34 @@
+* Qualcomm Atheros AR9330 High-Speed UART
+
+Required properties:
+
+- compatible: Must be "qca,ar9330-uart"
+
+- reg: Specifies the physical base address of the controller and
+  the length of the memory mapped region.
+
+- interrupt-parent: The phandle for the interrupt controller that
+  services interrupts for this device.
+
+- interrupts: Specifies the interrupt source of the parent interrupt
+  controller. The format of the interrupt specifier depends on the
+  parent interrupt controller.
+
+Additional requirements:
+
+  Each UART port must have an alias correctly numbered in "aliases"
+  node.
+
+Example:
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	uart0: uart@18020000 {
+		compatible = "qca,ar9330-uart";
+		reg = <0x18020000 0x14>;
+
+		interrupt-parent = <&intc>;
+		interrupts = <3>;
+	};
diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
index 0437a9e..acd03af 100644
--- a/drivers/tty/serial/ar933x_uart.c
+++ b/drivers/tty/serial/ar933x_uart.c
@@ -17,6 +17,8 @@
 #include <linux/sysrq.h>
 #include <linux/delay.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
 #include <linux/serial_core.h>
@@ -623,13 +625,24 @@ static int ar933x_uart_probe(struct platform_device *pdev)
 	struct uart_port *port;
 	struct resource *mem_res;
 	struct resource *irq_res;
+	struct device_node *np;
 	unsigned int baud;
 	int id;
 	int ret;
 
-	id = pdev->id;
-	if (id == -1)
-		id = 0;
+	np = pdev->dev.of_node;
+	if (config_enabled(CONFIG_OF) && np) {
+		id = of_alias_get_id(np, "serial");
+		if (id < 0) {
+			dev_err(&pdev->dev, "unable to get alias id, err=%d\n",
+				id);
+			return id;
+		}
+	} else {
+		id = pdev->id;
+		if (id == -1)
+			id = 0;
+	}
 
 	if (id > CONFIG_SERIAL_AR933X_NR_UARTS)
 		return -EINVAL;
@@ -713,12 +726,21 @@ static int ar933x_uart_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id ar933x_uart_of_ids[] = {
+	{ .compatible = "qca,ar9330-uart" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ar933x_uart_of_ids);
+#endif
+
 static struct platform_driver ar933x_uart_platform_driver = {
 	.probe		= ar933x_uart_probe,
 	.remove		= ar933x_uart_remove,
 	.driver		= {
 		.name		= DRIVER_NAME,
 		.owner		= THIS_MODULE,
+		.of_match_table = of_match_ptr(ar933x_uart_of_ids),
 	},
 };
 
-- 
1.7.10

Re: [PATCH v2] tty: ar933x_uart: add device tree support and binding documentation

[Greg Kroah-Hartman]

On Thu, Aug 29, 2013 at 11:48:15AM +0200, Gabor Juhos wrote:
> Modify the probe routine to get the port line number
> from device tree if the 'of_node' is populated in the
> platform device. The driver can be built as module,
> thus add an OF specific module device table as well
> to support module auto loading.
> 
> This makes it possible to use the driver for AR9330
> UART devices specified in device tree.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> ---
> Changes since v1:
>   - move the doc from bindings/tty/serial to bindings/serial
> 
> The patch is agains the 'tty-next' branch of the
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tree

No it isn't, it fails to apply here :(

Re: [PATCH v2] tty: ar933x_uart: add device tree support and binding documentation

[Gabor Juhos]

2013.08.30. 23:13 keltezéssel, Greg Kroah-Hartman írta:
> On Thu, Aug 29, 2013 at 11:48:15AM +0200, Gabor Juhos wrote:
>> Modify the probe routine to get the port line number
>> from device tree if the 'of_node' is populated in the
>> platform device. The driver can be built as module,
>> thus add an OF specific module device table as well
>> to support module auto loading.
>>
>> This makes it possible to use the driver for AR9330
>> UART devices specified in device tree.
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
>> ---
>> Changes since v1:
>>   - move the doc from bindings/tty/serial to bindings/serial
>>
>> The patch is agains the 'tty-next' branch of the
>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tree
> 
> No it isn't, it fails to apply here :(

Well, it was created on top of ...

  commit 9dfe59f15195b8c5bd4c2950266877c51fc329f0
  Author: Stephen Boyd <sboyd@codeaurora.org>
  Date:   Wed Aug 28 13:32:41 2013 -0700

      ARM: dts: msm: Update uartdm compatible strings

      Let's follow the ratified DT binding and use uartdm instead of
      hsuart. This does break backwards compatibility but this
      shouldn't be a problem because the uart driver isn't probing on
      these devices without adding clock support (which isn't merged so
      far).

      Cc: David Brown <davidb@codeaurora.org>
      Acked-by: Kumar Gala <galak@codeaurora.org>
      Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
      Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

... and i can still apply the patch on top of that commit.

However the current tty-next tree contains the previous version of the patch:

  commit dd910d98a2e240443db0090b0ae8a719a4a460f8
  Author: Gabor Juhos <juhosg@openwrt.org>
  Date:   Thu Aug 29 08:44:01 2013 +0200

      tty: ar933x_uart: add device tree support and binding documentation

    Modify the probe routine to get the port line number
    from device tree if the 'of_node' is populated in the
    platform device. The driver can be built as module,
    thus add an OF specific module device table as well
    to support module auto loading.

    This makes it possible to use the driver for AR9330
    UART devices specified in device tree.

    Cc: devicetree@vger.kernel.org
    Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

And that is the reason why you can't apply v2. Probably you have missed my
preceding [1] mail.

What should I do now? Should I send a follow-up patch which moves the
documentation from 'bindings/tty/serial' to bindings/serial?

Thanks,
Gabor

http://marc.info/?l=linux-serial&m=137776943126968&w=2

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

Re: [PATCH v2] tty: ar933x_uart: add device tree support and binding documentation

[Greg Kroah-Hartman]

On Fri, Aug 30, 2013 at 11:49:41PM +0200, Gabor Juhos wrote:
> 2013.08.30. 23:13 keltezéssel, Greg Kroah-Hartman írta:
> > On Thu, Aug 29, 2013 at 11:48:15AM +0200, Gabor Juhos wrote:
> >> Modify the probe routine to get the port line number
> >> from device tree if the 'of_node' is populated in the
> >> platform device. The driver can be built as module,
> >> thus add an OF specific module device table as well
> >> to support module auto loading.
> >>
> >> This makes it possible to use the driver for AR9330
> >> UART devices specified in device tree.
> >>
> >> Cc: devicetree@vger.kernel.org
> >> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> >> ---
> >> Changes since v1:
> >>   - move the doc from bindings/tty/serial to bindings/serial
> >>
> >> The patch is agains the 'tty-next' branch of the
> >> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tree
> > 
> > No it isn't, it fails to apply here :(
> 
> Well, it was created on top of ...
> 
>   commit 9dfe59f15195b8c5bd4c2950266877c51fc329f0
>   Author: Stephen Boyd <sboyd@codeaurora.org>
>   Date:   Wed Aug 28 13:32:41 2013 -0700
> 
>       ARM: dts: msm: Update uartdm compatible strings
> 
>       Let's follow the ratified DT binding and use uartdm instead of
>       hsuart. This does break backwards compatibility but this
>       shouldn't be a problem because the uart driver isn't probing on
>       these devices without adding clock support (which isn't merged so
>       far).
> 
>       Cc: David Brown <davidb@codeaurora.org>
>       Acked-by: Kumar Gala <galak@codeaurora.org>
>       Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>       Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ... and i can still apply the patch on top of that commit.
> 
> However the current tty-next tree contains the previous version of the patch:
> 
>   commit dd910d98a2e240443db0090b0ae8a719a4a460f8
>   Author: Gabor Juhos <juhosg@openwrt.org>
>   Date:   Thu Aug 29 08:44:01 2013 +0200
> 
>       tty: ar933x_uart: add device tree support and binding documentation
> 
>     Modify the probe routine to get the port line number
>     from device tree if the 'of_node' is populated in the
>     platform device. The driver can be built as module,
>     thus add an OF specific module device table as well
>     to support module auto loading.
> 
>     This makes it possible to use the driver for AR9330
>     UART devices specified in device tree.
> 
>     Cc: devicetree@vger.kernel.org
>     Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
>     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> And that is the reason why you can't apply v2. Probably you have missed my
> preceding [1] mail.
> 
> What should I do now? Should I send a follow-up patch which moves the
> documentation from 'bindings/tty/serial' to bindings/serial?

As I can't apply this one, I guess a new patch that I can apply will be
the best, right?  :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] tty: ar933x_uart: add device tree support and binding documentation

[Gabor Juhos]

2013.08.31. 3:31 keltezéssel, Greg Kroah-Hartman írta:

<snip>

>>
>> What should I do now? Should I send a follow-up patch which moves the
>> documentation from 'bindings/tty/serial' to bindings/serial?
> 
> As I can't apply this one, I guess a new patch that I can apply will be
> the best, right?  :)

Right. Sent a new one. :)

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

Re: [PATCH v2] tty: ar933x_uart: add device tree support and binding documentation

[Kumar Gala]

On Aug 31, 2013, at 2:19 AM, Gabor Juhos wrote:

> 2013.08.31. 3:31 keltezéssel, Greg Kroah-Hartman írta:
> 
> <snip>
> 
>>> 
>>> What should I do now? Should I send a follow-up patch which moves the
>>> documentation from 'bindings/tty/serial' to bindings/serial?
>> 
>> As I can't apply this one, I guess a new patch that I can apply will be
>> the best, right?  :)
> 
> Right. Sent a new one. :)
> 
> -Gabor

Did I miss a DT maintainer ACK on the binding?

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

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

Re: [PATCH v2] tty: ar933x_uart: add device tree support and binding documentation

[Kumar Gala]

On Aug 29, 2013, at 4:48 AM, Gabor Juhos wrote:

> Modify the probe routine to get the port line number
> from device tree if the 'of_node' is populated in the
> platform device. The driver can be built as module,
> thus add an OF specific module device table as well
> to support module auto loading.
> 
> This makes it possible to use the driver for AR9330
> UART devices specified in device tree.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> ---
> Changes since v1:
>  - move the doc from bindings/tty/serial to bindings/serial
> 
> The patch is agains the 'tty-next' branch of the
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tree
> ---
> .../devicetree/bindings/serial/qca,ar9330-uart.txt |   34 ++++++++++++++++++++
> drivers/tty/serial/ar933x_uart.c                   |   28 ++++++++++++++--
> 2 files changed, 59 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
> new file mode 100644
> index 0000000..c5e032c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
> @@ -0,0 +1,34 @@
> +* Qualcomm Atheros AR9330 High-Speed UART
> +
> +Required properties:
> +
> +- compatible: Must be "qca,ar9330-uart"

If the driver is called ar933x, is there a reason not to do so with the compatible and binding spec?

> +
> +- reg: Specifies the physical base address of the controller and
> +  the length of the memory mapped region.
> +
> +- interrupt-parent: The phandle for the interrupt controller that
> +  services interrupts for this device.
> +
> +- interrupts: Specifies the interrupt source of the parent interrupt
> +  controller. The format of the interrupt specifier depends on the
> +  parent interrupt controller.
> +
> +Additional requirements:
> +
> +  Each UART port must have an alias correctly numbered in "aliases"
> +  node.
> +
> +Example:
> +
> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	uart0: uart@18020000 {
> +		compatible = "qca,ar9330-uart";
> +		reg = <0x18020000 0x14>;
> +
> +		interrupt-parent = <&intc>;
> +		interrupts = <3>;
> +	};
> diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
> index 0437a9e..acd03af 100644
> --- a/drivers/tty/serial/ar933x_uart.c
> +++ b/drivers/tty/serial/ar933x_uart.c
> @@ -17,6 +17,8 @@
> #include <linux/sysrq.h>
> #include <linux/delay.h>
> #include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> #include <linux/tty.h>
> #include <linux/tty_flip.h>
> #include <linux/serial_core.h>
> @@ -623,13 +625,24 @@ static int ar933x_uart_probe(struct platform_device *pdev)
> 	struct uart_port *port;
> 	struct resource *mem_res;
> 	struct resource *irq_res;
> +	struct device_node *np;
> 	unsigned int baud;
> 	int id;
> 	int ret;
> 
> -	id = pdev->id;
> -	if (id == -1)
> -		id = 0;
> +	np = pdev->dev.of_node;
> +	if (config_enabled(CONFIG_OF) && np) {
> +		id = of_alias_get_id(np, "serial");
> +		if (id < 0) {
> +			dev_err(&pdev->dev, "unable to get alias id, err=%d\n",
> +				id);
> +			return id;
> +		}
> +	} else {
> +		id = pdev->id;
> +		if (id == -1)
> +			id = 0;
> +	}
> 
> 	if (id > CONFIG_SERIAL_AR933X_NR_UARTS)
> 		return -EINVAL;
> @@ -713,12 +726,21 @@ static int ar933x_uart_remove(struct platform_device *pdev)
> 	return 0;
> }
> 
> +#ifdef CONFIG_OF
> +static const struct of_device_id ar933x_uart_of_ids[] = {
> +	{ .compatible = "qca,ar9330-uart" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ar933x_uart_of_ids);
> +#endif
> +
> static struct platform_driver ar933x_uart_platform_driver = {
> 	.probe		= ar933x_uart_probe,
> 	.remove		= ar933x_uart_remove,
> 	.driver		= {
> 		.name		= DRIVER_NAME,
> 		.owner		= THIS_MODULE,
> +		.of_match_table = of_match_ptr(ar933x_uart_of_ids),
> 	},
> };
> 
> -- 
> 1.7.10
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation