| 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
| ||
|
Date: Thu, 7 Mar 2024 13:05:05 +0100
From: Sebastian Fricke <sebastian.fricke@...labora.com>
To: Devarsh Thakkar <devarsht@...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
Hey Devarsh,
On 06.03.2024 21:27, Devarsh Thakkar wrote:
>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@...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 ?
>>
>> 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@ti.com/ as here
>too nothing much has changed w.r.t initial posting apart from commit message.
Yes I'd assume so but I sadly got sick this week, I'll get to this as
soon as I can.
>
>Regards
>Devarsh
Greetings,
Sebastian
>
>> 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@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