Hi Krzysztof,
On 10/23/2023 12:06 PM, Krzysztof Kozlowski wrote:
On 23/10/2023 08:14, Jishnu Prakash wrote:
Hi Krzysztof,
On 7/9/2023 10:53 PM, Krzysztof Kozlowski wrote:
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.
You cannot remove constraints from an entry.
In this case, minItems: 1 will remain true for all other ADC devices
documented here, but it will not be true for ADC5 Gen3, as this one can
have multiple base addresses if more than one SDAM is used for ADC. I'll
update this separately for each compatible, keeping it the same for the
older ones, hope that should work.
'#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.
The property name suggests this is type of monitoring. Property
description says this will enable ADC_TM monitoring. These two do not match.
Except that I wonder now whether this is a property of hardware at
all... What is this monitoring? By the driver?
The property description is right, this property is used to indicate
that one of the configurable channels on the ADC SDAMs will be used for
ADC_TM functionality, for periodically monitoring this particular ADC
channel . This is the exact same functionality as in the existing QCOM
ADC_TM device, documented at
devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml. I'll mention this
too in the description.
It can be considered a property of the hardware as the monitoring is
done by a sequence under PBS (Programmable Boot Sequence, can be
considered firmware), which periodically gets the channel reading and
checks it against upper/lower thresholds set by clients of this driver,
for threshold violations.
...
then:
patternProperties:
@@ -299,7 +315,7 @@ examples:
label = "xo_therm";
};
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.
Just to be clear: your internal rules are your internal affair. We
expect here dual license.
I misunderstood what you meant earlier, I understand now that
"GPL-2.0-only" is wrong, I'll update it.
Thanks,
Jishnu
Best regards,
Krzysztof
