Re: [PATCH V7 3/5] dt-bindings: iio: adc: Add support for QCOM PMIC5 Gen3 ADC

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On Tue, Aug 26, 2025 at 02:06:55PM +0530, Jishnu Prakash wrote:
> For the PMIC5-Gen3 type PMICs, ADC peripheral is present in HW for the
> following PMICs: PMK8550, PM8550, PM8550B and PM8550VX PMICs.
> 
> It is similar to PMIC5-Gen2, with SW communication to ADCs on all PMICs
> going through PBS(Programmable Boot Sequence) firmware through a single
> register interface. This interface is implemented on SDAM (Shared
> Direct Access Memory) peripherals on the master PMIC PMK8550 rather
> than a dedicated ADC peripheral.
> 
> Add documentation for PMIC5 Gen3 ADC and macro definitions for ADC
> channels and virtual channels (combination of ADC channel number and
> PMIC SID number) per PMIC, to be used by clients of this device. Also
> update SPMI PMIC bindings to allow ADC5 Gen3 as adc@ subnode.
> 
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
> ---
> Changes since v6:
> - Updated SPMI PMIC bindings to allow ADC5 Gen3 as adc@ subnode and
>   copyright license in newly added files.

So you did not implement my requests/comments from v5? I did not request
above, I had many, many other comments.



> 
> Changes since v5:
> - Added more details in binding description explaining how number
>   of SDAM peripherals used for ADC is allocated per SoC.
> - Renamed per-PMIC binding files listing ADC channel macro names 
>   and addressed other reviewer comments.
> 
> Changes since v4:
> - Added ADC5 Gen3 documentation in a separate new file to avoid complicating
>   existing VADC documentation file further to accomodate this device, as
>   suggested by reviewer.
> 
> Changes since v3:
> - Added ADC5 Gen3 documentation changes in existing qcom,spmi-vadc.yaml file
>   instead of adding separate file and updated top-level constraints in documentation
>   file based on discussion with reviewers.
> - Dropped default SID definitions.
> - Addressed other reviewer comments.
> 
> Changes since v2:
> - Moved ADC5 Gen3 documentation into a separate new file.
> 
> Changes since v1:
> - Updated properties separately for all compatibles to clarify usage
>   of new properties and updates in usage of old properties for ADC5 Gen3.
> - Avoided updating 'adc7' name to 'adc5 gen2' and just left a comment
>   mentioning this convention.
> - Used predefined channel IDs in individual PMIC channel definitions
>   instead of numeric IDs.
> - Addressed other comments from reviewers.
> 
>  .../bindings/iio/adc/qcom,spmi-adc5-gen3.yaml | 155 ++++++++++++++++++
>  .../iio/adc/qcom,spmi-vadc-common.yaml        |   4 +-
>  .../bindings/iio/adc/qcom,spmi-vadc.yaml      |   2 +
>  .../bindings/mfd/qcom,spmi-pmic.yaml          |   1 +
>  .../iio/adc/qcom,pm8550-adc5-gen3.h           |  46 ++++++
>  .../iio/adc/qcom,pm8550b-adc5-gen3.h          |  85 ++++++++++
>  .../iio/adc/qcom,pm8550vx-adc5-gen3.h         |  22 +++
>  .../iio/adc/qcom,pmk8550-adc5-gen3.h          |  52 ++++++
>  include/dt-bindings/iio/adc/qcom,spmi-vadc.h  |  79 +++++++++
>  9 files changed, 444 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5-gen3.yaml
>  create mode 100644 include/dt-bindings/iio/adc/qcom,pm8550-adc5-gen3.h
>  create mode 100644 include/dt-bindings/iio/adc/qcom,pm8550b-adc5-gen3.h
>  create mode 100644 include/dt-bindings/iio/adc/qcom,pm8550vx-adc5-gen3.h
>  create mode 100644 include/dt-bindings/iio/adc/qcom,pmk8550-adc5-gen3.h
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5-gen3.yaml b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5-gen3.yaml
> new file mode 100644
> index 000000000000..40eb20b9d9de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5-gen3.yaml
> @@ -0,0 +1,155 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/qcom,spmi-adc5-gen3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm's SPMI PMIC ADC5 Gen3
> +
> +maintainers:
> +  - Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
> +
> +description: |
> +  SPMI PMIC5 Gen3 voltage ADC (ADC) provides interface to clients to read
> +  voltage. It is a 16-bit sigma-delta ADC. It also performs the same thermal
> +  monitoring function as the existing ADC_TM devices.
> +
> +  The interface is implemented on SDAM (Shared Direct Access Memory) peripherals
> +  on the master PMIC rather than a dedicated ADC peripheral. The number of PMIC
> +  SDAM peripherals allocated for ADC is not correlated with the PMIC used, it is
> +  programmed in FW (PBS) and is fixed per SOC, based on the SOC requirements.
> +  All boards using a particular (SOC + master PMIC) combination will have the
> +  same number of ADC SDAMs supported on that PMIC.
> +
> +properties:
> +  compatible:
> +    const: qcom,spmi-adc5-gen3
> +
> +  reg:
> +    items:
> +      - description: SDAM0 base address in the SPMI PMIC register map
> +      - description: SDAM1 base address
> +    minItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  '#io-channel-cells':
> +    const: 1
> +
> +  "#thermal-sensor-cells":

Nothing improved here, still mess with quotes.

I am not going to check the rest of comments, because:
1. Your changelog is vague and claims you did not implement them,
2. b4 diff does not work, base-commit is unknown.
3. Main changelog is even more vague.

You make it difficult for us to review your patches, fine. You will get:

NAK (plus one more comment below)

> diff --git a/include/dt-bindings/iio/adc/qcom,spmi-vadc.h b/include/dt-bindings/iio/adc/qcom,spmi-vadc.h
> index ef07ecd4d585..b1b89e874316 100644
> --- a/include/dt-bindings/iio/adc/qcom,spmi-vadc.h
> +++ b/include/dt-bindings/iio/adc/qcom,spmi-vadc.h
> @@ -300,4 +300,83 @@
>  #define ADC7_SBUx				0x94
>  #define ADC7_VBAT_2S_MID			0x96
>  
> +/* ADC channels for PMIC5 Gen3 */
> +
> +#define ADC5_GEN3_REF_GND			0x00
> +#define ADC5_GEN3_1P25VREF			0x01
> +#define ADC5_GEN3_VREF_VADC			0x02
> +#define ADC5_GEN3_DIE_TEMP			0x03
> +
> +#define ADC5_GEN3_AMUX1_THM			0x04
> +#define ADC5_GEN3_AMUX2_THM			0x05
> +#define ADC5_GEN3_AMUX3_THM			0x06
> +#define ADC5_GEN3_AMUX4_THM			0x07
> +#define ADC5_GEN3_AMUX5_THM			0x08
> +#define ADC5_GEN3_AMUX6_THM			0x09
> +#define ADC5_GEN3_AMUX1_GPIO			0x0a
> +#define ADC5_GEN3_AMUX2_GPIO			0x0b
> +#define ADC5_GEN3_AMUX3_GPIO			0x0c
> +#define ADC5_GEN3_AMUX4_GPIO			0x0d
> +
> +#define ADC5_GEN3_CHG_TEMP			0x10
> +#define ADC5_GEN3_USB_SNS_V_16			0x11
> +#define ADC5_GEN3_VIN_DIV16_MUX			0x12
> +#define ADC5_GEN3_VREF_BAT_THERM		0x15

You cannot have empty spaces in ID constants. These are abstract
numbers.

Otherwise please point me to driver using this constant.

Best regards,
Krzysztof