Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
* Informing common clock framework driver of externally-derived base clock frequency
@ 2019-01-04  2:35 ` Jonny Hall
  2019-01-04  9:30   ` Sylwester Nawrocki
  0 siblings, 1 reply; 5+ messages in thread
From: Jonny Hall @ 2019-01-04  2:35 UTC (permalink / raw)
  To: linux-clk

I've been tasked with writing a driver for the Silabs si5342
programmable jitter attenuator / clock multiplier. Looking at existing
common clock framework drivers for similar parts (such as
clk-si5351.c), most of this is relatively straightforward -- it's a
composite device, with multiplexers that can select a parent clock,
and fractional-N dividers that support the set_rate and round_rate
functions. However, in all the examples I've seen, the ultimate parent
/ root clock source is specified in the device tree as a fixed
frequency -- a crystal / oscillator / other known-at-design-time
frequency source. However, in my application, the input frequency of
the si5342 is not known in advance, is derived from an external
source, and can change during runtime. The si5342 will need to be
(re)configured during operation with the frequency of this root clock
source in order to properly lock onto it and generate the correct
output frequencies -- it cannot independently determine the input
frequency, only "locked" / "unlocked" state.  The application software
(other drivers and usermode application code) will know the input
clock rate.  I've though of a couple ways to do this:

-Implement the root clock source as an "imaginary" divider where the
set_rate call actually configures the input dividers of the si5342.
This approach seems to abuse the common clock framework, as I'm not
actually *setting* the rate, I'm *informing* the driver of a rate that
was determined independently.

-Have my si5342 driver create a /dev/si5342 file or something in /sys/
where userspace functions can write the frequency to a file to inform
the driver. This approach seems dirty in that it's using two different
methods (common clock framework and device nodes) to access the same
hardware.

-Something else involving adding an inform_rate function to the common
clock framework to support clock sources derived from external,
variable rate sources -- this sounds beyond my skillset, and
potentially expanding the scope of the common clock framework beyond
what is intended.

Any thoughts on what approach would be the "best" in terms of working,
keeping with the spirit of the common clock framework, and not
breaking things?

Thanks,
  -Jonny

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Informing common clock framework driver of externally-derived base clock frequency
  2019-01-04  2:35 ` Informing common clock framework driver of externally-derived base clock frequency Jonny Hall
@ 2019-01-04  9:30   ` Sylwester Nawrocki
  2019-01-04 21:29     ` Jonny Hall
  0 siblings, 1 reply; 5+ messages in thread
From: Sylwester Nawrocki @ 2019-01-04  9:30 UTC (permalink / raw)
  To: Jonny Hall, linux-clk; +Cc: Stephen Boyd, Michael Turquette

Hi,

Cc: CCF maintainers

On 1/4/19 03:35, Jonny Hall wrote:
[...]

> ... However, in my application, the input frequency of
> the si5342 is not known in advance, is derived from an external
> source, and can change during runtime. The si5342 will need to be
> (re)configured during operation with the frequency of this root clock
> source in order to properly lock onto it and generate the correct
> output frequencies -- it cannot independently determine the input
> frequency, only "locked" / "unlocked" state.  The application software
> (other drivers and usermode application code) will know the input
> clock rate.  I've though of a couple ways to do this:
> 
> -Implement the root clock source as an "imaginary" divider where the
> set_rate call actually configures the input dividers of the si5342.
> This approach seems to abuse the common clock framework, as I'm not
> actually *setting* the rate, I'm *informing* the driver of a rate that
> was determined independently.

It sounds like registering a clk notifier on the si5342 root source clock
might be a way to go, wouldn't that work for you?
For an example you could have a look at drivers/clk/sunxi-ng/ccu_mux.c.

--
Regards,
Sylwester

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Informing common clock framework driver of externally-derived base clock frequency
  2019-01-04  9:30   ` Sylwester Nawrocki
@ 2019-01-04 21:29     ` Jonny Hall
  2019-01-09 14:22       ` Sylwester Nawrocki
  0 siblings, 1 reply; 5+ messages in thread
From: Jonny Hall @ 2019-01-04 21:29 UTC (permalink / raw)
  To: Sylwester Nawrocki; +Cc: linux-clk, Stephen Boyd, Michael Turquette

On Fri, Jan 4, 2019 at 4:30 AM Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>
> Hi,
>
> Cc: CCF maintainers
>
> On 1/4/19 03:35, Jonny Hall wrote:
> [...]
>
> > ... However, in my application, the input frequency of
> > the si5342 is not known in advance, is derived from an external
> > source, and can change during runtime. The si5342 will need to be
> > (re)configured during operation with the frequency of this root clock
> > source in order to properly lock onto it and generate the correct
> > output frequencies -- it cannot independently determine the input
> > frequency, only "locked" / "unlocked" state.  The application software
> > (other drivers and usermode application code) will know the input
> > clock rate.  I've though of a couple ways to do this:
> >
> > -Implement the root clock source as an "imaginary" divider where the
> > set_rate call actually configures the input dividers of the si5342.
> > This approach seems to abuse the common clock framework, as I'm not
> > actually *setting* the rate, I'm *informing* the driver of a rate that
> > was determined independently.
>
> It sounds like registering a clk notifier on the si5342 root source clock
> might be a way to go, wouldn't that work for you?
> For an example you could have a look at drivers/clk/sunxi-ng/ccu_mux.c.
>
> --
> Regards,
> Sylwester

Maybe my understanding is incorrect, but to me it looks like a
notifier (callback registered with clk_notifier_register) is actually
the opposite case -- this method would be used by a clock consumer
that wants to be notified by the common clock framework that a clock
rate has changed (I, as an endpoint, want to know when my clock
source, which is controlled by CCF drivers, changes rate).  The use
case I'm looking for is when a clock chip needs to be notified by
*something else* that something in the system has changed (I, as a
clock chip, need to be informed by other parts of the system what my
input clock rate is).

To add a bit of clarity to the scenario -- in my use case, the si5342
is generating video pixel clocks for video outputs from the system.
The inputs of the si5342 are other video pixel clocks, from video
inputs to the system.  The parts of the system that are aware of the
input clock rates are things like HDMI subsystem drivers and usermode
application code.  I need a way to connect these
"parts-that-are-aware-of-the-clock-rate" to my si5342 driver so it can
properly configure that portion of the si5342 -- preferably within the
common clock framework, because on the output side of the si5342, our
video output drivers already work with the CCF.

It looks like some similar chips use recalc_rate as their function to
configure input dividers -- but there appears to be no way to call
recalc_rate through the clk.h API -- it looks like recalc_rate is only
used for keeping track of things internal to the CCF.

Thanks,
  -Jonny

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Informing common clock framework driver of externally-derived base clock frequency
  2019-01-04 21:29     ` Jonny Hall
@ 2019-01-09 14:22       ` Sylwester Nawrocki
  2019-01-29 22:26         ` Stephen Boyd
  0 siblings, 1 reply; 5+ messages in thread
From: Sylwester Nawrocki @ 2019-01-09 14:22 UTC (permalink / raw)
  To: Jonny Hall; +Cc: linux-clk, Stephen Boyd, Michael Turquette

On 1/4/19 22:29, Jonny Hall wrote:
> On Fri, Jan 4, 2019 at 4:30 AM Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
[...]
>> On 1/4/19 03:35, Jonny Hall wrote:
>> [...]
>>> ... However, in my application, the input frequency of
>>> the si5342 is not known in advance, is derived from an external
>>> source, and can change during runtime. The si5342 will need to be
>>> (re)configured during operation with the frequency of this root clock
>>> source in order to properly lock onto it and generate the correct
>>> output frequencies -- it cannot independently determine the input
>>> frequency, only "locked" / "unlocked" state.  The application software
>>> (other drivers and usermode application code) will know the input
>>> clock rate.  I've though of a couple ways to do this:
>>>
>>> -Implement the root clock source as an "imaginary" divider where the
>>> set_rate call actually configures the input dividers of the si5342.
>>> This approach seems to abuse the common clock framework, as I'm not
>>> actually *setting* the rate, I'm *informing* the driver of a rate that
>>> was determined independently.
>>
>> It sounds like registering a clk notifier on the si5342 root source clock
>> might be a way to go, wouldn't that work for you?
>> For an example you could have a look at drivers/clk/sunxi-ng/ccu_mux.c.

> Maybe my understanding is incorrect, but to me it looks like a
> notifier (callback registered with clk_notifier_register) is actually
> the opposite case -- this method would be used by a clock consumer
> that wants to be notified by the common clock framework that a clock
> rate has changed (I, as an endpoint, want to know when my clock
> source, which is controlled by CCF drivers, changes rate).  The use
> case I'm looking for is when a clock chip needs to be notified by
> *something else* that something in the system has changed (I, as a
> clock chip, need to be informed by other parts of the system what my
> input clock rate is).

How about modelling SI5432 input clocks as clock objects? Don't you do it 
already? These are clocks the SI5432 consumes so would be needed anyway.
 
> To add a bit of clarity to the scenario -- in my use case, the si5342
> is generating video pixel clocks for video outputs from the system.
> The inputs of the si5342 are other video pixel clocks, from video
> inputs to the system.  The parts of the system that are aware of the
> input clock rates are things like HDMI subsystem drivers and usermode
> application code.  I need a way to connect these
> "parts-that-are-aware-of-the-clock-rate" to my si5342 driver so it can
> properly configure that portion of the si5342 -- preferably within the
> common clock framework, because on the output side of the si5342, our
> video output drivers already work with the CCF.

Assuming the HDMI receiver is source of the clock, couldn't the HDMI 
driver just register a clock and modify its rate according to the HW
state changes or user ioctls, etc.? The rate is then propagated through
whole clock tree and any element can get notified about changes.

> It looks like some similar chips use recalc_rate as their function to
> configure input dividers -- but there appears to be no way to call
> recalc_rate through the clk.h API -- it looks like recalc_rate is only
> used for keeping track of things internal to the CCF.
-- 
Regards,
Sylwester

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Informing common clock framework driver of externally-derived base clock frequency
  2019-01-09 14:22       ` Sylwester Nawrocki
@ 2019-01-29 22:26         ` Stephen Boyd
  0 siblings, 0 replies; 5+ messages in thread
From: Stephen Boyd @ 2019-01-29 22:26 UTC (permalink / raw)
  To: Jonny Hall, Sylwester Nawrocki; +Cc: linux-clk, Michael Turquette

Quoting Sylwester Nawrocki (2019-01-09 06:22:05)
> On 1/4/19 22:29, Jonny Hall wrote:
> > On Fri, Jan 4, 2019 at 4:30 AM Sylwester Nawrocki
> > <s.nawrocki@samsung.com> wrote:
> [...]
> >> On 1/4/19 03:35, Jonny Hall wrote:
> >> [...]
> >>> ... However, in my application, the input frequency of
> >>> the si5342 is not known in advance, is derived from an external
> >>> source, and can change during runtime. The si5342 will need to be
> >>> (re)configured during operation with the frequency of this root clock
> >>> source in order to properly lock onto it and generate the correct
> >>> output frequencies -- it cannot independently determine the input
> >>> frequency, only "locked" / "unlocked" state.  The application software
> >>> (other drivers and usermode application code) will know the input
> >>> clock rate.  I've though of a couple ways to do this:
> >>>
> >>> -Implement the root clock source as an "imaginary" divider where the
> >>> set_rate call actually configures the input dividers of the si5342.
> >>> This approach seems to abuse the common clock framework, as I'm not
> >>> actually *setting* the rate, I'm *informing* the driver of a rate that
> >>> was determined independently.
> >>
> >> It sounds like registering a clk notifier on the si5342 root source clock
> >> might be a way to go, wouldn't that work for you?
> >> For an example you could have a look at drivers/clk/sunxi-ng/ccu_mux.c.
> 
> > Maybe my understanding is incorrect, but to me it looks like a
> > notifier (callback registered with clk_notifier_register) is actually
> > the opposite case -- this method would be used by a clock consumer
> > that wants to be notified by the common clock framework that a clock
> > rate has changed (I, as an endpoint, want to know when my clock
> > source, which is controlled by CCF drivers, changes rate).  The use
> > case I'm looking for is when a clock chip needs to be notified by
> > *something else* that something in the system has changed (I, as a
> > clock chip, need to be informed by other parts of the system what my
> > input clock rate is).
> 
> How about modelling SI5432 input clocks as clock objects? Don't you do it 
> already? These are clocks the SI5432 consumes so would be needed anyway.
>  
> > To add a bit of clarity to the scenario -- in my use case, the si5342
> > is generating video pixel clocks for video outputs from the system.
> > The inputs of the si5342 are other video pixel clocks, from video
> > inputs to the system.  The parts of the system that are aware of the
> > input clock rates are things like HDMI subsystem drivers and usermode
> > application code.  I need a way to connect these
> > "parts-that-are-aware-of-the-clock-rate" to my si5342 driver so it can
> > properly configure that portion of the si5342 -- preferably within the
> > common clock framework, because on the output side of the si5342, our
> > video output drivers already work with the CCF.
> 
> Assuming the HDMI receiver is source of the clock, couldn't the HDMI 
> driver just register a clock and modify its rate according to the HW
> state changes or user ioctls, etc.? The rate is then propagated through
> whole clock tree and any element can get notified about changes.
> 

It isn't super clear to me still what the scenario is but let me make
some general statements.

1) Try to avoid using clk notifiers. I'd like to get rid of them so new
users is frowned up, but accepted if necessary.

2) The root clk can be variable, it doesn't have to be fixed.

3) Adding a way for drivers to 'inject' a new rate seems weird too. We
already have a way for that to work, just call clk_set_rate() and then
the rate will be propagated down to all child clks during the set_rate
call.

So maybe that means your imaginary divider is the way to go? I'm not
sure, but I would say that whatever is informed about the external clk
input that's changing rate should be the piece of code that calls
clk_set_rate() on the external clk and informs the rest of the clk
children that the rate has changed. If clk drivers need to do something
else when their clk that uses the external clk changes rate, then they
can hook their clk's .set_rate op (basically implementing their own rate
change hook) and do things to keep providing the same rate desired by
whatever is consuming that clk.

Put another way, calling clk_set_rate() on something near the root of
the tree doesn't mean that downstream users should be put into an
inconsistent state. If that use-case isn't working for you we can look
into fixing it, maybe by using the clk rate locking or clk rate
constraint features.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20190104023702epcas4p4e6b225121db059cb6ed167581c303e92@epcas4p4.samsung.com>
2019-01-04  2:35 ` Informing common clock framework driver of externally-derived base clock frequency Jonny Hall
2019-01-04  9:30   ` Sylwester Nawrocki
2019-01-04 21:29     ` Jonny Hall
2019-01-09 14:22       ` Sylwester Nawrocki
2019-01-29 22:26         ` Stephen Boyd

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org linux-clk@archiver.kernel.org
	public-inbox-index linux-clk


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/ public-inbox