On Sat, 8 Jul 2023 12:58:25 +0530
Jishnu Prakash <quic_jprakash@xxxxxxxxxxx> wrote:
> The name used initially for this version of Qualcomm Technologies, Inc.
> PMIC ADC was ADC7, following the convention of calling the PMIC generation
> PMIC7. However, the names were later amended internally to ADC5 Gen2 and
> PMIC5 Gen2. In addition, the latest PMIC generation now is known as
> PMIC5 Gen3 with ADC5 Gen3 supported on it. With this addition, it makes more
> sense to correct the name for this version of ADCs to ADC5 Gen2 from ADC7.
> Since this affects ADC devices across some PMICs, update the names accordingly.
>
> In order to avoid breaking the existing implementations of ADC7, add
> support for ADC5 Gen2 first now and remove the ADC7 support in a later
> patch.
>
> Signed-off-by: Jishnu Prakash <quic_jprakash@xxxxxxxxxxx>
Hi Jishnu.
Whilst I can appreciate why you've picked this particular approach to
deal with the renames I'm not sure it's the smoothest path - or the
easiest to review.
If doing a single patch for the complete rename was too much, perhaps
doing one header (or if it makes sense set of headers)
at a time would be easier to read? With a final patch doing the compatible
addition. Maybe let's see what other reviewers think though.
A few other comments inline,
Jonathan
> ---
> .../bindings/iio/adc/qcom,spmi-vadc.yaml | 21 +++--
> .../bindings/thermal/qcom-spmi-adc-tm5.yaml | 16 ++--
> .../iio/qcom,spmi-adc5-gen2-pm8350.h | 64 +++++++++++++
> .../iio/qcom,spmi-adc5-gen2-pm8350b.h | 89 +++++++++++++++++++
> .../iio/qcom,spmi-adc5-gen2-pmk8350.h | 47 ++++++++++
> .../iio/qcom,spmi-adc5-gen2-pmr735a.h | 29 ++++++
> .../iio/qcom,spmi-adc5-gen2-pmr735b.h | 28 ++++++
> include/dt-bindings/iio/qcom,spmi-vadc.h | 77 ++++++++++++++++
> 8 files changed, 354 insertions(+), 17 deletions(-)
> create mode 100644 include/dt-bindings/iio/qcom,spmi-adc5-gen2-pm8350.h
> create mode 100644 include/dt-bindings/iio/qcom,spmi-adc5-gen2-pm8350b.h
> create mode 100644 include/dt-bindings/iio/qcom,spmi-adc5-gen2-pmk8350.h
> create mode 100644 include/dt-bindings/iio/qcom,spmi-adc5-gen2-pmr735a.h
> create mode 100644 include/dt-bindings/iio/qcom,spmi-adc5-gen2-pmr735b.h
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
> index ad7d6fc49de5..f886977de165 100644
> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
> @@ -13,7 +13,7 @@ maintainers:
> description: |
> SPMI PMIC voltage ADC (VADC) provides interface to clients to read
> voltage. The VADC is a 15-bit sigma-delta ADC.
> - SPMI PMIC5/PMIC7 voltage ADC (ADC) provides interface to clients to read
> + SPMI PMIC5/PMIC5 Gen2 voltage ADC (ADC) provides interface to clients to read
> voltage. The VADC is a 16-bit sigma-delta ADC.
>
> properties:
> @@ -27,6 +27,7 @@ properties:
> - qcom,spmi-adc5
> - qcom,spmi-adc-rev2
> - qcom,spmi-adc7
> + - qcom,spmi-adc5-gen2
Alphabetical order (roughly given currently list). So I'd stick
this after qcom,spmi-adc5
>
> reg:
> description: VADC base address in the SPMI PMIC register map
> @@ -71,7 +72,7 @@ patternProperties:
> description: |
> ADC channel number.
> See include/dt-bindings/iio/qcom,spmi-vadc.h
> - For PMIC7 ADC, the channel numbers are specified separately per PMIC
> + For PMIC5 Gen2 ADC, the channel numbers are specified separately per PMIC
> in the PMIC-specific files in include/dt-bindings/iio/.
>
> label:
> @@ -114,7 +115,7 @@ patternProperties:
> channel calibration. If property is not found, channel will be
> calibrated with 0.625V and 1.25V reference channels, also
> known as absolute calibration.
> - - For compatible property "qcom,spmi-adc5", "qcom,spmi-adc7" and
> + - For compatible property "qcom,spmi-adc5", "qcom,spmi-adc5-gen2" and
> "qcom,spmi-adc-rev2", if this property is specified VADC will use
> the VDD reference (1.875V) and GND for channel calibration. If
> property is not found, channel will be calibrated with 0V and 1.25V
> @@ -213,7 +214,9 @@ allOf:
> properties:
> compatible:
> contains:
> - const: qcom,spmi-adc7
> + enum :
> + - qcom,spmi-adc7
There is a deprecated marking for dt-bindings. Might be good to use it here.
> + - qcom,spmi-adc5-gen2
>
> then:
> patternProperties:
> @@ -277,8 +280,8 @@ examples:
> };
>U>;
> + io-channels = <&pmk8350_vadc PMK8350_ADC5_GEN2_AMUX_THM1_100K_PU>;
> qcom,decimation = <340>;
> qcom,ratiometric;
> qcom,hw-settle-time-us = <200>;
> @@ -251,7 +251,7 @@ examples:
>
> conn-therm@1 {
> reg = <1>;
> - io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
> + io-channels = <&pmk8350_vadc PM8350_ADC5_GEN2_AMUX_THM4_100K_PU(1)>;
> qcom,avg-samples = <2>;
> qcom,ratiometric;
> qcom,hw-settle-time-us = <200>;
