Received-SPF: pass (google.com: domain of linux-kernel+bounces-18912-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Sun, 7 Jan 2024 16:19:49 +0000
From: Jonathan Cameron <jic23@kernel.org>
To: <cy_huang@richtek.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley
 <conor+dt@kernel.org>, Lars-Peter Clausen <lars@metafoo.de>, Rob Herring
 <robh+dt@kernel.org>, Uwe =?UTF-8?B?S2xlaW5lLUvDtm5pZw==?=
 <u.kleine-koenig@pengutronix.de>, <linux-iio@vger.kernel.org>,
 <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 2/2] iio: adc: rtq6056: Add support for the whole
 RTQ6056 family
Message-ID: <20240107161949.04ec0011@jic23-huawei>
In-Reply-To: <2980b67de00bae1fc25004188e8aabf53073d940.1704357933.git.cy_huang@richtek.com>
References: <cover.1704357933.git.cy_huang@richtek.com>
	<2980b67de00bae1fc25004188e8aabf53073d940.1704357933.git.cy_huang@richtek.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Thu, 4 Jan 2024 17:03:31 +0800
<cy_huang@richtek.com> wrote:

> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> RTQ6053 and RTQ6059 are the same series of RTQ6056.
> 
> The respective differences with RTQ6056 are listed below
> RTQ6053
> - chip package type
> 
> RTQ6059
> - Reduce the pinout for vbus sensing pin
> - Some internal ADC scaling change
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
Hi. 

One last follow on comment based on v4 changes to the enum naming.
I think you missed one place they should be updated - the reg_field
definitions.

Thanks,

Jonathan


>  
> +/*
> + * The enum is to present the 0x00 CONFIG RG bitfield for the 16bit RG value
> + * field value order from LSB to MSB
> + * RTQ6053/6 is OPMODE->VSHUNTCT->VBUSCT->AVG->RESET
> + * RTQ6059 is OPMODE->SADC->BADC->PGA->RESET
> + */
>  enum {
>  	F_OPMODE = 0,
>  	F_VSHUNTCT,
> +	F_RTQ6059_SADC = F_VSHUNTCT,
>  	F_VBUSCT,
> +	F_RTQ6059_BADC = F_VBUSCT,
>  	F_AVG,
> +	F_RTQ6059_PGA = F_AVG,
>  	F_RESET,
>  	F_MAX_FIELDS
>  };

>  
> +static const struct reg_field rtq6059_reg_fields[F_MAX_FIELDS] = {
> +	[F_OPMODE] = REG_FIELD(RTQ6056_REG_CONFIG, 0, 2),
> +	[F_VSHUNTCT] = REG_FIELD(RTQ6056_REG_CONFIG, 3, 6),
> +	[F_VBUSCT] = REG_FIELD(RTQ6056_REG_CONFIG, 7, 10),
> +	[F_AVG]	= REG_FIELD(RTQ6056_REG_CONFIG, 11, 12),
> +	[F_RESET] = REG_FIELD(RTQ6056_REG_CONFIG, 15, 15),

Given these are the rtq6059 regfield definitions should they not be
using the new enum names?

> +};
> +