linux-kernel - Re: [PATCH v6 7/8] iio: dac: ad3552r: add high-speed platform driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2815c8b0-e2ad-47cb-b5aa-00297cf57899@baylibre.com>
Date: Tue, 15 Oct 2024 09:38:27 -0500
From: David Lechner <dlechner@...libre.com>
To: Nuno Sá <noname.nuno@...il.com>,
 Angelo Dureghello <adureghello@...libre.com>, Nuno Sá
 <nuno.sa@...log.com>, Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Jonathan Cameron <jic23@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Olivier Moysan <olivier.moysan@...s.st.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH v6 7/8] iio: dac: ad3552r: add high-speed platform driver

On 10/15/24 1:37 AM, Nuno Sá wrote:
> On Mon, 2024-10-14 at 16:15 -0500, David Lechner wrote:
>> On 10/14/24 5:08 AM, Angelo Dureghello wrote:
>>> From: Angelo Dureghello <adureghello@...libre.com>
>>>
>>> Add High Speed ad3552r platform driver.
>>>
>>
>> ...
>>
>>> +static int ad3552r_hs_read_raw(struct iio_dev *indio_dev,
>>> +			       struct iio_chan_spec const *chan,
>>> +			       int *val, int *val2, long mask)
>>> +{
>>> +	struct ad3552r_hs_state *st = iio_priv(indio_dev);
>>> +	int ret;
>>> +
>>> +	switch (mask) {
>>> +	case IIO_CHAN_INFO_SAMP_FREQ: {
>>> +		int sclk;
>>> +
>>> +		ret = iio_backend_read_raw(st->back, chan, &sclk, 0,
>>> +					   IIO_CHAN_INFO_FREQUENCY);
>>
>> FWIW, this still seems like an odd way to get the stream mode SCLK
>> rate from the backend to me. How does the backend know that we want
>> the stream mode clock rate and not some other frequency value? 
> 
> In this case the backend has a dedicated compatible so sky is the limit :). But yeah,
> I'm also not extremely happy with IIO_CHAN_INFO_FREQUENCY. But what do you have in
> mind? Using the sampling frequency INFO or a dedicated OP?
> 

It think it would be most straightforward to have something
like a iio_backend_get_data_stream_clock_rate() callback since
that is what we are getting.

Re: the other recent discussions about getting too many
callbacks. Instead of a dedicated function like this, we
could make a set of generic functions:

iio_backend_{g,s}et_property_{s,u}(8, 16, 32, 64}()

that take an enum parameter for the property. This way,
for each new property, we just have to add an enum member
instead of creating a get/set callback pair.

Unrelated to this particular case, but taking the idea even
farther, we could also do the same with enable/disable
functions. We talked before about cutting the number of
callbacks in half by using a bool parameter instead of
separate enable/disable callbacks. But we could cut it down
even more by having an enum parameter for the thing we are
enabling/disabling.