linux-kernel - Re: [PATCH 1/3] iio: frequency: adf41513: driver implementation

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 for Android: free password hash cracker in your pocket

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

Message-ID: <cbe4f6071952e0055f4df400b56fd283a1294115.camel@gmail.com>
Date: Thu, 13 Nov 2025 14:39:39 +0000
From: Nuno Sá <noname.nuno@...il.com>
To: Marcelo Schmitt <marcelo.schmitt1@...il.com>, rodrigo.alencar@...log.com
Cc: linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-doc@...r.kernel.org, Jonathan Cameron	
 <jic23@...nel.org>, David Lechner <dlechner@...libre.com>, Andy Shevchenko	
 <andy@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich	
 <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH 1/3] iio: frequency: adf41513: driver implementation

On Thu, 2025-11-13 at 10:13 -0300, Marcelo Schmitt wrote:
> Hi Rodrigo,
> 
> A couple of comments inline since this is on the mailing list.
> As mentioned in the other thread, we ought to continue the review of this internally.
> 
> On 11/10, Rodrigo Alencar via B4 Relay wrote:
> > From: Rodrigo Alencar <rodrigo.alencar@...log.com>
> > 
> > - ADF41513: 1 GHz to 26.5 GHz frequency range
> > - ADF41510: 1 GHz to 10 GHz frequency range
> > - Integer-N and fractional-N operation modes
> > - Ultra-low phase noise (-235 dBc/Hz integer-N, -231 dBc/Hz fractional-N)
> > - High maximum PFD frequency (250 MHz integer-N, 125 MHz fractional-N)
> > - 25-bit fixed modulus or 49-bit variable modulus fractional modes
> > - Programmable charge pump currents with 16x range
> > - Digital lock detect functionality
> > - Phase resync capability for consistent output phase
> > - Clock framework integration for system clock generation
> > 
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@...log.com>
> > ---
> ...
> > +
> > +static int adf41513_parse_uhz(const char *str, u64 *freq_uhz)
> > +{
> > +	u64 uhz = 0;
> > +	int f_count = ADF41513_HZ_DECIMAL_PRECISION;
> > +	bool frac_part = false;
> > +
> > +	if (str[0] == '+')
> > +		str++;
> > +
> > +	while (*str && f_count > 0) {
> > +		if ('0' <= *str && *str <= '9') {
> > +			uhz = uhz * 10 + *str - '0';
> > +			if (frac_part)
> > +				f_count--;
> > +		} else if (*str == '\n') {
> > +			if (*(str + 1) == '\0')
> > +				break;
> > +			return -EINVAL;
> > +		} else if (*str == '.' && !frac_part) {
> > +			frac_part = true;
> > +		} else {
> > +			return -EINVAL;
> > +		}
> > +		str++;
> > +	}
> > +
> > +	for (; f_count > 0; f_count--)
> > +		uhz *= 10;
> > +
> > +	*freq_uhz = uhz;
> > +
> > +	return 0;
> > +}
> didn't check the details, but can't the sub-Hz resolution be supported with
> .write_raw_get_fmt()?
> e.g.
> 
> static int adf41513_write_raw_get_fmt(struct iio_dev *indio_dev,
> 				    struct iio_chan_spec const *chan, long mask)
> {
> 	switch (mask) {
> 	case IIO_CHAN_INFO_FREQUENCY:
> 		return IIO_VAL_INT_64;
> 	default:
> 		return IIO_VAL_INT_PLUS_MICRO;

I think the above is already the default anyways... But the key here is that (I think) the goal
is to be able to do things like:

echo integer.fractional > out_altvoltage0_frequency

where integer can be u64. If I'm not missing anything, we cannot do that through the standard
interfaces.

- Nuno Sá

> 	}
> }
> 
> static const struct iio_info adf41513_info = {
> ...
> 	.write_raw_get_fmt = adf41513_write_raw_get_fmt(),
> };
> 
> ...
> > +
> > +static ssize_t adf41513_write(struct iio_dev *indio_dev,
> > +			      uintptr_t private,
> > +			      const struct iio_chan_spec *chan,
> > +			      const char *buf, size_t len)
> > +{
> > +	struct adf41513_state *st = iio_priv(indio_dev);
> > +	unsigned long long readin;
> > +	unsigned long tmp;
> > +	u64 freq_uhz;
> > +	int ret;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> > +	switch ((u32)private) {
> > +	case ADF41513_FREQ:
> > +		ret = adf41513_parse_uhz(buf, &freq_uhz);
> > +		if (ret)
> > +			return ret;
> > +		ret = adf41513_set_frequency(st, freq_uhz, ADF41513_SYNC_DIFF);
> > +		break;
> > +	case ADF41513_FREQ_REFIN:
> > +		ret = kstrtoull(buf, 10, &readin);
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (readin < ADF41513_MIN_REF_FREQ || readin > ADF41513_MAX_REF_FREQ) {
> Can, alternatively, this check be made with in_range() macro?
> If so, then
> #include <linux/minmax.h>
> 
> Same question/suggestion to other similar value bounds checks throughout the driver.
> 
> > +			ret = -EINVAL;
> > +			break;
> > +		}
> > +
> 
> With best regards,
> Marcelo