linux-kernel - RE: [PATCH v8 1/5] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <TYAPR01MB6201C2E34B969197C9E9DFB492C4A@TYAPR01MB6201.jpnprd01.prod.outlook.com>
Date:   Tue, 3 Oct 2023 23:10:10 +0000
From:   <yuji2.ishikawa@...hiba.co.jp>
To:     <krzysztof.kozlowski@...aro.org>, <hverkuil@...all.nl>,
        <laurent.pinchart@...asonboard.com>, <mchehab@...nel.org>,
        <robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
        <conor+dt@...nel.org>, <nobuhiro1.iwamatsu@...hiba.co.jp>
CC:     <linux-media@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v8 1/5] dt-bindings: media: platform: visconti: Add
 Toshiba Visconti Video Input Interface

Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> Sent: Thursday, September 28, 2023 2:32 PM
> To: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開)
> <yuji2.ishikawa@...hiba.co.jp>; Hans Verkuil <hverkuil@...all.nl>; Laurent
> Pinchart <laurent.pinchart@...asonboard.com>; Mauro Carvalho Chehab
> <mchehab@...nel.org>; Rob Herring <robh+dt@...nel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@...aro.org>; Conor Dooley
> <conor+dt@...nel.org>; iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○
> ＯＳＴ) <nobuhiro1.iwamatsu@...hiba.co.jp>
> Cc: linux-media@...r.kernel.org; devicetree@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v8 1/5] dt-bindings: media: platform: visconti: Add Toshiba
> Visconti Video Input Interface
> 
> On 26/09/2023 01:28, Yuji Ishikawa wrote:
> > Adds the Device Tree binding documentation that allows to describe the
> > Video Input Interface found in Toshiba Visconti SoCs.
> >
> 
> 
> > +  reg:
> > +    items:
> > +      - description: Registers for capture control
> > +      - description: Registers for CSI2 receiver control
> > +      - description: Registers for bus interface unit control
> > +      - description: Registers for Memory Protection Unit
> > +
> > +  interrupts:
> > +    items:
> > +      - description: Sync Interrupt
> > +      - description: Status (Error) Interrupt
> > +      - description: CSI2 Receiver Interrupt
> > +      - description: L1ISP Interrupt
> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/$defs/port-base
> > +    unevaluatedProperties: false
> > +    description: CSI-2 input port, with a single endpoint connected to the
> CSI-2 transmitter.
> > +
> > +    properties:
> > +      endpoint:
> > +        $ref: video-interfaces.yaml#
> > +        additionalProperties: false
> 
> This is a friendly reminder during the review process.
> 
> It seems my previous comments were not fully addressed. Maybe my feedback
> got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all requested
> changes or keep discussing them.
> 
> Thank you.

I'm very sorry that I misunderstood the intent of the last conversion.
https://lore.kernel.org/all/0aa471ce-da83-172d-d870-1ec7a562baf7@linaro.org/
I thought "additionalProperties: false" can be used and "xxx:true" should stay.

Let me confirm your intentions:
  - "unevaluatedProperties: false" should be used instead of "additionalProperties: false"
  - All of "xxx: true" should be removed
Are these two correct understandings?

> 
> > +
> > +        required:
> > +          - clock-noncontinuous
> > +          - link-frequencies
> > +          - remote-endpoint
> > +          - data-lanes
> 
> Not much improved here. required goes after properties, always. I pointed you
> last time the file which you should use as an example.

I'll check the overall structure of renesas,rzg2l-csi2.yaml.

> > +
> > +        properties:
> > +          data-lanes:
> > +            description: VIIF supports 1, 2, 3 or 4 data lanes
> > +            minItems: 1
> > +            items:
> > +              - const: 1
> > +              - const: 2
> > +              - const: 3
> > +              - const: 4
> > +
> > +          clock-noncontinuous: true
> 
> Drop
> 
> > +          link-frequencies: true
> 
> Drop
> > +          remote-endpoint: true
> 
> Drop
> 
> Best regards,
> Krzysztof

Best regards,
Yuji