Re: [PATCH v3 2/2] media: i2c: add Samsung S5KJN1 image sensor device driver

[Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>]

Hi Luca.

On 11/4/25 12:28, Luca Weiss wrote:
> Hi Vladimir,
> 
> On Thu Oct 23, 2025 at 4:53 AM CEST, Vladimir Zapolskiy wrote:
>> Samsung S5KJN1 is a 50MP image sensor, it produces Bayer GRBG (2x2)
>> frames in RAW10 output format, the maximum supported output resolution
>> is 8160x6144 at 10 frames per second rate.
>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>>   MAINTAINERS                |    8 +
>>   drivers/media/i2c/Kconfig  |   10 +
>>   drivers/media/i2c/Makefile |    1 +
>>   drivers/media/i2c/s5kjn1.c | 1407 ++++++++++++++++++++++++++++++++++++
>>   4 files changed, 1426 insertions(+)
>>   create mode 100644 drivers/media/i2c/s5kjn1.c
>>
>> diff --git a/drivers/media/i2c/s5kjn1.c b/drivers/media/i2c/s5kjn1.c
>> new file mode 100644
>> index 000000000000..9e2ac3d695ae
>> --- /dev/null
>> +++ b/drivers/media/i2c/s5kjn1.c
> 
> <snip>
> 
>> +static const char * const s5kjn1_supply_names[] = {
>> +	"afvdd",	/* Autofocus power */
>> +	"avdd",		/* Analog power */
>> +	"dovdd",	/* Digital I/O power */
>> +	"dvdd",		/* Digital core power */
> 
> My comments from the dt-bindings patch apply here as well of course.
> 
> Also my datasheet mentions that VDDD should be powered up before VDDA,
> and there should be a minimum time of 1 us between them.
> 

Good, thank you.

> And power down first VDDA then VDDD, again with 1 us minimum time
> between the two.
> 

Ack.

> Can't say much more about the rest of the driver, great to see this
> being worked on!
> 

What might be the most important, my plan is to keep the AF supply
on the list of the regulators. The testing has been done on SM8650
reference boards, you'll find the dts changes.

-- 
Best wishes,
Vladimir