[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Ding Tianhong]

This erratum describes a bug in logic outside the core, so MIDR can't be
used to identify its presence, and reading an SoC-specific revision
register from common arch timer code would be awkward.  So, describe it
in the device tree.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 Documentation/devicetree/bindings/arm/arch_timer.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index ef5fbe9..26bc837 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -31,6 +31,12 @@ to deliver its interrupts via SPIs.
   This also affects writes to the tval register, due to the implicit
   counter read.

+- hisilicon,erratum-161x01 : A boolean property. Indicates the presence of
+  QorIQ erratum 161201, which says that reading the counter is
+  unreliable unless the small range of value is returned by back-to-back reads.
+  This also affects writes to the tval register, due to the implicit
+  counter read.
+
 ** Optional properties:

 - arm,cpu-registers-not-fw-configured : Firmware does not initialize
-- 
1.9.0

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Shawn Guo]

On Sun, Oct 23, 2016 at 11:21:10AM +0800, Ding Tianhong wrote:
> This erratum describes a bug in logic outside the core, so MIDR can't be
> used to identify its presence, and reading an SoC-specific revision
> register from common arch timer code would be awkward.  So, describe it
> in the device tree.
> 
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index ef5fbe9..26bc837 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -31,6 +31,12 @@ to deliver its interrupts via SPIs.
>    This also affects writes to the tval register, due to the implicit
>    counter read.
> 
> +- hisilicon,erratum-161x01 : A boolean property. Indicates the presence of
> +  QorIQ erratum 161201, which says that reading the counter is

QorIQ is a Freescale/NXP specific name, and shouldn't be there.

Shawn

> +  unreliable unless the small range of value is returned by back-to-back reads.
> +  This also affects writes to the tval register, due to the implicit
> +  counter read.
> +
>  ** Optional properties:
> 
>  - arm,cpu-registers-not-fw-configured : Firmware does not initialize
> -- 
> 1.9.0
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Ding Tianhong]

On 2016/10/23 20:04, Shawn Guo wrote:
> On Sun, Oct 23, 2016 at 11:21:10AM +0800, Ding Tianhong wrote:
>> This erratum describes a bug in logic outside the core, so MIDR can't be
>> used to identify its presence, and reading an SoC-specific revision
>> register from common arch timer code would be awkward.  So, describe it
>> in the device tree.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>>  Documentation/devicetree/bindings/arm/arch_timer.txt | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> index ef5fbe9..26bc837 100644
>> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
>> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> @@ -31,6 +31,12 @@ to deliver its interrupts via SPIs.
>>    This also affects writes to the tval register, due to the implicit
>>    counter read.
>>
>> +- hisilicon,erratum-161x01 : A boolean property. Indicates the presence of
>> +  QorIQ erratum 161201, which says that reading the counter is
> 
> QorIQ is a Freescale/NXP specific name, and shouldn't be there.
> 
> Shawn
> 

Got it, will wait other feedback and fix them together, thanks.

Ding

>> +  unreliable unless the small range of value is returned by back-to-back reads.
>> +  This also affects writes to the tval register, due to the implicit
>> +  counter read.
>> +
>>  ** Optional properties:
>>
>>  - arm,cpu-registers-not-fw-configured : Firmware does not initialize
>> -- 
>> 1.9.0
>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> .
> 

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Marc Zyngier]

On 23/10/16 04:21, Ding Tianhong wrote:
> This erratum describes a bug in logic outside the core, so MIDR can't be
> used to identify its presence, and reading an SoC-specific revision
> register from common arch timer code would be awkward.  So, describe it
> in the device tree.
> 
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index ef5fbe9..26bc837 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -31,6 +31,12 @@ to deliver its interrupts via SPIs.
>    This also affects writes to the tval register, due to the implicit
>    counter read.
> 
> +- hisilicon,erratum-161x01 : A boolean property. Indicates the presence of
> +  QorIQ erratum 161201, which says that reading the counter is

Other than the copy/paste of the FSL erratum, please document the actual
erratum number. Is that 161x01 or 161201?

> +  unreliable unless the small range of value is returned by back-to-back reads.

That's a detail that doesn't belong in the DT, but that would be much
better next to the code doing the actual handling.

> +  This also affects writes to the tval register, due to the implicit
> +  counter read.
> +
>  ** Optional properties:
> 
>  - arm,cpu-registers-not-fw-configured : Firmware does not initialize
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Ding Tianhong]

On 2016/10/24 16:36, Marc Zyngier wrote:
> On 23/10/16 04:21, Ding Tianhong wrote:
>> This erratum describes a bug in logic outside the core, so MIDR can't be
>> used to identify its presence, and reading an SoC-specific revision
>> register from common arch timer code would be awkward.  So, describe it
>> in the device tree.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>>  Documentation/devicetree/bindings/arm/arch_timer.txt | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> index ef5fbe9..26bc837 100644
>> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
>> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> @@ -31,6 +31,12 @@ to deliver its interrupts via SPIs.
>>    This also affects writes to the tval register, due to the implicit
>>    counter read.
>>
>> +- hisilicon,erratum-161x01 : A boolean property. Indicates the presence of
>> +  QorIQ erratum 161201, which says that reading the counter is
> 
> Other than the copy/paste of the FSL erratum, please document the actual
> erratum number. Is that 161x01 or 161201?
> 

Sorry for the lazy behavior.

>> +  unreliable unless the small range of value is returned by back-to-back reads.
> 
> That's a detail that doesn't belong in the DT, but that would be much
> better next to the code doing the actual handling.
> 
Got it.

Thanks
Ding

>> +  This also affects writes to the tval register, due to the implicit
>> +  counter read.
>> +
>>  ** Optional properties:
>>
>>  - arm,cpu-registers-not-fw-configured : Firmware does not initialize
>>
> 
> Thanks,
> 
> 	M.
> 

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Mark Rutland]

On Sun, Oct 23, 2016 at 11:21:10AM +0800, Ding Tianhong wrote:
> +- hisilicon,erratum-161x01 : A boolean property. Indicates the presence of
> +  QorIQ erratum 161201, which says that reading the counter is
> +  unreliable unless the small range of value is returned by back-to-back reads.
> +  This also affects writes to the tval register, due to the implicit
> +  counter read.

Is "161x01" the *exact* erratum number, or is the 'x' a wildcard? Please
use the *exact* erratum number, even if that means we have to list
several.

Is "161x01" an *erratum* number, or the *part* number of affected
devices?

Thanks,
Mark.

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Ding Tianhong]

On 2016/10/24 19:16, Mark Rutland wrote:
> On Sun, Oct 23, 2016 at 11:21:10AM +0800, Ding Tianhong wrote:
>> +- hisilicon,erratum-161x01 : A boolean property. Indicates the presence of
>> +  QorIQ erratum 161201, which says that reading the counter is
>> +  unreliable unless the small range of value is returned by back-to-back reads.
>> +  This also affects writes to the tval register, due to the implicit
>> +  counter read.
> 
> Is "161x01" the *exact* erratum number, or is the 'x' a wildcard? Please
> use the *exact* erratum number, even if that means we have to list
> several.
> 

Hi Mark:

The 'x' is a wildcard, it will cover 161001 to 161601 several numbers, I will discuss to
the chip develop and get a exact erratum number.

Thanks.
Ding

> Is "161x01" an *erratum* number, or the *part* number of affected
> devices?
> 
> Thanks,
> Mark.
> 
> .
> 

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Mark Rutland]

On Mon, Oct 24, 2016 at 08:40:01PM +0800, Ding Tianhong wrote:
> On 2016/10/24 19:16, Mark Rutland wrote:
> > Is "161x01" the *exact* erratum number, or is the 'x' a wildcard?
> 
> The 'x' is a wildcard, it will cover 161001 to 161601 several numbers,

Given you're using a wildcard, I take it that this is a *part* number?

Thanks,
Mark.

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Ding Tianhong]

On 2016/10/24 21:16, Mark Rutland wrote:
> On Mon, Oct 24, 2016 at 08:40:01PM +0800, Ding Tianhong wrote:
>> On 2016/10/24 19:16, Mark Rutland wrote:
>>> Is "161x01" the *exact* erratum number, or is the 'x' a wildcard?
>>
>> The 'x' is a wildcard, it will cover 161001 to 161601 several numbers,
> 
> Given you're using a wildcard, I take it that this is a *part* number?
> 
Yes, I was doubt how to fix this, should I choose a better erratum number?

> Thanks,
> Mark.
> 
> .
> 

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Mark Rutland]

On Mon, Oct 24, 2016 at 09:23:10PM +0800, Ding Tianhong wrote:
> On 2016/10/24 21:16, Mark Rutland wrote:
> > On Mon, Oct 24, 2016 at 08:40:01PM +0800, Ding Tianhong wrote:
> >> On 2016/10/24 19:16, Mark Rutland wrote:
> >>> Is "161x01" the *exact* erratum number, or is the 'x' a wildcard?
> >>
> >> The 'x' is a wildcard, it will cover 161001 to 161601 several numbers,
> > 
> > Given you're using a wildcard, I take it that this is a *part* number?
> 
> Yes, I was doubt how to fix this, should I choose a better erratum number?

Typically, we expect that each vendor has some central database of their
errata, with each having a unique ID.

If Huawei do not have such a database, I do not think that we should
invent an erratum number here.

Thanks,
Mark.

[PATCH 1/3] arm64: arch_timer: Add device tree binding for hisilicon-161x01 erratum

[Ding Tianhong]

On 2016/10/24 21:39, Mark Rutland wrote:
> On Mon, Oct 24, 2016 at 09:23:10PM +0800, Ding Tianhong wrote:
>> On 2016/10/24 21:16, Mark Rutland wrote:
>>> On Mon, Oct 24, 2016 at 08:40:01PM +0800, Ding Tianhong wrote:
>>>> On 2016/10/24 19:16, Mark Rutland wrote:
>>>>> Is "161x01" the *exact* erratum number, or is the 'x' a wildcard?
>>>>
>>>> The 'x' is a wildcard, it will cover 161001 to 161601 several numbers,
>>>
>>> Given you're using a wildcard, I take it that this is a *part* number?
>>
>> Yes, I was doubt how to fix this, should I choose a better erratum number?
> 
> Typically, we expect that each vendor has some central database of their
> errata, with each having a unique ID.
> 
> If Huawei do not have such a database, I do not think that we should
> invent an erratum number here.
> 

Hi Marko<\x1a

After discussion with our chip developer, we decide the 161601 as the *exact* erratum number for this chip to cover
all the problem and register this in our company's database, thanks.

Ding

> Thanks,
> Mark.
> 
> .
>