[PATCH] Add devicetree support to altera_jtaguart

[PATCH] Add devicetree support to altera_jtaguart

[Walter Goossens]

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

This patch adds devicetree support to the altera_jtaguart driver.
Tested on hardware on the nios2 architecture.


[-- Attachment #2: altera_jtaguart_fdt.patch --]
[-- Type: text/plain, Size: 2705 bytes --]

diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c
index f9b49b5..b71ba92 100644
--- a/drivers/serial/altera_jtaguart.c
+++ b/drivers/serial/altera_jtaguart.c
@@ -431,21 +431,44 @@ static int __devinit altera_jtaguart_probe(struct platform_device *pdev)
 {
 	struct altera_jtaguart_platform_uart *platp = pdev->dev.platform_data;
 	struct uart_port *port;
-	int i;
-
-	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS && platp[i].mapbase; i++) {
-		port = &altera_jtaguart_ports[i].port;
-
-		port->line = i;
-		port->type = PORT_ALTERA_JTAGUART;
-		port->mapbase = platp[i].mapbase;
-		port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
-		port->iotype = SERIAL_IO_MEM;
-		port->irq = platp[i].irq;
-		port->ops = &altera_jtaguart_ops;
-		port->flags = ASYNC_BOOT_AUTOCONF;
-
-		uart_add_one_port(&altera_jtaguart_driver, port);
+	if(platp) {
+		int i;
+		for (i = 0; i < ALTERA_JTAGUART_MAXPORTS && platp[i].mapbase; i++) {
+			port = &altera_jtaguart_ports[i].port;
+
+			port->line = i;
+			port->type = PORT_ALTERA_JTAGUART;
+			port->mapbase = platp[i].mapbase;
+			port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
+			port->iotype = SERIAL_IO_MEM;
+			port->irq = platp[i].irq;
+			port->ops = &altera_jtaguart_ops;
+			port->flags = ASYNC_BOOT_AUTOCONF;
+
+			uart_add_one_port(&altera_jtaguart_driver, port);
+		}
+#ifdef CONFIG_OF
+	} else {
+		struct resource *res_irq;
+		struct resource *res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		if(res_mem)
+		{
+			res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+			if(res_irq)
+			{
+				port = &altera_jtaguart_ports[0].port;
+				port->line = 0;
+				port->type = PORT_ALTERA_JTAGUART;
+				port->mapbase = res_mem->start;
+				port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
+				port->iotype = SERIAL_IO_MEM;
+				port->irq = res_irq->start;
+				port->ops = &altera_jtaguart_ops;
+				port->flags = ASYNC_BOOT_AUTOCONF;
+				uart_add_one_port(&altera_jtaguart_driver, port);
+			}
+		}
+#endif
 	}
 
 	return 0;
@@ -464,6 +487,15 @@ static int __devexit altera_jtaguart_remove(struct platform_device *pdev)
 
 	return 0;
 }
+#ifdef CONFIG_OF
+static struct of_device_id altera_jtaguart_match[] = {
+	{ 
+		.compatible = "altera,altera_juart",
+	},
+	{},
+}
+MODULE_DEVICE_TABLE(of, altera_jtaguart_match);
+#endif /* CONFIG_OF */
 
 static struct platform_driver altera_jtaguart_platform_driver = {
 	.probe	= altera_jtaguart_probe,
@@ -471,6 +503,9 @@ static struct platform_driver altera_jtaguart_platform_driver = {
 	.driver	= {
 		.name	= DRV_NAME,
 		.owner	= THIS_MODULE,
+#ifdef CONFIG_OF
+		.of_match_table = altera_jtaguart_match,
+#endif
 	},
 };
 

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH] Add devicetree support to altera_jtaguart

[Grant Likely]

On Wed, Jan 12, 2011 at 11:17:31PM +0100, Walter Goossens wrote:
> This patch adds devicetree support to the altera_jtaguart driver.
> Tested on hardware on the nios2 architecture.
> 

Hi Walter, comments below...

> diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c
> index f9b49b5..b71ba92 100644
> --- a/drivers/serial/altera_jtaguart.c
> +++ b/drivers/serial/altera_jtaguart.c
> @@ -431,21 +431,44 @@ static int __devinit altera_jtaguart_probe(struct platform_device *pdev)
>  {
>  	struct altera_jtaguart_platform_uart *platp = pdev->dev.platform_data;
>  	struct uart_port *port;
> -	int i;
> -
> -	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS && platp[i].mapbase; i++) {
> -		port = &altera_jtaguart_ports[i].port;
> -
> -		port->line = i;
> -		port->type = PORT_ALTERA_JTAGUART;
> -		port->mapbase = platp[i].mapbase;
> -		port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
> -		port->iotype = SERIAL_IO_MEM;
> -		port->irq = platp[i].irq;
> -		port->ops = &altera_jtaguart_ops;
> -		port->flags = ASYNC_BOOT_AUTOCONF;
> -
> -		uart_add_one_port(&altera_jtaguart_driver, port);
> +	if(platp) {
> +		int i;
> +		for (i = 0; i < ALTERA_JTAGUART_MAXPORTS && platp[i].mapbase; i++) {
> +			port = &altera_jtaguart_ports[i].port;
> +
> +			port->line = i;
> +			port->type = PORT_ALTERA_JTAGUART;
> +			port->mapbase = platp[i].mapbase;
> +			port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
> +			port->iotype = SERIAL_IO_MEM;
> +			port->irq = platp[i].irq;
> +			port->ops = &altera_jtaguart_ops;
> +			port->flags = ASYNC_BOOT_AUTOCONF;
> +
> +			uart_add_one_port(&altera_jtaguart_driver, port);
> +		}
> +#ifdef CONFIG_OF
> +	} else {
> +		struct resource *res_irq;
> +		struct resource *res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +		if(res_mem)
> +		{
> +			res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +			if(res_irq)
> +			{
> +				port = &altera_jtaguart_ports[0].port;
> +				port->line = 0;
> +				port->type = PORT_ALTERA_JTAGUART;
> +				port->mapbase = res_mem->start;
> +				port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
> +				port->iotype = SERIAL_IO_MEM;
> +				port->irq = res_irq->start;
> +				port->ops = &altera_jtaguart_ops;
> +				port->flags = ASYNC_BOOT_AUTOCONF;
> +				uart_add_one_port(&altera_jtaguart_driver, port);
> +			}
> +		}
> +#endif

These two blocks are largely identical.  You should be able to
implement this with considerably less duplicated code between the two
drivers.  Since the scope of instantiating this particular device, I
recommend splitting it up so that there is only ever one device per
port, with the irq and register addresses stored in the resource table.
By making that change, exactly the same code would drive both OF and
non-OF use-cases.

Also, the way this is implemented precludes having more than
one instance of the device.  It would be better if the port structure
was dynamically allocated (but I do understand that the current driver
only handles one instance anyway, so it would make sense for that to
be a separate patch).

The rest of this patch look fine.

>  	}
>  
>  	return 0;
> @@ -464,6 +487,15 @@ static int __devexit altera_jtaguart_remove(struct platform_device *pdev)
>  
>  	return 0;
>  }
> +#ifdef CONFIG_OF
> +static struct of_device_id altera_jtaguart_match[] = {
> +	{ 
> +		.compatible = "altera,altera_juart",
> +	},
> +	{},
> +}
> +MODULE_DEVICE_TABLE(of, altera_jtaguart_match);
> +#endif /* CONFIG_OF */
>  
>  static struct platform_driver altera_jtaguart_platform_driver = {
>  	.probe	= altera_jtaguart_probe,
> @@ -471,6 +503,9 @@ static struct platform_driver altera_jtaguart_platform_driver = {
>  	.driver	= {
>  		.name	= DRV_NAME,
>  		.owner	= THIS_MODULE,
> +#ifdef CONFIG_OF
> +		.of_match_table = altera_jtaguart_match,
> +#endif
>  	},
>  };
>  

> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCHv2] Add devicetree support to altera_jtaguart

[Walter Goossens]

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

This patch adds devicetree support to the altera_jtaguart driver.
Tested on hardware on the nios2 architecture.

  


[-- Attachment #2: altera_jtaguart_fdt.patch --]
[-- Type: text/plain, Size: 1893 bytes --]

diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c
index f9b49b5..238348e 100644
--- a/drivers/serial/altera_jtaguart.c
+++ b/drivers/serial/altera_jtaguart.c
@@ -433,15 +433,33 @@ static int __devinit altera_jtaguart_probe(struct platform_device *pdev)
 	struct uart_port *port;
 	int i;
 
-	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS && platp[i].mapbase; i++) {
+	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS; i++) {
 		port = &altera_jtaguart_ports[i].port;
 
+		if(platp)
+		{
+			if(!platp[i].mapbase)
+				break;
+
+			port->mapbase = platp[i].mapbase;
+			port->irq = platp[i].irq;
+		} else {
+#ifdef CONFIG_OF
+			struct resource *res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+			struct resource *res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+			if((!res_mem) || (!res_irq))
+				break;
+
+			port->mapbase = res_mem->start;
+			port->irq = res_irq->start;
+#else
+			break;
+#endif
+		}
 		port->line = i;
 		port->type = PORT_ALTERA_JTAGUART;
-		port->mapbase = platp[i].mapbase;
 		port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
 		port->iotype = SERIAL_IO_MEM;
-		port->irq = platp[i].irq;
 		port->ops = &altera_jtaguart_ops;
 		port->flags = ASYNC_BOOT_AUTOCONF;
 
@@ -464,6 +482,15 @@ static int __devexit altera_jtaguart_remove(struct platform_device *pdev)
 
 	return 0;
 }
+#ifdef CONFIG_OF
+static struct of_device_id altera_jtaguart_match[] = {
+	{ 
+		.compatible = "altera,altera_juart",
+	},
+	{},
+}
+MODULE_DEVICE_TABLE(of, altera_jtaguart_match);
+#endif /* CONFIG_OF */
 
 static struct platform_driver altera_jtaguart_platform_driver = {
 	.probe	= altera_jtaguart_probe,
@@ -471,6 +498,9 @@ static struct platform_driver altera_jtaguart_platform_driver = {
 	.driver	= {
 		.name	= DRV_NAME,
 		.owner	= THIS_MODULE,
+#ifdef CONFIG_OF
+		.of_match_table = altera_jtaguart_match,
+#endif
 	},
 };
 

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [Nios2-dev] [PATCHv2] Add devicetree support to altera_jtaguart

[Tobias Klauser]

Hi Walter

A small comment on the patch is below, though I have not run it yet
(need some more time to get everything started with devicetree here).

On Thu, 13 Jan 2011 02:18:55 +0100 Walter Goossens <waltergoossens-CmkmPbn3yAE@public.gmane.org> wrote:
> This patch adds devicetree support to the altera_jtaguart driver.
> Tested on hardware on the nios2 architecture.
>
> diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c
> index f9b49b5..238348e 100644
> --- a/drivers/serial/altera_jtaguart.c
> +++ b/drivers/serial/altera_jtaguart.c
> @@ -433,15 +433,33 @@ static int __devinit altera_jtaguart_probe(struct platform_device *pdev)
>  	struct uart_port *port;
>  	int i;
>  
> -	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS && platp[i].mapbase; i++) {
> +	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS; i++) {
>  		port = &altera_jtaguart_ports[i].port;
>  
> +		if(platp)
> +		{
> +			if(!platp[i].mapbase)
> +				break;
> +
> +			port->mapbase = platp[i].mapbase;
> +			port->irq = platp[i].irq;
> +		} else {
> +#ifdef CONFIG_OF
> +			struct resource *res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +			struct resource *res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +			if((!res_mem) || (!res_irq))
> +				break;

We should return -ENODEV here if we fail to get the resources. If we
break here, we would return 0, which means the probe function was
successful, but this isn't the case.

> +
> +			port->mapbase = res_mem->start;
> +			port->irq = res_irq->start;
> +#else
> +			break;
> +#endif
> +		}
>  		port->line = i;
>  		port->type = PORT_ALTERA_JTAGUART;
> -		port->mapbase = platp[i].mapbase;
>  		port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
>  		port->iotype = SERIAL_IO_MEM;
> -		port->irq = platp[i].irq;
>  		port->ops = &altera_jtaguart_ops;
>  		port->flags = ASYNC_BOOT_AUTOCONF;
>  
> @@ -464,6 +482,15 @@ static int __devexit altera_jtaguart_remove(struct platform_device *pdev)
>  
>  	return 0;
>  }
> +#ifdef CONFIG_OF
> +static struct of_device_id altera_jtaguart_match[] = {
> +	{ 
> +		.compatible = "altera,altera_juart",
> +	},
> +	{},
> +}
> +MODULE_DEVICE_TABLE(of, altera_jtaguart_match);
> +#endif /* CONFIG_OF */
> 
>  static struct platform_driver altera_jtaguart_platform_driver = {
>  	.probe	= altera_jtaguart_probe,
> @@ -471,6 +498,9 @@ static struct platform_driver altera_jtaguart_platform_driver = {
>  	.driver	= {
>  		.name	= DRV_NAME,
>  		.owner	= THIS_MODULE,
> +#ifdef CONFIG_OF
> +		.of_match_table = altera_jtaguart_match,
> +#endif
>  	},
>  };

[PATCH v3] Add devicetree support to altera_jtaguart

[Walter Goossens]

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

New version of devicetree patch for altera jtag uart.
Fixed comments in v2 and added version nr as discussed in the altera_ps2 
patch.


[-- Attachment #2: altera_jtaguart_fdt.patch --]
[-- Type: text/plain, Size: 2218 bytes --]

diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c
index f9b49b5..a5902fb 100644
--- a/drivers/serial/altera_jtaguart.c
+++ b/drivers/serial/altera_jtaguart.c
@@ -432,23 +432,47 @@ static int __devinit altera_jtaguart_probe(struct platform_device *pdev)
 	struct altera_jtaguart_platform_uart *platp = pdev->dev.platform_data;
 	struct uart_port *port;
 	int i;
+	int res = 0;
 
-	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS && platp[i].mapbase; i++) {
+	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS; i++) {
 		port = &altera_jtaguart_ports[i].port;
 
+		if(platp)
+		{
+			if(!platp[i].mapbase) {
+				res = -ENODEV;
+				break;
+			}
+			port->mapbase = platp[i].mapbase;
+			port->irq = platp[i].irq;
+		} else {
+#ifdef CONFIG_OF
+			struct resource *res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+			struct resource *res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+			if((!res_mem) || (!res_irq))
+			{
+				res = -ENODEV;
+				break;
+			}
+
+			port->mapbase = res_mem->start;
+			port->irq = res_irq->start;
+#else
+			res = -ENODEV;
+			break;
+#endif
+		}
 		port->line = i;
 		port->type = PORT_ALTERA_JTAGUART;
-		port->mapbase = platp[i].mapbase;
 		port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
 		port->iotype = SERIAL_IO_MEM;
-		port->irq = platp[i].irq;
 		port->ops = &altera_jtaguart_ops;
 		port->flags = ASYNC_BOOT_AUTOCONF;
 
 		uart_add_one_port(&altera_jtaguart_driver, port);
 	}
 
-	return 0;
+	return res;
 }
 
 static int __devexit altera_jtaguart_remove(struct platform_device *pdev)
@@ -464,6 +488,15 @@ static int __devexit altera_jtaguart_remove(struct platform_device *pdev)
 
 	return 0;
 }
+#ifdef CONFIG_OF
+static struct of_device_id altera_jtaguart_match[] = {
+	{ 
+		.compatible = "altr,juart-1.0",
+	},
+	{},
+}
+MODULE_DEVICE_TABLE(of, altera_jtaguart_match);
+#endif /* CONFIG_OF */
 
 static struct platform_driver altera_jtaguart_platform_driver = {
 	.probe	= altera_jtaguart_probe,
@@ -471,6 +504,9 @@ static struct platform_driver altera_jtaguart_platform_driver = {
 	.driver	= {
 		.name	= DRV_NAME,
 		.owner	= THIS_MODULE,
+#ifdef CONFIG_OF
+		.of_match_table = altera_jtaguart_match,
+#endif
 	},
 };
 

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v3] Add devicetree support to altera_jtaguart

[Tobias Klauser]

Hi

On Sun, 23 Jan 2011 14:17:27 +0100 Walter Goossens <waltergoossens-CmkmPbn3yAE@public.gmane.org> wrote:
> New version of devicetree patch for altera jtag uart.
> Fixed comments in v2 and added version nr as discussed in the altera_ps2 
> patch.

Did you send the patch to linux-serial yet? The driver is in mainline
and changes should flow there as soon as possible too, I think.

While looking at the patch I noticed that we might want to switch the
driver to having one platform device per port (and thus eliminating the
need for the for loop. Anton Vorontsov submitted a patch to do that for
the altera_uart driver a while ago and it was integrated since.

If you are OK with this, I will change the altera_jtaguart driver in
that way too and submit the change (and also Walter's devicetree patch)
to mainline.

Thanks
Tobias

Re: [PATCH] Add devicetree support to altera_jtaguart

[Walter Goossens]

On 1/13/11 12:06 AM, Grant Likely wrote:
> On Wed, Jan 12, 2011 at 11:17:31PM +0100, Walter Goossens wrote:
>> This patch adds devicetree support to the altera_jtaguart driver.
>> Tested on hardware on the nios2 architecture.
[...]
> These two blocks are largely identical.  You should be able to
> implement this with considerably less duplicated code between the two
> drivers.  Since the scope of instantiating this particular device, I
> recommend splitting it up so that there is only ever one device per
> port, with the irq and register addresses stored in the resource table.
> By making that change, exactly the same code would drive both OF and
> non-OF use-cases.
>
> Also, the way this is implemented precludes having more than
> one instance of the device.  It would be better if the port structure
> was dynamically allocated (but I do understand that the current driver
> only handles one instance anyway, so it would make sense for that to
> be a separate patch).
Yes it's actually quite hard to see a system with more then one 
jtag_uart. It has to be a system with multiple interconnected FPGA's 
since each FPGA has one jtag interface and you can run just one 
jtag_uart per jtag interface.
But it's impossible so...
Not quite sure what the final solution should look like...
> The rest of this patch look fine.
>
>>   	}
>>
>>   	return 0;
>> @@ -464,6 +487,15 @@ static int __devexit altera_jtaguart_remove(struct platform_device *pdev)
>>
>>   	return 0;
>>   }
>> +#ifdef CONFIG_OF
>> +static struct of_device_id altera_jtaguart_match[] = {
>> +	{
>> +		.compatible = "altera,altera_juart",
>> +	},
>> +	{},
>> +}
>> +MODULE_DEVICE_TABLE(of, altera_jtaguart_match);
>> +#endif /* CONFIG_OF */
>>
>>   static struct platform_driver altera_jtaguart_platform_driver = {
>>   	.probe	= altera_jtaguart_probe,
>> @@ -471,6 +503,9 @@ static struct platform_driver altera_jtaguart_platform_driver = {
>>   	.driver	= {
>>   		.name	= DRV_NAME,
>>   		.owner	= THIS_MODULE,
>> +#ifdef CONFIG_OF
>> +		.of_match_table = altera_jtaguart_match,
>> +#endif
>>   	},
>>   };
>>
>> _______________________________________________
>> devicetree-discuss mailing list
>> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>