Re: [PATCH v4 1/4] arm64: dts: qcom: Add header file for ADC5 Gen3 channel macros

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Device tree binding header is improperly placed in an architecture-specific directory instead of `include/dt-bindings/`.
- [Medium] Function-like macros share exact names with object-like macros used in the driver, causing preprocessor namespace collisions.
--

--- Patch [1]: [PATCH v4 1/4] arm64: dts: qcom: Add header file for ADC5 Gen3 channel macros ---
commit 59d21969abff768469fefb21430c2e819299df4b
Author: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>

arm64: dts: qcom: Add header file for ADC5 Gen3 channel macros
    
This commit adds macro definitions for virtual channels (combining the
ADC channel number and PMIC SID number) to be used in the devicetree by
clients of the ADC5 GEN3 device.

> diff --git a/arch/arm64/boot/dts/qcom/qcom-adc5-gen3.h b/arch/arm64/boot/dts/qcom/qcom-adc5-gen3.h
> new file mode 100644
> index 0000000000000..aa8e54d7e786a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcom-adc5-gen3.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef __DTS_ARM64_QCOM_ADC5_GEN3_H__

[Severity: Medium]
Should this device tree binding header be placed in include/dt-bindings/
instead of arch/arm64/boot/dts/qcom/?

Headers that define hardware channel IDs form the ABI between the devicetree
and the operating system drivers. Placing this in an architecture-specific
directory makes it inaccessible to C drivers or other architectures.

Moving it to include/dt-bindings/ would allow it to be shared universally and
parsed by standard DT schema validation tools. The include guard also appears
to couple the file unnecessarily to ARM64.

> +#define __DTS_ARM64_QCOM_ADC5_GEN3_H__
> +
> +/* ADC channels for PMIC5 Gen3 */
> +
> +#define VIRT_CHAN(sid, chan)			((sid) << 8 | (chan))
> +
> +#define ADC5_GEN3_REF_GND(sid)			VIRT_CHAN(sid, 0x00)

[Severity: Medium]
Will this function-like macro collide with the object-like macros already
used in the driver?

The corresponding IIO ADC driver already uses identically named object-like
macros, such as #define ADC5_GEN3_REF_GND 0x00.

If the C driver includes this header to avoid duplicating channel
definitions, using the exact same identifier for both a function-like macro
and an object-like macro will trigger preprocessor redefinition errors.

Could the base channel IDs be defined as object-like constants, and a
distinctly named helper macro be provided to construct the virtual channel?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-adc5_gen3_dt-v4-0-355f0edece77@oss.qualcomm.com?part=1