| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <eff7080b-42e9-19ae-6022-bfbcc337b4a0@ti.com>
Date: Fri, 1 Mar 2024 22:02:03 +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,
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@...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.
>
> 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@...labora.com`
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@kernel.org/
>> [1] : https://lore.kernel.org/all/20240228141140.3530612-4-devarsht@ti.com/
>>
>>
>> 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@...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