Re: [PATCH v7 1/7] media: dt-bindings: venus: Add qcm2290 dt schema

[Krzysztof Kozlowski <krzk@kernel.org>]

On 17/07/2025 08:35, Jorge Ramirez wrote:
> On 17/07/25 00:22:53, Bryan O'Donoghue wrote:
>> On 15/07/2025 21:47, Jorge Ramirez-Ortiz wrote:
>>> Add a schema for the venus video encoder/decoder on the qcm2290.
>>>
>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> ---
>>>   .../bindings/media/qcom,qcm2290-venus.yaml    | 127 ++++++++++++++++++
>>>   1 file changed, 127 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml
>>> new file mode 100644
>>> index 000000000000..0371f8dd91a3
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/qcom,qcm2290-venus.yaml
>>> @@ -0,0 +1,127 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/media/qcom,qcm2290-venus.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm QCM2290 Venus video encode and decode accelerators
>>> +
>>> +maintainers:
>>> +  - Vikash Garodia <quic_vgarodia@quicinc.com>
>>
>> Shouldn't you be on this list ? If you upstream a file I think you should
>> list yourself as responsible for its glory or its mess.
> 
> happy to do it. The MAINTAINER's file covered all the files named

This should be the person(s) interested and caring about this hardware,
which means:
1. Subsystem maintainers: no
2. Driver maintainers: usually yes
3. Author(s) of new hardware support: usually yes

> schemas/media/*venus* so my understanding was that I shouldn't.

I cannot comment why people decided to go one way or another in other
code, but it as well could be just incorrect choice thinking only people
in MAINTAINERS care about hardware.

...

>>> +
>>> +        memory-region = <&pil_video_mem>;
>>> +        iommus = <&apps_smmu 0x860 0x0>,
>>> +                 <&apps_smmu 0x880 0x0>,
>>> +                 <&apps_smmu 0x861 0x04>,
>>> +                 <&apps_smmu 0x863 0x0>,
>>> +                 <&apps_smmu 0x804 0xe0>;
>>
>> You're listing five iommus.
>>
>> I understand there's some disagreement about whether or not to list all of
>> the potential use-cases but, TBH I don't think those are good arguments.
>>
>> Unless there's some technical prohibition I can't think of listing all five
>> maxItems:5 .. let's just do that.
> 
> since the device tree should describe hardware and not policy, and the
> driver seems to be able to ignore the unused SIDs I think this is the
> right thing to do.


It was never about the driver but about whether you should describe in
DTS for non-secure world the entries which are secure world. The answer
in general is that you can and there will be benefits (e.g. sharing DTS
with secure world implementations).

Best regards,
Krzysztof