Hi Krzysztof,
(Resending this mail for tracking on mailing lists, as it got rejected
from lists the first time due to HTML)
On 1/4/2024 1:48 PM, Krzysztof Kozlowski wrote:
On 31/12/2023 18:12, 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(Programmable Boot Sequence) firmware through a single
register interface. This interface is implemented on an SDAM (Shared
Direct Access Memory) peripheral 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.
Changes since v2:
- Moved ADC5 Gen3 documentation into a separate new file.
Changelog goes under ---.
Why did you do this? What is the rationale? Sorry, this patchset goes
nowhere.
I'll elaborate this more in the next patchset. There are two main
reasons for adding this documentation in a new file:
1.This device is not exactly like the existing QCOM VADC drivers as it
now combines VADC functionality (reading ADC channel on client request)
with ADC_TM functionality (thermal threshold monitoring).
2.Adding this device's bindings in the existing qcom,spmi-vadc.yaml file
is not possible as it would require updating some of the existing
top-level constraints. (for the older devices in that file, "reg" and
"interrupts" can have at most one item, while this device can have more
than one item under these properties.)
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.
+ per PMIC in the PMIC-specific files in include/dt-bindings/iio/adc.
+
+ label:
+ $ref: /schemas/types.yaml#/definitions/string
Why do you need it in the first place? Don't you miss some $ref?
This is just meant to show the ADC channel name in DT for our reference.
I'll check if I can use adc.yaml, which includes this property already,
as a reference in this case.
+ description: |
Do not need '|' unless you need to preserve formatting. Applies everywhere.
+ ADC input of the platform as seen in the schematics.
+ For thermistor inputs connected to generic AMUX or GPIO inputs
+ these can vary across platform for the same pins. Hence select
+ the platform schematics name for this channel.
+
+
+ qcom,adc-tm:
+ description: |
+ Indicates if ADC_TM monitoring is done on this channel.
+ Defined for compatible property "qcom,spmi-adc5-gen3".
+ This is the same functionality as in the existing QCOM ADC_TM
+ device, documented at devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml.
+ type: boolean
+
Why do you duplicate entire vadc file? Why it cannot be part of that
file? Oh wait, it was in v2.
You now duplicated a lot of property definitions without clear reason.
If this is intention, then you need to put them in common schema.
Many of the properties used for earlier QCOM VADC devices will be used
for this device too.....do you mean I can add a new schema file (named
something like qcom,vadc.yaml) and move common properties into it (like
qcom,hw-settle-time, qcom,decimation, etc) from this file and
qcom,spmi-vadc.yaml?
Dmitry suggests doing it in a separate patch, I'll follow his suggestion
if there's no issue.
diff --git a/include/dt-bindings/iio/adc/qcom,spmi-adc7-smb139x.h b/include/dt-bindings/iio/adc/qcom,spmi-adc7-smb139x.h
index c0680d1285cf..750a526af2c1 100644
--- a/include/dt-bindings/iio/adc/qcom,spmi-adc7-smb139x.h
+++ b/include/dt-bindings/iio/adc/qcom,spmi-adc7-smb139x.h
@@ -6,7 +6,7 @@
#ifndef _DT_BINDINGS_QCOM_SPMI_VADC_SMB139X_H
#define _DT_BINDINGS_QCOM_SPMI_VADC_SMB139X_H
-#include <dt-bindings/iio/qcom,spmi-vadc.h>
+#include <dt-bindings/iio/adc/qcom,spmi-vadc.h>
? How is it related?
This should have gone into patch 1, I'll fix it in the next patch series.
I'll address all your other comments in the next patchset.
Thanks,
Jishnu
#define SMB139x_1_ADC7_SMB_TEMP (SMB139x_1_SID << 8 | ADC7_SMB_TEMP)
#define SMB139x_1_ADC7_ICHG_SMB (SMB139x_1_SID << 8 | ADC7_ICHG_SMB)
diff --git a/include/dt-bindings/iio/adc/qcom,spmi-vadc.h b/include/dt-bindings/iio/adc/qcom,spmi-vadc.h
index ef07ecd4d585..cfe653d945a4 100644
--- a/include/dt-bindings/iio/adc/qcom,spmi-vadc.h
+++ b/include/dt-bindings/iio/adc/qcom,spmi-vadc.h
@@ -1,6 +1,8 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
* Copyright (c) 2012-2014,2018,2020 The Linux Foundation. All rights reserved.
+ *
Drop stray blank line
Best regards,
Krzysztof
