Date:	Wed, 27 May 2015 18:42:54 +0200
From:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
To:	York Sun <yorksun@...escale.com>,
	Guenter Roeck <linux@...ck-us.net>, mturquette@...aro.org
CC:	linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
	lee.jones@...aro.org, andrey@...hel.com, rabeeh@...id-run.com
Subject: Re: clock driver

On 27.05.2015 17:07, York Sun wrote:
> On 05/27/2015 12:09 AM, Sebastian Hesselbarth wrote:
[...]
>> Also, why should a user ever be able to mess with the clocks? If you
>> allow a user to change the clock rate of any output, you have to
>> consider that he will likely be able to crash your system easily.
>>
>> As long as you cannot give a clear requirement for user-configurable
>> clocks - especially in the detail of the driver you mentioned -
>> mainline kernel is not the place for such a driver.
>
> This driver I am proposing supports SI5338 in a generic way. It can take device
> tree as its default configuration. However I am using it differently, explained
> in detail below.
[...]
>> (a) Clocks are limited to the PCI card and only need a limited set of
>> configurable clocks. You should add functions to load the registers
>> with either the full register map or parts of it in a table based
>> approach. You don't expose the clocks with CCF but deal with rate
>> change requests internally in the PCI driver. You could also consider
>> to have the initial clock configuration as part of some firmware blob
>> you request with the PCI driver.
>
> That's right. I only need to change a small portion of the configuration, such
> as frequency, but keeping the reset the same, including output driver voltage,
> input clock, etc.
[...]
> My application has a host SoC booting up Linux. Then the clocks on PCIe (FPGA)
> cards get initialized with their clocks. The clocks are not used by host SoC, so
> setting the wrong clocks doesn't crash the system. Each PCIe card has up to four
> clock chips (with four clocks on each chip). It is required for users to be able
> to change the clocks after system boots up.

Consider a userspace configurable clock driver, load the FPGA design
which depends on a specific frequency generated by Si5338 and let the
user mess with your sysfs files - that will certainly crash your system.

Still, I do not see any requirement for a clock driver for that use
case. You have to load the FPGA design or at least configure it to
use the Si5338 generated clocks _after_ configuring Si5338. You'll
have to have a user interface for FPGA bitfile loading, so you can
add another one for the clock generator config.

> I wrote my driver for the PCIe cards so the clocks can be initialized using the
> data provided. But changing the clocks, or initializing with another set of
> configuration requires an interface. There are many ways to solve this. I would
> like to keep the clock driver generic so it can be reused. It looks like CCF may
> not be the best fit for such driver. What is an acceptable way to write this
> driver so it can be in the mainline kernel, or other maintained projects (I am
> not aware of any though)?

IMHO "generic" as in a generic mainline kernel clock driver just means
that other _drivers_ can request any clock rate from that chip. If you
want to write a CCF driver for Si5338, you'll have to make your PCIe
driver request that clock and hide the userspace configuration within
your PCIe driver.

Adding userspace interfaces to generic CCF clock drivers will not happen
just because messing with the clocks will break a running system. As I
said before, AFAIKS i2c-dev should give you enough of an interface to
configure the clock generator from userspace.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists