Received-SPF: pass (google.com: domain of linux-kernel+bounces-56599-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240201172224.574238-1-alexey.klimov@linaro.org>
 <20240201172224.574238-2-alexey.klimov@linaro.org> <CADrjBPpqHx1uoVZCYDX51kW+JdOr_-+4oryOjXcUMFkmLGTBLw@mail.gmail.com>
 <b199ba24-403b-44fa-b807-9b98f9e98913@linaro.org>
In-Reply-To: <b199ba24-403b-44fa-b807-9b98f9e98913@linaro.org>
From: Peter Griffin <peter.griffin@linaro.org>
Date: Wed, 7 Feb 2024 14:11:45 +0000
Message-ID: <CADrjBPpNukp+YQ0AmsZAE1f=MEk_auPPZit=tV8gk4szQ8MqRg@mail.gmail.com>
Subject: Re: [PATCH 2/4] arm64: dts: exynos: gs101: add chipid node
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Alexey Klimov <alexey.klimov@linaro.org>, alim.akhtar@samsung.com, 
	linux-samsung-soc@vger.kernel.org, semen.protsenko@linaro.org, 
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, 
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 
	linux-kernel@vger.kernel.org, klimov.linux@gmail.com, kernel-team@android.com, 
	tudor.ambarus@linaro.org, andre.draszik@linaro.org, saravanak@google.com, 
	willmcvicker@google.com, arnd@arndb.de
Content-Type: text/plain; charset="UTF-8"

Hi Krzysztof,

Thanks for your feedback.

On Tue, 6 Feb 2024 at 10:10, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 05/02/2024 15:36, Peter Griffin wrote:
> > Hi Alexey & Krysztof,
> >
> > On Thu, 1 Feb 2024 at 17:22, Alexey Klimov <alexey.klimov@linaro.org> wrote:
> >>
> >> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> >> ---
> >>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> >> index d838e3a7af6e..156fec2575bc 100644
> >> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> >> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> >> @@ -283,6 +283,11 @@ soc: soc@0 {
> >>                 #size-cells = <1>;
> >>                 ranges = <0x0 0x0 0x0 0x40000000>;
> >>
> >> +               chipid@10000000 {
> >> +                       compatible = "google,gs101-chipid";
> >> +                       reg = <0x10000000 0xd000>;
> >> +               };
> >> +
> >
> > I was wondering about the 0xd000 size here, as most upstream platforms
> > use a chipid size of 0x100 or 0x24. I see the downstream gs101 kernel
> > also uses 0xd000. Looking a bit more, that is because gs-chipid.c also
> > has support for dumping other areas of the OTP SFR bank like asv table
> > (offset 0x9000) hpm_asv (offset 0xa000) and hw_tune (0xc000).
> >
> > I checked Exynos850 and that also has ASV tables at those same offsets
> > above, but it currently uses a chipid size of 0x100 upstream.
> > Exynos-asv.c driver is part of exynos-chipid.c upstream so it seems
> > reasonable to have the increased size including those SFR registers.
> > Currently exynos-asv.c driver only supports Exynos5422 upstream.
> >
> > @Krzysztof - From a process PoV what is the best/correct thing to do
> > here? Have the increased size in DT that includes ASV parts of the OTP
> > bank from the get-go?
>
> ChipID so far had only size of 0x30 or something like that. What you
> refer to does not look like old ChipID but full blown OTP, which also
> includes ChipID.

OK so in some previous Exynos SoCs chipid had its own separate memory
mapped SFRs as well as being present in the OTP area?

>  Although I am not entirely sure about that, either.
> Depends whether they share clocks, for example.

This address is the OTP area, and I can't see chipid regs mentioned
anywhere else in the memory map other than OTP. Unfortunately there
are lots of separate docs for different IP blocks, so it isn't just a
case of searching a giant SoC TRM pdf.

e850 though looks to be the same (the address defined in DT is the otp
area), that is one large PDF and the chipid regs aren't mentioned
anywhere else, Given the chipid reg offset is the same (0x10000000)
for exynosautov9.dtsi, exynosautov920.dtsi, exynos850.dtsi, exynos7885
and exynos5433 I suspect this could be the same for all those SoCs as
well.

>
> I don't have any GS101 information so I don't know what's there. It
> seems you ask me to give you decision based on guessing... If you have
> one block, so if there is OTP, which contains ChipID, then define OTP.

I believe there is one block that contains ChipID, therefore based on
the above info we should define full OTP size?

> Not ChipID+OTP.
>
> I think Exynos850 DTSI is wrong here. That's OTP block, not ChipID.

Yes agreed, and quite possibly the other Exynos SoCs as well.

Thanks,

Peter.