public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: gregory.clement@free-electrons•com (Gregory CLEMENT)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH V3 4/5] arm: mm: Added support for PJ4B cpu and init routines
Date: Mon, 19 Nov 2012 16:16:58 +0100	[thread overview]
Message-ID: <50AA4D6A.2000707@free-electrons.com> (raw)
In-Reply-To: <20121119145018.GD4122@arm.com>

On 11/19/2012 03:50 PM, Catalin Marinas wrote:
> On Mon, Nov 19, 2012 at 12:18:14PM +0000, Gregory CLEMENT wrote:
>> On 11/19/2012 11:51 AM, Catalin Marinas wrote:
>>> On 14 November 2012 22:20, Gregory CLEMENT
>>> <gregory.clement@free-electrons•com> wrote:
>>>> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
>>>> index 846d279..a4c0ccf 100644
>>>> --- a/arch/arm/mm/proc-v7.S
>>>> +++ b/arch/arm/mm/proc-v7.S
>>>> @@ -169,6 +169,47 @@ __v7_ca15mp_setup:
>>>>         orreq   r0, r0, r10                     @ Enable CPU-specific SMP bits
>>>>         mcreq   p15, 0, r0, c1, c0, 1
>>>>  #endif
>>>> +
>>>> +__v7_pj4b_setup:
>>>> +#ifdef CONFIG_CPU_PJ4B
>>>> +
>>>> +#define SNOOP_DATA    (1 << 25) /* Dont interleave write and snoop data */
>>>> +#define CWF           (1 << 27) /* Disable Critical Word First feature */
>>>> +#define OUTSANDING_NC (1 << 29) /* Disable outstanding non cacheable request */
>>>> +#define L1_REP_RR     (1 << 30) /* L1 replacement - Strict round robin */
>>>> +
>>>> +#define AUX_DBG_CTRL2 (SNOOP_DATA | CWF | OUTSANDING_NC | L1_REP_RR)
>>>> +
>>>> +       /* Auxiliary Debug Modes Control 1 Register */
>>>> +       mrc     p15, 1, r0, c15, c1, 1
>>>> +       orr     r0, r0, #(1 << 16)     @ Disable data transfer for clean line.
>>>> +       orr     r0, r0, #(1 << 5)      @ Enable the back off of STREX instr
>>>> +       orr     r0, r0, #(1 << 8)      @ Disable Internal Parity Handling
>>>> +       bic     r0, r0, #(1 << 2)      @ Disable Static BP
>>>> +       mcr     p15, 1, r0, c15, c1, 1
>>>> +
>>>> +       /* Auxiliary Debug Modes Control 2 Register */
>>>> +       mrc     p15, 1, r0, c15, c1, 2
>>>> +       bic     r0, r0, #(1 << 23)   @ Enable fast LDR.
>>>> +       orr     r0, r0, #AUX_DBG_CTRL2
>>>> +       mcr     p15, 1, r0, c15, c1, 2
> 
> BTW, for some bits you defined macros while others are just immediate
> values. Just a cosmetic inconsistency (I would have preferred to use
> either macros or just immediate values).

Oh yes indeed: I made some change after Russell comments. But I should
have review the whole code to notice it. If I didn't have time to change
the code before the merge, I will fix it for the v3.8-rc1

> 
>>>> +
>>>> +       /* Auxiliary Functional Modes Control Register 0 */
>>>> +       mrc     p15, 1, r0, c15, c2, 0
>>>> +#ifdef CONFIG_SMP
>>>> +       orr     r0, r0, #(1 << 1)     @ Set SMP mode. Join the coherency fabric
>>>> +#endif
>>>> +       orr     r0, r0, #(1 << 2)     @ Support L1 parity checking
>>>> +       orr     r0, r0, #(1 << 8)     @ Broadcast Cache and TLB maintenance
>>>> +       mcr     p15, 1, r0, c15, c2, 0
>>>> +
>>>> +       /* Auxiliary Debug Modes Control 0 Register */
>>>> +       mrc     p15, 1, r0, c15, c1, 0
>>>> +       orr     r0, r0, #(1 << 22)   @ WFI/WFE - serve the DVM and back to idle
>>>> +       mcr     p15, 1, r0, c15, c1, 0
>>>
>>> Any chance that these could be set by the firmware prior to starting
>>> the kernel? We don't have any guidance for Linux here but longer term
>>> it seems to cause problems (i.e. you add some secure layer in a new
>>> CPU version).
>>
>> I will ask to Marvell engineers.
>>
>> However I hope it won't be show stopper for merging this code in 3.8.
>> This patch set was released 7 weeks ago, so would have though there was
>> a lot of time to raise the issue related to this code.
> 
> It shouldn't prevent the code being merged as we don't have clear
> requirement in Linux for this. But it would be good practice to push
> some sane defaults to the firmware. For example you set some option like
> L1 replacement which is optimal for your platform. Later you put the
> same CPU on a different SoC which may have different optimal value.
> There is no way in proc-v7.S to detect the SoC.
> 

OK I see your point. At this stage that would imply to change the bootloader
already deployed. I don't know how it will be easy to do it quickly. So
I am not sure that we can change it immediately, but maybe for future release.
However I still wait for more feedback on this subject for Marvell.

> Otherwise the patch looks fine to me.
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm•com>

Thanks a lot!


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

  reply	other threads:[~2012-11-19 15:16 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-14 22:20 [PATCH V3 0/5] SMP support for Armada XP Gregory CLEMENT
2012-11-14 22:20 ` [PATCH V3 1/5] arm: mvebu: Added support for coherency fabric in mach-mvebu Gregory CLEMENT
2012-11-14 22:20 ` [PATCH V3 2/5] arm: mvebu: Added initial support for power managmement service unit Gregory CLEMENT
2012-11-14 22:20 ` [PATCH V3 3/5] arm: mvebu: Added IPI support via doorbells Gregory CLEMENT
2012-11-14 22:20 ` [PATCH V3 4/5] arm: mm: Added support for PJ4B cpu and init routines Gregory CLEMENT
2012-11-14 22:31   ` Gregory CLEMENT
2012-11-16 21:46     ` Gregory CLEMENT
2012-11-19  3:12       ` Jason Cooper
2012-11-19  8:13         ` Gregory CLEMENT
2012-11-19 10:52           ` Will Deacon
2012-11-19  9:19         ` Russell King - ARM Linux
2012-11-19  9:23           ` Jason Cooper
2012-11-19 12:39           ` Gregory CLEMENT
2012-11-19 10:51   ` Catalin Marinas
2012-11-19 12:18     ` Gregory CLEMENT
2012-11-19 14:50       ` Catalin Marinas
2012-11-19 15:16         ` Gregory CLEMENT [this message]
2012-11-14 22:20 ` [PATCH V3 5/5] arm: mvebu: Added SMP support for Armada XP Gregory CLEMENT
2012-11-15  5:43   ` Hui Wang
2012-11-15  6:05     ` Hui Wang
2012-11-15  8:46 ` [PATCH V3 0/5] " Hui Wang
2012-11-15  8:50   ` Gregory CLEMENT
2012-11-15  8:56     ` Hui Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50AA4D6A.2000707@free-electrons.com \
    --to=gregory.clement@free-electrons$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox