lkml.org 
[lkml]   [2023]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH 06/11] iio: adc: Add QCOM PMIC5 Gen3 ADC bindings
From
Hi Krzysztof,

On 7/9/2023 10:53 PM, Krzysztof Kozlowski wrote:
> On 08/07/2023 09:28, 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 firmware through a single register interface. This
>> interface is implemented on an SDAM peripheral on the master PMIC PMK8550
>> rather than a dedicated ADC peripheral.
>>
>> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
>> ---
>> properties:
>> compatible:
>> @@ -27,10 +27,11 @@ properties:
>> - qcom,spmi-adc5
>> - qcom,spmi-adc-rev2
>> - qcom,spmi-adc5-gen2
>> + - qcom,spmi-adc5-gen3
>
> This could be ordered...


Yes, will do that in the next patchset.


>>
>> reg:
>> description: VADC base address in the SPMI PMIC register map
>> - maxItems: 1
>> + minItems: 1
> Why? This does not make any sense. With previous patches it looks like
> random set of changes.


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
>>
>>
>> + qcom,adc-tm-type:
>> + description: |
>> + Indicates if ADC_TM monitoring is done on this channel.
> Description does not match property name.


You mean it sounds more like an enum which can take several values
rather than just a boolean? I can update it to "qcom,adc-tm" if that
looks better.


>
>> + Defined for compatible property "qcom,spmi-adc5-gen3".
> Then you need if:then:.


Yes, will update this explicitly for ADC5 Gen3 in the next patchset.


>
>> + type: boolean
>> +
>> required:
>> - reg
>>
>> @@ -213,7 +227,9 @@ allOf:
>> properties:
>> compatible:
>> contains:
>> - const: qcom,spmi-adc5-gen2
>> + enum:
>> + - qcom,spmi-adc5-gen2
>> + - qcom,spmi-adc5-gen3
>>
>> then:
>> patternProperties:
>> @@ -299,7 +315,7 @@ examples:
>> label = "xo_therm";
>> };
>>
>> - channel@47 {
>> + channel@147 {
> Why?


It would be needed if this channel number was supposed to be the virtual
channel number made by combining PMIC SID and actual channel
number....but I could drop it for now and do it in a separate fix as
Jonathan suggested.


>
>> reg = <PM8350_ADC5_GEN2_AMUX_THM4_100K_PU(1)>;
>> qcom,ratiometric;
>> qcom,hw-settle-time = <200>;
>> @@ -307,3 +323,80 @@ examples:
>> };
>> };
>> };
>> +
>> + - |
>> + #include <dt-bindings/iio/qcom,spmi-adc5-gen3-pmk8550.h>
>> + #include <dt-bindings/iio/qcom,spmi-adc5-gen3-pm8550.h>
>> + #include <dt-bindings/iio/qcom,spmi-adc5-gen3-pm8550b.h>
>> + #include <dt-bindings/iio/qcom,spmi-adc5-gen3-pm8550vx.h>
>> +
>> + pmic {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + /* VADC node */
>> + pmk8550_vadc: vadc@9000 {
>> + compatible = "qcom,spmi-adc5-gen3";
> Don't add new examples which differ only in compatible.


This example does have differences unique to ADC5 Gen3 such as use of
"#thermal-sensor-cells" and "qcom,adc-tm-type" properties....to make it
clearer, I'll delete some of the excess nodes which don't highlight
these differences.

>
>
>> diff --git a/include/dt-bindings/iio/qcom,spmi-adc5-gen3-pm8550.h b/include/dt-bindings/iio/qcom,spmi-adc5-gen3-pm8550.h
>> new file mode 100644
>> index 000000000000..74e6e2f6f9ed
>> --- /dev/null
>> +++ b/include/dt-bindings/iio/qcom,spmi-adc5-gen3-pm8550.h
>> @@ -0,0 +1,48 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
> Dual license.


I think we do have an internal rule by which we do have to add these two
licenses....I'll check again and update them if required.
Thanks,

Jishnu

>
>
>
> Best regards,
> Krzysztof
>

\
 
 \ /
  Last update: 2023-10-23 08:15    [W:0.123 / U:1.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site