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

[Conor Dooley <conor@kernel.org>]

[-- 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 --]