Received: by 2002:a05:6a10:9e8c:0:0:0:0 with SMTP id y12csp1570074pxx; Fri, 30 Oct 2020 13:13:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx0FVGxB4iZ1/B0Yg2oCrXlp+5UxYvAsraplGSDCc/h/QnLHG/2ZRUMtSF6coNwYmm15Qzx X-Received: by 2002:a17:906:8610:: with SMTP id o16mr4119889ejx.209.1604088804769; Fri, 30 Oct 2020 13:13:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1604088804; cv=none; d=google.com; s=arc-20160816; b=AttEM7TpiFm6+LwmzozR4b9vYZJZ5+wcFSw2Z0+V/gN80W9M61vrLZJ9KD+qy11ypC oZHrrGtxf4OqSTE5yhaWFrwYPmE1F9Y5nmdFEcvSPzMrWYMDnj0CpD6xcAvO/kkVfpA8 jXdMMe1ahICowvUvAmiH/uG0UL6n3p2OFf0cwxWAALSgg2QCqRg92HE8BKaRfxnYrlqx n3T05aimgSDXqv6mzon0aEG/Z59pm4vBlDRWiipCMauw/hTB8n91q5CFBOkvnp6CSjwD bDW4MHxg8McLADaDiG4Zo3Jowx4Pg0YHkCR1b+xFoKodpzz7KIUvxrbknWxEDT/DeifN mxGA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=2dKqoVkVukaZQmHWcpeezbF5Ep9HUFnbSZB6Pl0w7Jk=; b=gwStO+5ZQFDSm3RB5MYauDU+4nwrGb6TBMKX/cW9Gw6Xw1I5QWS66LcGOT4i9trIM9 XcxoUpPysImkeHyCnxPBjzVpgzdbf4LNr6UTHRmJDT2jRzmsmHaYQEGr6daNRkMXopkD lagxAc2iFL0Oy0ZftVtskKLWdzVB2ZEsg6qOjWa+efCn/d1f6GZCkoQDhYnkI8CC6xtP H1i/s95yZr75obBKq9t4Rf0btCljyuvEOJqYD6xoIE/a4vF8OpJnfJiPUCEbw93dViya nVTD8PIm3GiM4YhjcO64JZjCTFlcXGlCk0kz+tDn/gfbmfIk9HS4B1IgvVS2h2UQCOF/ eQWQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=C7kxkKrx; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id y13si1438935eju.307.2020.10.30.13.12.57; Fri, 30 Oct 2020 13:13:24 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=C7kxkKrx; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727217AbgJ3UJm (ORCPT + 99 others); Fri, 30 Oct 2020 16:09:42 -0400 Received: from mail.kernel.org ([198.145.29.99]:44754 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727044AbgJ3UJY (ORCPT ); Fri, 30 Oct 2020 16:09:24 -0400 Received: from mail-oi1-f178.google.com (mail-oi1-f178.google.com [209.85.167.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id A3C8A20723; Fri, 30 Oct 2020 20:09:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604088562; bh=dsb2fGpwSeCxaqOUaIEQtAt8Ev0KBIXRf/w3waNhKws=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=C7kxkKrxR6DMsZCWkCPW2z3xH1cj5sJoA50H0JjBsmJ5y+NZkzIscLRN8uh9+LmvV rC5P1iFjJTq9PMYwOq6pLF4EksuZ9zTcv/WO0lE4JKrT+5pwQPfHId9AoRgXB1u1AJ qyx82wFKRb9ICX5wZkT44IM9eVY6IWMXRzsQqGHk= Received: by mail-oi1-f178.google.com with SMTP id j7so7841421oie.12; Fri, 30 Oct 2020 13:09:22 -0700 (PDT) X-Gm-Message-State: AOAM530GXqa8PqEBfmITeCu9GjzdWEpL8aO8zreuvqXoxNBnNaNEBHW2 8qLHqMTSyIhVXeXHzlY2mt7qACZZuqpez+/8ug== X-Received: by 2002:aca:5dc2:: with SMTP id r185mr2849061oib.106.1604088561838; Fri, 30 Oct 2020 13:09:21 -0700 (PDT) MIME-Version: 1.0 References: <1603188889-23664-1-git-send-email-hugues.fruchet@st.com> <1603188889-23664-3-git-send-email-hugues.fruchet@st.com> <20201021130033.GI2703@paasikivi.fi.intel.com> <657634eb-690a-53a6-2ac1-de3c06a1cec4@st.com> <20201021214058.GJ2703@paasikivi.fi.intel.com> <327ae9d5-8683-488f-7970-4983e2fec51d@st.com> <20201026141714.GA83693@bogus> <20201030174236.GV26150@paasikivi.fi.intel.com> In-Reply-To: <20201030174236.GV26150@paasikivi.fi.intel.com> From: Rob Herring Date: Fri, 30 Oct 2020 15:09:10 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 2/2] media: dt-bindings: media: st,stm32-dcmi: Add support of BT656 To: Sakari Ailus Cc: Hugues FRUCHET , Jacopo Mondi , Alexandre TORGUE , Mauro Carvalho Chehab , Hans Verkuil , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-media@vger.kernel.org" , "linux-stm32@st-md-mailman.stormreply.com" , Alain VOLMAT , Yannick FERTRE , Philippe CORNU Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 30, 2020 at 12:42 PM Sakari Ailus wrote: > > Hi Rob, > > On Mon, Oct 26, 2020 at 09:17:14AM -0500, Rob Herring wrote: > > On Thu, Oct 22, 2020 at 02:56:17PM +0000, Hugues FRUCHET wrote: > > > Hi Sakari, > > > > > > + Jacopo for his work on ov772x binding related to BT656 > > > > > > On 10/21/20 11:40 PM, Sakari Ailus wrote: > > > > Hi Hugues, > > > > > > > > On Wed, Oct 21, 2020 at 02:24:08PM +0000, Hugues FRUCHET wrote: > > > >> Hi Sakari, > > > >> > > > >> On 10/21/20 3:00 PM, Sakari Ailus wrote: > > > >>> Hi Hugues, > > > >>> > > > >>> On Tue, Oct 20, 2020 at 12:14:49PM +0200, Hugues Fruchet wrote: > > > >>>> Add support of BT656 parallel bus mode in DCMI. > > > >>>> This mode is enabled when hsync-active & vsync-active > > > >>>> fields are not specified. > > > >>>> > > > >>>> Signed-off-by: Hugues Fruchet > > > >>>> --- > > > >>>> .../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 ++++++++++++++++++++++ > > > >>>> 1 file changed, 30 insertions(+) > > > >>>> > > > >>>> diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml > > > >>>> index 3fe778c..1ee521a 100644 > > > >>>> --- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml > > > >>>> +++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml > > > >>>> @@ -44,6 +44,36 @@ properties: > > > >>>> bindings defined in > > > >>>> Documentation/devicetree/bindings/media/video-interfaces.txt. > > > >>>> > > > >>>> + properties: > > > >>>> + endpoint: > > > >>>> + type: object > > > >>>> + > > > >>>> + properties: > > > >>>> + bus-width: true > > > >>>> + > > > >>>> + hsync-active: > > > >>>> + description: > > > >>>> + If both HSYNC and VSYNC polarities are not specified, BT656 > > > >>>> + embedded synchronization is selected. > > > >>>> + default: 0 > > > >>>> + > > > >>>> + vsync-active: > > > >>>> + description: > > > >>>> + If both HSYNC and VSYNC polarities are not specified, BT656 > > > >>>> + embedded synchronization is selected. > > > >>>> + default: 0 > > > >>> > > > >>> Should I understand this as if the polarities were not specified, BT.656 > > > >>> will be used? > > > >> > > > >> Yes, this is what is documented in video-interfaces.txt: > > > >> " > > > >> Note, that if HSYNC and VSYNC polarities are not specified, embedded > > > >> synchronization may be required, where supported. > > > >> " > > > >> and > > > >> " > > > >> /* If hsync-active/vsync-active are missing, > > > >> embedded BT.656 sync is used */ > > > >> hsync-active = <0>; /* Active low */ > > > >> vsync-active = <0>; /* Active low */ > > > >> " > > > >> and I found also this in > > > >> Documentation/devicetree/bindings/media/renesas,vin.yaml > > > >> " > > > >> hsync-active: > > > >> description: > > > >> If both HSYNC and VSYNC polarities are not specified, > > > >> embedded > > > >> synchronization is selected. > > > >> default: 1 > > > >> > > > >> vsync-active: > > > >> description: > > > >> If both HSYNC and VSYNC polarities are not specified, > > > >> embedded > > > >> synchronization is selected. > > > >> default: 1 > > > > > > > > Having the defaults leads to somewhat weird behaviour: specifying the > > > > default value on either property changes the bus type. > > > > > > > >> " > > > >> > > > >> In the other hand I've found few occurences of "bus-type" > > > >> (marvell,mmp2-ccic.yaml), it is why I asked you if "bus-type" is the new > > > >> way to go versus previous way to signal BT656 (without hsync/vsync) ? > > > >> As explained previously, I prefer this last way for backward compatibility. > > > > > > > > If you have a default for bus-type (BT.601), this won't be a problem. > > > > > > > > The old DT bindings were somewhat, well, opportunistic. The v4l2-of > > > > framework-let did its best and sometimes it worked. The behaviour is still > > > > supported but not encouraged in new bindings. > > > > > > > > > > OK, so let's go for the new way. > > > I've found an interesting patch from Jacopo that is of great help: > > > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20200910162055.614089-4-jacopo+renesas@jmondi.org/ > > I wonder if Jacopo tested it. The idea seems interesting nonetheless. > > > > > > > Here is a draft proposal before I push a new version, please comment: > > > > > > properties: > > > bus-type: > > > enum: [5, 6] > > > default: 5 > > > > > > bus-width: > > > enum: [8, 10, 12, 14] > > > default: 8 > > > > > > hsync-active: > > > enum: [0, 1] > > > > For common properties, you can assume there's a common schema. As 0 and > > 1 are the only possible values, you don't need to define them here > > unless only a subset is valid for this device. > > > > > default: 0 > > > > > > vsync-active: > > > enum: [0, 1] > > > default: 0 > > > > > > pclk-sample: > > > enum: [0, 1] > > > default: 0 > > > > > > remote-endpoint: true > > > > > > allOf: > > > - if: > > > properties: > > > bus-type: > > > const: 6 > > > > To fix the error, you need: > > > > required: > > - bus-type > > > > The problem is the above schema is also true if the property > > is not present. > > Hmm. The idea was that we could keep this consistent with old bindings that > only documented parallel mode, and thus didn't need bus-type. This is > actually quite common --- adding support for something that wasn't known or > cared for during the original review. TBC, the 'required' here is required for the 'if' in the if/then schema to work as you want. It's not making 'bus-type' a required property. Rob