[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250201113358.565de2e1@jic23-huawei>
Date: Sat, 1 Feb 2025 11:33:58 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
agross@...nel.org, andersson@...nel.org, dmitry.baryshkov@...aro.org,
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,
quic_kamalw@...cinc.com, rui.zhang@...el.com, lukasz.luba@....com,
lars@...afoo.de, 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_skakitap@...cinc.com, neil.armstrong@...aro.org
Subject: Re: [PATCH V5 3/5] dt-bindings: iio: adc: Add support for QCOM
PMIC5 Gen3 ADC
On Sat, 1 Feb 2025 00:02:40 +0530
Jishnu Prakash <jishnu.prakash@....qualcomm.com> 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.
A few really minor comments inline given you are going to be doing a v6
for the build errors Rob's bot found. I guess this is racing
with some other changes.
>
> Signed-off-by: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
> ---
> 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 | 157 ++++++++++++++++++
> .../iio/adc/qcom,spmi-vadc-common.yaml | 4 +-
> .../bindings/iio/adc/qcom,spmi-vadc.yaml | 2 +
> .../iio/adc/qcom,spmi-adc5-gen3-pm8550.h | 46 +++++
> .../iio/adc/qcom,spmi-adc5-gen3-pm8550b.h | 85 ++++++++++
> .../iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h | 22 +++
> .../iio/adc/qcom,spmi-adc5-gen3-pmk8550.h | 52 ++++++
> include/dt-bindings/iio/adc/qcom,spmi-vadc.h | 81 +++++++++
> 8 files changed, 447 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,spmi-adc5-gen3-pm8550.h
> create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550b.h
> create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h
> create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pmk8550.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..d6f2d18623d4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5-gen3.yaml
> @@ -0,0 +1,157 @@
> +# 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
Trivial but slightly short wrap. I think dt bindings use 80 chars
as the limit.
> + 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.
> +
> +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":
> + const: 1
> +
> + interrupts:
> + items:
> + - description: SDAM0 end of conversion (EOC) interrupt
> + - description: SDAM1 EOC interrupt
> + minItems: 1
> +
> + interrupt-names:
> + items:
> + - const: sdam0
> + - const: sdam1
> + minItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - '#address-cells'
> + - '#size-cells'
> + - '#io-channel-cells'
> + - interrupts
> + - interrupt-names
> +
> +patternProperties:
> + "^channel@[0-9a-f]+$":
> + type: object
> + unevaluatedProperties: false
> + description: |
Doesn't look like it needs formatting. So no need for |
> + Represents the external channels which are connected to the ADC.
Maybe better to move ref before description.
> + $ref: /schemas/iio/adc/qcom,spmi-vadc-common.yaml
> +
> + properties:
> + qcom,decimation:
> + enum: [ 85, 340, 1360 ]
> + default: 1360
> +
> + qcom,hw-settle-time:
> + enum: [ 15, 100, 200, 300, 400, 500, 600, 700, 1000, 2000, 4000,
> + 8000, 16000, 32000, 64000, 128000 ]
Might be more readable as groups of 8. We love powers of two ;)
> + default: 15
> +
> + qcom,avg-samples:
> + enum: [ 1, 2, 4, 8, 16 ]
> + default: 1
> +
> + qcom,adc-tm:
> + description:
> + ADC_TM is a threshold monitoring feature in HW which can be enabled on any
> + ADC channel, to trigger an IRQ for threshold violation. In earlier ADC
> + generations, it was implemented in a separate device (documented in
> + Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml.)
> + In Gen3, this feature can be enabled in the same ADC device for any channel
> + and threshold monitoring and IRQ triggering are handled in FW (PBS) instead of
> + another dedicated HW block.
> + This property indicates ADC_TM monitoring is done on this channel.
> + type: boolean
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pmk8550.h>
> + #include <dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550.h>
> + #include <dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550b.h>
> + #include <dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + pmic {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@...0 {
> + compatible = "qcom,spmi-adc5-gen3";
> + reg = <0x9000>, <0x9100>;
> + interrupts = <0x0 0x90 0x1 IRQ_TYPE_EDGE_RISING>,
> + <0x0 0x91 0x1 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "sdam0", "sdam1";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #io-channel-cells = <1>;
> + #thermal-sensor-cells = <1>;
> +
> + /* PMK8550 Channel nodes */
> + channel@3 {
> + reg = <PMK8550_ADC5_GEN3_DIE_TEMP(0)>;
> + label = "pmk8550_die_temp";
> + qcom,pre-scaling = <1 1>;
> + };
> +
> + channel@44 {
> + reg = <PMK8550_ADC5_GEN3_AMUX_THM1_XO_THERM_100K_PU(0)>;
> + label = "pmk8550_xo_therm";
> + qcom,pre-scaling = <1 1>;
> + qcom,ratiometric;
> + qcom,hw-settle-time = <200>;
> + qcom,adc-tm;
> + };
> +
> + /* PM8550 Channel nodes */
> + channel@103 {
> + reg = <PM8550_ADC5_GEN3_DIE_TEMP(1)>;
> + label = "pm8550_die_temp";
> + qcom,pre-scaling = <1 1>;
> + };
> +
> + /* PM8550B Channel nodes */
> + channel@78f {
> + reg = <PM8550B_ADC5_GEN3_VBAT_SNS_QBG(7)>;
> + label = "pm8550b_vbat_sns_qbg";
> + qcom,pre-scaling = <1 3>;
> + };
> +
> + /* PM8550VS_C Channel nodes */
> + channel@203 {
> + reg = <PM8550VS_ADC5_GEN3_DIE_TEMP(2)>;
> + label = "pm8550vs_c_die_temp";
> + qcom,pre-scaling = <1 1>;
> + };
> + };
> + };
Powered by blists - more mailing lists