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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 28/10/2025 10:24, Luca Weiss wrote:
> Hi Krzysztof,
> 
> On Tue Oct 28, 2025 at 9:54 AM CET, Krzysztof Kozlowski wrote:
>> On Fri, Oct 24, 2025 at 02:23:59PM +0200, Luca Weiss wrote:
>>  +
>>> +  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
>>> +
>>> +  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: cam_ahb
>>> +      - const: cam_hf_0_mnoc
>>> +      - const: cam_sf_0_mnoc
>>> +      - const: cam_sf_icp_mnoc
>>
>> Please share the list with the previous generation of this device. Which
>> one was used here as "previous"? For example x1e has quite different
>> names - nothing with "cam". No "cam" in qcs8300, either.
> 
> sm8250 is the big sibling for sm6350, so it's matching the names from

Ah, ok, good to know.

> there upstream. These exact names are also used downstream for
> "qcom,msm-bus,name".
> 
> I don't think there's anything preventing removing the cam_ prefix though.

Let's use the X1E names here.

> 
>>
>>
>>> +
>>> +  iommus:
>>> +    maxItems: 4
>>
>> I was told iommus might differ. Are you sure all of them represent the
>> same (e.g. not specific iommus for specific purposes)?
> 
> I don't really know.
> 
> These 4 iommus are labelled 'msm_cam_smmu_ife' downstream. There's still
> more iommus for more hardware blocks: jpeg, icp, cpas_cdm and lrme.

OK, that's fine then.

> 
> Maybe someone from Qualcomm/Linaro can explain this further if
> necessary?
> 
>>
>>> +
>>> +  power-domains:
>>> +    items:
>>> +      - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
>>> +      - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
>>> +      - description: IFE2 GDSC - Image Front End, Global Distributed Switch Controller.
>>> +      - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
>>> +
>>> +  power-domain-names:
>>> +    items:
>>> +      - const: ife0
>>> +      - const: ife1
>>> +      - const: ife2
>>> +      - const: top
>>
>> Uh, not your fault, but who came with this list in previous generations?
>> Instead of simple and obvious "top+ifeX" which allows growing/shrinking,
>> someone put "top" at the end which means this cannot follow same order
>> as X1E for example... Heh, it follows at least sm8550.
> 
> Shall we put top as first power-domain? I don't think it's an issue to
> change the order.

Well, it matches sm8550, so I am just grumpy complaining. It's fine.

> 
>>
>>
>>> +
>>> +  vdda-0.9-supply:
>>
>> There are no dots in property names. Are you sure these are called
>> VDDA_0.9 in the device datasheet (not schematics)? Please look at other
>> bindings how this is being named, depending whether this is PHY or PLL
>> supply (or only PHY).
> 
> The following power supplies are mentioned:
> 
> * VDD_CAMSS_PLL_0P9 - Camera SS PLL 0.9 V circuits
>     (not referenced in downstream kernel, connected to vreg_s5a in
>     schematics)

So that's vdda-pll-supply

> * VDD_A_CSI_x_0P9 - MIPI CSIx 0.9 V circuits
>     With pad names VDD_A_CSI_0_0P9 to VDD_A_CSI_3_0P9

vdd-csiphy-0p9-supply

> * VDD_A_CSI_x_1P25 - MIPI CSIx 1.25 V circuits
>     With pad names VDD_A_CSI_0_1P25 to VDD_A_CSI_3_1P25

vdd-csiphy-1p25-supply


Best regards,
Krzysztof