Re: [PATCH v4 2/2] iio: adc: rtq6056: Add support for the whole RTQ6056 family

[Jonathan Cameron]

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

> From: ChiYuan Huang <cy_huang@xxxxxxxxxxx>
> 
> 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@xxxxxxxxxxx>
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?

> +};
> +