Re: [PATCH v3 2/6] dt-bindings: media: camss: Add qcom,kaanapali-camss binding

[Vijay Kumar Tumati]

On 10/28/2025 1:09 AM, Krzysztof Kozlowski wrote:
> On Thu, Oct 23, 2025 at 02:14:34AM -0700, Hangxiang Ma wrote:
> > Add bindings for qcom,kaanapali-camss in order to support the camera
> What is qcom,kaanapali-camss? Sounds like a compatible and you cannot
> add bindings for a compatible. Instead add bindings for hardware, so
> explain here hardware.
>
> You could easily use `git log` to see how such commits are written
> instead of pasting here your downstream practice.
>
> > subsystem for Kaanapali.
> >
> > Signed-off-by: Hangxiang Ma <hangxiang.ma@xxxxxxxxxxxxxxxx>
> > ---
> >   .../bindings/media/qcom,kaanapali-camss.yaml       | 369 +++++++++++++++++++++
> >   1 file changed, 369 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml
> > new file mode 100644
> > index 000000000000..82f427bd036b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-camss.yaml
> > @@ -0,0 +1,369 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/qcom,kaanapali-camss.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Kaanapali Camera Subsystem (CAMSS)
> > +
> > +maintainers:
> > +  - Hangxiang Ma <hangxiang.ma@xxxxxxxxxxxxxxxx>
> > +
> > +description:
> > +  The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms.
> > +
> > +properties:
> > +  compatible:
> > +    const: qcom,kaanapali-camss
> > +
> > +  reg:
> > +    maxItems: 16
> > +
> > +  reg-names:
> > +    items:
> > +      - const: csid0
> > +      - const: csid1
> > +      - const: csid2
> > +      - const: csid_lite0
> > +      - const: csid_lite1
> > +      - const: csiphy0
> > +      - const: csiphy1
> > +      - const: csiphy2
> > +      - const: csiphy3
> > +      - const: csiphy4
> > +      - const: csiphy5
> > +      - const: vfe0
> > +      - const: vfe1
> > +      - const: vfe2
> > +      - const: vfe_lite0
> > +      - const: vfe_lite1
> > +
> > +  clocks:
> > +    maxItems: 34
> > +
> > +  clock-names:
> > +    items:
> > +      - const: camnoc_nrt_axi
> > +      - const: camnoc_rt_axi
> > +      - const: camnoc_rt_vfe0
> > +      - const: camnoc_rt_vfe1
> > +      - const: camnoc_rt_vfe2
> > +      - const: camnoc_rt_vfe_lite
> > +      - const: cam_top_ahb
> > +      - const: cam_top_fast_ahb
> > +      - const: csid
> > +      - const: csid_csiphy_rx
> > +      - const: csiphy0
> > +      - const: csiphy0_timer
> > +      - const: csiphy1
> > +      - const: csiphy1_timer
> > +      - const: csiphy2
> > +      - const: csiphy2_timer
> > +      - const: csiphy3
> > +      - const: csiphy3_timer
> > +      - const: csiphy4
> > +      - const: csiphy4_timer
> > +      - const: csiphy5
> > +      - const: csiphy5_timer
> > +      - const: gcc_hf_axi
> > +      - const: qdss_debug_xo
> No, I told many times you are supposed to use same order as last
> generation. Stop doing this alphabetical ordering or ordering by value.
> The previous generation has here vfe0.
>
> > +      - const: vfe0
> > +      - const: vfe0_fast_ahb
> > +      - const: vfe1
> > +      - const: vfe1_fast_ahb
> > +      - const: vfe2
> > +      - const: vfe2_fast_ahb
> > +      - const: vfe_lite
> > +      - const: vfe_lite_ahb
> > +      - const: vfe_lite_cphy_rx
> > +      - const: vfe_lite_csid
> > +
> > +  interrupts:
> > +    maxItems: 16
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: csid0
> > +      - const: csid1
> > +      - const: csid2
> > +      - const: csid_lite0
> > +      - const: csid_lite1
> > +      - const: csiphy0
> > +      - const: csiphy1
> > +      - const: csiphy2
> > +      - const: csiphy3
> > +      - const: csiphy4
> > +      - const: csiphy5
> > +      - const: vfe0
> > +      - const: vfe1
> > +      - const: vfe2
> > +      - const: vfe_lite0
> > +      - const: vfe_lite1
> > +
> > +  interconnects:
> > +    maxItems: 2
> > +
> > +  interconnect-names:
> > +    items:
> > +      - const: ahb
> > +      - const: hf_mnoc
> Why previously this was called hf_0 but now hf?
Hi Krzysztof, the interconnect driver exposes only one node 'camnoc_hf' 
to the camera driver, with it internally managing the voting on hf_0 and 
hf_1 clients. The traffic from the Real Time blocks in camera go through 
both HF_0 and HF_1, with the former being the primary. This change 
correctly represents that the BW voting is for the whole of the HF 
client. Please let me know if you have any further questions and we 
would be happy to answer. Thank you.
> > +
> > +  iommus:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    items:
> > +      - description:
> > +          TFE0 GDSC - Thin Front End, Global Distributed Switch Controller.
> > +      - description:
> > +          TFE1 GDSC - Thin Front End, Global Distributed Switch Controller.
> > +      - description:
> > +          TFE2 GDSC - Thin Front End, Global Distributed Switch Controller.
> > +      - description:
> > +          Titan GDSC - Titan ISP Block Global Distributed Switch Controller.
> > +
> > +  power-domain-names:
> > +    items:
> > +      - const: tfe0
> > +      - const: tfe1
> > +      - const: tfe2
> Why not using the same names as before? It really does not matter that
> it is thin or image, all of them are the same because only the
> difference against top matters.
Right, this is done to maintain the consistency with the clock driver on 
Kaanapali along with the terminology used elsewhere in the Kaanapali 
camss patches (wherever feasible). However, we don't mind updating this 
if you think otherwise, please advise.
> > +      - const: top
> > +
> > +  vdd-csiphy-0p8-supply:
> > +    description:
> > +      Phandle to a 0.8V regulator supply to CSI PHYs core block.
> > +
> > +  vdd-csiphy-1p2-supply:
> > +    description:
> > +      Phandle to 1.2V regulator supply to CSI PHYs pll block.
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    description:
> > +      CSI input ports.
> > +
> > +    patternProperties:
> > +      "^port@[0-3]$":
> > +        $ref: /schemas/graph.yaml#/$defs/port-base
> > +        unevaluatedProperties: false
> > +        description:
> > +          Input port for receiving CSI data on CSI0.
> > +
> > +        properties:
> > +          endpoint:
> > +            $ref: video-interfaces.yaml#
> > +            unevaluatedProperties: false
> > +
> > +            properties:
> > +              data-lanes:
> > +                minItems: 1
> > +                maxItems: 4
> > +
> > +            required:
> > +              - data-lanes
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - clocks
> > +  - clock-names
> > +  - interrupts
> > +  - interrupt-names
> > +  - interconnects
> > +  - interconnect-names
> > +  - iommus
> > +  - power-domains
> > +  - power-domain-names
> > +  - vdd-csiphy-0p8-supply
> > +  - vdd-csiphy-1p2-supply
> > +  - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interconnect/qcom,icc.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/power/qcom,rpmhpd.h>
> > +
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        camss: isp@9253000 {
> Drop unused label
>
> > +            compatible = "qcom,kaanapali-camss";
> Best regards,
> Krzysztof