[PATCH 0/6] arm64: dts: qcom: sm8550-qrd/hdk: correct S5K3M5 properties

[PATCH 0/6] arm64: dts: qcom: sm8550-qrd/hdk: correct S5K3M5 properties

[Vladimir Zapolskiy]

The changeset updates voltage regulator property names and removes
an optional data-lanes property of Samsung S5K3M5 image sensor found
on SM8550-QRD and SM8550-HDK with Rear Camera Card boards.

This reflects the changes in the dt binding documentation of the sensor:
- https://lore.kernel.org/linux-media/20251203040241.71018-1-vladimir.zapolskiy@linaro.org

In addition to the changes above Konrad asks to place 'status' property
of camss device tree node as the last one on the list, do it as well.

Vladimir Zapolskiy (6):
  arm64: dts: qcom: sm8550-qrd: rename image sensor supply properties
  arm64: dts: qcom: sm8550-qrd: remove data-lanes property of image sensor
  arm64: dts: qcom: sm8550-qrd: move camss status property to the end
  arm64: dts: qcom: sm8550-hdk-rear-camera-card: rename supply properties
  arm64: dts: qcom: sm8550-hdk-rear-camera-card: remove optional property
  arm64: dts: qcom: sm8550-hdk-rear-camera-card: move camss status property

 .../boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso     | 10 ++++------
 arch/arm64/boot/dts/qcom/sm8550-qrd.dts                | 10 ++++------
 2 files changed, 8 insertions(+), 12 deletions(-)

-- 
2.49.0

[PATCH 1/6] arm64: dts: qcom: sm8550-qrd: rename image sensor supply properties

[Vladimir Zapolskiy]

At first voltage supply properties of Samsung S5K3M5 image sensor got
names following a pad name convention for Omnivision image sensors. Now
the property names were corrected in the sensor dt bindings documentation,
and it should be updated in the SM8550-QRD board dts file.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
index 9af2a4fd02ea..77a7e45660b6 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
@@ -748,9 +748,9 @@ sensor@10 {
 		pinctrl-0 = <&cam3_default>;
 		pinctrl-names = "default";
 		afvdd-supply = <&vreg_l7n_2p96>;
-		avdd-supply = <&vreg_l4m_2p8>;
-		dovdd-supply = <&vreg_l5n_1p8>;
-		dvdd-supply = <&vreg_l2m_1p056>;
+		vdda-supply = <&vreg_l4m_2p8>;
+		vddd-supply = <&vreg_l2m_1p056>;
+		vddio-supply = <&vreg_l5n_1p8>;
 
 		port {
 			cam_tele: endpoint {
-- 
2.49.0

[PATCH 2/6] arm64: dts: qcom: sm8550-qrd: remove data-lanes property of image sensor

[Vladimir Zapolskiy]

Samsung S5K3M5 image sensor supports only 4-lane MIPI CSI-2 interface,
which makes the sensor data-lanes property redundant, and it can be
safely removed from the board dts file. Noteworthy that the property
value is incorrect, because conventionally lanes enumeration of image
sensors starts from index 1.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
index 77a7e45660b6..48af6d114161 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
@@ -755,7 +755,6 @@ sensor@10 {
 		port {
 			cam_tele: endpoint {
 				link-frequencies = /bits/ 64 <602500000>;
-				data-lanes = <0 1 2 3>;
 				remote-endpoint = <&csiphy3_ep>;
 			};
 		};
-- 
2.49.0

[PATCH 3/6] arm64: dts: qcom: sm8550-qrd: move camss status property to the end

[Vladimir Zapolskiy]

Conventionally status property is the last one in the list, move it there.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
index 48af6d114161..b3eb0836c408 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
@@ -717,10 +717,9 @@ vreg_l7n_2p96: ldo7 {
 };
 
 &camss {
-	status = "okay";
-
 	vdda-phy-supply = <&vreg_l1e_0p88>;
 	vdda-pll-supply = <&vreg_l3e_1p2>;
+	status = "okay";
 
 	ports {
 		port@3 {
-- 
2.49.0

[PATCH 4/6] arm64: dts: qcom: sm8550-hdk-rear-camera-card: rename supply properties

[Vladimir Zapolskiy]

At first voltage supply properties of Samsung S5K3M5 image sensor got
names following a pad name convention for Omnivision image sensors. Now
the property names were corrected in the sensor dt bindings documentation,
and it should be updated in the SM8550-HDK with the Rear Camera Card
board dts file.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
index 66bec0fef766..e80de190eef9 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
+++ b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
@@ -52,9 +52,9 @@ sensor@10 {
 		pinctrl-0 = <&cam3_default>;
 		pinctrl-names = "default";
 		afvdd-supply = <&vreg_l7n_2p96>;
-		avdd-supply = <&vreg_l4m_2p8>;
-		dovdd-supply = <&vreg_l5n_1p8>;
-		dvdd-supply = <&vreg_l2m_1p056>;
+		vdda-supply = <&vreg_l4m_2p8>;
+		vddd-supply = <&vreg_l2m_1p056>;
+		vddio-supply = <&vreg_l5n_1p8>;
 
 		port {
 			cam_tele: endpoint {
-- 
2.49.0

[PATCH 5/6] arm64: dts: qcom: sm8550-hdk-rear-camera-card: remove optional property

[Vladimir Zapolskiy]

Samsung S5K3M5 image sensor supports only 4-lane MIPI CSI-2 interface,
which makes the sensor data-lanes property redundant, and it can be
safely removed from the board dts file. Noteworthy that the property
value is incorrect, because conventionally lanes enumeration of image
sensors starts from index 1.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
index e80de190eef9..21bfba6a1182 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
+++ b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
@@ -59,7 +59,6 @@ sensor@10 {
 		port {
 			cam_tele: endpoint {
 				link-frequencies = /bits/ 64 <602500000>;
-				data-lanes = <0 1 2 3>;
 				remote-endpoint = <&csiphy3_ep>;
 			};
 		};
-- 
2.49.0

[PATCH 6/6] arm64: dts: qcom: sm8550-hdk-rear-camera-card: move camss status property

[Vladimir Zapolskiy]

Conventionally status property of device tree nodes is the last one in
the list, move it there.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
index 21bfba6a1182..544cec93353a 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
+++ b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
@@ -13,10 +13,9 @@
 /plugin/;
 
 &camss {
-	status = "okay";
-
 	vdda-phy-supply = <&vreg_l1e_0p88>;
 	vdda-pll-supply = <&vreg_l3e_1p2>;
+	status = "okay";
 
 	ports {
 		#address-cells = <1>;
-- 
2.49.0

Re: [PATCH 0/6] arm64: dts: qcom: sm8550-qrd/hdk: correct S5K3M5 properties

[Neil Armstrong]

On 12/3/25 05:05, Vladimir Zapolskiy wrote:
> The changeset updates voltage regulator property names and removes
> an optional data-lanes property of Samsung S5K3M5 image sensor found
> on SM8550-QRD and SM8550-HDK with Rear Camera Card boards.
> 
> This reflects the changes in the dt binding documentation of the sensor:
> - https://lore.kernel.org/linux-media/20251203040241.71018-1-vladimir.zapolskiy@linaro.org
> 
> In addition to the changes above Konrad asks to place 'status' property
> of camss device tree node as the last one on the list, do it as well.
> 
> Vladimir Zapolskiy (6):
>    arm64: dts: qcom: sm8550-qrd: rename image sensor supply properties
>    arm64: dts: qcom: sm8550-qrd: remove data-lanes property of image sensor
>    arm64: dts: qcom: sm8550-qrd: move camss status property to the end
>    arm64: dts: qcom: sm8550-hdk-rear-camera-card: rename supply properties
>    arm64: dts: qcom: sm8550-hdk-rear-camera-card: remove optional property
>    arm64: dts: qcom: sm8550-hdk-rear-camera-card: move camss status property
> 
>   .../boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso     | 10 ++++------
>   arch/arm64/boot/dts/qcom/sm8550-qrd.dts                | 10 ++++------
>   2 files changed, 8 insertions(+), 12 deletions(-)
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH 3/6] arm64: dts: qcom: sm8550-qrd: move camss status property to the end

[Krzysztof Kozlowski]

On 03/12/2025 05:05, Vladimir Zapolskiy wrote:
> Conventionally status property is the last one in the list, move it there.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> index 48af6d114161..b3eb0836c408 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> @@ -717,10 +717,9 @@ vreg_l7n_2p96: ldo7 {
>  };
>  
>  &camss {
> -	status = "okay";
> -
>  	vdda-phy-supply = <&vreg_l1e_0p88>;
>  	vdda-pll-supply = <&vreg_l3e_1p2>;
> +	status = "okay";

Please do not do such changes node by node. Fix many or all devices here
or none. But actually even fixing all should be postponed - we discussed
this many times, first proper linter is needed.

Best regards,
Krzysztof

Re: [PATCH 6/6] arm64: dts: qcom: sm8550-hdk-rear-camera-card: move camss status property

[Krzysztof Kozlowski]

On 03/12/2025 05:05, Vladimir Zapolskiy wrote:
> Conventionally status property of device tree nodes is the last one in
> the list, move it there.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
> index 21bfba6a1182..544cec93353a 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
> +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
> @@ -13,10 +13,9 @@
>  /plugin/;
>  
>  &camss {
> -	status = "okay";
> -
>  	vdda-phy-supply = <&vreg_l1e_0p88>;
>  	vdda-pll-supply = <&vreg_l3e_1p2>;
> +	status = "okay";


NAK, making such changes in multiple patches is absolute churn.

Best regards,
Krzysztof

Re: [PATCH 1/6] arm64: dts: qcom: sm8550-qrd: rename image sensor supply properties

[Konrad Dybcio]

On 12/3/25 5:05 AM, Vladimir Zapolskiy wrote:
> At first voltage supply properties of Samsung S5K3M5 image sensor got
> names following a pad name convention for Omnivision image sensors. Now
> the property names were corrected in the sensor dt bindings documentation,
> and it should be updated in the SM8550-QRD board dts file.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 4/6] arm64: dts: qcom: sm8550-hdk-rear-camera-card: rename supply properties

[Konrad Dybcio]

On 12/3/25 5:05 AM, Vladimir Zapolskiy wrote:
> At first voltage supply properties of Samsung S5K3M5 image sensor got
> names following a pad name convention for Omnivision image sensors. Now
> the property names were corrected in the sensor dt bindings documentation,
> and it should be updated in the SM8550-HDK with the Rear Camera Card
> board dts file.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 2/6] arm64: dts: qcom: sm8550-qrd: remove data-lanes property of image sensor

[Konrad Dybcio]

On 12/3/25 5:05 AM, Vladimir Zapolskiy wrote:
> Samsung S5K3M5 image sensor supports only 4-lane MIPI CSI-2 interface,
> which makes the sensor data-lanes property redundant, and it can be
> safely removed from the board dts file. Noteworthy that the property
> value is incorrect, because conventionally lanes enumeration of image
> sensors starts from index 1.

This seems to vary between sensors

Konrad

Re: [PATCH 2/6] arm64: dts: qcom: sm8550-qrd: remove data-lanes property of image sensor

[Vladimir Zapolskiy]

On 12/3/25 13:11, Konrad Dybcio wrote:
> On 12/3/25 5:05 AM, Vladimir Zapolskiy wrote:
>> Samsung S5K3M5 image sensor supports only 4-lane MIPI CSI-2 interface,
>> which makes the sensor data-lanes property redundant, and it can be
>> safely removed from the board dts file. Noteworthy that the property
>> value is incorrect, because conventionally lanes enumeration of image
>> sensors starts from index 1.
> 
> This seems to vary between sensors
> 

It should not, from Documentation/devicetree/bindings/media/video-interfaces.yaml
the clock lane has index 0, data lanes enumeration starts from 1:

   data-lanes:
   ....
     description:
       An array of physical data lane indexes. Position of an entry determines
       the logical lane number, while the value of an entry indicates physical
       lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1 2>;",
       assuming the clock lane is on hardware lane 0.

-- 
Best wishes,
Vladimir

Re: [PATCH 2/6] arm64: dts: qcom: sm8550-qrd: remove data-lanes property of image sensor

[Konrad Dybcio]

On 12/3/25 12:41 PM, Vladimir Zapolskiy wrote:
> On 12/3/25 13:11, Konrad Dybcio wrote:
>> On 12/3/25 5:05 AM, Vladimir Zapolskiy wrote:
>>> Samsung S5K3M5 image sensor supports only 4-lane MIPI CSI-2 interface,
>>> which makes the sensor data-lanes property redundant, and it can be
>>> safely removed from the board dts file. Noteworthy that the property
>>> value is incorrect, because conventionally lanes enumeration of image
>>> sensors starts from index 1.
>>
>> This seems to vary between sensors
>>
> 
> It should not, from Documentation/devicetree/bindings/media/video-interfaces.yaml
> the clock lane has index 0, data lanes enumeration starts from 1:
> 
>   data-lanes:
>   ....
>     description:
>       An array of physical data lane indexes. Position of an entry determines
>       the logical lane number, while the value of an entry indicates physical
>       lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1 2>;",
>       assuming the clock lane is on hardware lane 0.

I interpret the last sentence as ">>if<< the clock lane is 0, then data lanes
start at 1. I'm not sure if the spec mandates that's the case though.

Konrad

Re: [PATCH 3/6] arm64: dts: qcom: sm8550-qrd: move camss status property to the end

[Vladimir Zapolskiy]

On 12/3/25 10:32, Krzysztof Kozlowski wrote:
> On 03/12/2025 05:05, Vladimir Zapolskiy wrote:
>> Conventionally status property is the last one in the list, move it there.
>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
>> index 48af6d114161..b3eb0836c408 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
>> @@ -717,10 +717,9 @@ vreg_l7n_2p96: ldo7 {
>>   };
>>   
>>   &camss {
>> -	status = "okay";
>> -
>>   	vdda-phy-supply = <&vreg_l1e_0p88>;
>>   	vdda-pll-supply = <&vreg_l3e_1p2>;
>> +	status = "okay";
> 
> Please do not do such changes node by node. Fix many or all devices here
> or none. But actually even fixing all should be postponed - we discussed
> this many times, first proper linter is needed.

For historical reference the corrected here issue has been spotted by Konrad,
and since the requested improvement could be done instantly I added these
changes to the series:

https://lore.kernel.org/linux-arm-msm/21430004-b186-4f28-be59-bcb3851237b8@oss.qualcomm.com/

Patches 3/6 and 6/6 can be dropped now while waiting for the linter or
be squashed together, or just included as is, whatever is preferred here.

-- 
Best wishes,
Vladimir

Re: [PATCH 2/6] arm64: dts: qcom: sm8550-qrd: remove data-lanes property of image sensor

[Vladimir Zapolskiy]

On 12/3/25 13:50, Konrad Dybcio wrote:
> On 12/3/25 12:41 PM, Vladimir Zapolskiy wrote:
>> On 12/3/25 13:11, Konrad Dybcio wrote:
>>> On 12/3/25 5:05 AM, Vladimir Zapolskiy wrote:
>>>> Samsung S5K3M5 image sensor supports only 4-lane MIPI CSI-2 interface,
>>>> which makes the sensor data-lanes property redundant, and it can be
>>>> safely removed from the board dts file. Noteworthy that the property
>>>> value is incorrect, because conventionally lanes enumeration of image
>>>> sensors starts from index 1.
>>>
>>> This seems to vary between sensors
>>>
>>
>> It should not, from Documentation/devicetree/bindings/media/video-interfaces.yaml
>> the clock lane has index 0, data lanes enumeration starts from 1:
>>
>>    data-lanes:
>>    ....
>>      description:
>>        An array of physical data lane indexes. Position of an entry determines
>>        the logical lane number, while the value of an entry indicates physical
>>        lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1 2>;",
>>        assuming the clock lane is on hardware lane 0.
> 
> I interpret the last sentence as ">>if<< the clock lane is 0, then data lanes
> start at 1. I'm not sure if the spec mandates that's the case though.
> 

Well, that's the absolute common practice for media devices, which I'd like
to follow without a deviation from it or introducing a novation.

The dt bindings documentation of this S5K3M5 sensor device describes the value
of the optional data-lanes property accordingly:

https://lore.kernel.org/linux-media/20251203040241.71018-2-vladimir.zapolskiy@linaro.org/

So, I belive, this change under review should be fine as is.

-- 
Best wishes,
Vladimir

Re: [PATCH 6/6] arm64: dts: qcom: sm8550-hdk-rear-camera-card: move camss status property

[Neil Armstrong]

On 12/3/25 09:32, Krzysztof Kozlowski wrote:
> On 03/12/2025 05:05, Vladimir Zapolskiy wrote:
>> Conventionally status property of device tree nodes is the last one in
>> the list, move it there.
>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
>> index 21bfba6a1182..544cec93353a 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
>> +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
>> @@ -13,10 +13,9 @@
>>   /plugin/;
>>   
>>   &camss {
>> -	status = "okay";
>> -
>>   	vdda-phy-supply = <&vreg_l1e_0p88>;
>>   	vdda-pll-supply = <&vreg_l3e_1p2>;
>> +	status = "okay";
> 
> 
> NAK, making such changes in multiple patches is absolute churn.

While it may be a "churn" for you, the patch is perfectly valid
and a NAK is not appropriate since Konrad asked this to be changed
on the already merged original patchset.

While you're right we should group such changes, it happens that
he sm8650 files already have an unified and clean style, and it's
not a churn to keep the style unified.

Neil

> 
> Best regards,
> Krzysztof

Re: [PATCH 6/6] arm64: dts: qcom: sm8550-hdk-rear-camera-card: move camss status property

[Krzysztof Kozlowski]

On 03/12/2025 13:02, Neil Armstrong wrote:
> On 12/3/25 09:32, Krzysztof Kozlowski wrote:
>> On 03/12/2025 05:05, Vladimir Zapolskiy wrote:
>>> Conventionally status property of device tree nodes is the last one in
>>> the list, move it there.
>>>
>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>> ---
>>>   arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
>>> index 21bfba6a1182..544cec93353a 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
>>> +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk-rear-camera-card.dtso
>>> @@ -13,10 +13,9 @@
>>>   /plugin/;
>>>   
>>>   &camss {
>>> -	status = "okay";
>>> -
>>>   	vdda-phy-supply = <&vreg_l1e_0p88>;
>>>   	vdda-pll-supply = <&vreg_l3e_1p2>;
>>> +	status = "okay";
>>
>>
>> NAK, making such changes in multiple patches is absolute churn.
> 
> While it may be a "churn" for you, the patch is perfectly valid
> and a NAK is not appropriate since Konrad asked this to be changed
> on the already merged original patchset.
> 
> While you're right we should group such changes, it happens that
> he sm8650 files already have an unified and clean style, and it's
> not a churn to keep the style unified.

We exactly had this discussion and the judgement expressed by Bjorn,
which I general support, was don't change existing source code till we
have the linter for that. And patch one-node-by-one is not valid. Such
patch is still NAK.

Best regards,
Krzysztof

Re: [PATCH 2/6] arm64: dts: qcom: sm8550-qrd: remove data-lanes property of image sensor

[Konrad Dybcio]

On 12/3/25 12:57 PM, Vladimir Zapolskiy wrote:
> On 12/3/25 13:50, Konrad Dybcio wrote:
>> On 12/3/25 12:41 PM, Vladimir Zapolskiy wrote:
>>> On 12/3/25 13:11, Konrad Dybcio wrote:
>>>> On 12/3/25 5:05 AM, Vladimir Zapolskiy wrote:
>>>>> Samsung S5K3M5 image sensor supports only 4-lane MIPI CSI-2 interface,
>>>>> which makes the sensor data-lanes property redundant, and it can be
>>>>> safely removed from the board dts file. Noteworthy that the property
>>>>> value is incorrect, because conventionally lanes enumeration of image
>>>>> sensors starts from index 1.
>>>>
>>>> This seems to vary between sensors
>>>>
>>>
>>> It should not, from Documentation/devicetree/bindings/media/video-interfaces.yaml
>>> the clock lane has index 0, data lanes enumeration starts from 1:
>>>
>>>    data-lanes:
>>>    ....
>>>      description:
>>>        An array of physical data lane indexes. Position of an entry determines
>>>        the logical lane number, while the value of an entry indicates physical
>>>        lane, e.g. for 2-lane MIPI CSI-2 bus we could have "data-lanes = <1 2>;",
>>>        assuming the clock lane is on hardware lane 0.
>>
>> I interpret the last sentence as ">>if<< the clock lane is 0, then data lanes
>> start at 1. I'm not sure if the spec mandates that's the case though.
>>
> 
> Well, that's the absolute common practice for media devices, which I'd like
> to follow without a deviation from it or introducing a novation.
> 
> The dt bindings documentation of this S5K3M5 sensor device describes the value
> of the optional data-lanes property accordingly:
> 
> https://lore.kernel.org/linux-media/20251203040241.71018-2-vladimir.zapolskiy@linaro.org/
> 
> So, I belive, this change under review should be fine as is.

What I'm trying to get an answer for, is that whether the lanes
are physically connected like this. You can find an answer for that
in the board schematics, specifically looking at the connection between
the PHY and the sensor.

Konrad