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

Anand Moon
2024-07-30T15:02:56Z

Hi Krzysztof,

On Tue, 30 Jul 2024 at 18:53, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 30/07/2024 15:20, Anand Moon wrote:
> > Hi Krzysztof,
> >
> > On Tue, 30 Jul 2024 at 17:57, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 30/07/2024 14:06, Anand Moon wrote:
> >>> Hi Marek,
> >>>
> >>> On Tue, 30 Jul 2024 at 17:14, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >>>>
> >>>>
> >>>> On 30.07.2024 11:13, Anand Moon wrote:
> >>>>> As per the 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@gmail.com>
> >>>>
> >>>>
> >>>> 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.
>
I will drop the icache and dcache values and just pass the L2_a7 and
L2_a15, value
Is this ok for you?

Earlier, I have tried to verify this information in /sys and /proc
to verify the changes as ARM does not populate this information.

> Best regards,
> Krzysztof
>
Thanks
-Anand