[RFC PATCH 1/3] dt-bindings: media: qcom: Add CAMSS Offline Processing Engine (OPE)

Loic Poulain posted 3 patches 1 week, 4 days ago
[RFC PATCH 1/3] dt-bindings: media: qcom: Add CAMSS Offline Processing Engine (OPE)
Posted by Loic Poulain 1 week, 4 days ago
Add Devicetree binding documentation for the Qualcomm Camera Subsystem
Offline Processing Engine (OPE) found on platforms such as Agatti.
The OPE is a memory-to-memory image processing block which operates
on frames read from and written back to system memory.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 .../bindings/media/qcom,camss-ope.yaml        | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,camss-ope.yaml

diff --git a/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
new file mode 100644
index 000000000000..509b4e89a88a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/qcom,camss-ope.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Camera Subsystem Offline Processing Engine
+
+maintainers:
+  - Loic Poulain <loic.poulain@oss.qualcomm.com>
+
+description:
+  The Qualcomm Camera Subsystem (CAMSS) Offline Processing Engine (OPE)
+  is a memory-to-memory image processing block used. It supports a
+  range of pixel-processing operations such as scaling, cropping, gain
+  adjustments, white-balancing, and various format conversions. The OPE
+  does not interface directly with image sensors, instead, it processes
+  frames sourced from and written back to system memory.
+
+properties:
+  compatible:
+    const: qcom,qcm2290-camss-ope
+
+  reg:
+    maxItems: 5
+
+  reg-names:
+    items:
+      - const: top
+      - const: bus_read
+      - const: bus_write
+      - const: pipeline
+      - const: qos
+
+  clocks:
+    maxItems: 5
+
+  clock-names:
+    items:
+      - const: axi
+      - const: core
+      - const: iface
+      - const: nrt
+      - const: top
+
+  interrupts:
+    maxItems: 1
+
+  interconnects:
+    maxItems: 2
+
+  interconnect-names:
+    items:
+      - const: config
+      - const: data
+
+  iommus:
+    maxItems: 2
+
+  operating-points-v2: true
+
+  opp-table:
+    type: object
+
+  power-domains:
+    maxItems: 2
+
+  power-domain-names:
+    items:
+      - const: camss
+      - const: cx
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - interrupts
+  - interconnects
+  - interconnect-names
+  - iommus
+  - power-domains
+  - power-domain-names
+
+additionalProperties: true
-- 
2.34.1
Re: [RFC PATCH 1/3] dt-bindings: media: qcom: Add CAMSS Offline Processing Engine (OPE)
Posted by Krzysztof Kozlowski 1 week, 4 days ago
On 23/03/2026 13:58, Loic Poulain wrote:
> Add Devicetree binding documentation for the Qualcomm Camera Subsystem
> Offline Processing Engine (OPE) found on platforms such as Agatti.
> The OPE is a memory-to-memory image processing block which operates
> on frames read from and written back to system memory.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

I don't see explanation in cover letter why this is RFC, so I assume
this is not ready, thus not a full review but just few nits to spare you
resubmits later when this becomes reviewable.

> ---
>  .../bindings/media/qcom,camss-ope.yaml        | 86 +++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
> new file mode 100644
> index 000000000000..509b4e89a88a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml

Filename must match compatible.

> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2

...
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interconnects
> +  - interconnect-names
> +  - iommus
> +  - power-domains
> +  - power-domain-names
> +
> +additionalProperties: true

There are no bindings like that. You cannot have here true.

Also, lack of example is a no-go.

BTW, also remember about proper versioning of your patchset. b4 would do
that for you, but since you did not use it, you must handle it.

Best regards,
Krzysztof
Re: [RFC PATCH 1/3] dt-bindings: media: qcom: Add CAMSS Offline Processing Engine (OPE)
Posted by Loic Poulain 1 week, 4 days ago
Hi Krzysztof,

On Mon, Mar 23, 2026 at 2:04 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 23/03/2026 13:58, Loic Poulain wrote:
> > Add Devicetree binding documentation for the Qualcomm Camera Subsystem
> > Offline Processing Engine (OPE) found on platforms such as Agatti.
> > The OPE is a memory-to-memory image processing block which operates
> > on frames read from and written back to system memory.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>
> I don't see explanation in cover letter why this is RFC, so I assume
> this is not ready, thus not a full review but just few nits to spare you
> resubmits later when this becomes reviewable.
>
> > ---
> >  .../bindings/media/qcom,camss-ope.yaml        | 86 +++++++++++++++++++
> >  1 file changed, 86 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
> > new file mode 100644
> > index 000000000000..509b4e89a88a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
>
> Filename must match compatible.

Some bindings (for example clock/qcom,mmcc.yaml) do not strictly
follow this rule and instead use a more generic filename that groups
multiple device-specific compatibles. I mention this because my
intention with a generic filename was to allow the binding to cover
additional compatibles in the future.

As I understand it, in the current state I should either:
- rename the file so that it matches the specific compatible, e.g.
qcom,qcm2290-camss-ope.yaml, or
- keep the generic filename (qcom,camss-ope.yaml) and add a top-level
const: qcom,camss-ope compatible to justify the generic naming.

Any preferred/valid direction?

>
> > @@ -0,0 +1,86 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
>
> ...
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - clocks
> > +  - clock-names
> > +  - interrupts
> > +  - interconnects
> > +  - interconnect-names
> > +  - iommus
> > +  - power-domains
> > +  - power-domain-names
> > +
> > +additionalProperties: true
>
> There are no bindings like that. You cannot have here true.

ok.

>
> Also, lack of example is a no-go.

Ouch, yes. Would it make sense to have dt_binding_check catch this
kind of issue?

>
> BTW, also remember about proper versioning of your patchset. b4 would do
> that for you, but since you did not use it, you must handle it.

ack.
Re: [RFC PATCH 1/3] dt-bindings: media: qcom: Add CAMSS Offline Processing Engine (OPE)
Posted by Krzysztof Kozlowski 1 week, 4 days ago
On 23/03/2026 17:03, Loic Poulain wrote:
> Hi Krzysztof,
> 
> On Mon, Mar 23, 2026 at 2:04 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 23/03/2026 13:58, Loic Poulain wrote:
>>> Add Devicetree binding documentation for the Qualcomm Camera Subsystem
>>> Offline Processing Engine (OPE) found on platforms such as Agatti.
>>> The OPE is a memory-to-memory image processing block which operates
>>> on frames read from and written back to system memory.
>>>
>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>
>> I don't see explanation in cover letter why this is RFC, so I assume
>> this is not ready, thus not a full review but just few nits to spare you
>> resubmits later when this becomes reviewable.
>>
>>> ---
>>>  .../bindings/media/qcom,camss-ope.yaml        | 86 +++++++++++++++++++
>>>  1 file changed, 86 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
>>> new file mode 100644
>>> index 000000000000..509b4e89a88a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
>>
>> Filename must match compatible.
> 
> Some bindings (for example clock/qcom,mmcc.yaml) do not strictly
> follow this rule and instead use a more generic filename that groups
> multiple device-specific compatibles. I mention this because my
> intention with a generic filename was to allow the binding to cover
> additional compatibles in the future.
> 
> As I understand it, in the current state I should either:
> - rename the file so that it matches the specific compatible, e.g.
> qcom,qcm2290-camss-ope.yaml, or

This one.

> - keep the generic filename (qcom,camss-ope.yaml) and add a top-level
> const: qcom,camss-ope compatible to justify the generic naming.

Because this would be a reverse logic... Name of file is never an
argument/reason to add a compatible.


> 
> Any preferred/valid direction?
> 
>>
>>> @@ -0,0 +1,86 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>
>> ...
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - reg-names
>>> +  - clocks
>>> +  - clock-names
>>> +  - interrupts
>>> +  - interconnects
>>> +  - interconnect-names
>>> +  - iommus
>>> +  - power-domains
>>> +  - power-domain-names
>>> +
>>> +additionalProperties: true
>>
>> There are no bindings like that. You cannot have here true.
> 
> ok.
> 
>>
>> Also, lack of example is a no-go.
> 
> Ouch, yes. Would it make sense to have dt_binding_check catch this
> kind of issue?

Not sure if worth implementing. Every new binding is a copy of existing
one and 99% of them have examples, so how new binding could be created
without one? This is highly unlikely and most likely there are other
issues as well, because process is broken, so dtschema won't help.

And with LLM you can write whatever will pass dtschema but still make
not sense.

Best regards,
Krzysztof
Re: [RFC PATCH 1/3] dt-bindings: media: qcom: Add CAMSS Offline Processing Engine (OPE)
Posted by Bryan O'Donoghue 1 week, 4 days ago
On 23/03/2026 12:58, Loic Poulain wrote:
> Add Devicetree binding documentation for the Qualcomm Camera Subsystem
> Offline Processing Engine (OPE) found on platforms such as Agatti.
> The OPE is a memory-to-memory image processing block which operates
> on frames read from and written back to system memory.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   .../bindings/media/qcom,camss-ope.yaml        | 86 +++++++++++++++++++
>   1 file changed, 86 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
> new file mode 100644
> index 000000000000..509b4e89a88a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,camss-ope.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,camss-ope.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Camera Subsystem Offline Processing Engine
> +
> +maintainers:
> +  - Loic Poulain <loic.poulain@oss.qualcomm.com>
> +
> +description:
> +  The Qualcomm Camera Subsystem (CAMSS) Offline Processing Engine (OPE)
> +  is a memory-to-memory image processing block used. It supports a
> +  range of pixel-processing operations such as scaling, cropping, gain
> +  adjustments, white-balancing, and various format conversions. The OPE
> +  does not interface directly with image sensors, instead, it processes
> +  frames sourced from and written back to system memory.
> +
> +properties:
> +  compatible:
> +    const: qcom,qcm2290-camss-ope
> +
> +  reg:
> +    maxItems: 5
> +
> +  reg-names:
> +    items:
> +      - const: top
> +      - const: bus_read
> +      - const: bus_write
> +      - const: pipeline
> +      - const: qos
> +
> +  clocks:
> +    maxItems: 5
> +
> +  clock-names:
> +    items:
> +      - const: axi
> +      - const: core
> +      - const: iface
> +      - const: nrt
> +      - const: top
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interconnects:
> +    maxItems: 2
> +
> +  interconnect-names:
> +    items:
> +      - const: config
> +      - const: data
> +
> +  iommus:
> +    maxItems: 2

These should be described.

> +
> +  operating-points-v2: true
> +
> +  opp-table:
> +    type: object
> +
> +  power-domains:
> +    maxItems: 2
> +
> +  power-domain-names:
> +    items:
> +      - const: camss
> +      - const: cx
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interconnects
> +  - interconnect-names
> +  - iommus
> +  - power-domains
> +  - power-domain-names
> +
> +additionalProperties: true
> --
> 2.34.1
>