linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Suman Anna <s-anna@ti.com>
To: "H. Nikolaus Schaller" <hns@goldelico.com>,
	"Andreas Färber" <afaerber@suse.de>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>
Cc: Hugues Fruchet <hugues.fruchet@st.com>,
	Mark Rutland <mark.rutland@arm.com>,
	devicetree <devicetree@vger.kernel.org>,
	Benjamin Gaignard <benjamin.gaignard@linaro.org>,
	Discussions about the Letux Kernel 
	<letux-kernel@openphoenux.org>,
	Alexandre Torgue <alexandre.torgue@st.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	Rob Herring <robh+dt@kernel.org>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Yannick Fertre <yannick.fertre@st.com>,
	Sylwester Nawrocki <sylvester.nawrocki@gmail.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Guennadi Liakhovetski <g.liakhovetski@gmx.de>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	<linux-media@vger.kernel.org>
Subject: Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module
Date: Fri, 23 Jun 2017 13:05:26 -0500	[thread overview]
Message-ID: <3f0dcd4f-92ef-a79d-b00b-1f348a201bd2@ti.com> (raw)
In-Reply-To: <05CBF9B8-2297-447B-860D-A89126B46FC9@goldelico.com>

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 <afaerber@suse.de>:
>>
>> Am 23.06.2017 um 16:53 schrieb H. Nikolaus Schaller:
>>> Hi Laurent,
>>>
>>>> Am 23.06.2017 um 13:58 schrieb Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
>>>>
>>>> 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 <afaerber@suse.de>:
>>>>>> 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

  reply	other threads:[~2017-06-23 18:06 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-22 15:05 [PATCH v1 0/6] Add support of OV9655 camera Hugues Fruchet
2017-06-22 15:05 ` [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module Hugues Fruchet
2017-06-23 10:25   ` H. Nikolaus Schaller
2017-06-23 10:46     ` Andreas Färber
2017-06-23 10:59       ` H. Nikolaus Schaller
2017-06-23 11:58         ` Laurent Pinchart
2017-06-23 14:53           ` H. Nikolaus Schaller
2017-06-23 14:57             ` Andreas Färber
2017-06-23 15:22               ` H. Nikolaus Schaller
2017-06-23 18:05                 ` Suman Anna [this message]
2017-06-23 18:59                   ` H. Nikolaus Schaller
2017-06-23 22:24                     ` Suman Anna
2017-06-26  6:00                       ` H. Nikolaus Schaller
2017-06-26 10:35     ` Hugues FRUCHET
2017-06-26 20:04       ` Sylwester Nawrocki
2017-06-27  5:48         ` H. Nikolaus Schaller
2017-06-27 22:57           ` Sylwester Nawrocki
2017-06-28  9:12             ` H. Nikolaus Schaller
2017-06-28 10:50               ` Sylwester Nawrocki
2017-06-28 11:24                 ` H. Nikolaus Schaller
2017-06-28 12:28                   ` Hugues FRUCHET
2017-06-26 18:56     ` Rob Herring
2017-06-26 18:54   ` Rob Herring
2017-06-22 15:05 ` [PATCH v1 2/6] [media] ov9650: add device tree support Hugues Fruchet
2017-06-26 16:31   ` Sakari Ailus
2017-06-26 17:46     ` H. Nikolaus Schaller
2017-06-27  5:36       ` Sakari Ailus
2017-06-27 10:14         ` Hugues FRUCHET
2017-06-22 15:05 ` [PATCH v1 3/6] [media] ov9650: select the nearest higher resolution Hugues Fruchet
2017-06-22 15:05 ` [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences Hugues Fruchet
2017-06-26 16:33   ` Sakari Ailus
2017-06-29 13:59     ` Hugues FRUCHET
2017-06-22 15:05 ` [PATCH v1 5/6] [media] ov9650: add multiple variant support Hugues Fruchet
2017-06-25 12:36   ` kbuild test robot
2017-06-22 15:05 ` [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant Hugues Fruchet
2017-06-25 16:07   ` [PATCH] ov9650: fix semicolon.cocci warnings kbuild test robot
2017-06-25 16:07   ` [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant kbuild test robot
2017-06-26  6:03   ` H. Nikolaus Schaller
2017-06-26 11:49     ` Hugues FRUCHET
2017-06-22 15:41 ` [PATCH v1 0/6] Add support of OV9655 camera H. Nikolaus Schaller
2017-06-23 10:25   ` H. Nikolaus Schaller
2017-06-25  9:18     ` omap3isp camera was " Pavel Machek
2017-06-26  6:05       ` H. Nikolaus Schaller
2017-06-26  8:39         ` Pavel Machek
2017-06-26  9:53           ` H. Nikolaus Schaller
2017-06-26 11:16             ` Pavel Machek
2017-06-27  5:49               ` H. Nikolaus Schaller
2017-06-26 13:19           ` Hugues FRUCHET
     [not found]             ` <E4E1E683-9CD0-4F23-AB03-080C52D0D73B@goldelico.com>
2017-06-27  7:57               ` Hugues FRUCHET
     [not found]                 ` <95B10899-9966-4844-9667-D2434968A492@goldelico.com>
     [not found]                   ` <85e62c47-e319-c801-376c-95f4d9cbf75a@st.com>
     [not found]                     ` <E183E921-B8F4-4F8A-A302-58297A874990@goldelico.com>
     [not found]                       ` <77bd3b8fe52643d68ecc821ec22bc0e6@SFHDAG5NODE1.st.com>
2017-07-01 21:00                         ` H. Nikolaus Schaller
2017-07-03  8:16                           ` Hugues FRUCHET
2017-07-03  9:14                             ` H. Nikolaus Schaller
2017-07-03 12:03                               ` Hugues FRUCHET
2017-07-03 12:23                                 ` H. Nikolaus Schaller
2017-07-05 14:02                                   ` H. Nikolaus Schaller
2017-07-08 20:55                                     ` Sakari Ailus
2017-06-26  6:02     ` H. Nikolaus Schaller
2017-06-26 10:05   ` Hugues FRUCHET

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3f0dcd4f-92ef-a79d-b00b-1f348a201bd2@ti.com \
    --to=s-anna@ti.com \
    --cc=afaerber@suse.de \
    --cc=alexandre.torgue@st.com \
    --cc=benjamin.gaignard@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=g.liakhovetski@gmx.de \
    --cc=hns@goldelico.com \
    --cc=hugues.fruchet@st.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=letux-kernel@openphoenux.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mchehab@kernel.org \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=sylvester.nawrocki@gmail.com \
    --cc=yannick.fertre@st.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).