linux-kernel - Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x 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: <mv2smx6m3iqocvplham74aqdcn2eaqmmj36tu7c7qzdjq7jlwu@w2zyijdslrpc>
Date: Wed, 9 Oct 2024 09:50:11 +0200
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>
Cc: Jonathan Cameron <jic23@...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>, Nuno Sa <nuno.sa@...log.com>, 
	Olivier Moysan <olivier.moysan@...s.st.com>, Andy Shevchenko <andy@...nel.org>, 
	David Lechner <dlechner@...libre.com>, Marcelo Schmitt <marcelo.schmitt@...log.com>, 
	Mike Looijmans <mike.looijmans@...ic.nl>, Marius Cristea <marius.cristea@...rochip.com>, 
	Dumitru Ceclan <mitrutzceclan@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Alisa-Dariana Roman <alisadariana@...il.com>, 
	Ivan Mikhaylov <fr0st61te@...il.com>, Sergiu Cuciurean <sergiu.cuciurean@...log.com>, 
	Dragos Bogdan <dragos.bogdan@...log.com>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v2 6/7] iio: adc: ad485x: add ad485x driver

Hello,

On Fri, Oct 04, 2024 at 05:07:55PM +0300, Antoniu Miclaus wrote:
> +static int ad485x_set_sampling_freq(struct ad485x_state *st, unsigned int freq)
> +{
> +	struct pwm_state cnv_state = {
> +		.duty_cycle = AD485X_T_CNVH_NS,
> +		.enabled = true,
> +	};
> +	int ret;
> +
> +	freq = clamp(freq, 0, st->info->throughput);

freq == 0 will become a problem in the next code line. Increase the
lower limit of the clamp to 1?!

> +	cnv_state.period = DIV_ROUND_CLOSEST_ULL(GIGA, freq);

Is ROUND_CLOSEST really the right thing here? The resulting period might
result in a frequency higher than freq, still more given that
pwm_apply_might_sleep() might round the period further down.

> +	ret = pwm_apply_might_sleep(st->cnv, &cnv_state);
> +	if (ret)
> +		return ret;
> +
> +	st->sampling_freq = freq;
> +
> +	return 0;
> +}
> [...]
> +static int ad485x_probe(struct spi_device *spi)
> +{
> [...]
> +	st->cnv = devm_pwm_get(&spi->dev, NULL);
> +	if (IS_ERR(st->cnv))
> +		return PTR_ERR(st->cnv);

devm_pwm_get() is silent on error, so adding an error message here would
be appropriate. I think the same applies to spi_get_device_match_data()
below.

> +	st->info = spi_get_device_match_data(spi);
> +	if (!st->info)
> +		return -ENODEV;
> [...]

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)