On 04/06/2024 10:55, AngeloGioacchino Del Regno wrote: > Il 01/06/24 17:32, Krzysztof Kozlowski ha scritto: >> On 30/05/2024 11:34, AngeloGioacchino Del Regno wrote: >>> Add a new binding for the MT6350 Series (MT6357/8/9) PMIC AUXADC, >>> providing various ADC channels for both internal temperatures and >>> voltages, audio accessory detection (hp/mic/hp+mic and buttons, >>> usually on a 3.5mm jack) other than some basic battery statistics >>> on boards where the battery is managed by this PMIC. >>> >>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> >>> --- >>> .../iio/adc/mediatek,mt6359-auxadc.yaml | 43 +++++++++++++++++++ >>> .../iio/adc/mediatek,mt6357-auxadc.h | 21 +++++++++ >>> .../iio/adc/mediatek,mt6358-auxadc.h | 22 ++++++++++ >>> .../iio/adc/mediatek,mt6359-auxadc.h | 22 ++++++++++ >>> 4 files changed, 108 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/adc/mediatek,mt6359-auxadc.yaml >>> create mode 100644 include/dt-bindings/iio/adc/mediatek,mt6357-auxadc.h >>> create mode 100644 include/dt-bindings/iio/adc/mediatek,mt6358-auxadc.h >>> create mode 100644 include/dt-bindings/iio/adc/mediatek,mt6359-auxadc.h >>> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt6359-auxadc.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt6359-auxadc.yaml >>> new file mode 100644 >>> index 000000000000..dd6c331905cf >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt6359-auxadc.yaml >>> @@ -0,0 +1,43 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/iio/adc/mediatek,mt6359-auxadc.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: MediaTek MT6350 series PMIC AUXADC >>> + >>> +maintainers: >>> + - AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> >>> + >>> +description: >>> + The Auxiliary Analog/Digital Converter (AUXADC) is an ADC found >>> + in some MediaTek PMICs, performing various PMIC related measurements >>> + such as battery and PMIC internal voltage regulators temperatures, >>> + accessory detection resistance (usually, for a 3.5mm audio jack) >>> + other than voltages for various PMIC internal components. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - mediatek,mt6357-auxadc >>> + - mediatek,mt6358-auxadc >>> + - mediatek,mt6359-auxadc >>> + >>> + "#io-channel-cells": >>> + const: 1 >>> + >>> +additionalProperties: false >> >> If there is going to be a re-spin, please move this below required: block. >> > > Yep, will do. Fixed up for v2. > >>> + >>> +required: >>> + - compatible >>> + - "#io-channel-cells" >>> + >>> +examples: >>> + - | >>> + pmic { >>> + pmic_adc: adc { >>> + compatible = "mediatek,mt6359-auxadc"; >>> + #io-channel-cells = <1>; >>> + }; >> >> This suggests that you should grow (make complete) the parent PMIC example. > > Uhm, should I instead add that to bindings/mfd/mediatek,mt6357.yaml and avoid > growing the parent example? No. You should avoid this example and grow the parent example. Not avoid parent example... > > adc: > type: object > $ref: /schemas/iio/adc/mediatek,mt6359-auxadc.yaml > unevaluatedProperties: false > >> >> Actually having this as a separate node is not really needed. Why it >> cannot be just part of the MFD/parent node? >> > > (glossary: PWRAP = PMIC [SPI] WRAPper) > > The top node is the PWRAP, an IP that is (mostly) used to dispatch commands to > the PMIC, but the AUXADC is not integrated into the PWRAP, but into the PMIC. Eh? mediatek,mt6357.yaml says it is a PMIC... > > Declaring the AUXADC as a PWRAP child would therefore be an incorrect description > of the hardware. > > P.S.: not necessary, but to "complete the circle" ... note that the PWRAP can be > skipped on some SoCs/firmwares/configurations, even though afaik that's only > mostly for debugging purposes, it's not granted that you have pwrap between > SoC and PMIC on all SoCs/fws/confs, even though, all of the boards that are > supported upstream do have it and do require it. Best regards, Krzysztof