Re: [PATCH 3/3] soc: mediatek: pwrap: add mt6572 support

[Roman Vivchar]

Hi AngeloGioacchino,

On Wednesday, July 15th, 2026 at 1:21 PM, AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:

> On 7/14/26 14:42, Roman Vivchar via B4 Relay wrote:

...
 
> >
> > +static const struct pmic_wrapper_type pwrap_mt6572 = {
> > +	.regs = mt6572_regs,
> 
> MT6572 only has PWRAP_OP_TYPE, PWRAP_MSB_FIRST, PWRAP_GPS_STA additional registers
> compared to MT2701, and the three are currently unused.

GPS_STA is indeed unused. As for others, downstream kernel includes mt6320 path.

//###############################
//Enable 6320 option
//###############################
#ifdef SLV_6320
WRAP_WR32(PMIC_WRAP_OP_TYPE ,OP_TYPE_CSL);
WRAP_WR32(PMIC_WRAP_MSB_FIRST , LSB);
#endif

Though I haven't seen any mt6572 device with the mt6320 so far, as well as
pwrap_init_chip_select_ext is configured for the mt6323.

> Instead of adding yet one more huge array for 3 more registers, since I'm mostly
> sure that those will never be used anyway (happy if you prove me wrong!), you can
> at this point just use mt2701_regs for mt6572.
> 
> Perhaps, add a comment saying that MT6572 does indeed have those additional regs
> but were omitted because currently unused.
> 
> So, something like
> 
> /*
>   * MT6572 has additional registers OP_TYPE (0x10), MSB_FIRST (0x14), GPS_STA (0x40)
>   * which were omitted as they're currently unused in this driver.
>   * Apart from that, the register map matches the one from MT2701.
>   */
> static const struct pmic_wrapper_type pwrap_mt6572 = {
> 	.regs = mt2701_regs,
>

I'm not a big fan of dead code, but at some point the mt6572+mt6320 may exist.
Would you suggest keeping mt6572 array or drop it until some mt6320
device appears?

Best regards,
Roman