On 05/27/2015 09:42 AM, Sebastian Hesselbarth wrote: > 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, Thanks for the insight. Looks like I should give up upstreaming this driver. I will find other ways to make this driver available if anyone wants to use it. York -- 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
