RE: [EXTERNAL] Re: Questions regarding regarding idt/renesas versaclock5 driver

["Fillion, Claude" <Claude.Fillion@mksinst.com>]

Hello Luca,

> -----Original Message-----
> From: Luca Ceresoli <luca@lucaceresoli.net>
> Sent: Tuesday, March 22, 2022 12:00 PM
> To: Fillion, Claude <Claude.Fillion@mksinst.com>; Adam Ford
> <aford173@gmail.com>
> Cc: Sean Anderson <sean.anderson@seco.com>; linux-clk <linux-
> clk@vger.kernel.org>
> Subject: Re: [EXTERNAL] Re: Questions regarding regarding idt/renesas
> versaclock5 driver
> 
> Hi Claude,
> 
> On 21/03/22 22:21, Fillion, Claude wrote:
> [...]
> >>>> Look at the use of devm_clk_get_optional and clk_prepare_enable
> >>>> from that patch.  (yes, there is a subsequent patch that fixes
> >>>> something I didn't quite do right, but the basics are here)
> >>>>
> >>>> The consumer drivers need to 'get' the clock so it can associate
> >>>> itself to the clock in question.  Once the relationship is
> >>>> established, the consumer needs to call clk_prepare_enable() which
> >>>> uses
> >> the clock system to turn the clock on.
> >>>> Without this step, it's likely the Versaclock won't generate a
> >>>> signal, because it doesn't know it needs to turn it on.
> >>>>
> >>>> adam
> >>>
> >>> Not sure I fully follow.  I see that clk out1 is enabled but the
> >>> other channels
> >> are not so it would seem my difficulty is with individual channels.
> >>
> >> Do the devices that need the clock from the versaclock reference the
> >> versaclock?  If so, to those drivers use the get and enable?  If not,
> >> the versaclock will stay off.  In the patch example I showed, I had
> >> to modify the Ethernet driver on a processor, because it didn't
> >> explicitly enable the reference clock.  That Ethernet driver expected
> >> the refclk was always present which was a false assumption.  Once I
> >> got the consumer device (in this case, Ethernet) to request and
> >> enable the clock, the clock subsystem enabled the corresponding output
> on the versaclock.
> >>
> >> For the Ethernet example I cited above, the corresponding device tree
> >> looks
> >> like:
> >>
> >> &avb {
> >>      clocks = <&cpg CPG_MOD 812>, <&versaclock5 4>;
> >>      clock-names = "fck", "refclk";
> >>      status = "okay";
> >> };
> >>
> >> With this device tree reference, the 'refclk' gets associated to
> >> versaclock ouput 4.  When the Ethernet needs the clock, it calls
> >> clk_prepare_enable on that clock reference, and the clock system
> >> calls on the versaclock driver to enable the output.  The reason I
> >> needed to submit that patch was that the consumer driver (the
> >> Ethernet in this
> >> case) wasn't calling the clk_prepare_enable, so the clock remained
> >> off.  It's likely that whatever devices that need the clock from the
> >> versaclock will need both a device tree reference to it as well as a
> >> call to clk_prepare_enable.
> >
> > For my application I would like the ability to set clock frequencies according
> to values that are stored in a json formatted config file. Is there a way to
> change these values from user space app code?  If not, would it be ok to add
> code to my consumer driver (say in the probe method) that would read the
> values from the json and then call set_rate() and prepare_enable()) ?
> 
> This is absolutely not a standard practice in the kernel. If drivers are well
> written then a consumer driver knows which clock it needs, it will ask the
> produced that frequency and configure the consumer device accordingly.
> 
> E.g. if the consumer device is an audio codec then user space will never say "I
> want you to ask for a 1 MHz clock" but rather "I want to playback at 48 kHz",
> then the driver will ask for a given clock frequency and configure the codec to
> use it, perhaps setting divisor/multiplier registers etc.
> 
> So I really suggest you to understand what the correct kernel-userspace
> interface would be for your consumer driver. If you want you can share what
> the consumer device is so we can be give some suggestions.
> 
> That said, if you don't want to mainline your consumer driver nobody will
> prevent you from doing anything "strange". But keep it limited to your driver,
> you don't want to have conflicts in the versaclock code when you upgrade to
> a more recent kernel.
> 
> Also, I don't think json is expected to be in the kernel. If you want to do
> something like that then maybe parse json at userspace level and then send
> just the frequency in Hz via a sysfs entry.
> 
> --
> Luca

Hello Luca,
Thank you for responding.  I will attempt to provide a little more detail on our use case.  Our products do a fair amount of signal processing, both in transmitting and receiving chains. The clocks we would like to adjust control ADC and DAC sample rates.  These rates do not change frequently, but depending on particular features we enable or on conditions that are specific to a certain customer there will be times where we will need to change sample rates. Our products have an extensive set of configuration parameters contained in json files that control many details of our signal processing chains.  Ideally we would like to add the ability to get/set clock rates through this mechanism.

When changing frequency we expect to reboot the device, but would like to avoid rebuilding/distributing new code.  Essentially  1 code package, configured multiple different ways.

The idea of parsing json at userspace and just sending the frequency in Hz via a sysfs entry seems appealing. I thought I would need to call the set_rate() method of the driver in order to change frequency.  If it is not too much trouble can you explain how I could change frequency via sysfs call?

This is what I see on the device in /sys:
      root@CLAUDE:/sys/class/i2c-dev/i2c-1/device/1-006a# ls
      driver     modalias   name       of_node    power      subsystem  uevent

From this I am not clear on how to set frequency.

Regards,
Claude

======================================================================
This message and any attachments are intended only for the designated recipient(s) and may contain confidential or proprietary information and be subject to the attorney-client privilege or other confidentiality protections.  If you are not a designated recipient, you may not review, use, copy or distribute this message or any attachments.  If you received this email in error, please notify the sender by reply e-mail and permanently delete the original and any copies of this message and any attachments thereto.  Thank you.