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

On 8/11/2026 2:59 PM, 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 <[email protected]>
>>> ---
>>>   .../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 <[email protected]>
>>> +
>>> +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 belongs to the consumer, not to the CAMNOC provider.
Each consumer (IFE, JPEG, BPS, etc.) has its own dedicated core clock
(opp-hz) and its own CAMNOC bandwidth requirement (opp-peak-kBps) that
depends on its operating point, resolution, and quality settings. These
differ per sub-device, so a single shared OPP table in cam_virt would
not be meaningful.

The cam_virt node only owns CAM_CC_CAMNOC_AXI_CLK and aggregates votes
from all consumers -- it has no knowledge of individual consumer
operating points.

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

Addressed in v2: qcom,camnoc.h now lists all SM8250 CAMNOC master ports
(IFE 0/1, IFE Lite 0-4, BPS, IPE 0/1, JPEG, JPEG DMA, CDM, FD, SBI,
ICP) with a short comment per group.

Thanks for the review.
Atanas

>>
>>> +#define SLAVE_CAMNOC_AXI    1
>>> +
>>> +#endif /* __DT_BINDINGS_INTERCONNECT_QCOM_CAMNOC_H */
>>> -- 
>>> 2.34.1
>>>
>>
>