Re: [PATCH 3/5] media: dt-bindings: Add Apple ISP

On 19/02/2025 10:26, Sasha Finkelstein via B4 Relay wrote:

+  reg-names:
+    items:
+      - const: coproc
+      - const: mbox
+      - const: gpio
+      - const: mbox2
+
+  iommus:
+    description: All 3 must be kept in sync
+    minItems: 3

Drop minItems

+    maxItems: 3
+
+  interrupts:
+    maxItems: 1
+
+  power-domains:
+    minItems: 1
+    maxItems: 20
+    description: All necessary power domains. Driver will enable them in order
+
+  memory-region:
+    maxItems: 1
+
+  apple,dart-vm-size:
+    description: Supported device memory range
+    $ref: /schemas/types.yaml#/definitions/uint64

That's deducible from comaptible.

+
+  apple,platform-id:
+    description: Platform id for firmware
+    $ref: /schemas/types.yaml#/definitions/uint32

No, use firmware-name.

+
+  apple,temporal-filter:
+    description: Whether temporal filter should be enabled in firmware
+    $ref: /schemas/types.yaml#/definitions/uint32

And why is this not enabled always? Why this is board specific?


You miss here ports or port. ISP usually gets signal from some camera or
other block.

+
+  sensor-presets:
+    additionalProperties: false
+
+    patternProperties:
+      '^preset[0-9]+$':
+        type: object
+
+        additionalProperties: false
+
+        properties:
+          apple,config-index:
+            description: Firmware config index
+            $ref: /schemas/types.yaml#/definitions/uint32

No duplicated indices. You have reg for this, assuming this is index.

+
+          apple,input-size:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            minItems: 2
+            maxItems: 2
+            description: Raw sensor size
+
+          apple,output-size:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            minItems: 2
+            maxItems: 2
+            description: Cropped and scaled image size
+
+          apple,crop:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            minItems: 4
+            maxItems: 4
+            description: Area to crop

All these do not look like hardware properties but rather configuration
of sensor which should be done runtime by OS, not by DT.

Best regards,
Krzysztof