Hi Sebastian, On 29/02/24 19:00, Sebastian Fricke wrote: > Hey Devarsh, > > On 29.02.2024 16:50, Devarsh Thakkar wrote: >> 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@xxxxxx> >>>> Signed-off-by: David Huang <d-huang@xxxxxx> >>>> Signed-off-by: Devarsh Thakkar <devarsht@xxxxxx> >>>> Reviewed-by: Rob Herring <robh@xxxxxxxxxx> >>> >>> 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. > > Yes thanks I was a bit too swift to write here, sorry for the noise. > We'll have a look. > Sorry for the back and forth, but on the hindsight and re-looking at the kernel patch guidelines [0] they suggest that Reviewed-By tag should only be removed if substantial changes were made in further revisions. So looks to me in-fact it was a mistake on my part to remove the Reviewed-by considering the change made in the following patch series was not a substantial one as seen in the range-diff [1]. Considering this, just wanted to check with you if it's possible for you to consider the Reviewed-by tag : `Reviewed-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx` if it helps consolidate things faster to get this series in given we are close to final RC's ? [0]: https://docs.kernel.org/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes:~:text=changed%20substantially [1]: https://gist.github.com/devarsht/c89180ac2b0d2814614f2b59d0705c19 Regards Devarsh > > Greetings, > Sebastian > >> >> [0] : >> https://lore.kernel.org/all/170716378412.295212.11603162949482063011.robh@xxxxxxxxxx/ >> [1] : https://lore.kernel.org/all/20240228141140.3530612-4-devarsht@xxxxxx/ >> >> >> Regards >> Devarsh >>>> --- >>>> 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@xxxxxx> >>>> + >>>> +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@xxxxxx> >>>> +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 >>>>
