Re: [PATCH 06/11] iio: adc: Add QCOM PMIC5 Gen3 ADC bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonathan,

On 7/8/2023 8:42 PM, Jonathan Cameron wrote:
On Sat, 8 Jul 2023 12:58:30 +0530
Jishnu Prakash <quic_jprakash@xxxxxxxxxxx> 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
reg:
      description: VADC base address in the SPMI PMIC register map
-    maxItems: 1
+    minItems: 1
?

The idea here is to convey that reg can have multiple values for ADC5 Gen3 as there can be more than one peripheral used for ADC, so there can be multiple base addresses. I'll try to make this more clear in the next patchset.



'#address-cells':
      const: 1
@@ -38,6 +39,12 @@ properties:
    '#size-cells':
      const: 0
+ "#thermal-sensor-cells":
+    const: 1
+    description:
+      Number of cells required to uniquely identify the thermal sensors. Since
+      we have multiple sensors this is set to 1.
+
Defined only for the new gen3?  If so make make it false for the other devices.


Yes, will do that in the next patchset.


    '#io-channel-cells':
      const: 1
@@ -71,8 +78,8 @@ patternProperties:
          description: |
            ADC channel number.
+ qcom,adc-tm-type:
+        description: |
+            Indicates if ADC_TM monitoring is done on this channel.
+            Defined for compatible property "qcom,spmi-adc5-gen3".
+        type: boolean
Enforce that in the binding, not via a comment. Once the binding
performs that check (set it to false for non matching compatibles) then
there is no need to also mention it in text.

Will do that in the next patchset.



+
      required:
        - reg
@@ -213,7 +227,9 @@ allOf:
        properties:
          compatible:
            contains:
-            const: qcom,spmi-adc5-gen2
+            enum:
+              - qcom,spmi-adc5-gen2
+              - qcom,spmi-adc5-gen3
Side note - it's fine to have a single element enum, so you could
use that option to reduce churn in this set...


I think we can remove this and instead specify properties explicitly for qcom,spmi-adc5-gen3 too separately in the next patchset.



then:
        patternProperties:
@@ -299,7 +315,7 @@ examples:
                  label = "xo_therm";
              };
- channel@47 {
+            channel@147 {
?  If that's a valid change, then it looks like a separate fix.


I think I can avoid this for now, although it would be needed if this channel number was the virtual channel number made by combining PMIC SID and actual channel number....maybe we can do it in a separate fix.



                  reg = <PM8350_ADC5_GEN2_AMUX_THM4_100K_PU(1)>;
                  qcom,ratiometric;
                  qcom,hw-settle-time = <200>;
@@ -307,3 +323,80 @@ examples:
              };
          };
      };
+
+#ifndef PM8550_SID
+#define PM8550_SID		1
+#endif
+
+/* ADC channels for PM8550_ADC for PMIC5 Gen3 */
+#define PM8550_ADC5_GEN3_OFFSET_REF			(PM8550_SID << 8 | 0x00)
can we do the naming for the 0x00 as per Dmitry's set? That is get them from
qcom,spmi-vadc.h

https://patchwork.kernel.org/project/linux-iio/patch/20230707123027.1510723-2-dmitry.baryshkov@xxxxxxxxxx/

Yes, will do that in the next patchset.

Thanks,

Jishnu





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux