2020-03-24 21:37:08

by Chris Packham

[permalink] [raw]
Subject: [PATCH] powerpc/fsl: Add cache properties for T2080/T2081

Add the d-cache/i-cache properties for the T208x SoCs. The L1 cache on
these SoCs is 32KiB and is split into 64 byte blocks (lines).

Signed-off-by: Chris Packham <[email protected]>
---
arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
index 3f745de44284..2ad27e16ac16 100644
--- a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
@@ -81,6 +81,10 @@ cpus {
cpu0: PowerPC,e6500@0 {
device_type = "cpu";
reg = <0 1>;
+ d-cache-line-size = <64>;
+ i-cache-line-size = <64>;
+ d-cache-size = <32768>;
+ i-cache-size = <32768>;
clocks = <&clockgen 1 0>;
next-level-cache = <&L2_1>;
fsl,portid-mapping = <0x80000000>;
@@ -88,6 +92,10 @@ cpu0: PowerPC,e6500@0 {
cpu1: PowerPC,e6500@2 {
device_type = "cpu";
reg = <2 3>;
+ d-cache-line-size = <64>;
+ i-cache-line-size = <64>;
+ d-cache-size = <32768>;
+ i-cache-size = <32768>;
clocks = <&clockgen 1 0>;
next-level-cache = <&L2_1>;
fsl,portid-mapping = <0x80000000>;
@@ -95,6 +103,10 @@ cpu1: PowerPC,e6500@2 {
cpu2: PowerPC,e6500@4 {
device_type = "cpu";
reg = <4 5>;
+ d-cache-line-size = <64>;
+ i-cache-line-size = <64>;
+ d-cache-size = <32768>;
+ i-cache-size = <32768>;
clocks = <&clockgen 1 0>;
next-level-cache = <&L2_1>;
fsl,portid-mapping = <0x80000000>;
@@ -102,6 +114,10 @@ cpu2: PowerPC,e6500@4 {
cpu3: PowerPC,e6500@6 {
device_type = "cpu";
reg = <6 7>;
+ d-cache-line-size = <64>;
+ i-cache-line-size = <64>;
+ d-cache-size = <32768>;
+ i-cache-size = <32768>;
clocks = <&clockgen 1 0>;
next-level-cache = <&L2_1>;
fsl,portid-mapping = <0x80000000>;
--
2.25.1


2020-03-25 02:00:07

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH] powerpc/fsl: Add cache properties for T2080/T2081

Chris Packham <[email protected]> writes:
> Add the d-cache/i-cache properties for the T208x SoCs. The L1 cache on
> these SoCs is 32KiB and is split into 64 byte blocks (lines).
>
> Signed-off-by: Chris Packham <[email protected]>
> ---
> arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)

LGTM.

I'll wait a few days to see if Scott wants to ack it.

cheers


> diff --git a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> index 3f745de44284..2ad27e16ac16 100644
> --- a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> @@ -81,6 +81,10 @@ cpus {
> cpu0: PowerPC,e6500@0 {
> device_type = "cpu";
> reg = <0 1>;
> + d-cache-line-size = <64>;
> + i-cache-line-size = <64>;
> + d-cache-size = <32768>;
> + i-cache-size = <32768>;
> clocks = <&clockgen 1 0>;
> next-level-cache = <&L2_1>;
> fsl,portid-mapping = <0x80000000>;
> @@ -88,6 +92,10 @@ cpu0: PowerPC,e6500@0 {
> cpu1: PowerPC,e6500@2 {
> device_type = "cpu";
> reg = <2 3>;
> + d-cache-line-size = <64>;
> + i-cache-line-size = <64>;
> + d-cache-size = <32768>;
> + i-cache-size = <32768>;
> clocks = <&clockgen 1 0>;
> next-level-cache = <&L2_1>;
> fsl,portid-mapping = <0x80000000>;
> @@ -95,6 +103,10 @@ cpu1: PowerPC,e6500@2 {
> cpu2: PowerPC,e6500@4 {
> device_type = "cpu";
> reg = <4 5>;
> + d-cache-line-size = <64>;
> + i-cache-line-size = <64>;
> + d-cache-size = <32768>;
> + i-cache-size = <32768>;
> clocks = <&clockgen 1 0>;
> next-level-cache = <&L2_1>;
> fsl,portid-mapping = <0x80000000>;
> @@ -102,6 +114,10 @@ cpu2: PowerPC,e6500@4 {
> cpu3: PowerPC,e6500@6 {
> device_type = "cpu";
> reg = <6 7>;
> + d-cache-line-size = <64>;
> + i-cache-line-size = <64>;
> + d-cache-size = <32768>;
> + i-cache-size = <32768>;
> clocks = <&clockgen 1 0>;
> next-level-cache = <&L2_1>;
> fsl,portid-mapping = <0x80000000>;
> --
> 2.25.1

2020-03-25 02:13:47

by Crystal Wood

[permalink] [raw]
Subject: Re: [PATCH] powerpc/fsl: Add cache properties for T2080/T2081

On Wed, 2020-03-25 at 12:59 +1100, Michael Ellerman wrote:
> Chris Packham <[email protected]> writes:
> > Add the d-cache/i-cache properties for the T208x SoCs. The L1 cache on
> > these SoCs is 32KiB and is split into 64 byte blocks (lines).
> >
> > Signed-off-by: Chris Packham <[email protected]>
> > ---
> > arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
>
> LGTM.
>
> I'll wait a few days to see if Scott wants to ack it.
>
> cheers
>
>
> > diff --git a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > index 3f745de44284..2ad27e16ac16 100644
> > --- a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > +++ b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > @@ -81,6 +81,10 @@ cpus {
> > cpu0: PowerPC,e6500@0 {
> > device_type = "cpu";
> > reg = <0 1>;
> > + d-cache-line-size = <64>;
> > + i-cache-line-size = <64>;
> > + d-cache-size = <32768>;
> > + i-cache-size = <32768>;
> > clocks = <&clockgen 1 0>;
> > next-level-cache = <&L2_1>;
> > fsl,portid-mapping = <0x80000000>;

U-Boot should be setting d/i-cache-size and d/i-cache-block-size -- are you
using something else?

The line size is the same as the block size so we don't need a separate d/i-
cache-line-size.

-Scott


2020-03-25 02:38:47

by Chris Packham

[permalink] [raw]
Subject: Re: [PATCH] powerpc/fsl: Add cache properties for T2080/T2081

On Tue, 2020-03-24 at 21:08 -0500, Scott Wood wrote:
> On Wed, 2020-03-25 at 12:59 +1100, Michael Ellerman wrote:
> > Chris Packham <[email protected]> writes:
> > > Add the d-cache/i-cache properties for the T208x SoCs. The L1
> > > cache on
> > > these SoCs is 32KiB and is split into 64 byte blocks (lines).
> > >
> > > Signed-off-by: Chris Packham <[email protected]>
> > > ---
> > > arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi | 16 ++++++++++++++++
> > > 1 file changed, 16 insertions(+)
> >
> > LGTM.
> >
> > I'll wait a few days to see if Scott wants to ack it.
> >
> > cheers
> >
> >
> > > diff --git a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > > b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > > index 3f745de44284..2ad27e16ac16 100644
> > > --- a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > > +++ b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > > @@ -81,6 +81,10 @@ cpus {
> > > cpu0: PowerPC,e6500@0 {
> > > device_type = "cpu";
> > > reg = <0 1>;
> > > + d-cache-line-size = <64>;
> > > + i-cache-line-size = <64>;
> > > + d-cache-size = <32768>;
> > > + i-cache-size = <32768>;
> > > clocks = <&clockgen 1 0>;
> > > next-level-cache = <&L2_1>;
> > > fsl,portid-mapping = <0x80000000>;
>
> U-Boot should be setting d/i-cache-size and d/i-cache-block-size --
> are you
> using something else?

Nope it is u-boot specifically

U-Boot 2017.01-rc3-dirty

I'm pretty sure the '-dirty' is just a change to use a different cross-
compiler but I can't confirm and I'm a little hesitant to try updating
as I've only got remote access to the board right now.

>
> The line size is the same as the block size so we don't need a
> separate d/i-
> cache-line-size.
>

I'm not sure that'll work looking at the code[1]. It has logic to set
bsizep to lsizep if no block size is set but not the other way round.
Looking at the spec from devicetree.org this actually seems wrong.
Perhaps that is the real source of the error.

--
[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/setup_64.c#n510


2020-03-25 02:51:01

by Chris Packham

[permalink] [raw]
Subject: Re: [PATCH] powerpc/fsl: Add cache properties for T2080/T2081

On Wed, 2020-03-25 at 15:38 +1300, Chris Packham wrote:
> On Tue, 2020-03-24 at 21:08 -0500, Scott Wood wrote:
> > On Wed, 2020-03-25 at 12:59 +1100, Michael Ellerman wrote:
> > > Chris Packham <[email protected]> writes:
> > > > Add the d-cache/i-cache properties for the T208x SoCs. The L1
> > > > cache on
> > > > these SoCs is 32KiB and is split into 64 byte blocks (lines).
> > > >
> > > > Signed-off-by: Chris Packham <[email protected]
> > > > >
> > > > ---
> > > > arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi | 16
> > > > ++++++++++++++++
> > > > 1 file changed, 16 insertions(+)
> > >
> > > LGTM.
> > >
> > > I'll wait a few days to see if Scott wants to ack it.
> > >
> > > cheers
> > >
> > >
> > > > diff --git a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > > > b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > > > index 3f745de44284..2ad27e16ac16 100644
> > > > --- a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > > > +++ b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > > > @@ -81,6 +81,10 @@ cpus {
> > > > cpu0: PowerPC,e6500@0 {
> > > > device_type = "cpu";
> > > > reg = <0 1>;
> > > > + d-cache-line-size = <64>;
> > > > + i-cache-line-size = <64>;
> > > > + d-cache-size = <32768>;
> > > > + i-cache-size = <32768>;
> > > > clocks = <&clockgen 1 0>;
> > > > next-level-cache = <&L2_1>;
> > > > fsl,portid-mapping = <0x80000000>;
> >
> > U-Boot should be setting d/i-cache-size and d/i-cache-block-size --
> > are you
> > using something else?
>
> Nope it is u-boot specifically
>
> U-Boot 2017.01-rc3-dirty
>
> I'm pretty sure the '-dirty' is just a change to use a different
> cross-
> compiler but I can't confirm and I'm a little hesitant to try
> updating
> as I've only got remote access to the board right now.
>
> >
> > The line size is the same as the block size so we don't need a
> > separate d/i-
> > cache-line-size.
> >
>
> I'm not sure that'll work looking at the code[1]. It has logic to set
> bsizep to lsizep if no block size is set but not the other way round.
> Looking at the spec from devicetree.org this actually seems wrong.
> Perhaps that is the real source of the error.

Sure enough without my change

# ls /sys/firmware/devicetree/base/cpus/PowerPC,e6500@0/
bus-frequency d-cache-size name
cache-stash-id device_type next-level-cache
clock-frequency enable-method phandle
clocks fsl,portid-mapping reg
cpu-release-addr i-cache-block-size status
d-cache-block-size i-cache-sets timebase-frequency
d-cache-sets i-cache-size

So it's the lack of handling the optional line-size. Different patch
incomming.

>
> --
> [1] -
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/setup_64.c#n510
>
>

2020-03-25 03:19:45

by Chris Packham

[permalink] [raw]
Subject: [PATCH v2] powerpc/setup_64: Set cache-line-size based on cache-block-size

If {i,d}-cache-block-size is set and {i,d}-cache-line-size is not, use
the block-size value for both. Per the devicetree spec cache-line-size
is only needed if it differs from the block size.

Signed-off-by: Chris Packham <[email protected]>
---
It looks as though the bsizep = lsizep is not required per the spec but it's
probably safer to retain it.

Changes in v2:
- Scott pointed out that u-boot should be filling in the cache properties
(which it does). But it does not specify a cache-line-size because it
provides a cache-block-size and the spec says you don't have to if they are
the same. So the error is in the parsing not in the devicetree itself.

arch/powerpc/kernel/setup_64.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index e05e6dd67ae6..dd8a238b54b8 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -516,6 +516,8 @@ static bool __init parse_cache_info(struct device_node *np,
lsizep = of_get_property(np, propnames[3], NULL);
if (bsizep == NULL)
bsizep = lsizep;
+ if (lsizep == NULL)
+ lsizep = bsizep;
if (lsizep != NULL)
lsize = be32_to_cpu(*lsizep);
if (bsizep != NULL)
--
2.25.1

2020-04-16 04:39:57

by Chris Packham

[permalink] [raw]
Subject: Re: [PATCH v2] powerpc/setup_64: Set cache-line-size based on cache-block-size

Hi All,

On Wed, 2020-03-25 at 16:18 +1300, Chris Packham wrote:
> If {i,d}-cache-block-size is set and {i,d}-cache-line-size is not,
> use
> the block-size value for both. Per the devicetree spec cache-line-
> size
> is only needed if it differs from the block size.
>
> Signed-off-by: Chris Packham <[email protected]>
> ---
> It looks as though the bsizep = lsizep is not required per the spec
> but it's
> probably safer to retain it.
>
> Changes in v2:
> - Scott pointed out that u-boot should be filling in the cache
> properties
> (which it does). But it does not specify a cache-line-size because
> it
> provides a cache-block-size and the spec says you don't have to if
> they are
> the same. So the error is in the parsing not in the devicetree
> itself.
>

Ping? This thread went kind of quiet.

> arch/powerpc/kernel/setup_64.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/setup_64.c
> b/arch/powerpc/kernel/setup_64.c
> index e05e6dd67ae6..dd8a238b54b8 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -516,6 +516,8 @@ static bool __init parse_cache_info(struct
> device_node *np,
> lsizep = of_get_property(np, propnames[3], NULL);
> if (bsizep == NULL)
> bsizep = lsizep;
> + if (lsizep == NULL)
> + lsizep = bsizep;
> if (lsizep != NULL)
> lsize = be32_to_cpu(*lsizep);
> if (bsizep != NULL)

2020-04-16 11:45:40

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2] powerpc/setup_64: Set cache-line-size based on cache-block-size

Chris Packham <[email protected]> writes:
> Hi All,
>
> On Wed, 2020-03-25 at 16:18 +1300, Chris Packham wrote:
>> If {i,d}-cache-block-size is set and {i,d}-cache-line-size is not,
>> use
>> the block-size value for both. Per the devicetree spec cache-line-
>> size
>> is only needed if it differs from the block size.
>>
>> Signed-off-by: Chris Packham <[email protected]>
>> ---
>> It looks as though the bsizep = lsizep is not required per the spec
>> but it's
>> probably safer to retain it.
>>
>> Changes in v2:
>> - Scott pointed out that u-boot should be filling in the cache
>> properties
>> (which it does). But it does not specify a cache-line-size because
>> it
>> provides a cache-block-size and the spec says you don't have to if
>> they are
>> the same. So the error is in the parsing not in the devicetree
>> itself.
>>
>
> Ping? This thread went kind of quiet.

I replied in the other thread:

https://lore.kernel.org/linuxppc-dev/[email protected]/

But then the merge window happened which is a busy time.

What I'd really like is a v3 that incorporates the info I wrote in the
other thread and a Fixes tag.

If you feel like doing that, that would be great. Otherwise I'll do it
tomorrow.

cheers

2020-04-16 21:32:59

by Chris Packham

[permalink] [raw]
Subject: Re: [PATCH v2] powerpc/setup_64: Set cache-line-size based on cache-block-size

On Thu, 2020-04-16 at 21:43 +1000, Michael Ellerman wrote:
> Chris Packham <[email protected]> writes:
> > Hi All,
> >
> > On Wed, 2020-03-25 at 16:18 +1300, Chris Packham wrote:
> > > If {i,d}-cache-block-size is set and {i,d}-cache-line-size is
> > > not,
> > > use
> > > the block-size value for both. Per the devicetree spec cache-
> > > line-
> > > size
> > > is only needed if it differs from the block size.
> > >
> > > Signed-off-by: Chris Packham <[email protected]>
> > > ---
> > > It looks as though the bsizep = lsizep is not required per the
> > > spec
> > > but it's
> > > probably safer to retain it.
> > >
> > > Changes in v2:
> > > - Scott pointed out that u-boot should be filling in the cache
> > > properties
> > > (which it does). But it does not specify a cache-line-size
> > > because
> > > it
> > > provides a cache-block-size and the spec says you don't have to
> > > if
> > > they are
> > > the same. So the error is in the parsing not in the devicetree
> > > itself.
> > >
> >
> > Ping? This thread went kind of quiet.
>
> I replied in the other thread:
>
>
> https://lore.kernel.org/linuxppc-dev/[email protected]/
>
> But then the merge window happened which is a busy time.
>

Yeah I figured that was the case.

> What I'd really like is a v3 that incorporates the info I wrote in
> the
> other thread and a Fixes tag.
>
> If you feel like doing that, that would be great. Otherwise I'll do
> it
> tomorrow.

I'll rebase against Linus's tree and have a go a adding some more words
to the commit message.

>
> cheers

2020-04-20 02:56:54

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2] powerpc/setup_64: Set cache-line-size based on cache-block-size

Chris Packham <[email protected]> writes:
> On Thu, 2020-04-16 at 21:43 +1000, Michael Ellerman wrote:
>> Chris Packham <[email protected]> writes:
>> > On Wed, 2020-03-25 at 16:18 +1300, Chris Packham wrote:
>> > > If {i,d}-cache-block-size is set and {i,d}-cache-line-size is
>> > > not,
>> > > use
>> > > the block-size value for both. Per the devicetree spec cache-
>> > > line-
>> > > size
>> > > is only needed if it differs from the block size.
>> > >
>> > > Signed-off-by: Chris Packham <[email protected]>
>> > > ---
>> > > It looks as though the bsizep = lsizep is not required per the
>> > > spec
>> > > but it's
>> > > probably safer to retain it.
>> > >
>> > > Changes in v2:
>> > > - Scott pointed out that u-boot should be filling in the cache
>> > > properties
>> > > (which it does). But it does not specify a cache-line-size
>> > > because
>> > > it
>> > > provides a cache-block-size and the spec says you don't have to
>> > > if
>> > > they are
>> > > the same. So the error is in the parsing not in the devicetree
>> > > itself.
>> > >
>> >
>> > Ping? This thread went kind of quiet.
>>
>> I replied in the other thread:
>>
>>
>> https://lore.kernel.org/linuxppc-dev/[email protected]/
>>
>> But then the merge window happened which is a busy time.
>>
>
> Yeah I figured that was the case.
>
>> What I'd really like is a v3 that incorporates the info I wrote in
>> the
>> other thread and a Fixes tag.
>>
>> If you feel like doing that, that would be great. Otherwise I'll do
>> it
>> tomorrow.
>
> I'll rebase against Linus's tree and have a go a adding some more words
> to the commit message.

Thanks.

cheers