From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 71548C4363D for ; Tue, 22 Sep 2020 14:24:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2E08E2395B for ; Tue, 22 Sep 2020 14:24:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600784682; bh=fyhGiT07YqgY1sX7H5cYuZEWhzgxX1cJqk4Q9y19gjs=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=1WJqkbzsaz5j2CiEjrRc6WBdMlp7Ijsds8oILMQtzRbGgzjj+OWvRw5PXXoEfgvhM jQCaIZB7OMoDNiT3kWGWWnzgzAEek9efPnYsTw+y1qEVdP/ycfP/okOrRG4iqNyz5U HPJzV6yTPnzxOJrhsqRYlJq10zYrITBAJmWtsPlA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726678AbgIVOYi (ORCPT ); Tue, 22 Sep 2020 10:24:38 -0400 Received: from mail.kernel.org ([198.145.29.99]:58218 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726593AbgIVOYd (ORCPT ); Tue, 22 Sep 2020 10:24:33 -0400 Received: from mail-oo1-f44.google.com (mail-oo1-f44.google.com [209.85.161.44]) (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 688C1239D2; Tue, 22 Sep 2020 14:24:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600784672; bh=fyhGiT07YqgY1sX7H5cYuZEWhzgxX1cJqk4Q9y19gjs=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=O4GhqNTLUyIkVNe1g35MNvHUi2N1TQP2CaB0iKDfqgMBCvw2bTHDR9akkw6s3alVj rte5MyJ4m9s6DATOviv0bJ9O4ed08Zub/j+0K/7dA5f1+zeN7EwZE9Ig+vmW+pWk38 mYaaC98M2jlAgvbVJaFDroKt8yB8NkeISVNCFAWw= Received: by mail-oo1-f44.google.com with SMTP id h8so4177572ooc.12; Tue, 22 Sep 2020 07:24:32 -0700 (PDT) X-Gm-Message-State: AOAM53040d8nvgSe7iqqayS7UFQEAeSD//7IaLaMbJWaBYkCmsTCIslM VHIM0Vi5v5NHJ8Y/uo2lUk6/zzOfOhWLWYs4Uw== X-Google-Smtp-Source: ABdhPJzUFEuKoDPplE32fF2OW24xWYgi/9lL9nNZnKQwkQwBHwCVey8YSOKSt3310TZYoa/pm+iYTw1x55KM9sBA3qs= X-Received: by 2002:a4a:b30d:: with SMTP id m13mr3077541ooo.50.1600784671655; Tue, 22 Sep 2020 07:24:31 -0700 (PDT) MIME-Version: 1.0 References: <20200722155533.252844-1-helen.koike@collabora.com> <20200722155533.252844-6-helen.koike@collabora.com> In-Reply-To: <20200722155533.252844-6-helen.koike@collabora.com> From: Rob Herring Date: Tue, 22 Sep 2020 08:24:20 -0600 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v5 5/9] media: staging: rkisp1: remove unecessary clocks To: Helen Koike Cc: devicetree@vger.kernel.org, Linux Media Mailing List , "open list:ARM/Rockchip SoC..." , "linux-kernel@vger.kernel.org" , "open list:STAGING SUBSYSTEM" , "heiko@sntech.de" , Hans Verkuil , Collabora Kernel ML , Dafna Hirschfeld , Ezequiel Garcia , Mark Rutland , karthik.poduval@gmail.com, Johan Jonker , Tomasz Figa , Eddie Cai , Shunqian Zheng , Robin Murphy Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On Wed, Jul 22, 2020 at 9:56 AM Helen Koike wrote: > > aclk_isp_wrap is a child of aclk_isp, and hclk_isp_wrap is a child of > hclk_isp, thus we can remove parents from the list. > > Also, for the isp0, we only need the ISP clock, ACLK and HCLK. > In the future we'll need a pixel clock for RK3288 and RK3399, and a JPEG > clock for RK3288. > > So with the goal to cleanup the dt-bindings and remove it from staging, > simplify clock names to isp, aclk and hclk. > > Assigned clocks are meant to refer to the full path in the clock tree, > i.e. the leaf in the tree. > For instance, in RK3399, the clock responsible for ACLK (ISP AXI CLOCK) > is aclk_isp0_wrapper. > > For reference, this is the isp clock topology on RK3399: > > xin24m > pll_npll > npll > clk_isp1 > clk_isp0 > pll_cpll > cpll > aclk_isp1 > aclk_isp1_noc > hclk_isp1 > aclk_isp1_wrapper > hclk_isp1_noc > aclk_isp0 > hclk_isp1_wrapper > aclk_isp0_wrapper > aclk_isp0_noc > hclk_isp0 > hclk_isp0_wrapper > hclk_isp0_noc > pclkin_isp1_wrapper > > Signed-off-by: Helen Koike > > --- > Changes in V5: > - Use if/then schema as suggested by Rob Herring on > https://patchwork.linuxtv.org/project/linux-media/patch/20200702191322.2639681-6-helen.koike@collabora.com/#119729 > > Changes in V4: > - update binding according to suggestion by Robin Murphy > on https://patchwork.kernel.org/patch/11475007/ > > Changes in V3: > - this is a new patch in the series > --- > .../bindings/media/rockchip-isp1.yaml | 50 ++++++++++++------- > drivers/staging/media/rkisp1/rkisp1-dev.c | 8 ++- > 2 files changed, 36 insertions(+), 22 deletions(-) > > diff --git a/drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.yaml > index 62a6b9c959498..23c677d15037a 100644 > --- a/drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.yaml > +++ b/drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.yaml > @@ -24,20 +24,10 @@ properties: > maxItems: 1 > > clocks: > - items: > - - description: ISP clock > - - description: ISP AXI clock clock > - - description: ISP AXI clock wrapper clock > - - description: ISP AHB clock clock > - - description: ISP AHB wrapper clock > + minItems: 3 You need maxItems here too or it will always be 3. > > clock-names: > - items: > - - const: clk_isp > - - const: aclk_isp > - - const: aclk_isp_wrap > - - const: hclk_isp > - - const: hclk_isp_wrap > + minItems: 3 > > iommus: > maxItems: 1 > @@ -116,6 +106,34 @@ required: > - power-domains > - ports > > +if: > + properties: > + compatible: > + contains: > + const: rockchip,rk3399-cif-isp > +then: > + properties: > + clocks: > + maxItems: 4 > + minItems: 3 For a single compatible you shouldn't really have a variable number of clocks. > + items: > + # isp0 and isp1 > + - description: ISP clock > + - description: ISP AXI clock > + - description: ISP AHB clock > + # only for isp1 > + - description: ISP Pixel clock > + clock-names: > + maxItems: 4 > + minItems: 3 > + items: > + # isp0 and isp1 > + - const: isp > + - const: aclk > + - const: hclk > + # only for isp1 > + - const: pclk_isp Don't you need an 'else' clause. For not rockchip,rk3399-cif-isp, there's no definition of what clocks there are. Rob