[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7a83fe91-5afa-6aee-a8a4-44f6e3d713c2@ti.com>
Date: Thu, 29 Feb 2024 16:50:38 +0530
From: Devarsh Thakkar <devarsht@...com>
To: Sebastian Fricke <sebastian.fricke@...labora.com>
CC: <mchehab@...nel.org>, <robh@...nel.org>,
<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
<hverkuil-cisco@...all.nl>, <linux-media@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<benjamin.gaignard@...labora.com>, <laurent.pinchart@...asonboard.com>,
<praneeth@...com>, <nm@...com>, <vigneshr@...com>, <a-bhatia1@...com>,
<j-luthra@...com>, <b-brnich@...com>, <detheridge@...com>,
<p-mantena@...com>, <vijayp@...com>, <andrzej.p@...labora.com>,
<nicolas@...fresne.ca>, <afd@...com>, <milkfafa@...il.com>
Subject: Re: [PATCH v6 1/3] media: dt-bindings: Add Imagination E5010 JPEG
Encoder
Hi Sebastian,
Thanks for the review.
On 29/02/24 15:56, Sebastian Fricke wrote:
> 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@...com>
>> Signed-off-by: David Huang <d-huang@...com>
>> Signed-off-by: Devarsh Thakkar <devarsht@...com>
>> Reviewed-by: Rob Herring <robh@...nel.org>
>
> hmmm when did Rob give his reviewed by on this patch? (As this is not a
> DT binding I find that odd)
[PATCH v6 1/3] media: dt-bindings: Add Imagination E5010 JPEG Encoder : This
is indeed the dt-binding patch. Also As shared in version history it is at V4
where Rob Herring added a Reviewed-By as seen here [0]
> And where is the Reviewed by tag from Benjamin that he provided on V5?
>
As captured in patch version history here [1] I thought to remove the
Reviewed-By since the Reviewed-By tag was on V5 and with V6 the driver got
updated with some changes to handle reported sparse warnings and so I have
asked Benjamin to check the range-diff and help with a quick review again if
possible.
Kindly let me know if I missed something or anything needs to be done from my end.
[0] :
https://lore.kernel.org/all/170716378412.295212.11603162949482063011.robh@kernel.org/
[1] : https://lore.kernel.org/all/20240228141140.3530612-4-devarsht@ti.com/
Regards
Devarsh
> 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@...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@...0000 {
>> + 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@...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
>>
Powered by blists - more mailing lists