Re: [PATCH v6 10/11] arm64: dts: mediatek: add properties for MT6359

Conor Dooley
2025-10-30T19:46:36Z

[-- Attachment #1: Type: text/plain, Size: 3932 bytes --]

On Thu, Oct 30, 2025 at 09:44:42PM +0800, Jack Hsu wrote:
> Add properties of rtc fg (Fuel Gauge), external crystal
> and auxadc definition for mt6359 pmic.
>
> Signed-off-by: Jack Hsu <jh.hsu@mediatek.com>
> ---
> arch/arm64/boot/dts/mediatek/mt6359.dtsi | 20 ++++++++++
> include/dt-bindings/iio/mt635x-auxadc.h | 50 ++++++++++++++++++++++++
> 2 files changed, 70 insertions(+)
> create mode 100644 include/dt-bindings/iio/mt635x-auxadc.h
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt6359.dtsi b/arch/arm64/boot/dts/mediatek/mt6359.dtsi
> index 467d8a4c2aa7..cc7053bdd292 100644
> --- a/arch/arm64/boot/dts/mediatek/mt6359.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt6359.dtsi
> @@ -3,6 +3,8 @@
> * Copyright (C) 2022 MediaTek Inc.
> */
>
> +#include <dt-bindings/iio/mt635x-auxadc.h>
> +
> &pwrap {
> pmic: pmic {
> compatible = "mediatek,mt6359";
> @@ -302,6 +304,24 @@ mt6359_vsram_others_sshub_ldo: ldo_vsram_others_sshub {
>
> mt6359rtc: rtc {
> compatible = "mediatek,mt6358-rtc";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + status = "disabled";
> +
> + fginit: fginit {
> + reg = <0 0x1>;
> + bits = <0 8>;
> + };

Uhhhh, what on earth is going on here? This is an RTC, what does
fuelgauge stuff have to do with it? Did you test this at all? What does
it even do? Very confused.

> +
> + fgsoc: fgsoc {
> + reg = <1 0x1>;
> + bits = <0 8>;
> + };
> +
> + ext32k: ext32k {
> + reg = <2 0x1>;
> + bits = <6 1>;
> + };
> };
> };
> };
> diff --git a/include/dt-bindings/iio/mt635x-auxadc.h b/include/dt-bindings/iio/mt635x-auxadc.h
> new file mode 100644
> index 000000000000..69ba13a7b9ec
> --- /dev/null
> +++ b/include/dt-bindings/iio/mt635x-auxadc.h
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
> + */
> +
> +#ifndef _DT_BINDINGS_MT635X_AUXADC_H
> +#define _DT_BINDINGS_MT635X_AUXADC_H
> +
> +/* PMIC MT635x AUXADC channels */
> +#define AUXADC_BATADC 0x00
> +#define AUXADC_ISENSE 0x01
> +#define AUXADC_VCDT 0x02
> +#define AUXADC_BAT_TEMP 0x03
> +#define AUXADC_BATID 0x04
> +#define AUXADC_CHIP_TEMP 0x05
> +#define AUXADC_VCORE_TEMP 0x06
> +#define AUXADC_VPROC_TEMP 0x07
> +#define AUXADC_VGPU_TEMP 0x08
> +#define AUXADC_ACCDET 0x09
> +#define AUXADC_VDCXO 0x0a
> +#define AUXADC_TSX_TEMP 0x0b
> +#define AUXADC_HPOFS_CAL 0x0c
> +#define AUXADC_DCXO_TEMP 0x0d
> +#define AUXADC_VBIF 0x0e
> +#define AUXADC_IMP 0x0f
> +#define AUXADC_IMIX_R 0x10
> +#define AUXADC_VTREF 0x11
> +#define AUXADC_VSYSSNS 0x12
> +#define AUXADC_VIN1 0x13
> +#define AUXADC_VIN2 0x14
> +#define AUXADC_VIN3 0x15
> +#define AUXADC_VIN4 0x16
> +#define AUXADC_VIN5 0x17
> +#define AUXADC_VIN6 0x18
> +#define AUXADC_VIN7 0x19

What has this fine got to do with the node you're adding above? I don't
understand why this is in this patch.
There's random whitespace problems in it, and half the defines look
bizarre. This seems like it should be in the same patch as some binding
changes for an iio device? The mt635x auxadc binding doesn't even permit
anything that would use any of the properties beyond this point.

NAK.

> +
> +#define AUXADC_CHAN_MIN AUXADC_BATADC
> +#define AUXADC_CHAN_MAX AUXADC_VIN7
> +
> +#define ADC_PURES_100K (0)
> +#define ADC_PURES_30K (1)
> +#define ADC_PURES_400K (2)
> +#define ADC_PURES_OPEN (3)
> +
> +#define ADC_PURES_100K_MASK (ADC_PURES_100K << 8)
> +#define ADC_PURES_30K_MASK (ADC_PURES_30K << 8)
> +#define ADC_PURES_400K_MASK (ADC_PURES_400K << 8)
> +#define ADC_PURES_OPEN_MASK (ADC_PURES_OPEN << 8)
> +
> +#endif /* _DT_BINDINGS_MT635X_AUXADC_H */
> --
> 2.45.2
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]