Re: [PATCH 2/5] arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: fix ov7251 lane properties

[neil.armstrong@linaro.org]

On 04/12/2024 14:30, Vladimir Zapolskiy wrote:
> On 12/4/24 15:16, neil.armstrong@linaro.org wrote:
>> On 04/12/2024 14:10, Vladimir Zapolskiy wrote:
>>> On 12/4/24 14:16, Neil Armstrong wrote:
>>>> On 04/12/2024 12:05, Dmitry Baryshkov wrote:
>>>>> On Wed, Dec 04, 2024 at 11:56:54AM +0100, Neil Armstrong wrote:
>>>>>> Bindings documents data-lanes as a single entry with a separate
>>>>>> clock-lanes property, but DT uses 2 entries in data-lanes.
>>>>>>
>>>>>> This would suggest clock-lanes is missing, fix the DT using the
>>>>>> bindings example.
>>>>>>
>>>>>> This fixes:
>>>>>> sdm845-db845c-navigation-mezzanine.dtso: camera@60: port:endpoint:data-lanes: [0, 1] is too long
>>>>>>      from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml#
>>>>>>
>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>> ---
>>>>>>     arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 3 ++-
>>>>>>     1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso
>>>>>> index 0a87df806cafc8e726aacc07a772ca478d0ee3df..5a16f4c2b346b314af3d614266e1ca034057e643 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso
>>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso
>>>>>> @@ -115,7 +115,8 @@ camera@60 {
>>>>>>             port {
>>>>>>                 ov7251_ep: endpoint {
>>>>>> -                data-lanes = <0 1>;
>>>>>> +                clock-lanes = <1>;
>>>>>> +                data-lanes = <0>;
>>>>>
>>>>> Is it really this way or the other way around, clock = <0>, data = <1>?
>>>>
>>>> No idea actually, on the schematics the lanes from the DB845 are :
>>>>
>>>> CSI0_P/N -> OV7251_CSI3_LANE0_P/N -> MIPI_CSI3_LANE0_P -> SoC
>>>> and
>>>> CLKP/N -> OV7251_CSI3_CLK_P/N -> MIPI_CSI3_CLK_P/N -> SoC
>>>>
>>>> So I assume the data-lane is 0, for clock-lane I just used
>>>> the example, but I found nothing in the code using those assignments
>>>>
>>>
>>> It's a sensor property, and OV7251 sensor has the non-selectable clock lane.
>>>
>>> If it's technically acceptable, I would rather suggest to deprecate and
>>> remove "clock-lanes" property and hard-code "data-lanes" value to <1>.
>>
>>
>> Ok indeed while looking at the OV7251 sensor datasheet, there's a single
>> fixed data lane and a single fixed clock lane, so on the sensor side we
>> can't select anything but how would we define lane0 is used on the SoC side ?
> 
> It's done right in the common way, there are clock-lanes and data-lanes
> properties on the ISP endpoint side.

For the ov8856 yes, but in the current state the ov7251 endpoint is not
connected to the csiphy3, so it's currently not usable and disabled,
so I'd rather remove this node completely instead of fixing an
untestable dtso.

Neil

> 
>>>
>>> By the way, this particular device tree bindings miss bus-type property,
>>> it could be either MIPI or LVDS serial output.
>>>
>