Re: [PATCH v2 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss

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

On 11/16/25 16:05, Bryan O'Donoghue wrote:
> On 14/11/2025 17:06, Vladimir Zapolskiy wrote:
>> On 11/14/25 18:09, Bryan O'Donoghue wrote:
>>> On 14/11/2025 13:06, Luca Weiss wrote:
>>>> Hi Vladimir,
>>>>
>>>> On Fri Nov 14, 2025 at 1:40 PM CET, Vladimir Zapolskiy wrote:
>>>>> Hi Luca.
>>>>>
>>>>> On 11/14/25 13:15, Luca Weiss wrote:
>>>>>> Add bindings for the Camera Subsystem on the SM6350 SoC.
>>>>>>
>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>>> ---
>>>>>>      .../bindings/media/qcom,sm6350-camss.yaml          | 349 ++++++
>>>>>> +++++++++++++++
>>>>>>      1 file changed, 349 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm6350-
>>>>>> camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm6350-
>>>>>> camss.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..d812b5b50c05
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/media/qcom,sm6350-camss.yaml
>>>>>> @@ -0,0 +1,349 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/media/qcom,sm6350-camss.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Qualcomm SM6350 Camera Subsystem (CAMSS)
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Luca Weiss <luca.weiss@fairphone.com>
>>>>>> +
>>>>>> +description:
>>>>>> +  The CAMSS IP is a CSI decoder and ISP present on Qualcomm
>>>>>> platforms.
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    const: qcom,sm6350-camss
>>>>>> +
>>>>>> +  reg:
>>>>>> +    maxItems: 12
>>>>>> +
>>>>>> +  reg-names:
>>>>>> +    items:
>>>>>> +      - const: csid0
>>>>>> +      - const: csid1
>>>>>> +      - const: csid2
>>>>>> +      - const: csid_lite
>>>>>> +      - const: csiphy0
>>>>>> +      - const: csiphy1
>>>>>> +      - const: csiphy2
>>>>>> +      - const: csiphy3
>>>>>> +      - const: vfe0
>>>>>> +      - const: vfe1
>>>>>> +      - const: vfe2
>>>>>> +      - const: vfe_lite
>>>>>> +
>>>>>> +  clocks:
>>>>>> +    maxItems: 30
>>>>>> +
>>>>>> +  clock-names:
>>>>>> +    items:
>>>>>> +      - const: cam_ahb_clk
>>>>>> +      - const: cam_axi
>>>>>> +      - const: soc_ahb
>>>>>> +      - const: camnoc_axi
>>>>>> +      - const: core_ahb
>>>>>> +      - const: cpas_ahb
>>>>>> +      - const: csiphy0
>>>>>> +      - const: csiphy0_timer
>>>>>> +      - const: csiphy1
>>>>>> +      - const: csiphy1_timer
>>>>>> +      - const: csiphy2
>>>>>> +      - const: csiphy2_timer
>>>>>> +      - const: csiphy3
>>>>>> +      - const: csiphy3_timer
>>>>>> +      - const: slow_ahb_src
>>>>>> +      - const: vfe0_axi
>>>>>> +      - const: vfe0
>>>>>> +      - const: vfe0_cphy_rx
>>>>>> +      - const: vfe0_csid
>>>>>> +      - const: vfe1_axi
>>>>>> +      - const: vfe1
>>>>>> +      - const: vfe1_cphy_rx
>>>>>> +      - const: vfe1_csid
>>>>>> +      - const: vfe2_axi
>>>>>> +      - const: vfe2
>>>>>> +      - const: vfe2_cphy_rx
>>>>>> +      - const: vfe2_csid
>>>>>> +      - const: vfe_lite
>>>>>> +      - const: vfe_lite_cphy_rx
>>>>>> +      - const: vfe_lite_csid
>>>>>
>>>>> The sorting order of this list does not follow the sorting order
>>>>> accepted
>>>>> in the past.
>>>>
>>>> What file should I best reference?
>>>
>>> Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml
>>>
>>>>>
>>>>> I'm very sorry for the vagueness, but I can not pronounce the accepted
>>>>> sorting order name, because it triggers people.
>>>>>
>>>>>> +
>>>>>> +  interrupts:
>>>>>> +    maxItems: 12
>>>>>> +
>>>>>> +  interrupt-names:
>>>>>> +    items:
>>>>>> +      - const: csid0
>>>>>> +      - const: csid1
>>>>>> +      - const: csid2
>>>>>> +      - const: csid_lite
>>>>>> +      - const: csiphy0
>>>>>> +      - const: csiphy1
>>>>>> +      - const: csiphy2
>>>>>> +      - const: csiphy3
>>>>>> +      - const: vfe0
>>>>>> +      - const: vfe1
>>>>>> +      - const: vfe2
>>>>>> +      - const: vfe_lite
>>>>>> +
>>>>>> +  interconnects:
>>>>>> +    maxItems: 4
>>>>>> +
>>>>>> +  interconnect-names:
>>>>>> +    items:
>>>>>> +      - const: ahb
>>>>>> +      - const: hf_mnoc
>>>>>> +      - const: sf_mnoc
>>>>>> +      - const: sf_icp_mnoc
>>>>>
>>>>> Please remove sf_mnoc and sf_icp_mnoc, they are not needed for enabling
>>>>> IP to produce raw images, and one day you may use them somewhere else.
>>>>
>>>> Ack, will give it a try.
>>>
>>> Disagree with this.
>>>
>>> See the Kanaapali patches. I'm asking new submissions to be as complete
>>> as possible, instead of limiting the hardware description to the RDI.
>>>
>>> So listing the ICP noc is the right thing to do.
>>>
>>> So please include register banks for
>>>
>>> - bps
>>> - cdm
>>> - icp
>>> - ipe
>>> - jpeg
>>> - lrme
>>
>> This suggests to get a DT backward compatibility lock forever,
> 
> I'm not entirely sure what this comment means.
> 
> The objective here is to get a full and complete DT describing camera
> hardware that can be consumed by
> 
> - CAMSS RDI
> - CAMSS RDI + future goodness

Here the problem is manifested, there is no even a raw idea of "future
goodness" and how it looks like practically.

Hardware can be represented in DT in many ways, why one arbitrary chosen,
and never tested or even verified for correctness, representation is
selected over any other one?

Let's start from the beginning, why putting everything into one unstructured
pile is better than accurate splitting of various CAMSS IPs into own
device tree nodes? Downstream has different device tree nodes layout,
why is it worse, should it be expected that Qualcomm in downstream moves
to a "better" proposed layout of device tree nodes for CAMSS or only
upstream Linux shall suffer?

Somebody said that there is an idea for "future goodness" to modularize
CAMSS, did it change?

> - FreeBSD
> - Any other DT consumer of upstream data perhaps even CamX

FreeBSD or CamX are software, and they shall be excluded from consideration.

There shall be no guesses about software usage, the only concern is if
the hardware desription in the shape of device tree nodes is proper or not.

Partial verifiable description is a proper description, and adding unknowns
of "future goodness" does not serve any reasonable technical purpose, it
adds an illusion of a proper description only, and it was proven that this
illision shatters even for CAMSS, see CAMSS CSIPHY case, when a wrong DT
choice can not be undone anymore for legacy platforms. It is good to acquire
an ability to learn from the past.

Everything can be added ad-hoc while keeping the hardware description in
a proper and correct state.

-- 
Best wishes,
Vladimir