| 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: Mon, 18 Sep 2023 08:49:54 +0200
From: Sebastian Fricke <sebastian.fricke@...labora.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
Nas Chung <nas.chung@...psnmedia.com>,
Sascha Hauer <s.hauer@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Shawn Guo <shawnguo@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Jackson Lee <jackson.lee@...psnmedia.com>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
NXP Linux Team <linux-imx@....com>,
Hans Verkuil <hverkuil@...all.nl>,
Conor Dooley <conor+dt@...nel.org>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
Robert Beckett <bob.beckett@...labora.com>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
kernel@...labora.com,
Nicolas Dufresne <nicolas.dufresne@...labora.com>
Subject: Re: [PATCH v12 6/7] dt-bindings: media: wave5: add yaml devicetree
bindings
Hey Krzysztof,
thanks for your review.
On 17.09.2023 09:56, Krzysztof Kozlowski wrote:
>On 15/09/2023 23:11, Sebastian Fricke wrote:
>> From: Robert Beckett <bob.beckett@...labora.com>
>>
>> Add bindings for the wave5 chips&media codec driver
>>
>> Signed-off-by: Robert Beckett <bob.beckett@...labora.com>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@...labora.com>
>> Signed-off-by: Sebastian Fricke <sebastian.fricke@...labora.com>
>
>So this is v12 and still no tested?
I have tested it, multiple times actually since V11. (For some reason
that indentation issue slipped by me though ...)
If you mean the tested by tag, the patch was completely unnoticed until
v10 by the community, which was partially because me and the previous
commiters didn't use the right recipients for this patch. So from that
point of view this is more like v2.
>
>A nit, subject: drop second/last, redundant "yaml devicetree indings".
>The "dt-bindings" prefix is already stating that these are bindings.
>Basically three words bringing zero information.
Okay so:
`dt-bindings: media: wave5: add devicetree`
?
>
>> ---
>> .../devicetree/bindings/media/cnm,wave5.yaml | 66 ++++++++++++++++++++++
>> 1 file changed, 66 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/cnm,wave5.yaml b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>> new file mode 100644
>> index 000000000000..b8f383621805
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/cnm,wave5.yaml
>> @@ -0,0 +1,66 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/cnm,wave5.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Chips&Media Wave 5 Series multi-standard codec IP
>> +
>> +maintainers:
>> + - Nas Chung <nas.chung@...psnmedia.com>
>> + - Jackson Lee <jackson.lee@...psnmedia.com>
>> +
>> +description: |-
>
>Do not need '|-' unless you need to preserve formatting.
Ack.
>
>> + The Chips&Media WAVE codec IP is a multi format video encoder/decoder
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - cnm,cm521c-vpu
>
>It does not look like you tested the bindings, at least after quick
>look. Please run `make dt_binding_check` (see
>Documentation/devicetree/bindings/writing-schema.rst for instructions).
>Maybe you need to update your dtschema and yamllint.
Here my testing output:
```
❯ make dt_binding_check DT_SCHEMA_FILES=cnm,wave5.yaml
HOSTCC scripts/basic/fixdep
HOSTCC scripts/dtc/dtc.o
HOSTCC scripts/dtc/flattree.o
HOSTCC scripts/dtc/fstree.o
HOSTCC scripts/dtc/data.o
HOSTCC scripts/dtc/livetree.o
HOSTCC scripts/dtc/treesource.o
HOSTCC scripts/dtc/srcpos.o
HOSTCC scripts/dtc/checks.o
HOSTCC scripts/dtc/util.o
LEX scripts/dtc/dtc-lexer.lex.c
YACC scripts/dtc/dtc-parser.tab.[ch]
HOSTCC scripts/dtc/dtc-lexer.lex.o
HOSTCC scripts/dtc/dtc-parser.tab.o
HOSTLD scripts/dtc/dtc
LINT Documentation/devicetree/bindings
./Documentation/devicetree/bindings/media/cnm,wave5.yaml:19:9: [warning] wrong indentation: expected 6 but found 8 (indentation)
CHKDT Documentation/devicetree/bindings/processed-schema.json
SCHEMA Documentation/devicetree/bindings/processed-schema.json
DTEX Documentation/devicetree/bindings/media/cnm,wave5.example.dts
DTC_CHK Documentation/devicetree/bindings/media/cnm,wave5.example.dtb
```
Again sorry about missing the indentation warning, but nothing else was
highlighted.
Both dtschema and yamllint seem to be up-to-date:
```
❯ python3 -m pip --version
pip 23.2.1 from /home/basti/.local/lib/python3.8/site-packages/pip (python 3.8)
❯ pip3 show dtschema
Name: dtschema
Version: 2023.7
Summary: DeviceTree validation schema and tools
Home-page: https://github.com/devicetree-org/dt-schema
Author: Rob Herring
Author-email: robh@...nel.org
License: BSD
Location: /home/basti/.local/lib/python3.8/site-packages
Requires: jsonschema, pylibfdt, rfc3987, ruamel.yaml
Required-by:
❯ pip3 show yamllint
Name: yamllint
Version: 1.32.0
Summary: A linter for YAML files.
Home-page:
Author: Adrien Vergé
Author-email:
License: GPL-3.0-only
Location: /home/basti/.local/lib/python3.8/site-packages
Requires: pathspec, pyyaml
Required-by:
```
>
>Missing blank line
Ack, will add that.
>
>> + reg:
>> + maxItems: 1
>> +
>> + clocks:
>> + items:
>> + - description: VCODEC clock
>> +
>> + clock-names:
>> + items:
>> + - const: vcodec
>
>Drop clock-names, not really useful for one entry.
Ack
>
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + power-domains:
>> + maxItems: 1
>> +
>> + resets:
>> + maxItems: 1
>> +
>> + sram:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> +
>
>Drop blank line
Ack
>
>> + description:
>> + The VPU uses the SRAM to store some of the reference data instead of
>> + storing it on DMA memory. It is mainly used for the purpose of reducing
>> + bandwidth.
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>
>Keep the same order as listed in properties:
Ack
>
>> + - clocks
>> + - clock-names
>> +
>> +additionalProperties: false
>> +
>
>Best regards,
>Krzysztof
Sincerely,
Sebastian
>
>_______________________________________________
>Kernel mailing list -- kernel@...lman.collabora.com
>To unsubscribe send an email to kernel-leave@...lman.collabora.com
Powered by blists - more mailing lists