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=-7.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 40C8EC433E7 for ; Tue, 20 Oct 2020 12:27:46 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CAA0E22265 for ; Tue, 20 Oct 2020 12:27:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="MdZVGf/c" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CAA0E22265 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=EZHfX6na5FClg5xqm47Mv302jld5IqjbW5DgysmF7/M=; b=MdZVGf/chh2xOlfdh/Y0hnWXo liZwCKd9asL1SIjX9GQA/qKV4iu/gmwj37PIT7HHFOtrv7bNFZQKiYnSc8EAVTBiPhn9XIX+haqOo 9b6mGbyNCTD493IqJctIglL0C48qjawXXSslZOiqb0LaWNI36Gqy/xbzEeu+C4TbdkeH5R6zXmAW+ P/i/Qj2ceuH/bOM4Ssqje5hCtRBcbJR0fS4sEpNao0pBhbhDLBwD9sy8h4ismWayHWBOV+B7e/rRm DjZ3B5U2RM3jGkE1vL96PTNEJp3OgqYklzudSCy0ta883cmLS5R1UnvoLQzLOevt+XE4ItAe80CE6 VtZ6S6XTA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kUqij-0005O3-A7; Tue, 20 Oct 2020 12:26:29 +0000 Received: from mail-wr1-f66.google.com ([209.85.221.66]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kUqig-0005NJ-NK for linux-arm-kernel@lists.infradead.org; Tue, 20 Oct 2020 12:26:27 +0000 Received: by mail-wr1-f66.google.com with SMTP id n6so1877297wrm.13 for ; Tue, 20 Oct 2020 05:26:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=m0pVRuj0M2nZqf8Wm9BAPNQzfIxN4rcRXkQ0qVAU77o=; b=dhTxkjSNh2yfBie4UpeL+UNm3zjNRrXdvwWjKj9V4tAsHsaeHshRX4iVJ5EXL+XQjZ vjGp8kHBTI4R6JI39ZgL5FZXM21F8Fio1q+LtuybMAGnVbQKdRa3PtHzLom56nIzkWGe pmOl+QhWAs7pc8H63M6bAAjJqA/61LRMgb09YuXP0lsa/VvqZsOuA0EUWc5WN0WkncPB SzPO0BK6O6F031a1MO50D+qGC7MM4zn44TjPqahQ17cG/bo5StwrzCe57T8JP2GWd9u2 KXRY8nqP1jLivuVW9J6PIOkPgzv4hXgBI8vmxaHXaAxTYi/BDrU1vT69w1w5Kqu7CUDO VoIA== X-Gm-Message-State: AOAM533ejWHuYpW3mov4HtckYhmrkCBlV7GT7v6KNVOOw5x20EcnNMOX jgMTQRYWJsKzeIR89QI6+WA= X-Google-Smtp-Source: ABdhPJzJ4IfSkdpsol8AyRt80yXAXWp/BGDLmqEcL3uejEZmveMHiGjUKxbbGboz4mTyukzsYyU83Q== X-Received: by 2002:adf:bb43:: with SMTP id x3mr3235381wrg.250.1603196785071; Tue, 20 Oct 2020 05:26:25 -0700 (PDT) Received: from kozik-lap ([194.230.155.171]) by smtp.googlemail.com with ESMTPSA id 67sm2492253wmb.31.2020.10.20.05.26.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Oct 2020 05:26:23 -0700 (PDT) Date: Tue, 20 Oct 2020 14:26:21 +0200 From: Krzysztof Kozlowski To: Sakari Ailus Subject: Re: [PATCH v5 1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor Message-ID: <20201020122621.GA126891@kozik-lap> References: <20201019170247.92002-1-krzk@kernel.org> <20201020103833.GT13341@paasikivi.fi.intel.com> <20201020120058.GU13341@paasikivi.fi.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201020120058.GU13341@paasikivi.fi.intel.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201020_082626_767386_4506BF24 X-CRM114-Status: GOOD ( 41.69 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, Rob Herring , Stephen Boyd , Shawn Guo , Sascha Hauer , "linux-kernel@vger.kernel.org" , linux-clk@vger.kernel.org, Rob Herring , NXP Linux Team , Pengutronix Kernel Team , Mauro Carvalho Chehab , Fabio Estevam , Michael Turquette , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Oct 20, 2020 at 03:00:58PM +0300, Sakari Ailus wrote: > Hi Krzysztof, > > On Tue, Oct 20, 2020 at 12:54:09PM +0200, Krzysztof Kozlowski wrote: > > On Tue, 20 Oct 2020 at 12:38, Sakari Ailus wrote: > > > > > > Hi Krzysztof, > > > > > > On Mon, Oct 19, 2020 at 07:02:44PM +0200, Krzysztof Kozlowski wrote: > > > > Add bindings for the IMX258 camera sensor. The bindings, just like the > > > > driver, are quite limited, e.g. do not support regulator supplies. > > > > > > > > Signed-off-by: Krzysztof Kozlowski > > > > Reviewed-by: Rob Herring > > > > > > > > --- > > > > > > > > Changes since v4: > > > > 1. Add clock-lanes, > > > > 2. Add Rob's review, > > > > 3. Add one more example and extend existing one, > > > > 4. Add common clock properties (assigned-*). > > > > > > Using the assigned-* clock properties may be workable for this driver at > > > the moment. But using these properties does not guarantee the external > > > clock frequency intended to be used on the hardware. > > > > It guarantees it. The clock frequency will be as expected (except if > > someone misconfigures the DTS). > > Is that guaranteed? > > I'm not saying no to the approach, but if we change how camera sensor DT > bindings are defined, I'd prefer an informed decision is made on the > matter. > > > > > > Using other > > > frequencies *is not* expected to work. That applies to this driver as well. > > > > This is the binding which is HW description. According to HW datasheet > > other frequencies from described range are accepted and expected to > > work. > > As per datasheet, yes, different external clock frequencies can be used. > But the link frequency is still not independent of the external clock > frequency. > > The properties of the sensor's PLL tree determines what can be achieved > given a certain external clock frequency. So picking a wrong external clock > frequency quite possibly means that none of the designated link frequencies > are available, rendering the sensor inoperable. The driver then controls the HW and knows exactly what is needed. If link frequency (which has its own DT property) requires some clock frequency, the driver will configure the clock to that value. The same going other direction. Driver has the knowledge about both its input clock and link frequency, therefore it can make the best decision. If someone configures DT or the driver to wrong frequencies (making the link frequencies not available), the solution is not to add more properties. It would be a band aid. The solution is to configure it correctly. > > > > > > This, instead of the clock-frequency property, effectively removes the > > > ability to set the correct frequency from the driver, at least with current > > > set of the used APIs. > > > > It seems you confuse DT bindings with some specific driver > > implementation. Bindings do not describe the driver behavior but the > > HW. The ability to set the correct frequency from the driver is not > > removed. It was never part of the bindings and never should. It is > > part of the driver. > > > > > > > > I suppose you could add a function to set the assigned clock frequency and > > > keep it, just as clk_set_rate_exclusive does? I did not reply to this comment, so let me know. Of course, one could add such functions. It's not a job for DT bindings, though. > > > > > > Cc the common clock framework list + maintainers. > > > > The bindings have Rob review which is the DT maintainer. His > > ack/review is needed for the bindings to be accepted. What more do you > > need? Shall I point to submitting-bindings document? > > > > I am really tired of discussing this. You raise some concerns about > > driver behavior in the wrong context - in the patch for device tree > > bindings. You use the arguments about the driver while we talk about > > bindings. This is clearly not correct. I am all the time repeating > > myself - the bindings describe the hardware, not the driver. > > My concerns are not related to the current driver implementation nor the > driver patches in this set. Yet you refer to driver related tasks (configuring provided clock) while discussing the bindings. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel