Re: [PATCH v2 2/5] nvmem: add Samsung Exynos OTP support

On 11/13/25 11:35 AM, Krzysztof Kozlowski wrote:

On 13/11/2025 10:28, Tudor Ambarus wrote:

quoted

On 11/13/25 10:30 AM, Krzysztof Kozlowski wrote:

quoted

On Wed, Nov 12, 2025 at 08:29:06AM +0000, Tudor Ambarus wrote:

quoted

Add initial support for the Samsung Exynos OTP controller. Read the
product and chip IDs from the OTP controller registers space and
register the SoC info to the SoC interface.

The driver can be extended to empower the controller become nvmem
provider. This is not in the scope of this patch because it seems the
OTP memory space is not yet used by any consumer, even downstream.

Quick look tells me you just duplicated existing Samsung ChipID driver.
Even actual product ID registers and masks are the same, with one
difference - you read CHIPID3... which is the same as in newer Exynos,
e.g. Exynos8895.

Yes, that's correct. It's very similar with the Samsung ChipID driver.

quoted

What is exactly the point of having this as separate driver? I think

The difference is that for gs101 the chipid info is part of the OTP
registers. GS101 OTP has a clock, an interrupt line, a register space 
(that contains product and chip ID, TMU data, ASV, etc) and a 32Kbit
memory space that can be read/program/locked with specific commands.

The ChipID driver handles older exynos platforms that have a dedicated
chipid device that references a SFR register space to get the product
and chip ID. On GS101 (but also for e850 and autov9 I assume) the
"ChipID block" is just an abstraction, it's not a physical device. The
ChipID info is from OTP. When the power-on sequence progresses, the OTP
chipid values are loaded to the OTP registers. We need the OTP clock to
be on in order to read them. So GS101 has an OTP device that also happens
to have chip ID info.

For now I just got the chipid info and registered it to the SoC interface
(which is very similar to that the exynos-chipid driver does), but this
driver can be extended to export both its memory space and register space

There is no code for that now and possibility of extension is not a
reason to duplicate yet.

quoted

as nvmem devices, if any consumer needs them. Downstream GS101 drivers
seem to use just the chip id info and a dvfs version from the OTP
registers. DVFS version is not going to be used upstream as we're defining
the OPPs in DT. So I was not interested in extending the driver with nvmem
provider support, because it seems we don't need it for GS101.

Do the above justify the point of having a dedicated driver?

Only partially, I asked about driver. I did not spot previously the
clock, so we have two differences - CHIPID3 register and clock - right?

clock and interrupts, but I don't use the interrupts because I just need
to read the OTP registers to get the chip id info.

I wonder why Exynos8895 and others, which are already supported, do not
use CHIPID3, but nevertheless these two differences can be easily
integrated into existing driver.

they can be integrated, but I want to make sure we're making the best
decision.

quoted

quoted

this can easily be just customized chipid driver - with different
implementation of exynos_chipid_get_chipid_info().

If the answer is no to my question above, how shall I model the device
that binds to the existing exynos-chipid driver?

Just extend the existing driver.

So you mean I shall have something like that in DT:

+		chipid@10000000 {
+			compatible = "google,gs101-chipid";
+			reg = <0x10000000 0xf084>;
+			clocks = <&cmu_misc CLK_GOUT_MISC_OTP_CON_TOP_PCLK>;
+			interrupts = <GIC_SPI 752 IRQ_TYPE_LEVEL_HIGH 0>;
+		};

Maybe remove the interrupts because I don't need them for reading OTP regs.

What happens in the maybe unlikely case we do want to add support for OTP
for GS101? How will we describe that in DT?

Thanks!
ta