[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250829-classic-dynamic-clam-addbd8@kuoka>
Date: Fri, 29 Aug 2025 09:19:56 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
Cc: jic23@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, agross@...nel.org, andersson@...nel.org, lumag@...nel.org,
dmitry.baryshkov@....qualcomm.com, konradybcio@...nel.org, daniel.lezcano@...aro.org,
sboyd@...nel.org, amitk@...nel.org, thara.gopinath@...il.com, lee@...nel.org,
rafael@...nel.org, subbaraman.narayanamurthy@....qualcomm.com,
david.collins@....qualcomm.com, anjelique.melendez@....qualcomm.com,
kamal.wadhwa@....qualcomm.com, rui.zhang@...el.com, lukasz.luba@....com,
devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, cros-qcom-dts-watchers@...omium.org,
quic_kotarake@...cinc.com, neil.armstrong@...aro.org, stephan.gerhold@...aro.org,
Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH V7 3/5] dt-bindings: iio: adc: Add support for QCOM PMIC5
Gen3 ADC
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@...wei.com>
> Signed-off-by: Jishnu Prakash <jishnu.prakash@....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@....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
Powered by blists - more mailing lists