Received-SPF: pass (google.com: domain of linux-kernel+bounces-86546-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 29 Feb 2024 11:26:23 +0100
From: Sebastian Fricke <sebastian.fricke@collabora.com>
To: Devarsh Thakkar <devarsht@ti.com>
Cc: mchehab@kernel.org, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	conor+dt@kernel.org, hverkuil-cisco@xs4all.nl,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, benjamin.gaignard@collabora.com,
	laurent.pinchart@ideasonboard.com, praneeth@ti.com, nm@ti.com,
	vigneshr@ti.com, a-bhatia1@ti.com, j-luthra@ti.com, b-brnich@ti.com,
	detheridge@ti.com, p-mantena@ti.com, vijayp@ti.com,
	andrzej.p@collabora.com, nicolas@ndufresne.ca, afd@ti.com,
	milkfafa@gmail.com
Subject: Re: [PATCH v6 1/3] media: dt-bindings: Add Imagination E5010 JPEG
 Encoder
Message-ID: <20240229102623.ihwhbba4qwzvxzzq@basti-XPS-13-9310>
References: <20240228141140.3530612-1-devarsht@ti.com>
 <20240228141140.3530612-2-devarsht@ti.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Disposition: inline
In-Reply-To: <20240228141140.3530612-2-devarsht@ti.com>

Hey Devarsh,

On 28.02.2024 19:41, Devarsh Thakkar wrote:
>Add dt-bindings for Imagination E5010 JPEG Encoder [1] which is implemented
>as stateful V4L2 M2M driver.
>
>The device supports baseline encoding with two different quantization
>tables and compression ratio as demanded.
>
>Minimum resolution supported is 64x64 and Maximum resolution supported is
>8192x8192.
>
>[1]:  AM62A TRM (Section 7.6 is for JPEG Encoder)
>Link: https://www.ti.com/lit/pdf/spruj16
>
>Co-developed-by: David Huang <d-huang@ti.com>
>Signed-off-by: David Huang <d-huang@ti.com>
>Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
>Reviewed-by: Rob Herring <robh@kernel.org>

hmmm when did Rob give his reviewed by on this patch? (As this is not a
DT binding I find that odd)
And where is the Reviewed by tag from Benjamin that he provided on V5?

Greetings,
Sebastian

>---
>V2: No change
>V3:
>- Add vendor specific compatible
>- Update reg names
>- Update clocks to 1
>- Fix dts example with proper naming
>V4:
> - Use ti-specific compatible ti,am62a-jpeg-enc as secondary one
> - Update commit message and title
> - Remove clock-names as only single clock
>V5:
> - Add Reviewed-By tag
>V6:
> - No change
>
> .../bindings/media/img,e5010-jpeg-enc.yaml    | 75 +++++++++++++++++++
> MAINTAINERS                                   |  5 ++
> 2 files changed, 80 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml
>
>diff --git a/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml
>new file mode 100644
>index 000000000000..085020cb9e61
>--- /dev/null
>+++ b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml
>@@ -0,0 +1,75 @@
>+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>+%YAML 1.2
>+---
>+$id: http://devicetree.org/schemas/media/img,e5010-jpeg-enc.yaml#
>+$schema: http://devicetree.org/meta-schemas/core.yaml#
>+
>+title: Imagination E5010 JPEG Encoder
>+
>+maintainers:
>+  - Devarsh Thakkar <devarsht@ti.com>
>+
>+description: |
>+  The E5010 is a JPEG encoder from Imagination Technologies implemented on
>+  TI's AM62A SoC. It is capable of real time encoding of YUV420 and YUV422
>+  inputs to JPEG and M-JPEG. It supports baseline JPEG Encoding up to
>+  8Kx8K resolution.
>+
>+properties:
>+  compatible:
>+    oneOf:
>+      - items:
>+          - const: ti,am62a-jpeg-enc
>+          - const: img,e5010-jpeg-enc
>+      - const: img,e5010-jpeg-enc
>+
>+  reg:
>+    items:
>+      - description: The E5010 core register region
>+      - description: The E5010 mmu register region
>+
>+  reg-names:
>+    items:
>+      - const: core
>+      - const: mmu
>+
>+  power-domains:
>+    maxItems: 1
>+
>+  resets:
>+    maxItems: 1
>+
>+  clocks:
>+    maxItems: 1
>+
>+  interrupts:
>+    maxItems: 1
>+
>+required:
>+  - compatible
>+  - reg
>+  - reg-names
>+  - interrupts
>+  - clocks
>+
>+additionalProperties: false
>+
>+examples:
>+  - |
>+    #include <dt-bindings/soc/ti,sci_pm_domain.h>
>+    #include <dt-bindings/interrupt-controller/arm-gic.h>
>+    #include <dt-bindings/interrupt-controller/irq.h>
>+
>+    soc {
>+      #address-cells = <2>;
>+      #size-cells = <2>;
>+      jpeg-encoder@fd20000 {
>+          compatible = "img,e5010-jpeg-enc";
>+          reg = <0x00 0xfd20000 0x00 0x100>,
>+                <0x00 0xfd20200 0x00 0x200>;
>+          reg-names = "core", "mmu";
>+          clocks = <&k3_clks 201 0>;
>+          power-domains = <&k3_pds 201 TI_SCI_PD_EXCLUSIVE>;
>+          interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
>+      };
>+    };
>diff --git a/MAINTAINERS b/MAINTAINERS
>index e1475ca38ff2..6b34ee8d92b5 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -10572,6 +10572,11 @@ S:	Maintained
> F:	Documentation/devicetree/bindings/auxdisplay/img,ascii-lcd.yaml
> F:	drivers/auxdisplay/img-ascii-lcd.c
>
>+IMGTEC JPEG ENCODER DRIVER
>+M:	Devarsh Thakkar <devarsht@ti.com>
>+S:	Supported
>+F:	Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml
>+
> IMGTEC IR DECODER DRIVER
> S:	Orphan
> F:	drivers/media/rc/img-ir/
>-- 
>2.39.1
>