| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b5707f37-cc5d-47fb-a8d6-a1da8a9a7ff1@oss.qualcomm.com>
Date: Wed, 26 Feb 2025 14:21:40 +0530
From: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: jic23@...nel.org, 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
Hi Krzysztof,
On 2/2/2025 7:08 PM, Krzysztof Kozlowski wrote:
> On Sat, Feb 01, 2025 at 12:02:40AM +0530, Jishnu Prakash wrote:
>
>> 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
>> + 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.
>
> Don't wrap at 61, but at 80. See Coding style.
>
>> +
>> +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
>
> Why is this flexible?
>
>> +
>> + '#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
>
> Same question.
To explain why "reg" and "interrupts" are flexible:
We need to add one item under each of these properties, per ADC SDAM. 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.
The number of ADC SDAMs used on a given SOC with a given PMIC (like PMK8550) will be fixed, but it is possible for
the same PMIC to have 1 of its SDAMs allocated for ADC when used on one SOC and 2 SDAMs allocated for ADC when used on another SOC.
All boards using a particular (SOC + PMIC) combination will have the same number of ADC SDAMs supported on that PMIC.
>
>> +
>> + interrupt-names:
>> + items:
>> + - const: sdam0
>> + - const: sdam1
>> + minItems: 1
>
> So basically interrupt 0 and 1, just drop the names property entirely.
>
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - '#address-cells'
>> + - '#size-cells'
>> + - '#io-channel-cells'
>> + - interrupts
>> + - interrupt-names
>> +
>> +patternProperties:
>
> required block always goes after all properties, not in the middle.
>
>> + "^channel@[0-9a-f]+$":
>> + type: object
>> + unevaluatedProperties: false
>> + description: |
>
> Do not need '|' unless you need to preserve formatting.
>
>> + Represents the external channels which are connected to the ADC.
>> + $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 ]
>> + 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>;
>> + };
>> + };
>> + };
>> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc-common.yaml b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc-common.yaml
>> index cd087911ee88..1531153e6ea8 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc-common.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc-common.yaml
>> @@ -17,8 +17,8 @@ properties:
>> description:
>> ADC channel number.
>> See include/dt-bindings/iio/adc/qcom,spmi-vadc.h
>> - For PMIC7 ADC, the channel numbers are specified separately per PMIC
>> - in the PMIC-specific files in include/dt-bindings/iio/adc.
>> + For PMIC7 ADC and PMIC5 Gen3 ADC, the channel numbers are specified
>> + separately per PMIC in the PMIC-specific files in include/dt-bindings/iio/adc.
>> maxItems: 1
>>
>> label:
>> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
>> index b0ccad00c1a6..b77af38440fe 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
>> @@ -15,6 +15,8 @@ description: |
>> voltage. The VADC is a 15-bit sigma-delta ADC.
>> SPMI PMIC5/PMIC7 voltage ADC (ADC) provides interface to clients to read
>> voltage. The VADC is a 16-bit sigma-delta ADC.
>> + Note that PMIC7 ADC is the generation between PMIC5 and PMIC5 Gen3 ADC,
>> + it can be considered like PMIC5 Gen2.
>>
>> properties:
>> compatible:
>> diff --git a/include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550.h b/include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550.h
>> new file mode 100644
>> index 000000000000..9940715683b4
>> --- /dev/null
>> +++ b/include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550.h
>
>
> Which is the name/model of the device? PM8550? If so, then this must
> follow standard compatible naming, so qcom,pm8550-foo-bar. Unless device
> name is somehow different.
>
> Also drop redundant pieces here - can it be anything else than SPMI?
> Like I2C?
OK, I'll update this file's name to qcom,pm8550-adc5-gen3.h and update the other file names similarly.
I'll also address your other comments in the next patch series.
Thanks,
Jishnu
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists