On 27.05.2015 02:32, York Sun wrote:
On 05/26/2015 05:20 PM, York Sun wrote:
On 05/26/2015 03:38 PM, Guenter Roeck wrote:
On Tue, May 26, 2015 at 12:12:11PM -0700, York Sun wrote:
I have rewritten a driver for Silicon Labs SI5338 programmable clock chip. The
original driver was written by Andrey (CC'ed), but was floatingn outside of the
kernel. The driver was written to use sysfs as the interface, not the common
clock framework. I wonder if I have to rewrite the driver following common clock
framework. One concern is to support a feature to accept ClockBuilder (TM)
output on sysfs. I don't see sysfs support on common clock framework. Please
correct me if I am wrong.
If not using common clock framework is acceptable, I would like to send a RFC
patch for review.
My original driver for si570 was rejected because it didn't support the clock
framework, so you might face an uphill battle.
SI provides a document for SI5338 describing how to configure it without
using clockbuilder [1]. Can that be used to implement generic code which
doesn't need clockbuilder ?
The driver is capable to handle the user's input and enable the clocks. Removing
the support of importing is a step back. At least it is a feature I am using. I
believe Andrey also used this feature when the driver was first drafted.
York,
IMHO what kind of driver you need for the clock generator clearly
depends on the use case you have in mind.
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.
That being said, my application relies on setting multiple clock chips on a PCIe
device. That means I cannot put the configuration into device tree. There may be
a way to fill device tree, but I am not ready to explore yet. Without a sysfs
interface, can I change the configuration for each clock?
If the clock chip is part of a PCI device, the PCI driver should include
functions to set the clock rate. If you expose each clock with common
clock framework or not depends on how dynamically you want your clocks
to be. IMHO you have these options:
(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.
(b) Clocks are driving the PCI card as a parent clock but are not
dynamic. You should use the boot loader to load the register map or you
could simply use i2c-dev to load the registers from userspace. You'll
need no driver at all.
(c) Clocks are driving clock inputs of your SoC and should be fully
dynamic, i.e. you cannot tell the rate that will be requested. Expose
each clock with CCF and find some good code to derive si5338 register
values from the requested rate. si5351 driver is an example but I know
silabs documentation isn't that good when it comes to dynamically
changing the clocks. With CCF you'll have no user interface at all
because users just shouldn't be able to mess with the clocks.
[...]
If converting my driver to common clock framework, I need to find a way to
configure the clocks without recompiling the kernel. I will have about 30 clock
chips (with different frequency) on multiple PCIe cards.
This is your specific use case of the clock driver, something that
clearly doesn't match a generic use case. If you describe your use
case in detail, we may be able to give you more hints on what kind
of driver you'll need and if that driver will ever be able to get
into mainline.
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
