linux-kernel - Re: [PATCH v8 6/8] iio: dac: ad3552r: extract common code (no changes in behavior intended)

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: <20241026184754.009ea6f7@jic23-huawei>
Date: Sat, 26 Oct 2024 18:47:54 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Angelo Dureghello <adureghello@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Nuno Sá <nuno.sa@...log.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Olivier Moysan
 <olivier.moysan@...s.st.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, Mark Brown
 <broonie@...nel.org>, dlechner@...libre.com
Subject: Re: [PATCH v8 6/8] iio: dac: ad3552r: extract common code (no
 changes in behavior intended)

On Fri, 25 Oct 2024 11:49:39 +0200
Angelo Dureghello <adureghello@...libre.com> wrote:

> From: Angelo Dureghello <adureghello@...libre.com>
> 
> Extracting common code, to share common code to be used later
> by the AXI driver version (ad3552r-axi.c).
> 
> Signed-off-by: Angelo Dureghello <adureghello@...libre.com>

Hi Angelo,

A few trivial things but one bigger one that actually only becomes a problem
in the next patch so I'll comment on that.

> +
> +MODULE_DESCRIPTION("ad3552r common functions");
Ah. This rang alarm bells.  I'll comment in next patch but you can't link
the same file twice.


> +MODULE_LICENSE("GPL");


> @@ -1072,3 +727,4 @@ module_spi_driver(ad3552r_driver);
>  MODULE_AUTHOR("Mihail Chindris <mihail.chindris@...log.com>");
>  MODULE_DESCRIPTION("Analog Device AD3552R DAC");
>  MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS(IIO_AD3552R);
> diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
> new file mode 100644
> index 000000000000..22bd9ad27c65
> --- /dev/null
> +++ b/drivers/iio/dac/ad3552r.h
> @@ -0,0 +1,226 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * AD3552R Digital <-> Analog converters common header
> + *
> + * Copyright 2021-2024 Analog Devices Inc.
> + * Author: Angelo Dureghello <adureghello@...libre.com>
> + */
> +
> +#ifndef __DRIVERS_IIO_DAC_AD3552R_H__
> +#define __DRIVERS_IIO_DAC_AD3552R_H__
> +
> +/* Register addresses */
> +/* Primary address space */

> +#define   AD3552R_MASK_MULTI_IO_MODE			GENMASK(7, 6)
> +#define   AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE		BIT(2)
> +#define AD3552R_REG_ADDR_INTERFACE_CONFIG_C		0x10
> +#define   AD3552R_MASK_CRC_ENABLE			(GENMASK(7, 6) |\
> +							 GENMASK(1, 0))
If for whatever reason we go around again, (otherwise I might tweak anyway)
#define   AD3552R_MASK_CRC_ENABLE	\
		(GENMASK(7, 6) | GENMASK(1, 0))


> +#define   AD3552R_MASK_CH_OUTPUT_RANGE			GENMASK(7, 0)
> +#define   AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch)		((ch) ? \
> +							 GENMASK(7, 4) : \
> +							 GENMASK(3, 0))
I may tweak this whilst applying to be something like

#define   AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch)	\
		((ch) ? GENMASK(7, 4) : GENMASK(3, 0))


> +/* Useful defines */
Made me laugh.  I hope we don't ever have a comment that says "Useless defines" :)

> +#define AD3552R_MAX_CH					2
> +#define AD3552R_MASK_CH(ch)				BIT(ch)
> +#define AD3552R_MASK_ALL_CH				GENMASK(1, 0)
> +#define AD3552R_MAX_REG_SIZE				3
> +#define AD3552R_READ_BIT				BIT(7)
> +#define AD3552R_ADDR_MASK				GENMASK(6, 0)
> +#define AD3552R_MASK_DAC_12B				GENMASK(15, 4)
> +#define AD3552R_DEFAULT_CONFIG_B_VALUE			0x8
> +#define AD3552R_SCRATCH_PAD_TEST_VAL1			0x34
> +#define AD3552R_SCRATCH_PAD_TEST_VAL2			0xB2
> +#define AD3552R_GAIN_SCALE				1000
> +#define AD3552R_LDAC_PULSE_US				100