[v1,2/2] arm: npcm: Enable L2 Cache in NPCM7xx

Message ID 1521155412-29229-3-git-send-email-tmaimon77@gmail.com
State New
Headers show
Series
  • arm: npcm: Modify NPCM7XX machine code
Related show

Commit Message

Tomer Maimon March 15, 2018, 11:10 p.m.
Enable L2 Cache in Nuvoton NPCM7xx BMC.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 arch/arm/mach-npcm/npcm7xx.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Brendan Higgins March 16, 2018, 5:52 a.m. | #1
On Thu, Mar 15, 2018 at 4:16 PM Tomer Maimon <tmaimon77@gmail.com> wrote:

> Enable L2 Cache in Nuvoton NPCM7xx BMC.

> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>   arch/arm/mach-npcm/npcm7xx.c | 2 ++
>   1 file changed, 2 insertions(+)

> diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
> index 5f7cd88103ef..c5f77d854c4f 100644
> --- a/arch/arm/mach-npcm/npcm7xx.c
> +++ b/arch/arm/mach-npcm/npcm7xx.c
> @@ -17,4 +17,6 @@ static const char *const npcm7xx_dt_match[] = {
>   DT_MACHINE_START(NPCM7XX_DT, "NPCM7XX Chip family")
>          .atag_offset    = 0x100,
>          .dt_compat      = npcm7xx_dt_match,
> +       .l2c_aux_val    = 0x0,
> +       .l2c_aux_mask   = ~0x0,

You need to limit this to the specific bit(s) you want to set and verify
that
the l2c driver does not already manage that bit appropriately and that it
can
not be specified via the dtsi.

We discussed this a little while ago with Rob here:
https://www.spinics.net/lists/arm-kernel/msg613372.html

>   MACHINE_END
> --
> 2.14.1


Cheers
Tomer Maimon March 16, 2018, 9:51 p.m. | #2
On 16 March 2018 at 07:52, Brendan Higgins <brendanhiggins@google.com>
wrote:

> On Thu, Mar 15, 2018 at 4:16 PM Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> > Enable L2 Cache in Nuvoton NPCM7xx BMC.
>
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >   arch/arm/mach-npcm/npcm7xx.c | 2 ++
> >   1 file changed, 2 insertions(+)
>
> > diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
> > index 5f7cd88103ef..c5f77d854c4f 100644
> > --- a/arch/arm/mach-npcm/npcm7xx.c
> > +++ b/arch/arm/mach-npcm/npcm7xx.c
> > @@ -17,4 +17,6 @@ static const char *const npcm7xx_dt_match[] = {
> >   DT_MACHINE_START(NPCM7XX_DT, "NPCM7XX Chip family")
> >          .atag_offset    = 0x100,
> >          .dt_compat      = npcm7xx_dt_match,
> > +       .l2c_aux_val    = 0x0,
> > +       .l2c_aux_mask   = ~0x0,
>
> You need to limit this to the specific bit(s) you want to set and verify
> that
> the l2c driver does not already manage that bit appropriately and that it
> can
> not be specified via the dtsi.
>

Do you mean that I need to specify it the same as been done in V7?
because when I run with the above l2c_aux_val and  l2c_aux_mask parameters
I get the same result.

Also if I am not adding the l2c_aux_val and  l2c_aux_mask parameters to
the  DT_MACHINE_START
the L2C cache does not initialize.

>
> We discussed this a little while ago with Rob here:
> https://www.spinics.net/lists/arm-kernel/msg613372.html


Sorry in this link I see only Russel king comment regarding the L2C

>
>
> >   MACHINE_END
> > --
> > 2.14.1
>
>
> Cheers
>

Cheers
<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 16 March 2018 at 07:52, Brendan Higgins <span dir="ltr">&lt;<a href="mailto:brendanhiggins@google.com" target="_blank">brendanhiggins@google.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On Thu, Mar 15, 2018 at 4:16 PM Tomer Maimon &lt;<a href="mailto:tmaimon77@gmail.com" target="_blank">tmaimon77@gmail.com</a>&gt; wrote:<br>
<br>
&gt; Enable L2 Cache in Nuvoton NPCM7xx BMC.<br>
<br>
&gt; Signed-off-by: Tomer Maimon &lt;<a href="mailto:tmaimon77@gmail.com" target="_blank">tmaimon77@gmail.com</a>&gt;<br>
&gt; ---<br>
&gt;   arch/arm/mach-npcm/npcm7xx.c | 2 ++<br>
&gt;   1 file changed, 2 insertions(+)<br>
<br>
&gt; diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c<br>
&gt; index 5f7cd88103ef..c5f77d854c4f 100644<br>
&gt; --- a/arch/arm/mach-npcm/npcm7xx.c<br>
&gt; +++ b/arch/arm/mach-npcm/npcm7xx.c<br>
&gt; @@ -17,4 +17,6 @@ static const char *const npcm7xx_dt_match[] = {<br>
&gt;   DT_MACHINE_START(NPCM7XX_DT, &quot;NPCM7XX Chip family&quot;)<br>
&gt;          .atag_offset    = 0x100,<br>
&gt;          .dt_compat      = npcm7xx_dt_match,<br>
&gt; +       .l2c_aux_val    = 0x0,<br>
&gt; +       .l2c_aux_mask   = ~0x0,<br>
<br>
</span>You need to limit this to the specific bit(s) you want to set and verify<br>
that<br>
the l2c driver does not already manage that bit appropriately and that it<br>
can<br>
not be specified via the dtsi.<br></blockquote><div><br></div><div>Do you mean that I need to specify it the same as been done in V7?</div><div>because when I run with the above <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration-style:initial;text-decoration-color:initial;background-color:rgb(255,255,255);float:none;display:inline">l2c_aux_val and <span> </span><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">l2c_aux_mask parameters</span></span>

 I get the same result.</div><div><br></div><div>Also if I am not adding the <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">l2c_aux_val and 

<span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">l2c_aux_mask parameters to the 

<span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">DT_MACHINE_START</span>

</span></span><br></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">the L2C cache does not initialize. </span></span></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
We discussed this a little while ago with Rob here:<br>
<a href="https://www.spinics.net/lists/arm-kernel/msg613372.html" rel="noreferrer" target="_blank">https://www.spinics.net/lists/<wbr>arm-kernel/msg613372.html</a></blockquote><div><br></div><div>Sorry in this link I see only Russel king comment regarding the L2C </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
&gt;   MACHINE_END<br>
&gt; --<br>
&gt; 2.14.1<br>
<br>
<br>
Cheers<br></blockquote><div><br></div><div>Cheers </div></div><br></div></div>
Arnd Bergmann April 5, 2018, 12:54 p.m. | #3
On Fri, Mar 16, 2018 at 10:51 PM, Tomer Maimon <tmaimon77@gmail.com> wrote:
>
>
> On 16 March 2018 at 07:52, Brendan Higgins <brendanhiggins@google.com>
> wrote:
>>
>> On Thu, Mar 15, 2018 at 4:16 PM Tomer Maimon <tmaimon77@gmail.com> wrote:
>>
>> > Enable L2 Cache in Nuvoton NPCM7xx BMC.
>>
>> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>> > ---
>> >   arch/arm/mach-npcm/npcm7xx.c | 2 ++
>> >   1 file changed, 2 insertions(+)
>>
>> > diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
>> > index 5f7cd88103ef..c5f77d854c4f 100644
>> > --- a/arch/arm/mach-npcm/npcm7xx.c
>> > +++ b/arch/arm/mach-npcm/npcm7xx.c
>> > @@ -17,4 +17,6 @@ static const char *const npcm7xx_dt_match[] = {
>> >   DT_MACHINE_START(NPCM7XX_DT, "NPCM7XX Chip family")
>> >          .atag_offset    = 0x100,
>> >          .dt_compat      = npcm7xx_dt_match,
>> > +       .l2c_aux_val    = 0x0,
>> > +       .l2c_aux_mask   = ~0x0,
>>
>> You need to limit this to the specific bit(s) you want to set and verify
>> that
>> the l2c driver does not already manage that bit appropriately and that it
>> can
>> not be specified via the dtsi.
>
>
> Do you mean that I need to specify it the same as been done in V7?
> because when I run with the above l2c_aux_val and  l2c_aux_mask parameters I
> get the same result.
>
> Also if I am not adding the l2c_aux_val and  l2c_aux_mask parameters to the
> DT_MACHINE_START
> the L2C cache does not initialize.
>>
>>
>> We discussed this a little while ago with Rob here:
>> https://www.spinics.net/lists/arm-kernel/msg613372.html
>
>
> Sorry in this link I see only Russel king comment regarding the L2C

I'm not sure what the outcome is, and the patch changelog doesn't
explain what the patch is for, so I've not applied it. If you need the
patch for correct operation, please resend it with a proper changelog
comment explaining why it's needed and why you ended up not setting
any of the bits.

The last email in that thread mentions
L310_AUX_CTRL_CACHE_REPLACE_RR, is that required after all?

     Arnd
Tomer Maimon April 5, 2018, 4:56 p.m. | #4
On 5 April 2018 at 15:54, Arnd Bergmann <arnd@arndb.de> wrote:

> On Fri, Mar 16, 2018 at 10:51 PM, Tomer Maimon <tmaimon77@gmail.com>
> wrote:
> >
> >
> > On 16 March 2018 at 07:52, Brendan Higgins <brendanhiggins@google.com>
> > wrote:
> >>
> >> On Thu, Mar 15, 2018 at 4:16 PM Tomer Maimon <tmaimon77@gmail.com>
> wrote:
> >>
> >> > Enable L2 Cache in Nuvoton NPCM7xx BMC.
> >>
> >> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> >> > ---
> >> >   arch/arm/mach-npcm/npcm7xx.c | 2 ++
> >> >   1 file changed, 2 insertions(+)
> >>
> >> > diff --git a/arch/arm/mach-npcm/npcm7xx.c
> b/arch/arm/mach-npcm/npcm7xx.c
> >> > index 5f7cd88103ef..c5f77d854c4f 100644
> >> > --- a/arch/arm/mach-npcm/npcm7xx.c
> >> > +++ b/arch/arm/mach-npcm/npcm7xx.c
> >> > @@ -17,4 +17,6 @@ static const char *const npcm7xx_dt_match[] = {
> >> >   DT_MACHINE_START(NPCM7XX_DT, "NPCM7XX Chip family")
> >> >          .atag_offset    = 0x100,
> >> >          .dt_compat      = npcm7xx_dt_match,
> >> > +       .l2c_aux_val    = 0x0,
> >> > +       .l2c_aux_mask   = ~0x0,
> >>
> >> You need to limit this to the specific bit(s) you want to set and verify
> >> that
> >> the l2c driver does not already manage that bit appropriately and that
> it
> >> can
> >> not be specified via the dtsi.
> >
> >
> > Do you mean that I need to specify it the same as been done in V7?
> > because when I run with the above l2c_aux_val and  l2c_aux_mask
> parameters I
> > get the same result.
> >
> > Also if I am not adding the l2c_aux_val and  l2c_aux_mask parameters to
> the
> > DT_MACHINE_START
> > the L2C cache does not initialize.
> >>
> >>
> >> We discussed this a little while ago with Rob here:
> >> https://www.spinics.net/lists/arm-kernel/msg613372.html
> >
> >
> > Sorry in this link I see only Russel king comment regarding the L2C
>
> I'm not sure what the outcome is, and the patch changelog doesn't
> explain what the patch is for, so I've not applied it. If you need the
> patch for correct operation, please resend it with a proper changelog
> comment explaining why it's needed and why you ended up not setting
> any of the bits.
>

Sorry for not explain it better,

It seems that I have to add l2c_aux_val, l2c_aux_mask parameters to the
DT_MACHINE_START
to enable the L2 Cache.

>
> The last email in that thread mentions
> L310_AUX_CTRL_CACHE_REPLACE_RR, is that required after all?
>

No it is not require this is why I added:
.l2c_aux_val    = 0x0,
.l2c_aux_mask   = ~0x0,



>
>      Arnd
>

Can I resend it on Sunday?

Thanks,

Tomer
<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 5 April 2018 at 15:54, Arnd Bergmann <span dir="ltr">&lt;<a href="mailto:arnd@arndb.de" target="_blank">arnd@arndb.de</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Mar 16, 2018 at 10:51 PM, Tomer Maimon &lt;<a href="mailto:tmaimon77@gmail.com">tmaimon77@gmail.com</a>&gt; wrote:<br>
&gt;<br>
&gt;<br>
&gt; On 16 March 2018 at 07:52, Brendan Higgins &lt;<a href="mailto:brendanhiggins@google.com">brendanhiggins@google.com</a>&gt;<br>
&gt; wrote:<br>
&gt;&gt;<br>
&gt;&gt; On Thu, Mar 15, 2018 at 4:16 PM Tomer Maimon &lt;<a href="mailto:tmaimon77@gmail.com">tmaimon77@gmail.com</a>&gt; wrote:<br>
&gt;&gt;<br>
&gt;&gt; &gt; Enable L2 Cache in Nuvoton NPCM7xx BMC.<br>
&gt;&gt;<br>
&gt;&gt; &gt; Signed-off-by: Tomer Maimon &lt;<a href="mailto:tmaimon77@gmail.com">tmaimon77@gmail.com</a>&gt;<br>
&gt;&gt; &gt; ---<br>
&gt;&gt; &gt;   arch/arm/mach-npcm/npcm7xx.c | 2 ++<br>
&gt;&gt; &gt;   1 file changed, 2 insertions(+)<br>
&gt;&gt;<br>
&gt;&gt; &gt; diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c<br>
&gt;&gt; &gt; index 5f7cd88103ef..c5f77d854c4f 100644<br>
&gt;&gt; &gt; --- a/arch/arm/mach-npcm/npcm7xx.c<br>
&gt;&gt; &gt; +++ b/arch/arm/mach-npcm/npcm7xx.c<br>
&gt;&gt; &gt; @@ -17,4 +17,6 @@ static const char *const npcm7xx_dt_match[] = {<br>
&gt;&gt; &gt;   DT_MACHINE_START(NPCM7XX_DT, &quot;NPCM7XX Chip family&quot;)<br>
&gt;&gt; &gt;          .atag_offset    = 0x100,<br>
&gt;&gt; &gt;          .dt_compat      = npcm7xx_dt_match,<br>
&gt;&gt; &gt; +       .l2c_aux_val    = 0x0,<br>
&gt;&gt; &gt; +       .l2c_aux_mask   = ~0x0,<br>
&gt;&gt;<br>
&gt;&gt; You need to limit this to the specific bit(s) you want to set and verify<br>
&gt;&gt; that<br>
&gt;&gt; the l2c driver does not already manage that bit appropriately and that it<br>
&gt;&gt; can<br>
&gt;&gt; not be specified via the dtsi.<br>
&gt;<br>
&gt;<br>
&gt; Do you mean that I need to specify it the same as been done in V7?<br>
&gt; because when I run with the above l2c_aux_val and  l2c_aux_mask parameters I<br>
&gt; get the same result.<br>
&gt;<br>
&gt; Also if I am not adding the l2c_aux_val and  l2c_aux_mask parameters to the<br>
&gt; DT_MACHINE_START<br>
&gt; the L2C cache does not initialize.<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; We discussed this a little while ago with Rob here:<br>
&gt;&gt; <a href="https://www.spinics.net/lists/arm-kernel/msg613372.html" rel="noreferrer" target="_blank">https://www.spinics.net/lists/<wbr>arm-kernel/msg613372.html</a><br>
&gt;<br>
&gt;<br>
&gt; Sorry in this link I see only Russel king comment regarding the L2C<br>
<br>
</div></div>I&#39;m not sure what the outcome is, and the patch changelog doesn&#39;t<br>
explain what the patch is for, so I&#39;ve not applied it. If you need the<br>
patch for correct operation, please resend it with a proper changelog<br>
comment explaining why it&#39;s needed and why you ended up not setting<br>
any of the bits.<br></blockquote><div> </div><div>Sorry for not explain it better,</div><div><br></div><div>It seems that I have to add <span style="color:rgb(80,0,80)">l2c_aux_val, </span><span style="color:rgb(80,0,80)">l2c_aux_mask parameters to the <span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">DT_MACHINE_START</span>

</span></div><div><span style="color:rgb(80,0,80)"><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">to enable the L2 Cache.</span></span></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The last email in that thread mentions<br>
L310_AUX_CTRL_CACHE_REPLACE_<wbr>RR, is that required after all?<br></blockquote><div><br></div><div>No it is not require this is why I added:</div><div><span style="color:rgb(80,0,80)">.l2c_aux_val    = 0x0,</span></div><div><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">.l2c_aux_mask   = ~0x0,</span></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
     Arnd<br>
</font></span></blockquote></div><br></div><div class="gmail_extra">Can I resend it on Sunday?</div><div class="gmail_extra"><br></div><div class="gmail_extra">Thanks,</div><div class="gmail_extra"><br></div><div class="gmail_extra">Tomer</div></div>
Arnd Bergmann April 5, 2018, 8:42 p.m. | #5
On Thu, Apr 5, 2018 at 6:56 PM, Tomer Maimon <tmaimon77@gmail.com> wrote:
> On 5 April 2018 at 15:54, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Fri, Mar 16, 2018 at 10:51 PM, Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> It seems that I have to add l2c_aux_val, l2c_aux_mask parameters to the
> DT_MACHINE_START
> to enable the L2 Cache.
>>
>>
>> The last email in that thread mentions
>> L310_AUX_CTRL_CACHE_REPLACE_RR, is that required after all?
>
>
> No it is not require this is why I added:
> .l2c_aux_val    = 0x0,
> .l2c_aux_mask   = ~0x0,
>
> Can I resend it on Sunday?

Yes, please resend it as soon as you have the time to improve the changelog,
no need to wait if you can do it right away. We can always merge bugfixes,
but doing it earlier rather than later is better.

      Arnd

Patch

diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
index 5f7cd88103ef..c5f77d854c4f 100644
--- a/arch/arm/mach-npcm/npcm7xx.c
+++ b/arch/arm/mach-npcm/npcm7xx.c
@@ -17,4 +17,6 @@  static const char *const npcm7xx_dt_match[] = {
 DT_MACHINE_START(NPCM7XX_DT, "NPCM7XX Chip family")
 	.atag_offset	= 0x100,
 	.dt_compat	= npcm7xx_dt_match,
+	.l2c_aux_val	= 0x0,
+	.l2c_aux_mask	= ~0x0,
 MACHINE_END