Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 6 Nov 2022 17:58:39 +0000
From:   Jonathan Cameron <jic23@kernel.org>
To:     Antoniu Miclaus <antoniu.miclaus@analog.com>
Cc:     <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
        <linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/4] Documentation: ABI: testing: adf4377: add ABI docs
Message-ID: <20221106175604.061b8662@jic23-huawei>
In-Reply-To: <20221104092802.90725-4-antoniu.miclaus@analog.com>
References: <20221104092802.90725-1-antoniu.miclaus@analog.com>
        <20221104092802.90725-4-antoniu.miclaus@analog.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 4 Nov 2022 11:28:01 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add documentation for the use of the output frequency and muxout select.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Hi Antoniu,

A few questions inline.
> ---
>  .../testing/sysfs-bus-iio-frequency-adf4377   | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4377
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4377 b/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4377
> new file mode 100644
> index 000000000000..0324de5c9328
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4377
> @@ -0,0 +1,32 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/frequency
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Stores the PLL frequency in Hz for output channels.
> +		Reading returns the frequency in Hz.

I'm a little confused. Should we not tie this to the output channels?
out_altvoltageX_frequency

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/muxout_select

Why would userspace change this? 
I'd sort of expect this to typically be chosen by what it is physically wired
up to.

You could treat muxout_low and muxout_high as a gpio (possibly high_z as well).
If that were the case, then device tree binding should allow you detect that usecase.

Lock detector is normally wired to a GPI so that software can check if the lock is
successful (or it's wired to some other circuits we can't see).  The two
clock dividers should be represented as a clock provider.

Note this is partly driven by my desire to keep custom ABI to the minimum because
it is rare that userspace has a clue what to do with it.  That is probably less
of a consideration here though as I doubt this device is going to be much used
with software stacks that don't know exactly what it is.

Jonathan

> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		The mux output allows the user to access various internal points on
> +		the chip. Valid values that can be written are:
> +		* high_z -> high impedance output
> +		* lock_detect -> digital lock detector output
> +		* muxout_low -> low output
> +		* f_div_rclk_2 -> fdiv_rclk/2
> +		* f_div_nclk_2 -> fdiv_nclk/2
> +		* muxout_high -> high output
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/muxout_select_available
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Reading this returns the valid values that can be written to the
> +		muxout_mode attribute:
> +		* high_z
> +		* lock_detect
> +		* muxout_low
> +		* f_div_rclk_2
> +		* f_div_nclk_2
> +		* muxout_high