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

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

[Chris Packham]

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 <chris.packham@alliedtelesis.co.nz>
---
 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

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

[Michael Ellerman]

Chris Packham <chris.packham@alliedtelesis.co.nz> 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 <chris.packham@alliedtelesis.co.nz>
> ---
>  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

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

[Scott Wood]

On Wed, 2020-03-25 at 12:59 +1100, Michael Ellerman wrote:
> Chris Packham <chris.packham@alliedtelesis.co.nz> 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 <chris.packham@alliedtelesis.co.nz>
> > ---
> >  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

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

[Chris Packham]

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 <chris.packham@alliedtelesis.co.nz> 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 <chris.packham@alliedtelesis.co.nz>
> > > ---
> > >  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

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

[Chris Packham]

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 <chris.packham@alliedtelesis.co.nz> 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 <chris.packham@alliedtelesis.co.nz
> > > > >
> > > > ---
> > > >  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
> 
> 

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

[Chris Packham]

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 <chris.packham@alliedtelesis.co.nz>
---
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

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

[Chris Packham]

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 <chris.packham@alliedtelesis.co.nz>
> ---
> 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)

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

[Michael Ellerman]

Chris Packham <Chris.Packham@alliedtelesis.co.nz> 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 <chris.packham@alliedtelesis.co.nz>
>> ---
>> 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/87369xx99u.fsf@mpe.ellerman.id.au/

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

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

[Chris Packham]

On Thu, 2020-04-16 at 21:43 +1000, Michael Ellerman wrote:
> Chris Packham <Chris.Packham@alliedtelesis.co.nz> 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 <chris.packham@alliedtelesis.co.nz>
> > > ---
> > > 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/87369xx99u.fsf@mpe.ellerman.id.au/
> 
> 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

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

[Michael Ellerman]

Chris Packham <Chris.Packham@alliedtelesis.co.nz> writes:
> On Thu, 2020-04-16 at 21:43 +1000, Michael Ellerman wrote:
>> Chris Packham <Chris.Packham@alliedtelesis.co.nz> 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 <chris.packham@alliedtelesis.co.nz>
>> > > ---
>> > > 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/87369xx99u.fsf@mpe.ellerman.id.au/
>> 
>> 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