On Tue, Jun 06, 2023 at 11:16:12AM +0200, Krzysztof Kozlowski wrote:

On 05/06/2023 18:20, Nícolas F. R. A. Prado wrote:

quoted

The binding expects the first register space to be VDEC_SYS. But on
mt8183, which uses the stateless decoders, this space is used only for
controlling clocks and resets, which are better described as separate
clock-controller and reset-controller nodes.

In fact, in mt8173's devicetree there are already such separate
clock-controller nodes, which cause duplicate addresses between the
vdecsys node and the vcodec node. But for this SoC, since the stateful
decoder code makes other uses of the VDEC_SYS register space, it's not
straightforward to remove it.

In order to avoid the same address conflict to happen on mt8183,
since the only current use of the VDEC_SYS register space in
the driver is to read the status of a clock that indicates the hardware
is active, remove the VDEC_SYS register space from the binding and
describe an extra clock that will be used to directly check the hardware
status.

Also add reg-names to be able to tell that this new register schema is
used, so the driver can keep backward compatibility.

Signed-off-by: Nícolas F. R. A. Prado <redacted>
---

 .../media/mediatek,vcodec-decoder.yaml        | 29 +++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
index 6447e6c86f29..36a53b2484d6 100644
--- a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml

@@ -21,17 +21,21 @@ properties:
       - mediatek,mt8183-vcodec-dec
 
   reg:
+    minItems: 11
     maxItems: 12
 
+  reg-names:
+    minItems: 11

maxItems

quoted

+
   interrupts:
     maxItems: 1
 
   clocks:
-    minItems: 1
+    minItems: 2

It does not make any sense. Just two patches ago you made it 1! Don't
add incorrect values which are immediately changed in the same patchset.

It's not an incorrect value. At the point that commit was added, the first reg
was still VDEC_SYS, so an active clock wasn't required. This commit removes the
VDEC_SYS reg, and so the active clock becomes necessary. Splitting it like this
made the most sense to me, and I thought it would also simplify review. But
since it seems to be a problem I'll merge commit 1 with this one in v2 to avoid
changing the number of clocks twice.

quoted

     maxItems: 8
 
   clock-names:
-    minItems: 1
+    minItems: 2
     maxItems: 8
 
   assigned-clocks: true

@@ -84,6 +88,24 @@ allOf:
         clock-names:
           items:
             - const: vdec
+            - const: active
+
+        reg:
+          maxItems: 11
+
+        reg-names:
+          items:
+            - const: misc
+            - const: ld
+            - const: top
+            - const: cm
+            - const: ad
+            - const: av
+            - const: pp
+            - const: hwd
+            - const: hwq
+            - const: hwb
+            - const: hwg
 
   - if:
       properties:

@@ -108,6 +130,9 @@ allOf:
             - const: venc_lt_sel
             - const: vdec_bus_clk_src
 
+        reg:
+          minItems: 12

so max can be 1000?

No, there's 'maxItems: 12' up above in the general case.

Thanks,
Nícolas

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel