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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 13/11/2025 10:28, Tudor Ambarus wrote:
> 
> 
> On 11/13/25 10:30 AM, Krzysztof Kozlowski wrote:
>> On Wed, Nov 12, 2025 at 08:29:06AM +0000, Tudor Ambarus wrote:
>>> 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.
> 
>>
>> 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.

> 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?
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.
 >
>> 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.

Best regards,
Krzysztof