Re: [PATCH 0/2] iio: frequency: add iio support for Amlogic clock measure

[Neil Armstrong <neil.armstrong@linaro.org>]

On 25/06/2024 11:53, Jerome Brunet wrote:
> On Tue 25 Jun 2024 at 11:38, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> 
>> Hi,
>>
>> [+cc people from linux-msm]
>>
>> On 24/06/2024 19:31, Jerome Brunet wrote:
>>> Add support for the HW found in most Amlogic SoC dedicated to measure
>>> system clocks.
>>> This drivers aims to replace the one found in
>>> drivers/soc/amlogic/meson-clk-measure.c with following improvements:
>>> * Access to the measurements through the IIO API:
>>>     Easier re-use of the results in userspace and other drivers
>>> * Controllable scale with raw measurements
>>> * Higher precision with processed measurements
>>> Jerome Brunet (2):
>>>     dt-bindings: iio: frequency: add clock measure support
>>>     iio: frequency: add amlogic clock measure support
>>>    .../iio/frequency/amlogic,clk-msr-io.yaml     |  50 ++
>>>    drivers/iio/frequency/Kconfig                 |  15 +
>>>    drivers/iio/frequency/Makefile                |   1 +
>>>    drivers/iio/frequency/amlogic-clk-msr-io.c    | 802 ++++++++++++++++++
>>>    4 files changed, 868 insertions(+)
>>>    create mode 100644 Documentation/devicetree/bindings/iio/frequency/amlogic,clk-msr-io.yaml
>>>    create mode 100644 drivers/iio/frequency/amlogic-clk-msr-io.c
>>>
>>
>> While I really appreciate the effort, and the code looks cool, the clkmsr is really
>> a debug tool, and I'm not sure IIO is the right place for such debug tool ?
> 
> The reason why I went through the trouble of doing an IIO port is
> because I need that for other purposes than debug. I need to to be able
> to check a frequency from another driver. I don't see a reason to invent
> another API when IIO provide a perfectly good one.
> 
> The HW does measurements. IIO seems like the best place for it.
> 
> For the record, I need this for a eARC support.
> eARC has a PLL that locks on incoming stream. eARC registers show wether
> the PLL is locked or not, but not at which rate. That information is
> needed in ASoC. Fortunately the eARC PLL is one of measured clock, which
> is a life saver in that case.

This is a very interesting use-case, and quite weird nothing is provided
on the eARC side.

So yes it's definitely a valid use-case, but:
- we should keep the debugfs interface, perhaps move it in the iio driver ?
- we should keep a single compatible, so simply update the current bindings with iio cells
- for s4 & c3, it's ok to either add a second reg entry in the bindings

Neil

> 
> Everything that was available through the old driver still is, with more
> precision and more control.
> 
>>
>> There's almost the same interface on qcom SoCs (https://github.com/linux-msm/debugcc) but
>> they chose to keep it in userspace until we find an appropriate way to expose
>> this from the kernel the right way.
>>
>> If it enabled us to monitor a frequency input for a product use-case, IIO would be
>> the appropriate interface, but AFAIK it's only internal clocks and thus I'm worried
>> it's not the best way to expose those clocks.
>>
>> Neil
>