clock driver

clock driver

[York Sun]

Linux experts,

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.

Regards,

York

Re: clock driver

[Guenter Roeck]

On Tue, May 26, 2015 at 12:12:11PM -0700, York Sun wrote:
> Linux experts,
> 
> 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 ?

Guenter

---
[1] https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5338-RM.pdf

Re: clock driver

[York Sun]

On 05/26/2015 03:38 PM, Guenter Roeck wrote:
> On Tue, May 26, 2015 at 12:12:11PM -0700, York Sun wrote:
>> Linux experts,
>>
>> 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.

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?

I also found COMMON_CLK is a bool, not tristate. It is only selected by others.
Is there a reason for doing so? My current platform (P1022DS) doesn't have
CONFIG_COMMON_CLK enabled.

York

Re: clock driver

[York Sun]

Michael,

Can you give me some guidance here?


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:
>>> Linux experts,
>>>
>>> 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.
> 
> 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?
> 
> I also found COMMON_CLK is a bool, not tristate. It is only selected by others.
> Is there a reason for doing so? My current platform (P1022DS) doesn't have
> CONFIG_COMMON_CLK enabled.
> 

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.

York

Re: clock driver

[Sebastian Hesselbarth]

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

Re: clock driver

[York Sun]

On 05/27/2015 12:09 AM, Sebastian Hesselbarth wrote:
> 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.

Sebastian,

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.

> 
>>> 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.

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.

> 
> (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.
> 

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.

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)?

York

Re: clock driver

[Sebastian Hesselbarth]

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

Re: clock driver

[York Sun]

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

Re: clock driver

[Guenter Roeck]

On Wed, May 27, 2015 at 09:46:02AM -0700, York Sun wrote:
> 
[ ... ]
> 
> 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.
> 

You might consider putting it on github. That is what I did with my si570
driver. It was then picked up by someone else and ported to the clock framework.
Win-win for everyone.

Thanks,
Guenter

Re: clock driver

[York Sun]

On 05/27/2015 10:09 AM, Guenter Roeck wrote:
> On Wed, May 27, 2015 at 09:46:02AM -0700, York Sun wrote:
>>
> [ ... ]
>>
>> 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.
>>
> 
> You might consider putting it on github. That is what I did with my si570
> driver. It was then picked up by someone else and ported to the clock framework.
> Win-win for everyone.
> 
> Thanks,
> Guenter

Thanks for the suggestion.

York

Re: clock driver

[Michael Turquette]

Quoting York Sun (2015-05-26 17:32:13)
> Michael,
> 
> Can you give me some guidance here?
> 
> 
> 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:
> >>> Linux experts,
> >>>
> >>> 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.

Can you give me a link to some info about ClockBuilder?

> >>>
> >>> 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.
> > 
> > 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?

CCF does not have any dependency on DeviceTree. Zero. If you don't want
to use DT, then that is great! The ARM community has chosen DT, and the
ARM community by and large uses CCF in Linux. But there is no
requirement to use DT if you want to use CCF.

Regarding sysfs, I really need to understand what interfaces you want
from sysfs. Can you provide a link to the ClockBuilder(TM) stuff?

It is certainly possible for you to create sysfs entries in your clock
driver. Depending on what you want to do, there may be no need to
involve the framework in this stuff. Do you think you can keep your
sysfs stuff localized in your driver?

> > 
> > I also found COMMON_CLK is a bool, not tristate. It is only selected by others.
> > Is there a reason for doing so? My current platform (P1022DS) doesn't have
> > CONFIG_COMMON_CLK enabled.
> > 
> 
> 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.

Sure. Let's figure out your requirements and decide if we can make CCF
work for you. I think we can.

I know that the Beagle Bone folks have been looking at modifying DT
blobs and changing them/loading them at run-time. That might be
interesting for your on-the-fly clock configuration.

If you don't like DT then perhaps you can export your sysfs clock stuff
via your clock driver, instead of the framework?

Regards,
Mike

> 
> York

Re: clock driver

[York Sun]

On 05/27/2015 10:30 AM, Michael Turquette wrote:
> Quoting York Sun (2015-05-26 17:32:13)
>> Michael,
>>
>> Can you give me some guidance here?
>>
>>
>> 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:
>>>>> Linux experts,
>>>>>
>>>>> 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.
> 
> Can you give me a link to some info about ClockBuilder?

It is a software provided by Silicon Labs to create configuration. See
http://www.silabs.com/products/clocksoscillators/Pages/Timing-Software-Development-Tools.aspx

> 
>>>>>
>>>>> 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.
>>>
>>> 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?
> 
> CCF does not have any dependency on DeviceTree. Zero. If you don't want
> to use DT, then that is great! The ARM community has chosen DT, and the
> ARM community by and large uses CCF in Linux. But there is no
> requirement to use DT if you want to use CCF.

That's good to know.

> 
> Regarding sysfs, I really need to understand what interfaces you want
> from sysfs. Can you provide a link to the ClockBuilder(TM) stuff?
> 
> It is certainly possible for you to create sysfs entries in your clock
> driver. Depending on what you want to do, there may be no need to
> involve the framework in this stuff. Do you think you can keep your
> sysfs stuff localized in your driver?

I understand that I can create sysfs entries in my driver. I brought it up
because I remember seeing your plan to add sysfs interface.

If I add sysfs for this driver, it will not be a generic driver.

> 
>>>
>>> I also found COMMON_CLK is a bool, not tristate. It is only selected by others.
>>> Is there a reason for doing so? My current platform (P1022DS) doesn't have
>>> CONFIG_COMMON_CLK enabled.
>>>
>>
>> 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.
> 
> Sure. Let's figure out your requirements and decide if we can make CCF
> work for you. I think we can.
> 
> I know that the Beagle Bone folks have been looking at modifying DT
> blobs and changing them/loading them at run-time. That might be
> interesting for your on-the-fly clock configuration.
> 
> If you don't like DT then perhaps you can export your sysfs clock stuff
> via your clock driver, instead of the framework?
> 

Like I explained in the other email of this thread, I plan to use the clock
driver to initialize clocks on PCIe (FPGA) cards which four chips on each card.
The clocks will be set by the host SoC for FPGA to use. Messing with the clocks
will not crash host OS. It is required to set the clocks with different
frequencies without recompiling/rebooting the host OS.

I wrote a driver for the PCIe card to load FPGA images. To setup the clocks, I
rewrote the SI5338 driver and set the desired frequencies using sysfs. This
driver has a feature to import/dump the raw configuration data. That's one
feature I plan to use, at least for debugging.

I don't think device tree is the best for my application because I will have
about 30 clock chips to program in the complete system. It is easier to use user
space application to do so from I2C bus.

Earlier Guenter helped me to comb through the idea and we concluded CCF may not
be the best fit for this application. I wonder if CCF is the only way to get
clock supported now.

York

Re: clock driver

[Guenter Roeck]

On Wed, May 27, 2015 at 10:45:32AM -0700, York Sun wrote:
> 
> 
> On 05/27/2015 10:30 AM, Michael Turquette wrote:
> > Quoting York Sun (2015-05-26 17:32:13)
> >> Michael,
> >>
> >> Can you give me some guidance here?
> >>
> >>
> >> 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:
> >>>>> Linux experts,
> >>>>>
> >>>>> 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.
> > 
> > Can you give me a link to some info about ClockBuilder?
> 
> It is a software provided by Silicon Labs to create configuration. See
> http://www.silabs.com/products/clocksoscillators/Pages/Timing-Software-Development-Tools.aspx
> 
> > 
> >>>>>
> >>>>> 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.
> >>>
> >>> 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?
> > 
> > CCF does not have any dependency on DeviceTree. Zero. If you don't want
> > to use DT, then that is great! The ARM community has chosen DT, and the
> > ARM community by and large uses CCF in Linux. But there is no
> > requirement to use DT if you want to use CCF.
> 
> That's good to know.
> 
> > 
> > Regarding sysfs, I really need to understand what interfaces you want
> > from sysfs. Can you provide a link to the ClockBuilder(TM) stuff?
> > 
> > It is certainly possible for you to create sysfs entries in your clock
> > driver. Depending on what you want to do, there may be no need to
> > involve the framework in this stuff. Do you think you can keep your
> > sysfs stuff localized in your driver?
> 
> I understand that I can create sysfs entries in my driver. I brought it up
> because I remember seeing your plan to add sysfs interface.
> 
> If I add sysfs for this driver, it will not be a generic driver.
> 
> > 
> >>>
> >>> I also found COMMON_CLK is a bool, not tristate. It is only selected by others.
> >>> Is there a reason for doing so? My current platform (P1022DS) doesn't have
> >>> CONFIG_COMMON_CLK enabled.
> >>>
> >>
> >> 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.
> > 
> > Sure. Let's figure out your requirements and decide if we can make CCF
> > work for you. I think we can.
> > 
> > I know that the Beagle Bone folks have been looking at modifying DT
> > blobs and changing them/loading them at run-time. That might be
> > interesting for your on-the-fly clock configuration.
> > 
> > If you don't like DT then perhaps you can export your sysfs clock stuff
> > via your clock driver, instead of the framework?
> > 
> 
> Like I explained in the other email of this thread, I plan to use the clock
> driver to initialize clocks on PCIe (FPGA) cards which four chips on each card.
> The clocks will be set by the host SoC for FPGA to use. Messing with the clocks
> will not crash host OS. It is required to set the clocks with different
> frequencies without recompiling/rebooting the host OS.
> 

Someone suggested earlier that the clocks should be set from the PCIe driver.
Question here is really if you need to control the clocks from user space.
Even if you do, the driver for the chip _using_ the clocks would be better
suited for changing the clock rates than the clock driver itself. This way it
can ensure that the clock rates are sane for the given use case, and the use
case is kept out of the clock driver.

> I wrote a driver for the PCIe card to load FPGA images. To setup the clocks, I
> rewrote the SI5338 driver and set the desired frequencies using sysfs. This
> driver has a feature to import/dump the raw configuration data. That's one
> feature I plan to use, at least for debugging.
> 
With the above in mind, the idea would be for the PCIe driver to set the clock
rates.

> I don't think device tree is the best for my application because I will have
> about 30 clock chips to program in the complete system. It is easier to use user
> space application to do so from I2C bus.
> 
Devicetree is static, so you might have to use devicetree overlays if the
programming changes at runtime. Not sure why the number of clock chips
would make that non-feasible, though.

On a side note, we are using devicetree a lot for PCIe devices.

> Earlier Guenter helped me to comb through the idea and we concluded CCF may not
> be the best fit for this application. I wonder if CCF is the only way to get

Well, you did ;-). I think it would be feasible, but you would have to change
your viewpoint (in respect to how to control the clocks).

Thanks,
Guenter

Re: clock driver

[York Sun]

On 05/27/2015 11:15 AM, Guenter Roeck wrote:
> On Wed, May 27, 2015 at 10:45:32AM -0700, York Sun wrote:
>>
>>
>> On 05/27/2015 10:30 AM, Michael Turquette wrote:
>>> Quoting York Sun (2015-05-26 17:32:13)
>>>> Michael,
>>>>
>>>> Can you give me some guidance here?
>>>>
>>>>
>>>> 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:
>>>>>>> Linux experts,
>>>>>>>
>>>>>>> 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.
>>>
>>> Can you give me a link to some info about ClockBuilder?
>>
>> It is a software provided by Silicon Labs to create configuration. See
>> http://www.silabs.com/products/clocksoscillators/Pages/Timing-Software-Development-Tools.aspx
>>
>>>
>>>>>>>
>>>>>>> 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.
>>>>>
>>>>> 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?
>>>
>>> CCF does not have any dependency on DeviceTree. Zero. If you don't want
>>> to use DT, then that is great! The ARM community has chosen DT, and the
>>> ARM community by and large uses CCF in Linux. But there is no
>>> requirement to use DT if you want to use CCF.
>>
>> That's good to know.
>>
>>>
>>> Regarding sysfs, I really need to understand what interfaces you want
>>> from sysfs. Can you provide a link to the ClockBuilder(TM) stuff?
>>>
>>> It is certainly possible for you to create sysfs entries in your clock
>>> driver. Depending on what you want to do, there may be no need to
>>> involve the framework in this stuff. Do you think you can keep your
>>> sysfs stuff localized in your driver?
>>
>> I understand that I can create sysfs entries in my driver. I brought it up
>> because I remember seeing your plan to add sysfs interface.
>>
>> If I add sysfs for this driver, it will not be a generic driver.
>>
>>>
>>>>>
>>>>> I also found COMMON_CLK is a bool, not tristate. It is only selected by others.
>>>>> Is there a reason for doing so? My current platform (P1022DS) doesn't have
>>>>> CONFIG_COMMON_CLK enabled.
>>>>>
>>>>
>>>> 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.
>>>
>>> Sure. Let's figure out your requirements and decide if we can make CCF
>>> work for you. I think we can.
>>>
>>> I know that the Beagle Bone folks have been looking at modifying DT
>>> blobs and changing them/loading them at run-time. That might be
>>> interesting for your on-the-fly clock configuration.
>>>
>>> If you don't like DT then perhaps you can export your sysfs clock stuff
>>> via your clock driver, instead of the framework?
>>>
>>
>> Like I explained in the other email of this thread, I plan to use the clock
>> driver to initialize clocks on PCIe (FPGA) cards which four chips on each card.
>> The clocks will be set by the host SoC for FPGA to use. Messing with the clocks
>> will not crash host OS. It is required to set the clocks with different
>> frequencies without recompiling/rebooting the host OS.
>>
> 
> Someone suggested earlier that the clocks should be set from the PCIe driver.
> Question here is really if you need to control the clocks from user space.
> Even if you do, the driver for the chip _using_ the clocks would be better
> suited for changing the clock rates than the clock driver itself. This way it
> can ensure that the clock rates are sane for the given use case, and the use
> case is kept out of the clock driver.
> 
>> I wrote a driver for the PCIe card to load FPGA images. To setup the clocks, I
>> rewrote the SI5338 driver and set the desired frequencies using sysfs. This
>> driver has a feature to import/dump the raw configuration data. That's one
>> feature I plan to use, at least for debugging.
>>
> With the above in mind, the idea would be for the PCIe driver to set the clock
> rates.

I am interested in this path. Can you explain a little bit more about setting
the clock rates? I have no experience on CCF.

> 
>> I don't think device tree is the best for my application because I will have
>> about 30 clock chips to program in the complete system. It is easier to use user
>> space application to do so from I2C bus.
>>
> Devicetree is static, so you might have to use devicetree overlays if the
> programming changes at runtime. Not sure why the number of clock chips
> would make that non-feasible, though.

I mean the existence is unknown for many chips. The baseline is I can't use
static data.

> 
> On a side note, we are using devicetree a lot for PCIe devices.

It's tempting. I want to explore this direction at a later phase of my project.
I will appreciate if you can point me to something.

> 
>> Earlier Guenter helped me to comb through the idea and we concluded CCF may not
>> be the best fit for this application. I wonder if CCF is the only way to get
> 
> Well, you did ;-). I think it would be feasible, but you would have to change
> your viewpoint (in respect to how to control the clocks).

Right, I did. I will revisit this after bring up the platform initially, when I
have more knowledge about those clocks. Maybe I should add an interface for my
FPGA driver to request clock rates, instead of setting them from clock driver.
It sounds better.

York

Re: clock driver

[Guenter Roeck]

On Wed, May 27, 2015 at 11:24:50AM -0700, York Sun wrote:
> 
> > 
> > Someone suggested earlier that the clocks should be set from the PCIe driver.
> > Question here is really if you need to control the clocks from user space.
> > Even if you do, the driver for the chip _using_ the clocks would be better
> > suited for changing the clock rates than the clock driver itself. This way it
> > can ensure that the clock rates are sane for the given use case, and the use
> > case is kept out of the clock driver.
> > 
> >> I wrote a driver for the PCIe card to load FPGA images. To setup the clocks, I
> >> rewrote the SI5338 driver and set the desired frequencies using sysfs. This
> >> driver has a feature to import/dump the raw configuration data. That's one
> >> feature I plan to use, at least for debugging.
> >>
> > With the above in mind, the idea would be for the PCIe driver to set the clock
> > rates.
> 
> I am interested in this path. Can you explain a little bit more about setting
> the clock rates? I have no experience on CCF.
> 
The API functions are documented in include/linux/clk.h. What you are looking
for here would be clk_set_rate() and related functions such as clk_round_rate(),
and of course support functions such as devm_clk_get(), clk_prepare_enable(),
and clk_disable_unprepare().

You can find lots of examples on how this works if you search for clk_set_rate()
in the kernel source.

> > 
> >> I don't think device tree is the best for my application because I will have
> >> about 30 clock chips to program in the complete system. It is easier to use user
> >> space application to do so from I2C bus.
> >>
> > Devicetree is static, so you might have to use devicetree overlays if the
> > programming changes at runtime. Not sure why the number of clock chips
> > would make that non-feasible, though.
> 
> I mean the existence is unknown for many chips. The baseline is I can't use
> static data.
> 
> > 
> > On a side note, we are using devicetree a lot for PCIe devices.
> 
> It's tempting. I want to explore this direction at a later phase of my project.
> I will appreciate if you can point me to something.
> 
I can send you some devicetree files if you think that would help. Note that we
are heavily using devicetree overlays, so this is all a bit WIP.

> > 
> >> Earlier Guenter helped me to comb through the idea and we concluded CCF may not
> >> be the best fit for this application. I wonder if CCF is the only way to get
> > 
> > Well, you did ;-). I think it would be feasible, but you would have to change
> > your viewpoint (in respect to how to control the clocks).
> 
> Right, I did. I will revisit this after bring up the platform initially, when I
> have more knowledge about those clocks. Maybe I should add an interface for my
> FPGA driver to request clock rates, instead of setting them from clock driver.
> It sounds better.
> 
Yes, that would be the idea. Essentially your FPGA driver could either determine
the clock rate(s) it needs internally, or you could set the clock rate(s)
through sysfs attributes attached to the FPGA driver. The FPGA driver would then
use clk_set_rate() to set the rate in the clock chip.

Guenter

Re: clock driver

[York Sun]

On 05/27/2015 11:54 AM, Guenter Roeck wrote:
> On Wed, May 27, 2015 at 11:24:50AM -0700, York Sun wrote:
>>
>>>
>>> Someone suggested earlier that the clocks should be set from the PCIe driver.
>>> Question here is really if you need to control the clocks from user space.
>>> Even if you do, the driver for the chip _using_ the clocks would be better
>>> suited for changing the clock rates than the clock driver itself. This way it
>>> can ensure that the clock rates are sane for the given use case, and the use
>>> case is kept out of the clock driver.
>>>
>>>> I wrote a driver for the PCIe card to load FPGA images. To setup the clocks, I
>>>> rewrote the SI5338 driver and set the desired frequencies using sysfs. This
>>>> driver has a feature to import/dump the raw configuration data. That's one
>>>> feature I plan to use, at least for debugging.
>>>>
>>> With the above in mind, the idea would be for the PCIe driver to set the clock
>>> rates.
>>
>> I am interested in this path. Can you explain a little bit more about setting
>> the clock rates? I have no experience on CCF.
>>
> The API functions are documented in include/linux/clk.h. What you are looking
> for here would be clk_set_rate() and related functions such as clk_round_rate(),
> and of course support functions such as devm_clk_get(), clk_prepare_enable(),
> and clk_disable_unprepare().
> 
> You can find lots of examples on how this works if you search for clk_set_rate()
> in the kernel source.

Yes I saw them. I have no experience with CCF. It will take some learning time.
I am trying to find an SI5xx EVB board so I can try it out.

> 
>>>
>>>> I don't think device tree is the best for my application because I will have
>>>> about 30 clock chips to program in the complete system. It is easier to use user
>>>> space application to do so from I2C bus.
>>>>
>>> Devicetree is static, so you might have to use devicetree overlays if the
>>> programming changes at runtime. Not sure why the number of clock chips
>>> would make that non-feasible, though.
>>
>> I mean the existence is unknown for many chips. The baseline is I can't use
>> static data.
>>
>>>
>>> On a side note, we are using devicetree a lot for PCIe devices.
>>
>> It's tempting. I want to explore this direction at a later phase of my project.
>> I will appreciate if you can point me to something.
>>
> I can send you some devicetree files if you think that would help. Note that we
> are heavily using devicetree overlays, so this is all a bit WIP.

Yes, please send them to me.

> 
>>>
>>>> Earlier Guenter helped me to comb through the idea and we concluded CCF may not
>>>> be the best fit for this application. I wonder if CCF is the only way to get
>>>
>>> Well, you did ;-). I think it would be feasible, but you would have to change
>>> your viewpoint (in respect to how to control the clocks).
>>
>> Right, I did. I will revisit this after bring up the platform initially, when I
>> have more knowledge about those clocks. Maybe I should add an interface for my
>> FPGA driver to request clock rates, instead of setting them from clock driver.
>> It sounds better.
>>
> Yes, that would be the idea. Essentially your FPGA driver could either determine
> the clock rate(s) it needs internally, or you could set the clock rate(s)
> through sysfs attributes attached to the FPGA driver. The FPGA driver would then
> use clk_set_rate() to set the rate in the clock chip.
> 

Sounds promising. Thanks for the guidance.

York

Re: clock driver

[Guenter Roeck]

On 05/27/2015 12:44 PM, andrey wrote:
> Hello all,
> Let me add a comment on using sysfs to simplify user space access to the clock features as opposed to controlling them from a driver that uses the clock chip driver.
>
> It is common to use such advanced clock chips with the FPGA devices (as me and York do), and a lot of development (HDL code) is done before a fancy higher-level driver is even started. And it is not just a temporary stage needed by a small minority of developers - as HDL coding gets more to the the core of many new devices running Linux kernel, it makes sense to create the chip drivers more developer-friendly, not just for the final use in a higher level device driver - modification of the HDL code (most modern FPGA are programmed at runtime) makes it a new device that may need a new driver.
> I'm sure that it is not just for me, when it starts with the chip driver that supports low-level functionality exposing it to the user space, and then working on the HDL code using Python scripts at that stage. And only later in the development designing the higher level device drivers that may not need all of the chip functionality. And such higher level driver will work for our systems, but other developers who work on their embedded systems will again need access to low level chip functionality, and will have to redo the same work all over again. This I believe is a rationale of exposing such chip-specific hardware features (not all of them are probably easy to fit into a specific standard model) to the user space scrtipts.
>
> I wrote the initial driver code for our system ( https://github.com/Elphel/linux-elphel/blob/master/src/drivers/misc/si5338.c ) and being very far from being a kernel developer myself (I'm more of a hardware guy) I didn't even try to satisfy the required coding style and submit it, so I'm very thankful to York who re-wrote the code and is trying to make it usable to others.
>

Line wraps at ~75 columns would make this a bet easier to read.

A more generic solution to your problem might be to implement a driver
similar to i2c-dev, which exports raw i2c device information to user space.
In your case, you would export information about the clocks in the system,
possibly through sysfs (i2c-dev uses ioctl which is a bit old-fashioned).

This would be a driver independent solution, and work for all clock drivers.
It might still not be accepted by Mike and Stephen, due to the risk, but it
might be worth a try. After all, using i2c-dev to access i2c devices directly
is just as risky.

In my opinion, it is always better to have a driver in the upstream kernel,
if possible one that uses a standard framework. That makes it much easier
to support going forward.

Guenter

Re: clock driver

[andrey]

---- On Wed, 27 May 2015 16:08:12 -0700 Guenter Roeck<linux@roeck-us.net> wrote ---- 

 > On 05/27/2015 12:44 PM, andrey wrote: 
 > > Hello all, 
 > > Let me add a comment on using sysfs to simplify user space access to the clock
 > > features as opposed to controlling them from a driver that uses the clock chip driver. 
 > > 
 > > It is common to use such advanced clock chips with the FPGA devices (as me and
 > > York do), and a lot of development (HDL code) is done before a fancy higher-level
 > > driver is even started. And it is not just a temporary stage needed by a small minority
 > > of developers - as HDL coding gets more to the the core of many new devices running
 > > Linux kernel, it makes sense to create the chip drivers more developer-friendly, not
 > > just for the final use in a higher level device driver - modification of the HDL code
 > > (most modern FPGA are programmed at runtime) makes it a new device that may
 > > need a new driver. 
 > > I'm sure that it is not just for me, when it starts with the chip driver that supports
 > > low-level functionality exposing it to the user space, and then working on the HDL
 > > code using Python scripts at that stage. And only later in the development designing
 > > the higher level device drivers that may not need all of the chip functionality. And such
 > > higher level driver will work for our systems, but other developers who work on their
 > > embedded systems will again need access to low level chip functionality, and will have
 > > to redo the same work all over again. This I believe is a rationale of exposing such
 > > chip-specific hardware features (not all of them are probably easy to fit into a specific
 > > standard model) to the user space scripts. 
 > > 
 > > I wrote the initial driver code for our system
 > > ( https://github.com/Elphel/linux-elphel/blob/master/src/drivers/misc/si5338.c ) and
 > > being very far from being a kernel developer myself (I'm more of a hardware guy)
 > > I didn't even try to satisfy the required coding style and submit it, so I'm very thankful
 > > to York who re-wrote the code and is trying to make it usable to others. 
 > > 
 >  
 > Line wraps at ~75 columns would make this a bet easier to read.

Guenter, I'm sorry for using "rich text" email settings. 

 >  
 > A more generic solution to your problem might be to implement a driver 
 > similar to i2c-dev, which exports raw i2c device information to user space. 
 > In your case, you would export information about the clocks in the system, 
 > possibly through sysfs (i2c-dev uses ioctl which is a bit old-fashioned).

I was trying to make it safer to use low-level functionality of the particular
(and rather popular) clock chip and to avoid using SiLabs proprietary tools to
generate required settings offline. Using just raw i2c would require to have
large user space program to calculate valid settings for the device.

I would consider this chip as both a generic clock device that can fit into
a standard framework and simultaneously a unique device that offers specific
functionality outside of the framework. I thought that sysfs (instead of
"old-fashioned" ioctl I used in such cases before) can offer
hardware developer-friendly solution as a supplement to in-framework
basic functionality.

Device driver for this chip makes it possible to avoid proprietary configuration
software and calculate register settings at runtime, minimizing requirements to
the user space software and so the time developers of the new embedded
systems will need to (re-)implement these important chip-specific  features.

Andrey

 >  
 > This would be a driver independent solution, and work for all clock drivers. 
 > It might still not be accepted by Mike and Stephen, due to the risk, but it 
 > might be worth a try. After all, using i2c-dev to access i2c devices directly 
 > is just as risky. 
 >  
 > In my opinion, it is always better to have a driver in the upstream kernel, 
 > if possible one that uses a standard framework. That makes it much easier 
 > to support going forward. 
 >  
 > Guenter 
 >  
 > 

Re: clock driver

[Guenter Roeck]

On 05/27/2015 04:58 PM, andrey wrote:
>
>
> ---- On Wed, 27 May 2015 16:08:12 -0700 Guenter Roeck<linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote ----
>
>   > On 05/27/2015 12:44 PM, andrey wrote:
>   > > Hello all,
>   > > Let me add a comment on using sysfs to simplify user space access to the clock
>   > > features as opposed to controlling them from a driver that uses the clock chip driver.
>   > >
>   > > It is common to use such advanced clock chips with the FPGA devices (as me and
>   > > York do), and a lot of development (HDL code) is done before a fancy higher-level
>   > > driver is even started. And it is not just a temporary stage needed by a small minority
>   > > of developers - as HDL coding gets more to the the core of many new devices running
>   > > Linux kernel, it makes sense to create the chip drivers more developer-friendly, not
>   > > just for the final use in a higher level device driver - modification of the HDL code
>   > > (most modern FPGA are programmed at runtime) makes it a new device that may
>   > > need a new driver.
>   > > I'm sure that it is not just for me, when it starts with the chip driver that supports
>   > > low-level functionality exposing it to the user space, and then working on the HDL
>   > > code using Python scripts at that stage. And only later in the development designing
>   > > the higher level device drivers that may not need all of the chip functionality. And such
>   > > higher level driver will work for our systems, but other developers who work on their
>   > > embedded systems will again need access to low level chip functionality, and will have
>   > > to redo the same work all over again. This I believe is a rationale of exposing such
>   > > chip-specific hardware features (not all of them are probably easy to fit into a specific
>   > > standard model) to the user space scripts.
>   > >
>   > > I wrote the initial driver code for our system
>   > > ( https://github.com/Elphel/linux-elphel/blob/master/src/drivers/misc/si5338.c ) and
>   > > being very far from being a kernel developer myself (I'm more of a hardware guy)
>   > > I didn't even try to satisfy the required coding style and submit it, so I'm very thankful
>   > > to York who re-wrote the code and is trying to make it usable to others.
>   > >
>   >
>   > Line wraps at ~75 columns would make this a bet easier to read.
>
> Guenter, I'm sorry for using "rich text" email settings.
>
>   >
>   > A more generic solution to your problem might be to implement a driver
>   > similar to i2c-dev, which exports raw i2c device information to user space.
>   > In your case, you would export information about the clocks in the system,
>   > possibly through sysfs (i2c-dev uses ioctl which is a bit old-fashioned).
>
> I was trying to make it safer to use low-level functionality of the particular
> (and rather popular) clock chip and to avoid using SiLabs proprietary tools to
> generate required settings offline. Using just raw i2c would require to have
> large user space program to calculate valid settings for the device.
>
> I would consider this chip as both a generic clock device that can fit into
> a standard framework and simultaneously a unique device that offers specific
> functionality outside of the framework. I thought that sysfs (instead of
> "old-fashioned" ioctl I used in such cases before) can offer
> hardware developer-friendly solution as a supplement to in-framework
> basic functionality.
>
> Device driver for this chip makes it possible to avoid proprietary configuration
> software and calculate register settings at runtime, minimizing requirements to
> the user space software and so the time developers of the new embedded
> systems will need to (re-)implement these important chip-specific  features.
>

I think we are in violent agreement ;-). Only question was how to implement
sysfs (or user space access) support, where my preference would be a more
generic solution.

Thanks,
Guenter

> Andrey
>
>   >
>   > This would be a driver independent solution, and work for all clock drivers.
>   > It might still not be accepted by Mike and Stephen, due to the risk, but it
>   > might be worth a try. After all, using i2c-dev to access i2c devices directly
>   > is just as risky.
>   >
>   > In my opinion, it is always better to have a driver in the upstream kernel,
>   > if possible one that uses a standard framework. That makes it much easier
>   > to support going forward.
>   >
>   > Guenter
>   >
>   >
>
>
>
>

Re: clock driver

[andrey]

---- On Wed, 27 May 2015 17:10:06 -0700 Guenter Roeck<linux@roeck-us.net> wrote ---- 
 > On 05/27/2015 04:58 PM, andrey wrote: 
 > > 
 > > 
 > > ---- On Wed, 27 May 2015 16:08:12 -0700 Guenter Roeck<linux@roeck-us.net> wrote ---- 
 > > 
 > >   > On 05/27/2015 12:44 PM, andrey wrote: 
 > >   > > Hello all, 
 > >   > > Let me add a comment on using sysfs to simplify user space access to the clock 
 > >   > > features as opposed to controlling them from a driver that uses the clock chip driver. 
 > >   > > 
 > >   > > It is common to use such advanced clock chips with the FPGA devices (as me and 
 > >   > > York do), and a lot of development (HDL code) is done before a fancy higher-level 
 > >   > > driver is even started. And it is not just a temporary stage needed by a small minority 
 > >   > > of developers - as HDL coding gets more to the the core of many new devices running 
 > >   > > Linux kernel, it makes sense to create the chip drivers more developer-friendly, not 
 > >   > > just for the final use in a higher level device driver - modification of the HDL code 
 > >   > > (most modern FPGA are programmed at runtime) makes it a new device that may 
 > >   > > need a new driver. 
 > >   > > I'm sure that it is not just for me, when it starts with the chip driver that supports 
 > >   > > low-level functionality exposing it to the user space, and then working on the HDL 
 > >   > > code using Python scripts at that stage. And only later in the development designing 
 > >   > > the higher level device drivers that may not need all of the chip functionality. And such 
 > >   > > higher level driver will work for our systems, but other developers who work on their 
 > >   > > embedded systems will again need access to low level chip functionality, and will have 
 > >   > > to redo the same work all over again. This I believe is a rationale of exposing such 
 > >   > > chip-specific hardware features (not all of them are probably easy to fit into a specific 
 > >   > > standard model) to the user space scripts. 
 > >   > > 
 > >   > > I wrote the initial driver code for our system 
 > >   > > ( https://github.com/Elphel/linux-elphel/blob/master/src/drivers/misc/si5338.c ) and 
 > >   > > being very far from being a kernel developer myself (I'm more of a hardware guy) 
 > >   > > I didn't even try to satisfy the required coding style and submit it, so I'm very thankful 
 > >   > > to York who re-wrote the code and is trying to make it usable to others. 
 > >   > > 
 > >   > 
 > >   > Line wraps at ~75 columns would make this a bet easier to read. 
 > > 
 > > Guenter, I'm sorry for using "rich text" email settings. 
 > > 
 > >   > 
 > >   > A more generic solution to your problem might be to implement a driver 
 > >   > similar to i2c-dev, which exports raw i2c device information to user space. 
 > >   > In your case, you would export information about the clocks in the system, 
 > >   > possibly through sysfs (i2c-dev uses ioctl which is a bit old-fashioned). 
 > > 
 > > I was trying to make it safer to use low-level functionality of the particular 
 > > (and rather popular) clock chip and to avoid using SiLabs proprietary tools to 
 > > generate required settings offline. Using just raw i2c would require to have 
 > > large user space program to calculate valid settings for the device. 
 > > 
 > > I would consider this chip as both a generic clock device that can fit into 
 > > a standard framework and simultaneously a unique device that offers specific 
 > > functionality outside of the framework. I thought that sysfs (instead of 
 > > "old-fashioned" ioctl I used in such cases before) can offer 
 > > hardware developer-friendly solution as a supplement to in-framework 
 > > basic functionality. 
 > > 
 > > Device driver for this chip makes it possible to avoid proprietary configuration 
 > > software and calculate register settings at runtime, minimizing requirements to 
 > > the user space software and so the time developers of the new embedded 
 > > systems will need to (re-)implement these important chip-specific  features. 
 > > 
 >  
 > I think we are in violent agreement ;-). Only question was how to implement 
 > sysfs (or user space access) support, where my preference would be a more 
 > generic solution. 

Guenter,

I just considered this chip as a "frontier" device, not yet a member of an established
class of similar ones. It may be possible to generalize later, extracting common
functionality to a more abstract interface. But we just need this device support now,
and when this one will become a member of some generic class - "frontier" will again
move a step farther, new devices will emerge that stick out of the nice frameworks.

Andrey

 >  
 > Thanks, 
 > Guenter 
 >  
 > > Andrey 
 > > 
 > >   > 
 > >   > This would be a driver independent solution, and work for all clock drivers. 
 > >   > It might still not be accepted by Mike and Stephen, due to the risk, but it 
 > >   > might be worth a try. After all, using i2c-dev to access i2c devices directly 
 > >   > is just as risky. 
 > >   > 
 > >   > In my opinion, it is always better to have a driver in the upstream kernel, 
 > >   > if possible one that uses a standard framework. That makes it much easier 
 > >   > to support going forward. 
 > >   > 
 > >   > Guenter 
 > >   > 
 > >   > 
 > > 
 > > 
 > > 
 > > 
 >  
 > 

Re: clock driver

[Michael Turquette]

Quoting andrey (2015-05-27 17:29:00)
> 
> 
> ---- On Wed, 27 May 2015 17:10:06 -0700 Guenter Roeck<linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote ---- 
>  > On 05/27/2015 04:58 PM, andrey wrote: 
>  > > 
>  > > 
>  > > ---- On Wed, 27 May 2015 16:08:12 -0700 Guenter Roeck<linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote ---- 
>  > > 
>  > >   > On 05/27/2015 12:44 PM, andrey wrote: 
>  > >   > > Hello all, 
>  > >   > > Let me add a comment on using sysfs to simplify user space access to the clock 
>  > >   > > features as opposed to controlling them from a driver that uses the clock chip driver. 
>  > >   > > 
>  > >   > > It is common to use such advanced clock chips with the FPGA devices (as me and 
>  > >   > > York do), and a lot of development (HDL code) is done before a fancy higher-level 
>  > >   > > driver is even started. And it is not just a temporary stage needed by a small minority 
>  > >   > > of developers - as HDL coding gets more to the the core of many new devices running 
>  > >   > > Linux kernel, it makes sense to create the chip drivers more developer-friendly, not 
>  > >   > > just for the final use in a higher level device driver - modification of the HDL code 
>  > >   > > (most modern FPGA are programmed at runtime) makes it a new device that may 
>  > >   > > need a new driver. 
>  > >   > > I'm sure that it is not just for me, when it starts with the chip driver that supports 
>  > >   > > low-level functionality exposing it to the user space, and then working on the HDL 
>  > >   > > code using Python scripts at that stage. And only later in the development designing 
>  > >   > > the higher level device drivers that may not need all of the chip functionality. And such 
>  > >   > > higher level driver will work for our systems, but other developers who work on their 
>  > >   > > embedded systems will again need access to low level chip functionality, and will have 
>  > >   > > to redo the same work all over again. This I believe is a rationale of exposing such 
>  > >   > > chip-specific hardware features (not all of them are probably easy to fit into a specific 
>  > >   > > standard model) to the user space scripts. 
>  > >   > > 
>  > >   > > I wrote the initial driver code for our system 
>  > >   > > ( https://github.com/Elphel/linux-elphel/blob/master/src/drivers/misc/si5338.c ) and 
>  > >   > > being very far from being a kernel developer myself (I'm more of a hardware guy) 
>  > >   > > I didn't even try to satisfy the required coding style and submit it, so I'm very thankful 
>  > >   > > to York who re-wrote the code and is trying to make it usable to others. 
>  > >   > > 
>  > >   > 
>  > >   > Line wraps at ~75 columns would make this a bet easier to read. 
>  > > 
>  > > Guenter, I'm sorry for using "rich text" email settings. 
>  > > 
>  > >   > 
>  > >   > A more generic solution to your problem might be to implement a driver 
>  > >   > similar to i2c-dev, which exports raw i2c device information to user space. 
>  > >   > In your case, you would export information about the clocks in the system, 
>  > >   > possibly through sysfs (i2c-dev uses ioctl which is a bit old-fashioned). 
>  > > 
>  > > I was trying to make it safer to use low-level functionality of the particular 
>  > > (and rather popular) clock chip and to avoid using SiLabs proprietary tools to 
>  > > generate required settings offline. Using just raw i2c would require to have 
>  > > large user space program to calculate valid settings for the device. 
>  > > 
>  > > I would consider this chip as both a generic clock device that can fit into 
>  > > a standard framework and simultaneously a unique device that offers specific 
>  > > functionality outside of the framework. I thought that sysfs (instead of 
>  > > "old-fashioned" ioctl I used in such cases before) can offer 
>  > > hardware developer-friendly solution as a supplement to in-framework 
>  > > basic functionality. 
>  > > 
>  > > Device driver for this chip makes it possible to avoid proprietary configuration 
>  > > software and calculate register settings at runtime, minimizing requirements to 
>  > > the user space software and so the time developers of the new embedded 
>  > > systems will need to (re-)implement these important chip-specific  features. 
>  > > 
>  >  
>  > I think we are in violent agreement ;-). Only question was how to implement 
>  > sysfs (or user space access) support, where my preference would be a more 
>  > generic solution. 
> 
> Guenter,
> 
> I just considered this chip as a "frontier" device, not yet a member of an established
> class of similar ones. It may be possible to generalize later, extracting common
> functionality to a more abstract interface. But we just need this device support now,
> and when this one will become a member of some generic class - "frontier" will again
> move a step farther, new devices will emerge that stick out of the nice frameworks.

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).

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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

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

Regards,
Mike

> 
> Andrey
> 
>  >  
>  > Thanks, 
>  > Guenter 
>  >  
>  > > Andrey 
>  > > 
>  > >   > 
>  > >   > This would be a driver independent solution, and work for all clock drivers. 
>  > >   > It might still not be accepted by Mike and Stephen, due to the risk, but it 
>  > >   > might be worth a try. After all, using i2c-dev to access i2c devices directly 
>  > >   > is just as risky. 
>  > >   > 
>  > >   > In my opinion, it is always better to have a driver in the upstream kernel, 
>  > >   > if possible one that uses a standard framework. That makes it much easier 
>  > >   > to support going forward. 
>  > >   > 
>  > >   > Guenter 
>  > >   > 
>  > >   > 
>  > > 
>  > > 
>  > > 
>  > > 
>  >  
>  > 
> 
> 

Re: clock driver

[York Sun]

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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> 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

Re: clock driver

[York Sun]

Michael,

I have rewritten the driver to use CCF. It will be sent for review when ready.

I have some questions, hoping you can shed some light on them.

Q1: What does of_clk_add_provider do?
I read the code and comment. It registers a clock provider for a node. How is it
used after registration?

Q2: What do you suggest to name multiple clocks on PCIe (FPGA) cards?
I will have multiple cards with multiple clock chips on each. The clock driver
can handle clocks on each chip. Is it recommended to create unique names by the
driver? Is there any example to follow?

Q3: Is there a guideline or an API to create sub folder under debugfs "clk"?

Thanks a lot.

York