public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: York Sun <yorksun@freescale.com>
To: Michael Turquette <mturquette@linaro.org>,
	andrey <andrey@elphel.com>, Guenter Roeck <linux@roeck-us.net>
Cc: <linux-i2c@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<lee.jones@linaro.org>, <sebastian.hesselbarth@gmail.com>,
	<rabeeh@solid-run.com>
Subject: Re: clock driver
Date: Thu, 28 May 2015 08:24:24 -0700	[thread overview]
Message-ID: <55673328.5050206@freescale.com> (raw)
In-Reply-To: <20150528061121.22384.20782@quantum>



On 05/27/2015 11:11 PM, Michael Turquette wrote:

<snip>

> 
> Hi Andrey,
> 
> I think this is a cool problem to solve. As far as frontier devices go I
> really can't say. Other companies create similar clock generators that
> are programmed at run-time over i2c. And we already have support merged
> for Silicon Labs 5351 and 570 devices:
> 
> 	drivers/clk/clk-si5351.c
> 	drivers/clk/clk-si5351.h
> 	drivers/clk/clk-si570.c
> 
> I'm not sure that we need to group such devices into a new "class". The
> Linux common clock framework (ccf) has two classes: clock providers and
> clock consumers. We haven't needed any more classification than that
> thus far.
> 
> I took a look at your github code and it looks like you expose registers
> individually as sysfs files. That is a start, but a better abstraction
> is to consider the clock input signals that your pcie/fpga device will
> take as input. With a proper clock driver for the silabs part your
> pcie/fpga driver could hopefully just call clk_prepare_enable and
> clk_set_rate and clk_set_parent as needed on those clocks to configure
> them for consumption by the downstream fpga.
> 
> According to the data sheet[0] it looks like there are not many clock
> outputs (CLK0{A,B}, CLK1{A,B}, CLK2{A,B}, and CLK3{A,B}).
> 
> At what point do you know how the clocks should be configured? I am
> guessing that your fpga driver (e.g. the clock consumer) figures that
> out as bytestream blobs are loaded? So that seems like the right call
> site to start enabling clocks and configuring rates, instead of stuffing
> that into the silabs driver (e.g. the clock provider).

I think it works for my case. I will explore this direction.

> 
> York,
> 
> There is already a way to clock drivers to extend the debugfs interfaces
> for per-driver stuff. The Nvidia Tegra guys do this already in their
> out-of-tree test modules. Do you think such an interface might be
> helpful to you? See:
> 
> clk_debugfs_add_file in drivers/clk/clk.c
> and,
> http://lkml.kernel.org/r/<1403794853-16928-1-git-send-email-pdeschrijver@nvidia.com>
> 
> So your silabs clock generator driver could export some custom knobs
> which help out in the early phases of development.
> 
> Ideally this interface would not be a register-level interface, but an
> output signal type interface. Here is an example of the files you will
> have by default:
> 
> $ ls /sys/kernel/debug/clk/clk0a/
> clk_accuracy      clk_flags           clk_phase          clk_rate
> clk_enable_count  clk_notifier_count  clk_prepare_count
> 
> With the custom debugfs interface you might add a "clk_set_rate" file
> where user space can write to it and change the frequency of that clock
> signal. You can do the same to change mux settings and clock gates as
> well.
> 
> A userspace tool might even be able to take the clockbuilder data to do
> this, if someone is willing to write it.
> 
> [0] https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5338.pdf

Thanks for the suggestion. I think I can limit the features of this clock chip
and fit it into CCF. Earlier I thought too much about exposing all features for
general use, which shouldn't be the case for the clocks. I will see if I can
remove those features or reserve them for debug use.

York




  reply	other threads:[~2015-05-28 15:24 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-26 19:12 clock driver York Sun
2015-05-26 22:38 ` Guenter Roeck
2015-05-27  0:20   ` York Sun
2015-05-27  0:32     ` York Sun
2015-05-27  7:09       ` Sebastian Hesselbarth
2015-05-27 15:07         ` York Sun
2015-05-27 16:42           ` Sebastian Hesselbarth
2015-05-27 16:46             ` York Sun
2015-05-27 17:09               ` Guenter Roeck
2015-05-27 17:10                 ` York Sun
2015-05-27 17:30       ` Michael Turquette
2015-05-27 17:45         ` York Sun
2015-05-27 18:15           ` Guenter Roeck
2015-05-27 18:24             ` York Sun
2015-05-27 18:54               ` Guenter Roeck
2015-05-27 18:58                 ` York Sun
     [not found]                   ` <14d96cd6d64.f62a1a09739217.9114963256886461171@elphel.com>
2015-05-27 23:08                     ` Guenter Roeck
2015-05-27 23:58                       ` andrey
2015-05-28  0:10                         ` Guenter Roeck
2015-05-28  0:29                           ` andrey
2015-05-28  6:11                             ` Michael Turquette
2015-05-28 15:24                               ` York Sun [this message]
2015-06-10  0:40         ` York Sun

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=55673328.5050206@freescale.com \
    --to=yorksun@freescale.com \
    --cc=andrey@elphel.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mturquette@linaro.org \
    --cc=rabeeh@solid-run.com \
    --cc=sebastian.hesselbarth@gmail.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