linux-kernel - Re: [PATCH v2 12/17] iio: adc: axp20x

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220608143514.00005b9b@Huawei.com>
Date:   Wed, 8 Jun 2022 14:35:14 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Aidan MacDonald <aidanmacdonald.0x0@...il.com>
CC:     <linus.walleij@...aro.org>, <brgl@...ev.pl>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <wens@...e.org>,
        <jic23@...nel.org>, <lee.jones@...aro.org>, <sre@...nel.org>,
        <broonie@...nel.org>, <gregkh@...uxfoundation.org>,
        <lgirdwood@...il.com>, <lars@...afoo.de>, <rafael@...nel.org>,
        <quic_gurus@...cinc.com>, <linux-gpio@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-iio@...r.kernel.org>, <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2 12/17] iio: adc: axp20x_adc: Add support for AXP192

On Tue,  7 Jun 2022 16:53:19 +0100
Aidan MacDonald <aidanmacdonald.0x0@...il.com> wrote:

> The AXP192 is identical to the AXP20x, except for the addition of
> two more GPIO ADC channels.
> 
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@...il.com>

Hi Aidan,

A few things follow through from review of previous 2 patches.
Otherwise looks good to me.

Thanks,

Jonathan


>  	AXP20X_ADC_CHANNEL(AXP20X_ACIN_V, "acin_v", IIO_VOLTAGE,
>  			   AXP20X_ACIN_V_ADC_H),
> @@ -250,6 +314,15 @@ static int axp20x_adc_raw(struct iio_dev *indio_dev,
>  	int ret, size;
>  
>  	switch (info->data->axp20x_id) {
> +	case AXP192_ID:
> +		/* Battery current ADCs on the AXP192 are 13 bits. */
> +		if (chan->type == IIO_CURRENT &&
> +		    (chan->channel == AXP20X_BATT_CHRG_I || chan->channel == AXP20X_BATT_DISCHRG_I))

Ah, you'll be needing two _res variables as suggested in earlier patch.

> +			size = 13;
> +		else
> +			size = 12;
> +		break;
> +
>  	case AXP202_ID:
>  	case AXP209_ID:
>  		/*
> @@ -276,6 +349,44 @@ static int axp20x_adc_raw(struct iio_dev *indio_dev,
>  	return IIO_VAL_INT;
>  }

...

> +static int axp192_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
> +				     int *val)
> +{
> +	struct axp20x_adc_iio *info = iio_priv(indio_dev);
> +	unsigned int regval;
> +	int ret;
> +
> +	ret = regmap_read(info->regmap, AXP192_GPIO30_IN_RANGE, &regval);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (channel) {
> +	case AXP192_GPIO0_V:
> +		regval &= AXP192_GPIO30_IN_RANGE_GPIO0;

As per earlier patch, FIELD_GET() would act as 'documentation'
of what is going on here, even though it'll make no real difference.

> +		break;
> +
> +	case AXP192_GPIO1_V:
> +		regval &= AXP192_GPIO30_IN_RANGE_GPIO1;
> +		break;
> +
> +	case AXP192_GPIO2_V:
> +		regval &= AXP192_GPIO30_IN_RANGE_GPIO2;
> +		break;
> +
> +	case AXP192_GPIO3_V:
> +		regval &= AXP192_GPIO30_IN_RANGE_GPIO3;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	*val = regval ? 700000 : 0;
> +	return IIO_VAL_INT;
> +}
> +