lkml.org 
[lkml]   [2023]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v8 1/5] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface
Date
Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Thursday, September 28, 2023 2:32 PM
> To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開)
> <yuji2.ishikawa@toshiba.co.jp>; Hans Verkuil <hverkuil@xs4all.nl>; Laurent
> Pinchart <laurent.pinchart@ideasonboard.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; iwamatsu nobuhiro(岩松 信洋 ○DITC□DIT○
> OST) <nobuhiro1.iwamatsu@toshiba.co.jp>
> Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.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
\
 
 \ /
  Last update: 2023-10-04 01:16    [W:0.395 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site