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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DDTUHFIN3IEK.3FY5IS9S73ASO@fairphone.com>
Date: Tue, 28 Oct 2025 10:24:07 +0100
From: "Luca Weiss" <luca.weiss@...rphone.com>
To: "Krzysztof Kozlowski" <krzk@...nel.org>, "Luca Weiss"
 <luca.weiss@...rphone.com>
Cc: "Bryan O'Donoghue" <bod@...nel.org>, "Robert Foss" <rfoss@...nel.org>,
 "Todor Tomov" <todor.too@...il.com>, "Vladimir Zapolskiy"
 <vladimir.zapolskiy@...aro.org>, "Mauro Carvalho Chehab"
 <mchehab@...nel.org>, "Rob Herring" <robh@...nel.org>, "Krzysztof
 Kozlowski" <krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>,
 "Bryan O'Donoghue" <bryan.odonoghue@...aro.org>, "Bjorn Andersson"
 <andersson@...nel.org>, "Konrad Dybcio" <konradybcio@...nel.org>,
 <~postmarketos/upstreaming@...ts.sr.ht>, <phone-devel@...r.kernel.org>,
 <linux-arm-msm@...r.kernel.org>, <linux-media@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] dt-bindings: media: camss: Add qcom,sm6350-camss

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
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.

>
>
>> +
>> +  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.

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.

>
>
>> +
>> +  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)
* 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_A_CSI_x_1P25 - MIPI CSIx 1.25 V circuits
    With pad names VDD_A_CSI_0_1P25 to VDD_A_CSI_3_1P25

Regards
Luca

>
>
>> +    description:
>> +      Phandle to a 0.9V regulator supply to a PHY.
>> +
>> +  vdda-1.25-supply:
>> +    description:
>> +      Phandle to a 1.25V regulator supply to a PHY.
>
> Best regards,
> Krzysztof