diff mbox

[4/7] tty: 8250: omap eliminate use of of_machine_is_compatible()

Message ID bf1f3478de98a74a3c92246d6a5d86bc71aa0cf8.1436174801.git.nsekhar@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sekhar Nori July 6, 2015, 9:47 a.m. UTC
Use of of_machine_is_compatible() for AM335x specific DMA
quirk in 8250_omap driver makes it ugly to extend the
quirk for other platforms. Instead use a new compatible.

The new compatible will also make it easier to care of other
quirks specific to AM335x and like SoCs.

This patch does break backward DTB compatibility for users of
8250_omap driver on AM335x boards. However, the 8250_omap driver
is new and omap_serial is still the default choice driver for UART
and so choosing to break compatibility over keeping the code
around forever.

Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 .../devicetree/bindings/serial/omap_serial.txt     |  1 +
 arch/arm/boot/dts/am33xx.dtsi                      | 12 ++++----
 drivers/tty/serial/8250/8250_omap.c                | 35 +++++++++++++---------
 3 files changed, 28 insertions(+), 20 deletions(-)

Comments

Peter Hurley July 9, 2015, midnight UTC | #1
Hi Sekhar,

On 07/06/2015 05:47 AM, Sekhar Nori wrote:
> Use of of_machine_is_compatible() for AM335x specific DMA
> quirk in 8250_omap driver makes it ugly to extend the
> quirk for other platforms. Instead use a new compatible.
> 
> The new compatible will also make it easier to care of other
> quirks specific to AM335x and like SoCs.
> 
> This patch does break backward DTB compatibility for users of
> 8250_omap driver on AM335x boards.

Not ok.

8250_omap was released with 3.19 and has been the default driver for
BeagleBone since 4.0.

Regards,
Peter Hurley

> However, the 8250_omap driver
> is new and omap_serial is still the default choice driver for UART
> and so choosing to break compatibility over keeping the code
> around forever.
> 
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  .../devicetree/bindings/serial/omap_serial.txt     |  1 +
>  arch/arm/boot/dts/am33xx.dtsi                      | 12 ++++----
>  drivers/tty/serial/8250/8250_omap.c                | 35 +++++++++++++---------
>  3 files changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
> index d3bd2b1ec401..0ee88209b341 100644
> --- a/Documentation/devicetree/bindings/serial/omap_serial.txt
> +++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
> @@ -5,6 +5,7 @@ Required properties:
>  - compatible : should be "ti,omap3-uart" for OMAP3 controllers
>  - compatible : should be "ti,omap4-uart" for OMAP4 controllers
>  - compatible : should be "ti,am4372-uart" for AM437x controllers
> +- compatible : should be "ti,am3352-uart" for AM335x controllers
>  - reg : address and length of the register space
>  - interrupts or interrupts-extended : Should contain the uart interrupt
>                                        specifier or both the interrupt
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 21fcc440fc1a..b76f9a2ce05d 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -210,7 +210,7 @@
>  		};
>  
>  		uart0: serial@44e09000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart1";
>  			clock-frequency = <48000000>;
>  			reg = <0x44e09000 0x2000>;
> @@ -221,7 +221,7 @@
>  		};
>  
>  		uart1: serial@48022000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart2";
>  			clock-frequency = <48000000>;
>  			reg = <0x48022000 0x2000>;
> @@ -232,7 +232,7 @@
>  		};
>  
>  		uart2: serial@48024000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart3";
>  			clock-frequency = <48000000>;
>  			reg = <0x48024000 0x2000>;
> @@ -243,7 +243,7 @@
>  		};
>  
>  		uart3: serial@481a6000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart4";
>  			clock-frequency = <48000000>;
>  			reg = <0x481a6000 0x2000>;
> @@ -252,7 +252,7 @@
>  		};
>  
>  		uart4: serial@481a8000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart5";
>  			clock-frequency = <48000000>;
>  			reg = <0x481a8000 0x2000>;
> @@ -261,7 +261,7 @@
>  		};
>  
>  		uart5: serial@481aa000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart6";
>  			clock-frequency = <48000000>;
>  			reg = <0x481aa000 0x2000>;
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index d9c96b993a84..52566387ec37 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -16,6 +16,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_irq.h>
>  #include <linux/delay.h>
> @@ -537,14 +538,14 @@ static void omap_serial_fill_features_erratas(struct uart_8250_port *up,
>  
>  	switch (revision) {
>  	case OMAP_UART_REV_46:
> -		priv->habit = UART_ERRATA_i202_MDR1_ACCESS;
> +		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS;
>  		break;
>  	case OMAP_UART_REV_52:
> -		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
> +		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS |
>  				OMAP_UART_WER_HAS_TX_WAKEUP;
>  		break;
>  	case OMAP_UART_REV_63:
> -		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
> +		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS |
>  			OMAP_UART_WER_HAS_TX_WAKEUP;
>  		break;
>  	default:
> @@ -1061,6 +1062,17 @@ static int omap8250_no_handle_irq(struct uart_port *port)
>  	return 0;
>  }
>  
> +static const u8 am3352_habit = OMAP_DMA_TX_KICK;
> +
> +static const struct of_device_id omap8250_dt_ids[] = {
> +	{ .compatible = "ti,omap2-uart" },
> +	{ .compatible = "ti,omap3-uart" },
> +	{ .compatible = "ti,omap4-uart" },
> +	{ .compatible = "ti,am3352-uart", .data = &am3352_habit, },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
> +
>  static int omap8250_probe(struct platform_device *pdev)
>  {
>  	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -1125,11 +1137,17 @@ static int omap8250_probe(struct platform_device *pdev)
>  	up.port.unthrottle = omap_8250_unthrottle;
>  
>  	if (pdev->dev.of_node) {
> +		const struct of_device_id *id;
> +
>  		ret = of_alias_get_id(pdev->dev.of_node, "serial");
>  
>  		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
>  				     &up.port.uartclk);
>  		priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
> +
> +		id = of_match_device(of_match_ptr(omap8250_dt_ids), &pdev->dev);
> +		if (id && id->data)
> +			priv->habit |= *(u8 *)id->data;
>  	} else {
>  		ret = pdev->id;
>  	}
> @@ -1184,9 +1202,6 @@ static int omap8250_probe(struct platform_device *pdev)
>  			priv->omap8250_dma.rx_size = RX_TRIGGER;
>  			priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
>  			priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
> -
> -			if (of_machine_is_compatible("ti,am33xx"))
> -				priv->habit |= OMAP_DMA_TX_KICK;
>  		}
>  	}
>  #endif
> @@ -1374,14 +1389,6 @@ static const struct dev_pm_ops omap8250_dev_pm_ops = {
>  	.complete       = omap8250_complete,
>  };
>  
> -static const struct of_device_id omap8250_dt_ids[] = {
> -	{ .compatible = "ti,omap2-uart" },
> -	{ .compatible = "ti,omap3-uart" },
> -	{ .compatible = "ti,omap4-uart" },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
> -
>  static struct platform_driver omap8250_platform_driver = {
>  	.driver = {
>  		.name		= "omap8250",
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori July 9, 2015, 11:18 a.m. UTC | #2
Hi Peter,

On Thursday 09 July 2015 05:30 AM, Peter Hurley wrote:
> Hi Sekhar,
> 
> On 07/06/2015 05:47 AM, Sekhar Nori wrote:
>> Use of of_machine_is_compatible() for AM335x specific DMA
>> quirk in 8250_omap driver makes it ugly to extend the
>> quirk for other platforms. Instead use a new compatible.
>>
>> The new compatible will also make it easier to care of other
>> quirks specific to AM335x and like SoCs.
>>
>> This patch does break backward DTB compatibility for users of
>> 8250_omap driver on AM335x boards.
> 
> Not ok.
> 
> 8250_omap was released with 3.19 and has been the default driver for
> BeagleBone since 4.0.

Alright, I guess I was really stretching my luck here. Will keep the
of_machine_is_compatible() around so there should be no backward
compatibility issue.

Thanks,
Sekhar

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

Patch

diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
index d3bd2b1ec401..0ee88209b341 100644
--- a/Documentation/devicetree/bindings/serial/omap_serial.txt
+++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
@@ -5,6 +5,7 @@  Required properties:
 - compatible : should be "ti,omap3-uart" for OMAP3 controllers
 - compatible : should be "ti,omap4-uart" for OMAP4 controllers
 - compatible : should be "ti,am4372-uart" for AM437x controllers
+- compatible : should be "ti,am3352-uart" for AM335x controllers
 - reg : address and length of the register space
 - interrupts or interrupts-extended : Should contain the uart interrupt
                                       specifier or both the interrupt
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 21fcc440fc1a..b76f9a2ce05d 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -210,7 +210,7 @@ 
 		};
 
 		uart0: serial@44e09000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart1";
 			clock-frequency = <48000000>;
 			reg = <0x44e09000 0x2000>;
@@ -221,7 +221,7 @@ 
 		};
 
 		uart1: serial@48022000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart2";
 			clock-frequency = <48000000>;
 			reg = <0x48022000 0x2000>;
@@ -232,7 +232,7 @@ 
 		};
 
 		uart2: serial@48024000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart3";
 			clock-frequency = <48000000>;
 			reg = <0x48024000 0x2000>;
@@ -243,7 +243,7 @@ 
 		};
 
 		uart3: serial@481a6000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart4";
 			clock-frequency = <48000000>;
 			reg = <0x481a6000 0x2000>;
@@ -252,7 +252,7 @@ 
 		};
 
 		uart4: serial@481a8000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart5";
 			clock-frequency = <48000000>;
 			reg = <0x481a8000 0x2000>;
@@ -261,7 +261,7 @@ 
 		};
 
 		uart5: serial@481aa000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart6";
 			clock-frequency = <48000000>;
 			reg = <0x481aa000 0x2000>;
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index d9c96b993a84..52566387ec37 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -16,6 +16,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/delay.h>
@@ -537,14 +538,14 @@  static void omap_serial_fill_features_erratas(struct uart_8250_port *up,
 
 	switch (revision) {
 	case OMAP_UART_REV_46:
-		priv->habit = UART_ERRATA_i202_MDR1_ACCESS;
+		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS;
 		break;
 	case OMAP_UART_REV_52:
-		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
+		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS |
 				OMAP_UART_WER_HAS_TX_WAKEUP;
 		break;
 	case OMAP_UART_REV_63:
-		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
+		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS |
 			OMAP_UART_WER_HAS_TX_WAKEUP;
 		break;
 	default:
@@ -1061,6 +1062,17 @@  static int omap8250_no_handle_irq(struct uart_port *port)
 	return 0;
 }
 
+static const u8 am3352_habit = OMAP_DMA_TX_KICK;
+
+static const struct of_device_id omap8250_dt_ids[] = {
+	{ .compatible = "ti,omap2-uart" },
+	{ .compatible = "ti,omap3-uart" },
+	{ .compatible = "ti,omap4-uart" },
+	{ .compatible = "ti,am3352-uart", .data = &am3352_habit, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
+
 static int omap8250_probe(struct platform_device *pdev)
 {
 	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1125,11 +1137,17 @@  static int omap8250_probe(struct platform_device *pdev)
 	up.port.unthrottle = omap_8250_unthrottle;
 
 	if (pdev->dev.of_node) {
+		const struct of_device_id *id;
+
 		ret = of_alias_get_id(pdev->dev.of_node, "serial");
 
 		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
 				     &up.port.uartclk);
 		priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
+
+		id = of_match_device(of_match_ptr(omap8250_dt_ids), &pdev->dev);
+		if (id && id->data)
+			priv->habit |= *(u8 *)id->data;
 	} else {
 		ret = pdev->id;
 	}
@@ -1184,9 +1202,6 @@  static int omap8250_probe(struct platform_device *pdev)
 			priv->omap8250_dma.rx_size = RX_TRIGGER;
 			priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
 			priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
-
-			if (of_machine_is_compatible("ti,am33xx"))
-				priv->habit |= OMAP_DMA_TX_KICK;
 		}
 	}
 #endif
@@ -1374,14 +1389,6 @@  static const struct dev_pm_ops omap8250_dev_pm_ops = {
 	.complete       = omap8250_complete,
 };
 
-static const struct of_device_id omap8250_dt_ids[] = {
-	{ .compatible = "ti,omap2-uart" },
-	{ .compatible = "ti,omap3-uart" },
-	{ .compatible = "ti,omap4-uart" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
-
 static struct platform_driver omap8250_platform_driver = {
 	.driver = {
 		.name		= "omap8250",