From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754452AbdFWSGb (ORCPT ); Fri, 23 Jun 2017 14:06:31 -0400 Received: from fllnx210.ext.ti.com ([198.47.19.17]:50843 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753786AbdFWSG3 (ORCPT ); Fri, 23 Jun 2017 14:06:29 -0400 Subject: Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module To: "H. Nikolaus Schaller" , =?UTF-8?Q?Andreas_F=c3=a4rber?= , Laurent Pinchart CC: Hugues Fruchet , Mark Rutland , devicetree , Benjamin Gaignard , Discussions about the Letux Kernel , Alexandre Torgue , LKML , Hans Verkuil , Rob Herring , Maxime Coquelin , Yannick Fertre , Sylwester Nawrocki , Mauro Carvalho Chehab , Guennadi Liakhovetski , linux-arm-kernel , References: <1498143942-12682-1-git-send-email-hugues.fruchet@st.com> <3E7B1344-ECE6-4CCC-9E9D-7521BB566CDE@goldelico.com> <13144955.Kq5qljPvgI@avalon> <24C976BF-52FD-4509-BCE4-9AE41B335482@goldelico.com> <88d0e8ea-74e4-6845-4e0d-8cd0f3a054be@suse.de> <05CBF9B8-2297-447B-860D-A89126B46FC9@goldelico.com> From: Suman Anna Message-ID: <3f0dcd4f-92ef-a79d-b00b-1f348a201bd2@ti.com> Date: Fri, 23 Jun 2017 13:05:26 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <05CBF9B8-2297-447B-860D-A89126B46FC9@goldelico.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [128.247.58.153] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Nikolaus, On 06/23/2017 10:22 AM, H. Nikolaus Schaller wrote: > Hi, > >> Am 23.06.2017 um 16:57 schrieb Andreas Färber : >> >> Am 23.06.2017 um 16:53 schrieb H. Nikolaus Schaller: >>> Hi Laurent, >>> >>>> Am 23.06.2017 um 13:58 schrieb Laurent Pinchart : >>>> >>>> Hi Nikolaus, >>>> >>>> On Friday 23 Jun 2017 12:59:24 H. Nikolaus Schaller wrote: >>>>> Am 23.06.2017 um 12:46 schrieb Andreas Färber : >>>>>> Am 23.06.2017 um 12:25 schrieb H. Nikolaus Schaller: >>>>>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov965x.txt >>>>>>>> b/Documentation/devicetree/bindings/media/i2c/ov965x.txt new file mode >>>>>>>> 100644 >>>>>>>> index 0000000..0e0de1f >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov965x.txt >>>>>>>> @@ -0,0 +1,37 @@ >>>>>>>> +* Omnivision OV9650/9652/9655 CMOS sensor >>>>>>>> + >>>>>>>> +The Omnivision OV965x sensor support multiple resolutions output, such >>>>>>>> as >>>>>>>> +CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB >>>>>>>> +output format. >>>>>>>> + >>>>>>>> +Required Properties: >>>>>>>> +- compatible: should be one of >>>>>>>> + "ovti,ov9650" >>>>>>>> + "ovti,ov9652" >>>>>>>> + "ovti,ov9655" >>>>>>>> +- clocks: reference to the mclk input clock. >>>>>>> >>>>>>> I wonder why you have removed the clock-frequency property? >>>>>>> >>>>>>> In some situations the camera driver must be able to tell the clock >>>>>>> source which frequency it wants to see. >>>>>> >>>>>> That's what assigned-clock-rates property is for: >>>>>> >>>>>> https://www.kernel.org/doc/Documentation/devicetree/bindings/clock/clock-b >>>>>> indings.txt >>>>>> >>>>>> AFAIU clock-frequency on devices is deprecated and equivalent to having >>>>>> a clocks property pointing to a fixed-clock, which is different from a >>>>>> clock with varying rate. >>>>> >>>>> I am not sure if that helps here. The OMAP3-ISP does not have a fixed clock >>>>> rate so we can only have the driver define what it wants to see. >>>>> >>>>> And common practise for OMAP3-ISP based camera modules (e.g. N900, N9) is >>>>> that they do it in the driver. >>>>> >>>>> Maybe ISP developers can comment? >>>> >>>> The OMAP3 ISP is a variable-frequency clock provider. The clock frequency is >>>> controlled by the clock consumer. As such, it's up to the consumer to decide >>>> whether to compute and request the clock rate dynamically at runtime, or use >>>> the assigned-clock-rates property in DT. >>>> >>>> Some ISPs include a clock generator, others don't. It should make no >>>> difference whether the clock is provided by the ISP, by a dedicated clock >>>> source in the SoC or by a discrete on-board adjustable clock source. >>> >>> Thanks for explaining the background. >>> >>> Do you have an hint or example how to use the assigned-clock-rates property in >>> a DT for a camera module connected to the omap3isp? >>> >>> Or does it just mean that it defines the property name? >> >> Please read the documentation link I sent - it's in the very bottom and >> should have an example. > > I have seen it but it does not give me a good clue how to translate that into > correct omap3isp node setup in a specific DT. Rather it raises more questions. > Maybe because I don't understand completely what it is talking about. > > The fundamental question is if this "assigned-clock-rates" is already > handled by ov965x->clk = devm_clk_get(&client->dev, NULL); ? > > Or should we define that for the omap3isp node? > > Then of course we need no new code and just use the right property names. > And N900, N9 camera DTs should be updated. Look up of_clk_set_defaults() function in drivers/clk/clk-conf.c. This function gets invoked usually during clock registration, and also gets called in platform_drv_probe(), so the parents and clocks do get configured before your driver gets probed. So, this provides a default configuration if these properties are supplied (in either clock nodes or actual device nodes), and if your driver needs to change the rates at runtime, then you would have to do that in the driver itself. regards Suman From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suman Anna Subject: Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Date: Fri, 23 Jun 2017 13:05:26 -0500 Message-ID: <3f0dcd4f-92ef-a79d-b00b-1f348a201bd2@ti.com> References: <1498143942-12682-1-git-send-email-hugues.fruchet@st.com> <3E7B1344-ECE6-4CCC-9E9D-7521BB566CDE@goldelico.com> <13144955.Kq5qljPvgI@avalon> <24C976BF-52FD-4509-BCE4-9AE41B335482@goldelico.com> <88d0e8ea-74e4-6845-4e0d-8cd0f3a054be@suse.de> <05CBF9B8-2297-447B-860D-A89126B46FC9@goldelico.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <05CBF9B8-2297-447B-860D-A89126B46FC9@goldelico.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: "H. Nikolaus Schaller" , =?UTF-8?Q?Andreas_F=c3=a4rber?= , Laurent Pinchart Cc: Mark Rutland , devicetree , Mauro Carvalho Chehab , Alexandre Torgue , LKML , Guennadi Liakhovetski , Hans Verkuil , Rob Herring , linux-arm-kernel , Benjamin Gaignard , Hugues Fruchet , Sylwester Nawrocki , Discussions about the Letux Kernel , Yannick Fertre , Maxime Coquelin , linux-media@vger.kernel.org List-Id: devicetree@vger.kernel.org SGkgTmlrb2xhdXMsCgpPbiAwNi8yMy8yMDE3IDEwOjIyIEFNLCBILiBOaWtvbGF1cyBTY2hhbGxl ciB3cm90ZToKPiBIaSwKPiAKPj4gQW0gMjMuMDYuMjAxNyB1bSAxNjo1NyBzY2hyaWViIEFuZHJl YXMgRsOkcmJlciA8YWZhZXJiZXJAc3VzZS5kZT46Cj4+Cj4+IEFtIDIzLjA2LjIwMTcgdW0gMTY6 NTMgc2NocmllYiBILiBOaWtvbGF1cyBTY2hhbGxlcjoKPj4+IEhpIExhdXJlbnQsCj4+Pgo+Pj4+ IEFtIDIzLjA2LjIwMTcgdW0gMTM6NTggc2NocmllYiBMYXVyZW50IFBpbmNoYXJ0IDxsYXVyZW50 LnBpbmNoYXJ0QGlkZWFzb25ib2FyZC5jb20+Ogo+Pj4+Cj4+Pj4gSGkgTmlrb2xhdXMsCj4+Pj4K Pj4+PiBPbiBGcmlkYXkgMjMgSnVuIDIwMTcgMTI6NTk6MjQgSC4gTmlrb2xhdXMgU2NoYWxsZXIg d3JvdGU6Cj4+Pj4+IEFtIDIzLjA2LjIwMTcgdW0gMTI6NDYgc2NocmllYiBBbmRyZWFzIEbDpHJi ZXIgPGFmYWVyYmVyQHN1c2UuZGU+Ogo+Pj4+Pj4gQW0gMjMuMDYuMjAxNyB1bSAxMjoyNSBzY2hy aWViIEguIE5pa29sYXVzIFNjaGFsbGVyOgo+Pj4+Pj4+PiBkaWZmIC0tZ2l0IGEvRG9jdW1lbnRh dGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL21lZGlhL2kyYy9vdjk2NXgudHh0Cj4+Pj4+Pj4+IGIv RG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL21lZGlhL2kyYy9vdjk2NXgudHh0IG5l dyBmaWxlIG1vZGUKPj4+Pj4+Pj4gMTAwNjQ0Cj4+Pj4+Pj4+IGluZGV4IDAwMDAwMDAuLjBlMGRl MWYKPj4+Pj4+Pj4gLS0tIC9kZXYvbnVsbAo+Pj4+Pj4+PiArKysgYi9Eb2N1bWVudGF0aW9uL2Rl dmljZXRyZWUvYmluZGluZ3MvbWVkaWEvaTJjL292OTY1eC50eHQKPj4+Pj4+Pj4gQEAgLTAsMCAr MSwzNyBAQAo+Pj4+Pj4+PiArKiBPbW5pdmlzaW9uIE9WOTY1MC85NjUyLzk2NTUgQ01PUyBzZW5z b3IKPj4+Pj4+Pj4gKwo+Pj4+Pj4+PiArVGhlIE9tbml2aXNpb24gT1Y5NjV4IHNlbnNvciBzdXBw b3J0IG11bHRpcGxlIHJlc29sdXRpb25zIG91dHB1dCwgc3VjaAo+Pj4+Pj4+PiBhcwo+Pj4+Pj4+ PiArQ0lGLCBTVkdBLCBVWEdBLiBJdCBhbHNvIGNhbiBzdXBwb3J0IFlVVjQyMi80MjAsIFJHQjU2 NS81NTUgb3IgcmF3IFJHQgo+Pj4+Pj4+PiArb3V0cHV0IGZvcm1hdC4KPj4+Pj4+Pj4gKwo+Pj4+ Pj4+PiArUmVxdWlyZWQgUHJvcGVydGllczoKPj4+Pj4+Pj4gKy0gY29tcGF0aWJsZTogc2hvdWxk IGJlIG9uZSBvZgo+Pj4+Pj4+PiArCSJvdnRpLG92OTY1MCIKPj4+Pj4+Pj4gKwkib3Z0aSxvdjk2 NTIiCj4+Pj4+Pj4+ICsJIm92dGksb3Y5NjU1Igo+Pj4+Pj4+PiArLSBjbG9ja3M6IHJlZmVyZW5j ZSB0byB0aGUgbWNsayBpbnB1dCBjbG9jay4KPj4+Pj4+Pgo+Pj4+Pj4+IEkgd29uZGVyIHdoeSB5 b3UgaGF2ZSByZW1vdmVkIHRoZSBjbG9jay1mcmVxdWVuY3kgcHJvcGVydHk/Cj4+Pj4+Pj4KPj4+ Pj4+PiBJbiBzb21lIHNpdHVhdGlvbnMgdGhlIGNhbWVyYSBkcml2ZXIgbXVzdCBiZSBhYmxlIHRv IHRlbGwgdGhlIGNsb2NrCj4+Pj4+Pj4gc291cmNlIHdoaWNoIGZyZXF1ZW5jeSBpdCB3YW50cyB0 byBzZWUuCj4+Pj4+Pgo+Pj4+Pj4gVGhhdCdzIHdoYXQgYXNzaWduZWQtY2xvY2stcmF0ZXMgcHJv cGVydHkgaXMgZm9yOgo+Pj4+Pj4KPj4+Pj4+IGh0dHBzOi8vd3d3Lmtlcm5lbC5vcmcvZG9jL0Rv Y3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9jbG9jay9jbG9jay1iCj4+Pj4+PiBpbmRp bmdzLnR4dAo+Pj4+Pj4KPj4+Pj4+IEFGQUlVIGNsb2NrLWZyZXF1ZW5jeSBvbiBkZXZpY2VzIGlz IGRlcHJlY2F0ZWQgYW5kIGVxdWl2YWxlbnQgdG8gaGF2aW5nCj4+Pj4+PiBhIGNsb2NrcyBwcm9w ZXJ0eSBwb2ludGluZyB0byBhIGZpeGVkLWNsb2NrLCB3aGljaCBpcyBkaWZmZXJlbnQgZnJvbSBh Cj4+Pj4+PiBjbG9jayB3aXRoIHZhcnlpbmcgcmF0ZS4KPj4+Pj4KPj4+Pj4gSSBhbSBub3Qgc3Vy ZSBpZiB0aGF0IGhlbHBzIGhlcmUuIFRoZSBPTUFQMy1JU1AgZG9lcyBub3QgaGF2ZSBhIGZpeGVk IGNsb2NrCj4+Pj4+IHJhdGUgc28gd2UgY2FuIG9ubHkgaGF2ZSB0aGUgZHJpdmVyIGRlZmluZSB3 aGF0IGl0IHdhbnRzIHRvIHNlZS4KPj4+Pj4KPj4+Pj4gQW5kIGNvbW1vbiBwcmFjdGlzZSBmb3Ig T01BUDMtSVNQIGJhc2VkIGNhbWVyYSBtb2R1bGVzIChlLmcuIE45MDAsIE45KSBpcwo+Pj4+PiB0 aGF0IHRoZXkgZG8gaXQgaW4gdGhlIGRyaXZlci4KPj4+Pj4KPj4+Pj4gTWF5YmUgSVNQIGRldmVs b3BlcnMgY2FuIGNvbW1lbnQ/Cj4+Pj4KPj4+PiBUaGUgT01BUDMgSVNQIGlzIGEgdmFyaWFibGUt ZnJlcXVlbmN5IGNsb2NrIHByb3ZpZGVyLiBUaGUgY2xvY2sgZnJlcXVlbmN5IGlzIAo+Pj4+IGNv bnRyb2xsZWQgYnkgdGhlIGNsb2NrIGNvbnN1bWVyLiBBcyBzdWNoLCBpdCdzIHVwIHRvIHRoZSBj b25zdW1lciB0byBkZWNpZGUgCj4+Pj4gd2hldGhlciB0byBjb21wdXRlIGFuZCByZXF1ZXN0IHRo ZSBjbG9jayByYXRlIGR5bmFtaWNhbGx5IGF0IHJ1bnRpbWUsIG9yIHVzZSAKPj4+PiB0aGUgYXNz aWduZWQtY2xvY2stcmF0ZXMgcHJvcGVydHkgaW4gRFQuCj4+Pj4KPj4+PiBTb21lIElTUHMgaW5j bHVkZSBhIGNsb2NrIGdlbmVyYXRvciwgb3RoZXJzIGRvbid0LiBJdCBzaG91bGQgbWFrZSBubyAK Pj4+PiBkaWZmZXJlbmNlIHdoZXRoZXIgdGhlIGNsb2NrIGlzIHByb3ZpZGVkIGJ5IHRoZSBJU1As IGJ5IGEgZGVkaWNhdGVkIGNsb2NrIAo+Pj4+IHNvdXJjZSBpbiB0aGUgU29DIG9yIGJ5IGEgZGlz Y3JldGUgb24tYm9hcmQgYWRqdXN0YWJsZSBjbG9jayBzb3VyY2UuCj4+Pgo+Pj4gVGhhbmtzIGZv ciBleHBsYWluaW5nIHRoZSBiYWNrZ3JvdW5kLgo+Pj4KPj4+IERvIHlvdSBoYXZlIGFuIGhpbnQg b3IgZXhhbXBsZSBob3cgdG8gdXNlIHRoZSBhc3NpZ25lZC1jbG9jay1yYXRlcyBwcm9wZXJ0eSBp bgo+Pj4gYSBEVCBmb3IgYSBjYW1lcmEgbW9kdWxlIGNvbm5lY3RlZCB0byB0aGUgb21hcDNpc3A/ Cj4+Pgo+Pj4gT3IgZG9lcyBpdCBqdXN0IG1lYW4gdGhhdCBpdCBkZWZpbmVzIHRoZSBwcm9wZXJ0 eSBuYW1lPwo+Pgo+PiBQbGVhc2UgcmVhZCB0aGUgZG9jdW1lbnRhdGlvbiBsaW5rIEkgc2VudCAt IGl0J3MgaW4gdGhlIHZlcnkgYm90dG9tIGFuZAo+PiBzaG91bGQgaGF2ZSBhbiBleGFtcGxlLgo+ IAo+IEkgaGF2ZSBzZWVuIGl0IGJ1dCBpdCBkb2VzIG5vdCBnaXZlIG1lIGEgZ29vZCBjbHVlIGhv dyB0byB0cmFuc2xhdGUgdGhhdCBpbnRvCj4gY29ycmVjdCBvbWFwM2lzcCBub2RlIHNldHVwIGlu IGEgc3BlY2lmaWMgRFQuIFJhdGhlciBpdCByYWlzZXMgbW9yZSBxdWVzdGlvbnMuCj4gTWF5YmUg YmVjYXVzZSBJIGRvbid0IHVuZGVyc3RhbmQgY29tcGxldGVseSB3aGF0IGl0IGlzIHRhbGtpbmcg YWJvdXQuCj4gCj4gVGhlIGZ1bmRhbWVudGFsIHF1ZXN0aW9uIGlzIGlmIHRoaXMgImFzc2lnbmVk LWNsb2NrLXJhdGVzIiBpcyBhbHJlYWR5Cj4gaGFuZGxlZCBieSBvdjk2NXgtPmNsayA9IGRldm1f Y2xrX2dldCgmY2xpZW50LT5kZXYsIE5VTEwpOyA/Cj4gCj4gT3Igc2hvdWxkIHdlIGRlZmluZSB0 aGF0IGZvciB0aGUgb21hcDNpc3Agbm9kZT8KPiAKPiBUaGVuIG9mIGNvdXJzZSB3ZSBuZWVkIG5v IG5ldyBjb2RlIGFuZCBqdXN0IHVzZSB0aGUgcmlnaHQgcHJvcGVydHkgbmFtZXMuCj4gQW5kIE45 MDAsIE45IGNhbWVyYSBEVHMgc2hvdWxkIGJlIHVwZGF0ZWQuCgpMb29rIHVwIG9mX2Nsa19zZXRf ZGVmYXVsdHMoKSBmdW5jdGlvbiBpbiBkcml2ZXJzL2Nsay9jbGstY29uZi5jLiBUaGlzCmZ1bmN0 aW9uIGdldHMgaW52b2tlZCB1c3VhbGx5IGR1cmluZyBjbG9jayByZWdpc3RyYXRpb24sIGFuZCBh bHNvIGdldHMKY2FsbGVkIGluIHBsYXRmb3JtX2Rydl9wcm9iZSgpLCBzbyB0aGUgcGFyZW50cyBh bmQgY2xvY2tzIGRvIGdldApjb25maWd1cmVkIGJlZm9yZSB5b3VyIGRyaXZlciBnZXRzIHByb2Jl ZC4gU28sIHRoaXMgcHJvdmlkZXMgYSBkZWZhdWx0CmNvbmZpZ3VyYXRpb24gaWYgdGhlc2UgcHJv cGVydGllcyBhcmUgc3VwcGxpZWQgKGluIGVpdGhlciBjbG9jayBub2RlcyBvcgphY3R1YWwgZGV2 aWNlIG5vZGVzKSwgYW5kIGlmIHlvdXIgZHJpdmVyIG5lZWRzIHRvIGNoYW5nZSB0aGUgcmF0ZXMg YXQKcnVudGltZSwgdGhlbiB5b3Ugd291bGQgaGF2ZSB0byBkbyB0aGF0IGluIHRoZSBkcml2ZXIg aXRzZWxmLgoKcmVnYXJkcwpTdW1hbgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX18KbGludXgtYXJtLWtlcm5lbCBtYWlsaW5nIGxpc3QKbGludXgtYXJtLWtl cm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxt YW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from fllnx210.ext.ti.com ([198.47.19.17]:50843 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753786AbdFWSG3 (ORCPT ); Fri, 23 Jun 2017 14:06:29 -0400 Subject: Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module To: "H. Nikolaus Schaller" , =?UTF-8?Q?Andreas_F=c3=a4rber?= , Laurent Pinchart CC: Hugues Fruchet , Mark Rutland , devicetree , Benjamin Gaignard , Discussions about the Letux Kernel , Alexandre Torgue , LKML , Hans Verkuil , Rob Herring , Maxime Coquelin , Yannick Fertre , Sylwester Nawrocki , Mauro Carvalho Chehab , Guennadi Liakhovetski , linux-arm-kernel , References: <1498143942-12682-1-git-send-email-hugues.fruchet@st.com> <3E7B1344-ECE6-4CCC-9E9D-7521BB566CDE@goldelico.com> <13144955.Kq5qljPvgI@avalon> <24C976BF-52FD-4509-BCE4-9AE41B335482@goldelico.com> <88d0e8ea-74e4-6845-4e0d-8cd0f3a054be@suse.de> <05CBF9B8-2297-447B-860D-A89126B46FC9@goldelico.com> From: Suman Anna Message-ID: <3f0dcd4f-92ef-a79d-b00b-1f348a201bd2@ti.com> Date: Fri, 23 Jun 2017 13:05:26 -0500 MIME-Version: 1.0 In-Reply-To: <05CBF9B8-2297-447B-860D-A89126B46FC9@goldelico.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-media-owner@vger.kernel.org List-ID: Hi Nikolaus, On 06/23/2017 10:22 AM, H. Nikolaus Schaller wrote: > Hi, > >> Am 23.06.2017 um 16:57 schrieb Andreas Färber : >> >> Am 23.06.2017 um 16:53 schrieb H. Nikolaus Schaller: >>> Hi Laurent, >>> >>>> Am 23.06.2017 um 13:58 schrieb Laurent Pinchart : >>>> >>>> Hi Nikolaus, >>>> >>>> On Friday 23 Jun 2017 12:59:24 H. Nikolaus Schaller wrote: >>>>> Am 23.06.2017 um 12:46 schrieb Andreas Färber : >>>>>> Am 23.06.2017 um 12:25 schrieb H. Nikolaus Schaller: >>>>>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov965x.txt >>>>>>>> b/Documentation/devicetree/bindings/media/i2c/ov965x.txt new file mode >>>>>>>> 100644 >>>>>>>> index 0000000..0e0de1f >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov965x.txt >>>>>>>> @@ -0,0 +1,37 @@ >>>>>>>> +* Omnivision OV9650/9652/9655 CMOS sensor >>>>>>>> + >>>>>>>> +The Omnivision OV965x sensor support multiple resolutions output, such >>>>>>>> as >>>>>>>> +CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB >>>>>>>> +output format. >>>>>>>> + >>>>>>>> +Required Properties: >>>>>>>> +- compatible: should be one of >>>>>>>> + "ovti,ov9650" >>>>>>>> + "ovti,ov9652" >>>>>>>> + "ovti,ov9655" >>>>>>>> +- clocks: reference to the mclk input clock. >>>>>>> >>>>>>> I wonder why you have removed the clock-frequency property? >>>>>>> >>>>>>> In some situations the camera driver must be able to tell the clock >>>>>>> source which frequency it wants to see. >>>>>> >>>>>> That's what assigned-clock-rates property is for: >>>>>> >>>>>> https://www.kernel.org/doc/Documentation/devicetree/bindings/clock/clock-b >>>>>> indings.txt >>>>>> >>>>>> AFAIU clock-frequency on devices is deprecated and equivalent to having >>>>>> a clocks property pointing to a fixed-clock, which is different from a >>>>>> clock with varying rate. >>>>> >>>>> I am not sure if that helps here. The OMAP3-ISP does not have a fixed clock >>>>> rate so we can only have the driver define what it wants to see. >>>>> >>>>> And common practise for OMAP3-ISP based camera modules (e.g. N900, N9) is >>>>> that they do it in the driver. >>>>> >>>>> Maybe ISP developers can comment? >>>> >>>> The OMAP3 ISP is a variable-frequency clock provider. The clock frequency is >>>> controlled by the clock consumer. As such, it's up to the consumer to decide >>>> whether to compute and request the clock rate dynamically at runtime, or use >>>> the assigned-clock-rates property in DT. >>>> >>>> Some ISPs include a clock generator, others don't. It should make no >>>> difference whether the clock is provided by the ISP, by a dedicated clock >>>> source in the SoC or by a discrete on-board adjustable clock source. >>> >>> Thanks for explaining the background. >>> >>> Do you have an hint or example how to use the assigned-clock-rates property in >>> a DT for a camera module connected to the omap3isp? >>> >>> Or does it just mean that it defines the property name? >> >> Please read the documentation link I sent - it's in the very bottom and >> should have an example. > > I have seen it but it does not give me a good clue how to translate that into > correct omap3isp node setup in a specific DT. Rather it raises more questions. > Maybe because I don't understand completely what it is talking about. > > The fundamental question is if this "assigned-clock-rates" is already > handled by ov965x->clk = devm_clk_get(&client->dev, NULL); ? > > Or should we define that for the omap3isp node? > > Then of course we need no new code and just use the right property names. > And N900, N9 camera DTs should be updated. Look up of_clk_set_defaults() function in drivers/clk/clk-conf.c. This function gets invoked usually during clock registration, and also gets called in platform_drv_probe(), so the parents and clocks do get configured before your driver gets probed. So, this provides a default configuration if these properties are supplied (in either clock nodes or actual device nodes), and if your driver needs to change the rates at runtime, then you would have to do that in the driver itself. regards Suman From mboxrd@z Thu Jan 1 00:00:00 1970 From: s-anna@ti.com (Suman Anna) Date: Fri, 23 Jun 2017 13:05:26 -0500 Subject: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module In-Reply-To: <05CBF9B8-2297-447B-860D-A89126B46FC9@goldelico.com> References: <1498143942-12682-1-git-send-email-hugues.fruchet@st.com> <3E7B1344-ECE6-4CCC-9E9D-7521BB566CDE@goldelico.com> <13144955.Kq5qljPvgI@avalon> <24C976BF-52FD-4509-BCE4-9AE41B335482@goldelico.com> <88d0e8ea-74e4-6845-4e0d-8cd0f3a054be@suse.de> <05CBF9B8-2297-447B-860D-A89126B46FC9@goldelico.com> Message-ID: <3f0dcd4f-92ef-a79d-b00b-1f348a201bd2@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Nikolaus, On 06/23/2017 10:22 AM, H. Nikolaus Schaller wrote: > Hi, > >> Am 23.06.2017 um 16:57 schrieb Andreas F?rber : >> >> Am 23.06.2017 um 16:53 schrieb H. Nikolaus Schaller: >>> Hi Laurent, >>> >>>> Am 23.06.2017 um 13:58 schrieb Laurent Pinchart : >>>> >>>> Hi Nikolaus, >>>> >>>> On Friday 23 Jun 2017 12:59:24 H. Nikolaus Schaller wrote: >>>>> Am 23.06.2017 um 12:46 schrieb Andreas F?rber : >>>>>> Am 23.06.2017 um 12:25 schrieb H. Nikolaus Schaller: >>>>>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov965x.txt >>>>>>>> b/Documentation/devicetree/bindings/media/i2c/ov965x.txt new file mode >>>>>>>> 100644 >>>>>>>> index 0000000..0e0de1f >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov965x.txt >>>>>>>> @@ -0,0 +1,37 @@ >>>>>>>> +* Omnivision OV9650/9652/9655 CMOS sensor >>>>>>>> + >>>>>>>> +The Omnivision OV965x sensor support multiple resolutions output, such >>>>>>>> as >>>>>>>> +CIF, SVGA, UXGA. It also can support YUV422/420, RGB565/555 or raw RGB >>>>>>>> +output format. >>>>>>>> + >>>>>>>> +Required Properties: >>>>>>>> +- compatible: should be one of >>>>>>>> + "ovti,ov9650" >>>>>>>> + "ovti,ov9652" >>>>>>>> + "ovti,ov9655" >>>>>>>> +- clocks: reference to the mclk input clock. >>>>>>> >>>>>>> I wonder why you have removed the clock-frequency property? >>>>>>> >>>>>>> In some situations the camera driver must be able to tell the clock >>>>>>> source which frequency it wants to see. >>>>>> >>>>>> That's what assigned-clock-rates property is for: >>>>>> >>>>>> https://www.kernel.org/doc/Documentation/devicetree/bindings/clock/clock-b >>>>>> indings.txt >>>>>> >>>>>> AFAIU clock-frequency on devices is deprecated and equivalent to having >>>>>> a clocks property pointing to a fixed-clock, which is different from a >>>>>> clock with varying rate. >>>>> >>>>> I am not sure if that helps here. The OMAP3-ISP does not have a fixed clock >>>>> rate so we can only have the driver define what it wants to see. >>>>> >>>>> And common practise for OMAP3-ISP based camera modules (e.g. N900, N9) is >>>>> that they do it in the driver. >>>>> >>>>> Maybe ISP developers can comment? >>>> >>>> The OMAP3 ISP is a variable-frequency clock provider. The clock frequency is >>>> controlled by the clock consumer. As such, it's up to the consumer to decide >>>> whether to compute and request the clock rate dynamically at runtime, or use >>>> the assigned-clock-rates property in DT. >>>> >>>> Some ISPs include a clock generator, others don't. It should make no >>>> difference whether the clock is provided by the ISP, by a dedicated clock >>>> source in the SoC or by a discrete on-board adjustable clock source. >>> >>> Thanks for explaining the background. >>> >>> Do you have an hint or example how to use the assigned-clock-rates property in >>> a DT for a camera module connected to the omap3isp? >>> >>> Or does it just mean that it defines the property name? >> >> Please read the documentation link I sent - it's in the very bottom and >> should have an example. > > I have seen it but it does not give me a good clue how to translate that into > correct omap3isp node setup in a specific DT. Rather it raises more questions. > Maybe because I don't understand completely what it is talking about. > > The fundamental question is if this "assigned-clock-rates" is already > handled by ov965x->clk = devm_clk_get(&client->dev, NULL); ? > > Or should we define that for the omap3isp node? > > Then of course we need no new code and just use the right property names. > And N900, N9 camera DTs should be updated. Look up of_clk_set_defaults() function in drivers/clk/clk-conf.c. This function gets invoked usually during clock registration, and also gets called in platform_drv_probe(), so the parents and clocks do get configured before your driver gets probed. So, this provides a default configuration if these properties are supplied (in either clock nodes or actual device nodes), and if your driver needs to change the rates at runtime, then you would have to do that in the driver itself. regards Suman