All of lore.kernel.org
 help / color / mirror / Atom feed
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clk: si570: Add a driver for SI570 oscillators
Date: Mon, 16 Sep 2013 12:37:43 -0600	[thread overview]
Message-ID: <52374FF7.8040604@wwwdotorg.org> (raw)
In-Reply-To: <20130916173540.GA3481@roeck-us.net>

On 09/16/2013 11:35 AM, Guenter Roeck wrote:
> On Mon, Sep 16, 2013 at 10:59:58AM -0600, Stephen Warren wrote:
>> On 09/16/2013 10:49 AM, Guenter Roeck wrote:
>>> On Mon, Sep 16, 2013 at 10:34:28AM -0600, Stephen Warren wrote:
>>>> On 09/12/2013 06:55 PM, Soren Brinkmann wrote:
>>>>> Add a driver for SILabs 570, 571, 598, 599 programmable oscillators.
>>>>> The devices generate low-jitter clock signals and are reprogrammable via
>>>>> an I2C interface.
...
>>>>> +Optional properties:
>>>>> + - initial-fout: Initial output frequency to set during probe
>>>>
>>>> "probe" is a Linux-specific concept. This property should be removed. If
>>>> the driver is asked to set a specific frequency, it should do so, but I
>>>> don't think it should program something pro-actively just because it
>>>> starts up.
>>>>
>>>> If this property is acceptable, it'd be better to describe it more along
>>>> the lines of the following:
>>>>
>>>> initial-fout: The frequency at which the system requires the clock to
>>>> operate.
>>>
>>> It should probably be something like "clock-frequency". In many use cases
>>> the programmed frequency is set to a constant frequency at system startup
>>> and never changed, similar to other clocks.
>>
>> I was going to suggest that too, but re-considered since I think
>> clock-frequency is more appropriate for fixed-frequency clocks, rather
>> than to specify the value at which a programmable clock generator should
>> operate?
>>
>> I don't think we have a good story yet for how to represent
>> how-we-want-the-clock-tree-configured, as opposed to representing the HW
>> itself (which is what DT should be more about).
>
> In many cases the chip _is_ used to generate a fixed frequency, so we will
> have to have a means to describe it. That it _can_ be used differently is a
> different matter. After all, that is true for many clock generators.

Perhaps if clock-frequency is specified, the driver should refuse to
provide anything else. If clock-frequency isn't specified, the driver
shouldn't touch the HW when it initializes, but should honor any
requests that come in from other drivers? That would maintain what I
feel is clock-frequency's connection to being a fixed clock.

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
Cc: Soren Brinkmann
	<soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>,
	Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	Rob Landley <rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org>,
	Mike Turquette
	<mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Grant Likely
	<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Hyun Kwon <hyunk-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH] clk: si570: Add a driver for SI570 oscillators
Date: Mon, 16 Sep 2013 12:37:43 -0600	[thread overview]
Message-ID: <52374FF7.8040604@wwwdotorg.org> (raw)
In-Reply-To: <20130916173540.GA3481-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

On 09/16/2013 11:35 AM, Guenter Roeck wrote:
> On Mon, Sep 16, 2013 at 10:59:58AM -0600, Stephen Warren wrote:
>> On 09/16/2013 10:49 AM, Guenter Roeck wrote:
>>> On Mon, Sep 16, 2013 at 10:34:28AM -0600, Stephen Warren wrote:
>>>> On 09/12/2013 06:55 PM, Soren Brinkmann wrote:
>>>>> Add a driver for SILabs 570, 571, 598, 599 programmable oscillators.
>>>>> The devices generate low-jitter clock signals and are reprogrammable via
>>>>> an I2C interface.
...
>>>>> +Optional properties:
>>>>> + - initial-fout: Initial output frequency to set during probe
>>>>
>>>> "probe" is a Linux-specific concept. This property should be removed. If
>>>> the driver is asked to set a specific frequency, it should do so, but I
>>>> don't think it should program something pro-actively just because it
>>>> starts up.
>>>>
>>>> If this property is acceptable, it'd be better to describe it more along
>>>> the lines of the following:
>>>>
>>>> initial-fout: The frequency at which the system requires the clock to
>>>> operate.
>>>
>>> It should probably be something like "clock-frequency". In many use cases
>>> the programmed frequency is set to a constant frequency at system startup
>>> and never changed, similar to other clocks.
>>
>> I was going to suggest that too, but re-considered since I think
>> clock-frequency is more appropriate for fixed-frequency clocks, rather
>> than to specify the value at which a programmable clock generator should
>> operate?
>>
>> I don't think we have a good story yet for how to represent
>> how-we-want-the-clock-tree-configured, as opposed to representing the HW
>> itself (which is what DT should be more about).
>
> In many cases the chip _is_ used to generate a fixed frequency, so we will
> have to have a means to describe it. That it _can_ be used differently is a
> different matter. After all, that is true for many clock generators.

Perhaps if clock-frequency is specified, the driver should refuse to
provide anything else. If clock-frequency isn't specified, the driver
shouldn't touch the HW when it initializes, but should honor any
requests that come in from other drivers? That would maintain what I
feel is clock-frequency's connection to being a fixed clock.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Soren Brinkmann <soren.brinkmann@xilinx.com>,
	Rob Herring <rob.herring@calxeda.com>,
	Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Rob Landley <rob@landley.net>,
	Mike Turquette <mturquette@linaro.org>,
	Grant Likely <grant.likely@linaro.org>,
	devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Hyun Kwon <hyunk@xilinx.com>
Subject: Re: [PATCH] clk: si570: Add a driver for SI570 oscillators
Date: Mon, 16 Sep 2013 12:37:43 -0600	[thread overview]
Message-ID: <52374FF7.8040604@wwwdotorg.org> (raw)
In-Reply-To: <20130916173540.GA3481@roeck-us.net>

On 09/16/2013 11:35 AM, Guenter Roeck wrote:
> On Mon, Sep 16, 2013 at 10:59:58AM -0600, Stephen Warren wrote:
>> On 09/16/2013 10:49 AM, Guenter Roeck wrote:
>>> On Mon, Sep 16, 2013 at 10:34:28AM -0600, Stephen Warren wrote:
>>>> On 09/12/2013 06:55 PM, Soren Brinkmann wrote:
>>>>> Add a driver for SILabs 570, 571, 598, 599 programmable oscillators.
>>>>> The devices generate low-jitter clock signals and are reprogrammable via
>>>>> an I2C interface.
...
>>>>> +Optional properties:
>>>>> + - initial-fout: Initial output frequency to set during probe
>>>>
>>>> "probe" is a Linux-specific concept. This property should be removed. If
>>>> the driver is asked to set a specific frequency, it should do so, but I
>>>> don't think it should program something pro-actively just because it
>>>> starts up.
>>>>
>>>> If this property is acceptable, it'd be better to describe it more along
>>>> the lines of the following:
>>>>
>>>> initial-fout: The frequency at which the system requires the clock to
>>>> operate.
>>>
>>> It should probably be something like "clock-frequency". In many use cases
>>> the programmed frequency is set to a constant frequency at system startup
>>> and never changed, similar to other clocks.
>>
>> I was going to suggest that too, but re-considered since I think
>> clock-frequency is more appropriate for fixed-frequency clocks, rather
>> than to specify the value at which a programmable clock generator should
>> operate?
>>
>> I don't think we have a good story yet for how to represent
>> how-we-want-the-clock-tree-configured, as opposed to representing the HW
>> itself (which is what DT should be more about).
>
> In many cases the chip _is_ used to generate a fixed frequency, so we will
> have to have a means to describe it. That it _can_ be used differently is a
> different matter. After all, that is true for many clock generators.

Perhaps if clock-frequency is specified, the driver should refuse to
provide anything else. If clock-frequency isn't specified, the driver
shouldn't touch the HW when it initializes, but should honor any
requests that come in from other drivers? That would maintain what I
feel is clock-frequency's connection to being a fixed clock.

  reply	other threads:[~2013-09-16 18:37 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-13  0:55 [PATCH] SI570 clock driver Soren Brinkmann
2013-09-13  0:55 ` Soren Brinkmann
2013-09-13  0:55 ` [PATCH] clk: si570: Add a driver for SI570 oscillators Soren Brinkmann
2013-09-13  0:55   ` Soren Brinkmann
2013-09-13  0:55   ` Soren Brinkmann
2013-09-13 17:00   ` Guenter Roeck
2013-09-13 17:00     ` Guenter Roeck
2013-09-13 17:26     ` Sören Brinkmann
2013-09-13 17:26       ` Sören Brinkmann
2013-09-13 17:26       ` Sören Brinkmann
2013-09-13 19:48       ` Guenter Roeck
2013-09-13 19:48         ` Guenter Roeck
2013-09-13 21:05         ` Sören Brinkmann
2013-09-13 21:05           ` Sören Brinkmann
2013-09-13 21:05           ` Sören Brinkmann
2013-09-13 21:14           ` Guenter Roeck
2013-09-13 21:14             ` Guenter Roeck
2013-09-14  8:01       ` Sebastian Hesselbarth
2013-09-14  8:01         ` Sebastian Hesselbarth
2013-09-14  8:01         ` Sebastian Hesselbarth
2013-09-16 16:34   ` Stephen Warren
2013-09-16 16:34     ` Stephen Warren
2013-09-16 16:49     ` Guenter Roeck
2013-09-16 16:49       ` Guenter Roeck
2013-09-16 16:59       ` Stephen Warren
2013-09-16 16:59         ` Stephen Warren
2013-09-16 16:59         ` Stephen Warren
2013-09-16 17:35         ` Guenter Roeck
2013-09-16 17:35           ` Guenter Roeck
2013-09-16 17:35           ` Guenter Roeck
2013-09-16 18:37           ` Stephen Warren [this message]
2013-09-16 18:37             ` Stephen Warren
2013-09-16 18:37             ` Stephen Warren
2013-09-16 19:14             ` Guenter Roeck
2013-09-16 19:14               ` Guenter Roeck
2013-09-17  7:59             ` Sebastian Hesselbarth
2013-09-17  7:59               ` Sebastian Hesselbarth
2013-09-17 13:08               ` Guenter Roeck
2013-09-17 13:08                 ` Guenter Roeck
2013-09-17 16:14               ` Stephen Warren
2013-09-17 16:14                 ` Stephen Warren
2013-09-18 20:41                 ` Sören Brinkmann
2013-09-18 20:41                   ` Sören Brinkmann
2013-09-18 20:41                   ` Sören Brinkmann

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=52374FF7.8040604@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.