public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel•org>
To: Roman Vivchar via B4 Relay <devnull+rva333.protonmail.com@kernel•org>
Cc: rva333@protonmail•com, "David Lechner" <dlechner@baylibre•com>,
	"Nuno Sá" <nuno.sa@analog•com>,
	"Andy Shevchenko" <andy@kernel•org>,
	"Rob Herring" <robh@kernel•org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel•org>,
	"Conor Dooley" <conor+dt@kernel•org>,
	"Matthias Brugger" <matthias.bgg@gmail•com>,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora•com>,
	"Lee Jones" <lee@kernel•org>,
	linux-iio@vger•kernel.org, devicetree@vger•kernel.org,
	linux-kernel@vger•kernel.org,
	linux-arm-kernel@lists•infradead.org,
	linux-mediatek@lists•infradead.org,
	"Ben Grisdale" <bengris32@protonmail•ch>
Subject: Re: [PATCH 2/4] iio: adc: mt6323-auxadc: add mt6323 PMIC AUXADC driver
Date: Wed, 3 Jun 2026 14:41:32 +0100	[thread overview]
Message-ID: <20260603144132.6c5daea8@jic23-huawei> (raw)
In-Reply-To: <20260602-mt6323-adc-v1-2-68ec737508ee@protonmail.com>

On Tue, 02 Jun 2026 15:46:55 +0300
Roman Vivchar via B4 Relay <devnull+rva333.protonmail.com@kernel•org> wrote:

> From: Roman Vivchar <rva333@protonmail•com>
> 
> The mt6323 AUXADC is a 15-bit ADC used for system monitoring. This driver
> provides support for reading various channels including battery and
> charger voltages, battery and chip temperature, current sensing and
> accessory detection.
> 
> Add a driver for the AUXADC found in the MediaTek mt6323 PMIC.
> 
> Tested-by: Ben Grisdale <bengris32@protonmail•ch> # Amazon Echo Dot (2nd Generation)
> Signed-off-by: Roman Vivchar <rva333@protonmail•com>
Trivial stuff inline + a question from the sashiko bot you may have
missed.

Jonathan

> diff --git a/drivers/iio/adc/mt6323-auxadc.c b/drivers/iio/adc/mt6323-auxadc.c
> new file mode 100644
> index 000000000000..da6c11a5079c
> --- /dev/null
> +++ b/drivers/iio/adc/mt6323-auxadc.c
> @@ -0,0 +1,299 @@


> +static int mt6323_auxadc_request(struct mt6323_auxadc *auxadc,
> +				 unsigned long channel)
> +{
> +	struct regmap *map = auxadc->regmap;
> +	int ret;
> +
> +	ret = regmap_set_bits(map, MT6323_AUXADC_CON11, AUXADC_CON11_VBUF_EN);

See below. Sashiko asked if lack of turning this off again when done
with a read is wasting power or similar.

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_clear_bits(map, MT6323_AUXADC_CON22, BIT(channel));
> +	if (ret)
> +		return ret;
> +
> +	return regmap_set_bits(map, MT6323_AUXADC_CON22, BIT(channel));
> +}
> +
> +static int mt6323_auxadc_read(struct mt6323_auxadc *auxadc,
> +			      const struct iio_chan_spec *chan, int *out)
> +{
> +	struct regmap *map = auxadc->regmap;
> +	u32 reg = chan->address;
It's only used one. I'd probably put it inline and skip the local variable.
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read_poll_timeout(map, reg, val, (val & AUXADC_READY_MASK),
> +				       1 * USEC_PER_MSEC, 100 * USEC_PER_MSEC);
> +	if (ret)
> +		return ret;
> +
> +	*out = FIELD_GET(AUXADC_DATA_MASK, val);
> +
> +	return 0;
> +}
> +
> +static int mt6323_auxadc_read_raw(struct iio_dev *indio_dev,
> +				  const struct iio_chan_spec *chan,
> +				  int *val, int *val2, long mask)
> +{
> +	struct mt6323_auxadc *auxadc = iio_priv(indio_dev);
> +	int ret, mult;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->channel == MT6323_AUXADC_ISENSE ||
> +		    chan->channel == MT6323_AUXADC_BATSNS)
> +			mult = 4;
> +		else
> +			mult = 1;
> +
> +		/* 1800mV full range with 15-bit resolution. */
> +		*val = mult * 1800;
> +		*val2 = 15;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_RAW:
> +		scoped_guard(mutex, &auxadc->lock) {
> +			ret = mt6323_auxadc_prepare_channel(auxadc);
> +			if (ret)
> +				return ret;
> +
> +			ret = mt6323_auxadc_request(auxadc, chan->channel);
Sashiko asks:
"Does this leak power by leaving AUXADC_CON11_VBUF_EN enabled after the
reading finishes?
It appears the voltage buffer is enabled in mt6323_auxadc_request() but never
disabled once the conversion completes and exits here. Also, the active
channel is never cleared."
https://sashiko.dev/#/patchset/20260602-mt6323-adc-v1-0-68ec737508ee%40protonmail.com

Seems like a reasonable point.


> +			if (ret)
> +				return ret;
> +
> +			/* Hardware limitation: the AUXADC needs a delay to become ready. */
> +			fsleep(300);
> +
> +			ret = mt6323_auxadc_read(auxadc, chan, val);
> +			if (ret)
> +				return ret;
> +		}
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +static int mt6323_auxadc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mt6323_auxadc *auxadc;
> +	struct iio_dev *iio;
> +	struct regmap *regmap;

When no other particular ordering preference in IIO is for reverse xmas
tree. 

> +	int ret;
> +
> +	regmap = dev_get_regmap(dev->parent->parent, NULL);
> +	if (!regmap)
> +		return dev_err_probe(dev, -ENODEV, "failed to get regmap\n");
> +
> +	iio = devm_iio_device_alloc(dev, sizeof(*auxadc));
> +	if (!iio)
> +		return -ENOMEM;
> +
> +	auxadc = iio_priv(iio);
> +	auxadc->regmap = regmap;
> +
> +	ret = devm_mutex_init(dev, &auxadc->lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = mt6323_auxadc_init(auxadc);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to initialize auxadc\n");
> +
> +	iio->name = "mt6323-auxadc";
> +	iio->info = &mt6323_auxadc_iio_info;
> +	iio->modes = INDIO_DIRECT_MODE;
> +	iio->channels = mt6323_auxadc_channels;
> +	iio->num_channels = ARRAY_SIZE(mt6323_auxadc_channels);
> +
> +	return devm_iio_device_register(dev, iio);
> +}


  parent reply	other threads:[~2026-06-03 13:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-02 12:46 [PATCH 0/4] AUXADC driver for the MediaTek mt6323 PMIC Roman Vivchar via B4 Relay
2026-06-02 12:46 ` [PATCH 1/4] dt-bindings: iio: adc: mediatek,mt6359-auxadc: add mt6323 PMIC AUXADC Roman Vivchar via B4 Relay
2026-06-02 16:21   ` Conor Dooley
2026-06-02 12:46 ` [PATCH 2/4] iio: adc: mt6323-auxadc: add mt6323 PMIC AUXADC driver Roman Vivchar via B4 Relay
2026-06-02 16:43   ` Nuno Sá
2026-06-03 11:24     ` Roman Vivchar
2026-06-03  1:11   ` Sanjay Chitroda
2026-06-03  5:51     ` Andy Shevchenko
2026-06-03 13:41   ` Jonathan Cameron [this message]
2026-06-02 12:46 ` [PATCH 3/4] mfd: mt6397-core: add mt6323 AUXADC support Roman Vivchar via B4 Relay
2026-06-02 12:46 ` [PATCH 4/4] ARM: dts: mediatek: mt6323: add " Roman Vivchar via B4 Relay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260603144132.6c5daea8@jic23-huawei \
    --to=jic23@kernel$(echo .)org \
    --cc=andy@kernel$(echo .)org \
    --cc=angelogioacchino.delregno@collabora$(echo .)com \
    --cc=bengris32@protonmail$(echo .)ch \
    --cc=conor+dt@kernel$(echo .)org \
    --cc=devicetree@vger$(echo .)kernel.org \
    --cc=devnull+rva333.protonmail.com@kernel$(echo .)org \
    --cc=dlechner@baylibre$(echo .)com \
    --cc=krzk+dt@kernel$(echo .)org \
    --cc=lee@kernel$(echo .)org \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    --cc=linux-iio@vger$(echo .)kernel.org \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=linux-mediatek@lists$(echo .)infradead.org \
    --cc=matthias.bgg@gmail$(echo .)com \
    --cc=nuno.sa@analog$(echo .)com \
    --cc=robh@kernel$(echo .)org \
    --cc=rva333@protonmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox