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

[Krzysztof Kozlowski]

On 23/03/2026 17:03, Loic Poulain wrote:
> Hi Krzysztof,
> 
> On Mon, Mar 23, 2026 at 2:04 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
>>
>> 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