From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=100.103.45.18 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1793749366; b=l0F6uxpodZMFqEkrQI/fv9liqXd42oKj0X4IffDht6bsN6l0SnDGG/Yy/Ci+KOsSaTLv 80XcSEw8lApJx3PlLja+jfA2YeNzutaagy4H+OILtSRiQSAee6Mf7ONl7KQFIFURfq7HS UiBH8b+6EMS/yC8yKfl75nxYEsVV7WdecHMY1fn95prpeIk1MIaLZeQXK5rJapILXPCah 2kjoZs5/EEfzJfLmOby/tH6duaiU2tLLiJFPUtK4gBAyDaxw7MT/pEylReewIu6XHluGm /V5ljJFnLf04Ii4Xzxf8GCN6rx3JhzlKUMNfE5ReNNA+A6bfWMDw+u8LfUc3uyUpgQA== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1793749366; h=Received:DKIM-Signature:Date:From:To:Cc:Subject:Message-ID: In-Reply-To:References:X-Mailer:MIME-Version:Content-Type: Content-Transfer-Encoding; bh=RLfbVHWPETTb3o8HlMj+HxgwepLgdhKx1KwHT8pQtec=; b=uJTv+vAh87Apb12xHL/CXvKpSwjPBsbsQgzgsb5o7sH76WlYGQidnzDswC42HoVxqE5r XiTtP6OQy4iR+5HEnAL57CiZ2Yn9SQW9ecza+uxC1BLWZgwKuGGe65i/wt0DQKai/yOml +viF3bcPRioTh/09EsrKEcLeqMiyB0Wcpj1X5b/ze04+xQqQwPKnVS9SyRx0bzDxQtj9/ frNsmeCi911wFOZxyTWoc20SU4d+/qLPqtNzRG6Qeh7bFcoJ3AJY6dw9UtmwI5EucNc3m gMO47HmoJkp61VDhLf7E7ZkrL19bKKHiUjuuC6yA24Elzq6bCKE8guBZ+l7hiI1E1TQ== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=bYupiRwf; arc=none smtp.remote-ip=100.103.45.18 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BF706606A31B; Tue, 3 Nov 2026 23:42:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key, unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=bYupiRwf Received: by smtp.kernel.org (Postfix) with ESMTPSA id 137351F00919; Tue, 3 Nov 2026 23:42:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1793749365; bh=RLfbVHWPETTb3o8HlMj+HxgwepLgdhKx1KwHT8pQtec=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=bYupiRwfmK2OTY4lnJPjOJ4JTaoY6XpvLrbIsLCs6hYMbIH3TcpTcEiFSVlDV0eFV WQf0cJnjSevfXPfjuKkCoj3Lqg4jZm2gRNSiQK27suv6r3Mslf15jwlC7fQXNbh72f cDHsc/PPj+ZLdMfnb5HhUFxxDUCPW1gou9GXj4r1xn7089R6u+zOe7NhgmKQXmlsOf Vgehv+BaVp59DQ+HqE5m3b+6i4zQZ88ddiKtOg+Jk8Veo2+vLnSiHkRjxNAcs3pm9x r5rf1ceA+4pSHc9tV1afPPP+M+8t9b2H11ft/qBFPa2/qhh0pxIrFd9zLBQQ6adUMA PDnRlb6dAKFTQ== Date: Tue, 3 Nov 2026 23:42:37 +0000 From: Jonathan Cameron To: Roman Vivchar via B4 Relay Cc: rva333@protonmail.com, David Lechner , Nuno =?UTF-8?B?U8Oh?= , Andy Shevchenko , Matthias Brugger , AngeloGioacchino Del Regno , "=?UTF-8?B?TsOtY29sYXM=?= F. R. A. Prado" , Chen-Yu Tsai , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org Subject: Re: [PATCH 1/3] iio: adc: mt6359-auxadc: correct channel types to IIO_VOLTAGE Message-ID: <20261103234237.0dbbe924@jic23-hlaptop> In-Reply-To: <20261103-6768-6358-fixes-v1-1-2a868a33152b@protonmail.com> References: <20261103-6768-6358-fixes-v1-0-2a868a33152b@protonmail.com> <20261103-6768-6358-fixes-v1-1-2a868a33152b@protonmail.com> X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-pc-linux-gnu) X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Tue, 03 Nov 2026 18:32:07 +0200 Roman Vivchar via B4 Relay wrote: > From: Roman Vivchar > > All mt6357, mt6358, mt6359 and mt6363 declare their channel types as > voltage, resistance and temperature. This is wrong, since the PMIC > doesn't include any built-in conversion block. It simply measures > voltage, while consumers are supposed to convert these readings to > the actual temperature or resistance using efuse calibration. Hi Roman, Useful to have an example of the consumer that is doing this. > > Correct all channel types to IIO_VOLTAGE. > > Fixes: 3587914bf61d ("iio: adc: Add support for MediaTek MT6357/8/9 Auxiliary ADC") > Fixes: d6f49313272b ("iio: adc: mt6359: Add support for MediaTek MT6363 PMIC AUXADC") > Signed-off-by: Roman Vivchar This one makes me very nervous, not because it is wrong but rather because there may be a bunch of userspace relying on these wrong channel types. It is horrible but I think we can't touch that :( What we can do is put the right interface in place as well. Thankfully it looks like the channel index is independent of the channel type so we can duplicate the wrong channels with voltage ones + some comments to make sure this doesn't propagate to any new supported devices. AngeloGioacchino, why did you go with the different channel types? If we do decide to go ahead with this approach, are there other channels that are validly using the channel type for non voltage? If not change the macro to drop the parameter. Thanks, Jonathan > --- > drivers/iio/adc/mt6359-auxadc.c | 78 ++++++++++++++++++++--------------------- > 1 file changed, 39 insertions(+), 39 deletions(-) > > diff --git a/drivers/iio/adc/mt6359-auxadc.c b/drivers/iio/adc/mt6359-auxadc.c > index 88b0e26309e5..951d236f972b 100644 > --- a/drivers/iio/adc/mt6359-auxadc.c > +++ b/drivers/iio/adc/mt6359-auxadc.c > @@ -205,18 +205,18 @@ struct mtk_pmic_auxadc_info { > } > > static const struct iio_chan_spec mt6357_auxadc_channels[] = { > - MTK_PMIC_IIO_CHAN(MT6357, bat_adc, BATADC, 0, 15, IIO_RESISTANCE), > - MTK_PMIC_IIO_CHAN(MT6357, isense, ISENSE, 1, 12, IIO_CURRENT), > - MTK_PMIC_IIO_CHAN(MT6357, cdt_v, VCDT, 2, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6357, batt_temp, BAT_TEMP, 3, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6357, chip_temp, CHIP_TEMP, 4, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6357, acc_det, ACCDET, 5, 12, IIO_RESISTANCE), > + MTK_PMIC_IIO_CHAN(MT6357, bat_adc, BATADC, 0, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6357, isense, ISENSE, 1, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6357, cdt_v, VCDT, 2, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6357, batt_temp, BAT_TEMP, 3, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6357, chip_temp, CHIP_TEMP, 4, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6357, acc_det, ACCDET, 5, 12, IIO_VOLTAGE), > MTK_PMIC_IIO_CHAN(MT6357, dcxo_v, VDCXO, 6, 12, IIO_VOLTAGE), > - MTK_PMIC_IIO_CHAN(MT6357, tsx_temp, TSX_TEMP, 7, 15, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6357, hp_ofs_cal, HPOFS_CAL, 9, 15, IIO_RESISTANCE), > - MTK_PMIC_IIO_CHAN(MT6357, dcxo_temp, DCXO_TEMP, 36, 15, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6357, vcore_temp, VCORE_TEMP, 40, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6357, vproc_temp, VPROC_TEMP, 41, 12, IIO_TEMP), > + MTK_PMIC_IIO_CHAN(MT6357, tsx_temp, TSX_TEMP, 7, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6357, hp_ofs_cal, HPOFS_CAL, 9, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6357, dcxo_temp, DCXO_TEMP, 36, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6357, vcore_temp, VCORE_TEMP, 40, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6357, vproc_temp, VPROC_TEMP, 41, 12, IIO_VOLTAGE), If this is all of the channels (I haven't checked) you should drop the ability to configure the channel type from the macro... However, see discussion at the top. > > /* Battery impedance channels */ > MTK_PMIC_IIO_CHAN(MT6357, batt_v, VBAT, 0, 15, IIO_VOLTAGE), > @@ -251,19 +251,19 @@ static const u16 mt6357_auxadc_regs[] = { > }; > > static const struct iio_chan_spec mt6358_auxadc_channels[] = { > - MTK_PMIC_IIO_CHAN(MT6358, bat_adc, BATADC, 0, 15, IIO_RESISTANCE), > - MTK_PMIC_IIO_CHAN(MT6358, cdt_v, VCDT, 2, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6358, batt_temp, BAT_TEMP, 3, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6358, chip_temp, CHIP_TEMP, 4, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6358, acc_det, ACCDET, 5, 12, IIO_RESISTANCE), > + MTK_PMIC_IIO_CHAN(MT6358, bat_adc, BATADC, 0, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6358, cdt_v, VCDT, 2, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6358, batt_temp, BAT_TEMP, 3, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6358, chip_temp, CHIP_TEMP, 4, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6358, acc_det, ACCDET, 5, 12, IIO_VOLTAGE), > MTK_PMIC_IIO_CHAN(MT6358, dcxo_v, VDCXO, 6, 12, IIO_VOLTAGE), > - MTK_PMIC_IIO_CHAN(MT6358, tsx_temp, TSX_TEMP, 7, 15, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6358, hp_ofs_cal, HPOFS_CAL, 9, 15, IIO_RESISTANCE), > - MTK_PMIC_IIO_CHAN(MT6358, dcxo_temp, DCXO_TEMP, 10, 15, IIO_TEMP), > + MTK_PMIC_IIO_CHAN(MT6358, tsx_temp, TSX_TEMP, 7, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6358, hp_ofs_cal, HPOFS_CAL, 9, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6358, dcxo_temp, DCXO_TEMP, 10, 15, IIO_VOLTAGE), > MTK_PMIC_IIO_CHAN(MT6358, bif_v, VBIF, 11, 12, IIO_VOLTAGE), > - MTK_PMIC_IIO_CHAN(MT6358, vcore_temp, VCORE_TEMP, 38, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6358, vproc_temp, VPROC_TEMP, 39, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6358, vgpu_temp, VGPU_TEMP, 40, 12, IIO_TEMP), > + MTK_PMIC_IIO_CHAN(MT6358, vcore_temp, VCORE_TEMP, 38, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6358, vproc_temp, VPROC_TEMP, 39, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6358, vgpu_temp, VGPU_TEMP, 40, 12, IIO_VOLTAGE), > > /* Battery impedance channels */ > MTK_PMIC_IIO_CHAN(MT6358, batt_v, VBAT, 0, 15, IIO_VOLTAGE), > @@ -299,18 +299,18 @@ static const u16 mt6358_auxadc_regs[] = { > }; > > static const struct iio_chan_spec mt6359_auxadc_channels[] = { > - MTK_PMIC_IIO_CHAN(MT6359, bat_adc, BATADC, 0, 15, IIO_RESISTANCE), > - MTK_PMIC_IIO_CHAN(MT6359, batt_temp, BAT_TEMP, 3, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6359, chip_temp, CHIP_TEMP, 4, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6359, acc_det, ACCDET, 5, 12, IIO_RESISTANCE), > + MTK_PMIC_IIO_CHAN(MT6359, bat_adc, BATADC, 0, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6359, batt_temp, BAT_TEMP, 3, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6359, chip_temp, CHIP_TEMP, 4, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6359, acc_det, ACCDET, 5, 12, IIO_VOLTAGE), > MTK_PMIC_IIO_CHAN(MT6359, dcxo_v, VDCXO, 6, 12, IIO_VOLTAGE), > - MTK_PMIC_IIO_CHAN(MT6359, tsx_temp, TSX_TEMP, 7, 15, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6359, hp_ofs_cal, HPOFS_CAL, 9, 15, IIO_RESISTANCE), > - MTK_PMIC_IIO_CHAN(MT6359, dcxo_temp, DCXO_TEMP, 10, 15, IIO_TEMP), > + MTK_PMIC_IIO_CHAN(MT6359, tsx_temp, TSX_TEMP, 7, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6359, hp_ofs_cal, HPOFS_CAL, 9, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6359, dcxo_temp, DCXO_TEMP, 10, 15, IIO_VOLTAGE), > MTK_PMIC_IIO_CHAN(MT6359, bif_v, VBIF, 11, 12, IIO_VOLTAGE), > - MTK_PMIC_IIO_CHAN(MT6359, vcore_temp, VCORE_TEMP, 30, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6359, vproc_temp, VPROC_TEMP, 31, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6359, vgpu_temp, VGPU_TEMP, 32, 12, IIO_TEMP), > + MTK_PMIC_IIO_CHAN(MT6359, vcore_temp, VCORE_TEMP, 30, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6359, vproc_temp, VPROC_TEMP, 31, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6359, vgpu_temp, VGPU_TEMP, 32, 12, IIO_VOLTAGE), > > /* Battery impedance channels */ > MTK_PMIC_IIO_CHAN(MT6359, batt_v, VBAT, 0, 15, IIO_VOLTAGE), > @@ -349,15 +349,15 @@ static const u16 mt6359_auxadc_regs[] = { > }; > > static const struct iio_chan_spec mt6363_auxadc_channels[] = { > - MTK_PMIC_IIO_CHAN(MT6363, bat_adc, BATADC, 0, 15, IIO_RESISTANCE), > - MTK_PMIC_IIO_CHAN(MT6363, cdt_v, VCDT, 2, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6363, batt_temp, BAT_TEMP, 3, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6363, chip_temp, CHIP_TEMP, 4, 12, IIO_TEMP), > + MTK_PMIC_IIO_CHAN(MT6363, bat_adc, BATADC, 0, 15, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6363, cdt_v, VCDT, 2, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6363, batt_temp, BAT_TEMP, 3, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6363, chip_temp, CHIP_TEMP, 4, 12, IIO_VOLTAGE), > MTK_PMIC_IIO_CHAN(MT6363, sys_sns_v, VSYSSNS, 6, 15, IIO_VOLTAGE), > MTK_PMIC_IIO_CHAN(MT6363, tref_v, VTREF, 11, 12, IIO_VOLTAGE), > - MTK_PMIC_IIO_CHAN(MT6363, vcore_temp, VCORE_TEMP, 38, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6363, vproc_temp, VPROC_TEMP, 39, 12, IIO_TEMP), > - MTK_PMIC_IIO_CHAN(MT6363, vgpu_temp, VGPU_TEMP, 40, 12, IIO_TEMP), > + MTK_PMIC_IIO_CHAN(MT6363, vcore_temp, VCORE_TEMP, 38, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6363, vproc_temp, VPROC_TEMP, 39, 12, IIO_VOLTAGE), > + MTK_PMIC_IIO_CHAN(MT6363, vgpu_temp, VGPU_TEMP, 40, 12, IIO_VOLTAGE), > > /* For VIN, ADC12 holds the result depending on which GPIO was activated */ > MTK_PMIC_IIO_CHAN(MT6363, in1_v, VIN1, 45, 15, IIO_VOLTAGE), >