Re: clock driver

From: Sebastian Hesselbarth
Date: Wed May 27 2015 - 12:43:12 EST


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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/