amba uarts - arm type uarts fifo size provided by device tree

amba uarts - arm type uarts fifo size provided by device tree

[Mark Rutland]

Hi,

Please Cc lakml for patches in this area in future.

On Wed, Feb 18, 2015 at 06:53:11PM +0000, Jorge Ramirez-Ortiz wrote:
> [PATCH 1/2] Documentation: bindings: add optional fifo size property
> [PATCH 2/2] drivers/tty: pl011: read fifo size from OF if present
> 
> 
> These patches aim at resolving an issue present on some of the amba pl011
> uarts.  

Which systems are affected? Neither of your two patches add the property
to any dts file.

> Some of these arm type uarts have their fifo sizes not defined in terms of the
> amba_rev register. 

What does said register actually contain?

Is this a full PL011, or a cut-down SBSA UART?

Mark.

> Those uarts should be able to declare their fifo sizes using the optional
> property "fifo-size" in the device tree. This device tree setting shall take
> precedence over any other values. 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

amba uarts - arm type uarts fifo size provided by device tree

[Jorge Ramirez-Ortiz]

On 02/18/2015 02:06 PM, Mark Rutland wrote:
> Hi,
>
> Please Cc lakml for patches in this area in future.
>
> On Wed, Feb 18, 2015 at 06:53:11PM +0000, Jorge Ramirez-Ortiz wrote:
>> [PATCH 1/2] Documentation: bindings: add optional fifo size property
>> [PATCH 2/2] drivers/tty: pl011: read fifo size from OF if present
>>
>>
>> These patches aim at resolving an issue present on some of the amba pl011
>> uarts.  
> Which systems are affected? Neither of your two patches add the property
> to any dts file.


yes, the system affected is still under revision (I should have mentioned)
https://lkml.org/lkml/2015/2/5/144

this is the board I am using to test
https://www.96boards.org/products/hikey/

>
>> Some of these arm type uarts have their fifo sizes not defined in terms of the
>> amba_rev register. 
> What does said register actually contain?

the amba_rev contains '1' for the Hisilicon Hi6220 SoC

>
> Is this a full PL011, or a cut-down SBSA UART?

I believe is full PL011 (ARM type) - that is what is being run in my tests.

>
> Mark.
>
>> Those uarts should be able to declare their fifo sizes using the optional
>> property "fifo-size" in the device tree. This device tree setting shall take
>> precedence over any other values. 
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

amba uarts - arm type uarts fifo size provided by device tree

[Russell King - ARM Linux]

On Wed, Feb 18, 2015 at 02:33:42PM -0500, Jorge Ramirez-Ortiz wrote:
> On 02/18/2015 02:06 PM, Mark Rutland wrote:
> > Hi,
> >
> > Please Cc lakml for patches in this area in future.
> >
> > On Wed, Feb 18, 2015 at 06:53:11PM +0000, Jorge Ramirez-Ortiz wrote:
> >> [PATCH 1/2] Documentation: bindings: add optional fifo size property
> >> [PATCH 2/2] drivers/tty: pl011: read fifo size from OF if present
> >>
> >>
> >> These patches aim at resolving an issue present on some of the amba pl011
> >> uarts.  
> > Which systems are affected? Neither of your two patches add the property
> > to any dts file.
> 
> 
> yes, the system affected is still under revision (I should have mentioned)
> https://lkml.org/lkml/2015/2/5/144

Sorry, I can't follow that thread.  What FIFO size do your UARTs have?
What is the full ID of the UARTs?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

amba uarts - arm type uarts fifo size provided by device tree

[Jorge Ramirez-Ortiz]

On 02/18/2015 07:34 PM, Russell King - ARM Linux wrote:
> On Wed, Feb 18, 2015 at 02:33:42PM -0500, Jorge Ramirez-Ortiz wrote:
>> On 02/18/2015 02:06 PM, Mark Rutland wrote:
>>> Hi,
>>>
>>> Please Cc lakml for patches in this area in future.
>>>
>>> On Wed, Feb 18, 2015 at 06:53:11PM +0000, Jorge Ramirez-Ortiz wrote:
>>>> [PATCH 1/2] Documentation: bindings: add optional fifo size property
>>>> [PATCH 2/2] drivers/tty: pl011: read fifo size from OF if present
>>>>
>>>>
>>>> These patches aim at resolving an issue present on some of the amba pl011
>>>> uarts.  
>>> Which systems are affected? Neither of your two patches add the property
>>> to any dts file.
>>
>> yes, the system affected is still under revision (I should have mentioned)
>> https://lkml.org/lkml/2015/2/5/144
> Sorry, I can't follow that thread.  What FIFO size do your UARTs have?
> What is the full ID of the UARTs?
>

I just added a temporary printk to my logs for clarity

[    3.328845] name = f7111000.uart, id = 0x241011, amba_rev = 2, fifo size = 64

[PATCH 2/2] drivers/tty: pl011: read fifo size from OF if present

[Mark Rutland]

On Wed, Feb 18, 2015 at 06:53:13PM +0000, Jorge Ramirez-Ortiz wrote:
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 02016fc..23fef63 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -84,6 +84,12 @@ struct vendor_data {
>  
>  static unsigned int get_fifosize_arm(struct amba_device *dev)
>  {
> +	const void *prop;
> +
> +	prop = of_get_property(dev->dev.of_node, "fifo-size", NULL);
> +	if (prop)
> +		return of_read_ulong(prop, 1);

Use of_property_read_u32.

You will need to sanity check the result, also.

What value do you need for your platform?

Mark.

> +
>  	return amba_rev(dev) < 3 ? 16 : 32;
>  }
>  
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[version 2] amba uarts - arm type uarts fifo size provided by device tree

[Jorge Ramirez-Ortiz]

[PATCH 1/2] Documentation: bindings: add optional fifo size property
[PATCH 2/2] drivers/tty: pl011: read fifo size from OF if present [updated]

These patches aim at resolving an issue present on some of the amba pl011 uarts.

They were developed for the Hisilicon Hi6220 SoC where the fifo size on the
uarts is 64 despite the amba revision having been set to 1. 

This makes the current way of detecting the fifo size based on the value of the
amba_rev non usable.

A mechanism for those uarts to retrieve the fifo sizes using the optional
property "fifo-size" in the device tree declared in PATCH1/2 is implemented
under PATCH2/2.

This device tree setting shall take precedence over any other calculated values.

[PATCH 2/2] drivers/tty: pl011: read fifo size from OF if present

[Jorge Ramirez-Ortiz]

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
---
 drivers/tty/serial/amba-pl011.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 02016fc..c6a2dd8 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -84,6 +84,11 @@ struct vendor_data {
 
 static unsigned int get_fifosize_arm(struct amba_device *dev)
 {
+	u32 val;
+
+	if (!of_property_read_u32(dev->dev.of_node, "fifo-size", &val))
+		return val;
+
 	return amba_rev(dev) < 3 ? 16 : 32;
 }
 
-- 
1.9.1

[PATCH 2/2] drivers/tty: pl011: read fifo size from OF if present

[Jorge Ramirez-Ortiz]

On 02/18/2015 03:07 PM, Jorge Ramirez-Ortiz wrote:
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 02016fc..c6a2dd8 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -84,6 +84,11 @@ struct vendor_data {
>  
>  static unsigned int get_fifosize_arm(struct amba_device *dev)
>  {
> +	u32 val;
> +
> +	if (!of_property_read_u32(dev->dev.of_node, "fifo-size", &val))
> +		return val;
> +
>  	return amba_rev(dev) < 3 ? 16 : 32;
>  }
>  

just following up on this patch. is anything else required?