Re: [PATCH v1 1/3] dt-bindings: interconnect: Add Qualcomm CAMNOC ICC binding

[Dmitry Baryshkov]

On Tue, Aug 11, 2026 at 02:59:04PM +0300, Atanas Filipov wrote:
> On 8/8/2026 12:12 AM, Dmitry Baryshkov wrote:
> > On Fri, Aug 07, 2026 at 02:49:49PM +0300, Atanas Filipov wrote:
> > > Add a device tree binding for the Qualcomm Camera Network-on-Chip
> > > (CAMNOC) interconnect provider. CAMNOC is the internal AXI
> > > interconnect within the camera subsystem that arbitrates bandwidth
> > > between camera sub-devices and the external memory interconnect.
> > > 
> > > The CAMNOC ICC provider aggregates bandwidth requests from multiple
> > > independent consumers and scales the CAMNOC AXI clock accordingly,
> > > avoiding the race conditions that arise when each consumer calls
> > > clk_set_rate() directly on the shared clock.
> > > 
> > > Signed-off-by: Atanas Filipov <atanas.filipov@xxxxxxxxxxxxxxxx>
> > > ---
> > >   .../bindings/interconnect/qcom,camnoc.yaml    | 98 +++++++++++++++++++
> > >   .../dt-bindings/interconnect/qcom,camnoc.h    | 12 +++
> > >   2 files changed, 110 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,camnoc.yaml
> > >   create mode 100644 include/dt-bindings/interconnect/qcom,camnoc.h
> > > 
> > > diff --git a/Documentation/devicetree/bindings/interconnect/qcom,camnoc.yaml b/Documentation/devicetree/bindings/interconnect/qcom,camnoc.yaml
> > > new file mode 100644
> > > index 000000000000..b2f5c140ff80
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/interconnect/qcom,camnoc.yaml
> > > @@ -0,0 +1,98 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/interconnect/qcom,camnoc.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Qualcomm Camera NOC (CAMNOC) Interconnect Provider
> > > +
> > > +maintainers:
> > > +  - Aleksandar Filipov <quic_afilipov@xxxxxxxxxxx>
> > > +
> > > +description:
> > > +  The Camera Network-on-Chip (CAMNOC) is the internal AXI interconnect
> > > +  within the Qualcomm camera subsystem. It arbitrates AXI bandwidth
> > > +  between camera sub-devices (IFE, JPEG, BPS, etc.) and the external
> > > +  memory interconnect. Multiple consumers may independently request
> > > +  bandwidth; the CAMNOC ICC provider aggregates these requests and
> > > +  scales CAM_CC_CAMNOC_AXI_CLK accordingly.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - qcom,sm8250-cam-virt
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: CAMNOC AXI clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: camnoc_axi
> > > +
> > > +  '#interconnect-cells':
> > > +    const: 1
> > > +
> > > +required:
> > > +  - compatible
> > > +  - clocks
> > > +  - clock-names
> > > +  - '#interconnect-cells'
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/qcom,camcc-sm8250.h>
> > > +    #include <dt-bindings/interconnect/qcom,camnoc.h>
> > > +
> > > +    camnoc_icc: interconnect {
> > > +        compatible = "qcom,sm8250-cam-virt";
> > > +        clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>;
> > > +        clock-names = "camnoc_axi";
> > > +        #interconnect-cells = <1>;
> > > +    };
> > > +
> > > +    /* Example camera sub-device consuming the CAMNOC provider above */
> > > +    camera-subsystem@ac53000 {
> > > +        reg = <0x0ac53000 0x1000>;
> > > +
> > > +        interconnects = <&camnoc_icc MASTER_CAMNOC_JPEG
> > > +                         &camnoc_icc SLAVE_CAMNOC_AXI>;
> > > +        interconnect-names = "camnoc";
> > > +
> > > +        /*
> > > +         * Optional: an operating-points-v2 table lets the consumer
> > > +         * drive its CAMNOC bandwidth vote from the same OPP that
> > > +         * selects its core clock rate and RPMh performance state.
> > 
> > Is this a table for the camera-subsystem or for the CAMNOC_AXI_CLK? In
> > the latter case, the OPP table should be a part of the camnoc device.
> > 
> 
> The OPP table in the example belongs to the consumer device, not to the
> CAMNOC provider. It describes the operating points
> of the camera sub-device itself: core clock frequency, RPMh performance
> state, and CAMNOC bandwidth vote bundled together in
> a single OPP entry. The opp-peak-kBps field is the bandwidth the consumer
> passes to icc_set_bw() when it transitions to that
> operating point.
> 
> This follows the standard ICC consumer pattern: the consumer owns the OPP
> table and uses opp-peak-kBps to drive its bandwidth
> vote. The CAMNOC provider has no per-consumer bandwidth table of its own —
> it only aggregates the votes it receives.

And because we only select the max of the votes (via the peak kBps)
instead of summing (via average kBps), we can't end up in a situation
when the final CAMNOC vote would require higher voltage than the one
specied by any of the consumers.

I agree here, but it would be nice to point it out in the commit
message.

> 
> The comment in the example was perhaps misleading. I will reword it in v2 to
> make it clear that the OPP table belongs to the
> consumer and that opp-peak-kBps is the ICC bandwidth vote, not a clock rate
> table for CAMNOC_AXI_CLK.
> 
> > > +         */
> > > +        operating-points-v2 = <&camera_opp_table>;
> > > +
> > > +        camera_opp_table: opp-table {
> > > +            compatible = "operating-points-v2";
> > > +
> > > +            opp-100000000 {
> > > +                opp-hz = /bits/ 64 <100000000>;
> > > +                opp-peak-kBps = <22246>;
> > > +                required-opps = <&rpmhpd_opp_min_svs>;
> > > +            };
> > > +
> > > +            opp-200000000 {
> > > +                opp-hz = /bits/ 64 <200000000>;
> > > +                opp-peak-kBps = <44492>;
> > > +                required-opps = <&rpmhpd_opp_low_svs>;
> > > +            };
> > > +
> > > +            opp-400000000 {
> > > +                opp-hz = /bits/ 64 <400000000>;
> > > +                opp-peak-kBps = <88983>;
> > > +                required-opps = <&rpmhpd_opp_svs>;
> > > +            };
> > > +
> > > +            opp-480000000 {
> > > +                opp-hz = /bits/ 64 <480000000>;
> > > +                opp-peak-kBps = <106780>;
> > > +                required-opps = <&rpmhpd_opp_svs_l1>;
> > > +            };
> > > +        };
> > > +    };
> > > diff --git a/include/dt-bindings/interconnect/qcom,camnoc.h b/include/dt-bindings/interconnect/qcom,camnoc.h
> > > new file mode 100644
> > > index 000000000000..a03f5e3f6446
> > > --- /dev/null
> > > +++ b/include/dt-bindings/interconnect/qcom,camnoc.h
> > > @@ -0,0 +1,12 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> > > +/*
> > > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > > + */
> > > +
> > > +#ifndef __DT_BINDINGS_INTERCONNECT_QCOM_CAMNOC_H
> > > +#define __DT_BINDINGS_INTERCONNECT_QCOM_CAMNOC_H
> > > +
> > > +#define MASTER_CAMNOC_JPEG	0
> > 
> > I guess, there are other connection ports. Please describe them.
> > 
> > > +#define SLAVE_CAMNOC_AXI	1
> > > +
> > > +#endif /* __DT_BINDINGS_INTERCONNECT_QCOM_CAMNOC_H */
> > > -- 
> > > 2.34.1
> > > 
> > 
> 

-- 
With best wishes
Dmitry