Re: [PATCH v1 2/2] ARM: dts: samsung: Add cache information to the Exynos542x SoC

[Krzysztof Kozlowski]

On 30/07/2024 15:20, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Tue, 30 Jul 2024 at 17:57, Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
>>
>> On 30/07/2024 14:06, Anand Moon wrote:
>>> Hi Marek,
>>>
>>> On Tue, 30 Jul 2024 at 17:14, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
>>>>
>>>>
>>>> On 30.07.2024 11:13, Anand Moon wrote:
>>>>> As per Exynos 5422 user manual add missing cache information to
>>>>> the Exynos542x SoC.
>>>>>
>>>>> - Each Cortex-A7 core has 32 KB of instruction cache and
>>>>>       32 KB of L1 data cache available.
>>>>> - Each Cortex-A15 core has 32 KB of L1 instruction cache and
>>>>>       32 KB of L1 data cache available.
>>>>> - The little (A7) cluster has 512 KB of unified L2 cache available.
>>>>> - The big (A15) cluster has 2 MB of unified L2 cache available.
>>>>>
>>>>> Features:
>>>>> - Exynos 5422 support cache coherency interconnect (CCI) bus with
>>>>>    L2 cache snooping capability. This hardware automatic L2 cache
>>>>>    snooping removes the efforts of synchronizing the contents of the
>>>>>    two L2 caches in core switching event.
>>>>>
>>>>> Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx>
>>>>
>>>>
>>>> The provided values are not correct. Please refer to commit 5f41f9198f29
>>>> ("ARM: 8864/1: Add workaround for I-Cache line size mismatch between CPU
>>>> cores"), which adds workaround for different l1 icache line size between
>>>> big and little CPUs. This workaround gets enabled on all Exynos542x/5800
>>>> boards.
>>>>
>>> Ok, I have just referred to the Exynos 5422 user manual for this patch,
>>> This patch is just updating the cache size for CPU for big.litle architecture..
>>>
>>
>> Let me get it right. Marek's comment was that you used wrong values.
>> Marek also provided rationale for this. Now your reply is that you
>> update cache size? Sorry, I fail how you address Marek's comment.
>>
>> Do not repeat what the patch is doing. We all can see it. Instead
>> respond to the comment with some sort of arguments.
>>
> 
> Ok, If I am not wrong  icache_size is hard-coded in the above commit.
> 
> +#ifdef CONFIG_CPU_ICACHE_MISMATCH_WORKAROUND
> +.globl icache_size
> +       .data
> +       .align  2
> +icache_size:
> +       .long   64
> +       .text
> +#endif
> 
> In the check_cpu_icache_size function, we read the control reg
> and recalculate the icache_size.
> if there mismatch we re-apply the Icache_size,
> 
> So dts passed values do not apply over here,

So you provide incorrect values in terms of them being ignored? Then do
not provide at all.

Best regards,
Krzysztof