Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add Samsung S5KJN1 image sensor

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

On 10/21/25 12:10, Sakari Ailus wrote:
> Hi Vladimir,
> 
> On Tue, Oct 21, 2025 at 11:00:35AM +0300, Vladimir Zapolskiy wrote:
>> Hi Sakari.
>>
>> On 10/20/25 23:23, Sakari Ailus wrote:
>>> Hi Vladimir,
>>>
>>> Thanks for the set.
>>>
>>> On Thu, Oct 16, 2025 at 05:04:18AM +0300, Vladimir Zapolskiy wrote:
>>>> Add device tree bindings documentation for Samsung S5KJN1 image sensor.
>>>>
>>>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>> ---
>>>>    .../bindings/media/i2c/samsung,s5kjn1.yaml    | 95 +++++++++++++++++++
>>>>    1 file changed, 95 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/media/i2c/samsung,s5kjn1.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/samsung,s5kjn1.yaml b/Documentation/devicetree/bindings/media/i2c/samsung,s5kjn1.yaml
>>>> new file mode 100644
>>>> index 000000000000..2220b3e528d4
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/media/i2c/samsung,s5kjn1.yaml
>>>> @@ -0,0 +1,95 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/media/i2c/samsung,s5kjn1.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Samsung S5KJN1 Image Sensor
>>>> +
>>>> +description:
>>>> +  Samsung S5KJN1 (ISOCELL JN1) image sensor is a 50MP image sensor.
>>>> +  The sensor is controlled over a serial camera control bus protocol,
>>>> +  the widest supported output image frame size is 8160x6144 at 10 frames
>>>> +  per second rate, data output format is RAW10 transferred over 4-lane
>>>> +  MIPI D-PHY interface.
>>>
>>> Can the sensor work with fewer lanes? This is almost always the case. In
>>> this case you'd need data-lanes property but feel free to make 4 the
>>> default if you like.
>>
>> As usual I don't have access to the sensor datasheet, what is known to me
>> is that
>>
>> 1) there is no examples in the downstream, when MIPI CSI interface is
>> configured in any other mode but 4 lanes D-PHY RAW10,
>>
>> 2) right the same information is given in the official scarce booklet:
>>
>> https://semiconductor.samsung.com/image-sensor/mobile-image-sensor/isocell-jn1/
>>
>> The same reasoning as above is directly applicable to the second sent
>> sensor driver of Samsung S5K3M5.
>>
>> There is a known practical pattern that if it happens to be of necessity
>> any new properties can be added to device's dt bindings later on, thus
>> it should be safe to omit any presumably non-configurable hardware
>> properties from the description on an early stage.
> 
> Even if the driver supports four lanes only, it's very unlikely the sensor

Well, the second point given above is not about the driver, but it leads
to the shortest possible sensor hardware spec:

https://semiconductor.samsung.com/image-sensor/mobile-image-sensor/isocell-jn1/

If you scroll right to the bottom, it says

Interface: 4 lanes (2.15 Gbps per lane)

It does not completely or clearly exclude 1 or 2 lane configuration,
I know, but "exclusions" are not documented anyway, only something
presenting gets documented.

> is limited to this. There are two options here:
> 
> 1. make data-lanes mandatory or
> 
> 2. add data-lanes as optional with default of four, which the driver
>     supports and which is known to function.

If you ask, I'd rather prefer to implement the second option in the dt
binding documentation and driver, let me know if there are any other
asked changes to be done in a bulk.

Thank you for review!

-- 
Best wishes,
Vladimir