Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28DEFC38142 for ; Wed, 1 Feb 2023 09:46:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231769AbjBAJqF (ORCPT ); Wed, 1 Feb 2023 04:46:05 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33494 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231709AbjBAJqC (ORCPT ); Wed, 1 Feb 2023 04:46:02 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1B4C73589; Wed, 1 Feb 2023 01:46:01 -0800 (PST) Received: from pendragon.ideasonboard.com (unknown [193.209.96.36]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 765BB505; Wed, 1 Feb 2023 10:45:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1675244759; bh=ae5B3Tp2ry8b8G71s40ax+Ovn2zEm/amT6bkxrEQuxE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=foPZ/2HPjQSpoOqncY+pjjiigYN5eqgZ3CcVr/AHeSvrOVPPTbWYMDjX7G++JMu4L MB73dAt6eexFHleB/XQDlRNL0csmsQRoYpbmgpKxTVyKOKs1N24OFJzLIuLWpZ+re2 xf9PLAxj3R38ar43TcZbh8wQrgcWZ1/H6oc/rybM= Date: Wed, 1 Feb 2023 11:45:57 +0200 From: Laurent Pinchart To: yuji2.ishikawa@toshiba.co.jp Cc: krzysztof.kozlowski@linaro.org, hverkuil@xs4all.nl, mchehab@kernel.org, nobuhiro1.iwamatsu@toshiba.co.jp, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, rafael.j.wysocki@intel.com, broonie@kernel.org, linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v5 1/6] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface bindings Message-ID: References: <20230111022433.25950-1-yuji2.ishikawa@toshiba.co.jp> <20230111022433.25950-2-yuji2.ishikawa@toshiba.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 30, 2023 at 09:06:25AM +0000, yuji2.ishikawa@toshiba.co.jp wrote: > On Monday, January 23, 2023 4:26 AM, Laurent Pinchart wrote: > > On Tue, Jan 17, 2023 at 06:01:27PM +0100, Krzysztof Kozlowski wrote: > > > On 17/01/2023 16:58, Laurent Pinchart wrote: > > > > On Tue, Jan 17, 2023 at 04:42:51PM +0100, Krzysztof Kozlowski wrote: > > > >> On 17/01/2023 16:26, Laurent Pinchart wrote: > > > >>> > > > >>>> + > > > >>>> + clock-lanes: > > > >>>> + description: VIIF supports 1 clock line > > > >>> > > > >>> s/line/lane/ > > Sorry for a late reply. > I'll fix the description. > > > > >>> > > > >>>> + const: 0 > > > >>> > > > >>> I would also add > > > >>> > > > >>> clock-noncontinuous: true > > > >>> link-frequencies: true > > > >>> > > > >>> to indicate that the above two properties are used by this device. > > > >> > > > >> No, these are coming from other schema and there is never need to > > > >> mention some property to indicate it is more used than other case. > > > >> None of the bindings are created such way, so this should not be exception. > > > > > > > > There are some bindings that do so, but that may not be a good > > > > enough reason, as there's a chance I wrote those myself :-) > > > > > > > > I would have sworn that at some point in the past the schema > > > > wouldn't have validated the example with this omitted. I'm not sure > > > > if something changed or if I got this wrong. > > > > > > You probably think about case when using additionalProperties:false, > > > where one has to explicitly list all valid properties. But not for > > > unevaluatedProperties:false. > > > > Possibly, yes. > > > > > > video-interfaces.yaml defines lots of properties applicable to > > > > endpoints. For a given device, those properties should be required > > > > > > required: > > > - foo > > > > > > > (easy, that's defined in the bindings), optional, > > > > > > by default (with unevaluatedProperties:false) or explicitly mention > > > "foo: true (with additionalProperties:false) > > > > > > > or forbidden. How do > > > > > > foo: false (with unevaluatedProperties:false) or by default (with > > > additionalProperties:false) > > > > I think we should default to the latter. video-interfaces.yaml contains lots of > > properties endpoint properties, most bindings will use less than half of them, so > > having to explicitly list all the ones that are not used with "foo: false" would be > > quite inconvenient. Furthermore, I expect more properties to be added to > > video-interfaces.yaml over time, and those shouldn't be accepted by default in > > existing bindings. > > > > I caught up with this discussion after some exercise on JSON schema validator. > I'll remove "unevaluatedProperties: false" at the "endpoint" and add "aditionalProperties: false" instead. > Furthermore, I'll explicitly declare required properties (required: ["foo"]) and optional properties (properties: {foo: true}) for Visconti. > Is this correct understanding? Looks very good to me ! > Are these changes also applied to "port", which is the parent node of > the "endpoint" ? That shouldn't be needed, as the "port" node should only have "endpoint" children and no other properties (except for reg, and possibly #address-cells and #size-cells of course). > > > > we differentiate between the latter two cases ? -- Regards, Laurent Pinchart