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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0bf4ab2e-9846-4f8b-ad72-e9db6fb7d08e@kernel.org>
Date: Tue, 28 Oct 2025 10:46:08 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: 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

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