Hi Sebastian, On 01/03/24 22:36, Sebastian Fricke wrote: > Hey Devarsh, > > On 01.03.2024 22:02, Devarsh Thakkar wrote: >> 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 ? > > Yup I think we can keep it as the changes are very minor. Otherwise the > series is pretty much good to go, I'll prepare the PR asap. > Thanks, so I assume this could still make it to 6.9 merge window if it gets pulled in before 6.8 rc8 ? Also do you think it would be possible to pull in https://lore.kernel.org/all/20240305160529.4152865-1-devarsht@xxxxxx/ as here too nothing much has changed w.r.t initial posting apart from commit message. Regards Devarsh > Greetings, > Sebastian > >> >> [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 >>>>>>
