[U-Boot] [PATCH] ARM: socfpga: Configure PL310 latencies

[Dinh Nguyen]

On 3/1/19 10:09 AM, Marek Vasut wrote:
> On 3/1/19 4:19 PM, Dinh Nguyen wrote:
>>
>>
>> On 3/1/19 3:40 AM, Marek Vasut wrote:
>>> On 3/1/19 12:59 AM, Dinh Nguyen wrote:
>>>> Hi Marek,
>>>>
>>>> On 2/19/19 4:01 AM, Simon Goldschmidt wrote:
>>>>> On Tue, Feb 19, 2019 at 1:44 AM Marek Vasut <marex at denx.de> wrote:
>>>>>>
>>>>>> Configure the PL310 tag and data latency registers, which slightly
>>>>>> improves performance and aligns the behavior with Linux.
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>>>> Cc: Dalon Westergreen <dwesterg at gmail.com>
>>>>>> Cc: Dinh Nguyen <dinguyen at kernel.org>
>>>>>> ---
>>>>>>  arch/arm/mach-socfpga/misc.c | 3 +++
>>>>>>  1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
>>>>>> index 78fbe28724..1ea4e32c11 100644
>>>>>> --- a/arch/arm/mach-socfpga/misc.c
>>>>>> +++ b/arch/arm/mach-socfpga/misc.c
>>>>>> @@ -62,6 +62,9 @@ void v7_outer_cache_enable(void)
>>>>>>         /* Disable the L2 cache */
>>>>>>         clrbits_le32(&pl310->pl310_ctrl, L2X0_CTRL_EN);
>>>>>>
>>>>>> +       writel(0x111, &pl310->pl310_tag_latency_ctrl);
>>>>>> +       writel(0x121, &pl310->pl310_data_latency_ctrl);
>>>>>
>>>>> Would it make sense to add defines as named constants for this?
>>>>> OTOH, in Linux, the values in the devicetree aren't really described,
>>>>> either, so:
>>>>
>>>> I was thinking the same, so I'm working on a patch to get these values
>>>> from the device tree.
>>>>
>>>> So while I was doing that, I realized that this patch is wrong.
>>>>
>>>> The patch should be like this:
>>>>
>>>> 	writel(0x0, &pl310->pl310_tag_latency_ctrl);
>>>> 	writel(0x010, &pl310->pl310_data_latency_ctrl);
>>>>
>>>> The reason is for the latency values:
>>>>
>>>> 000 = 1 cycle of latency, there is no additional latency.
>>>> 001 = 2 cycles of latency.
>>>> 010 = 3 cycles of latency.
>>>> 011 = 4 cycles of latency.
>>>> 100 = 5 cycles of latency.
>>>> 101 = 6 cycles of latency.
>>>> 110 = 7 cycles of latency.
>>>> 111 = 8 cycles of latency.
>>>>
>>>> So from the values in the device tree, they should be n-1.
>>>>
>>>> It looks like you've already sent the patch to Tom. I'll send a follow
>>>> up patch to fix that when it lands.
>>>
>>> Drat, thanks.
>>>
>>> Better yet, pull the latency config into a function, so it can be used
>>> by other platforms. The prototype should take 7 parameters, address and
>>> latency in cycles, so that it shields the users from this n-1 stuff.
>>>
>>
>> Agreed. I'm working on an RFC patch that creates a UBOOT_MISC driver to
>> handle all of the pl310 settings. Hope to send it out sometime next week.
> 
> I'd like a simpler fix for this release if possible, and a subsequent
> patch for the DM conversion.
> 

ok..

Dinh