RE: [PATCH mfd 1/1] mfd/ptp: clockmatrix: support 32-bit address space

[Min Li]

> On Tue, 21 Mar 2023 15:10:06 -0400 Min Li wrote:
> > -		buf[0] = (u8)(page & 0xff);
> > -		buf[1] = (u8)((page >> 8) & 0xff);
> > +		buf[0] = (u8)(page & 0xFF);
> > +		buf[1] = (u8)((page >> 8) & 0xFF);
> 
> why did you decide to change from 0xff to 0xFF as part of this big change?
> It's unnecessary churn.

I will revert it back to 0xff

> > -		err = idtcm_write(idtcm, 0, HW_Q8_CTRL_SPARE,
> > +		err = idtcm_write(idtcm, HW_Q8_CTRL_SPARE, 0,
> >  				  &temp, sizeof(temp));
> >  		if (err)
> >  			return err;
> >
> >  		temp |= Q9_TO_Q8_SYNC_TRIG;
> >
> > -		err = idtcm_write(idtcm, 0, HW_Q8_CTRL_SPARE,
> > +		err = idtcm_write(idtcm, HW_Q8_CTRL_SPARE, 0,
> >  				  &temp, sizeof(temp));
> >  		if (err)
> >  			return err;
> 
> Why are you flipping all these arguments?
> Isn't HW_Q8_CTRL_SPARE regaddr?

static inline int idtcm_write(struct idtcm *idtcm,
-			      u16 module,
+			      u32 module,
 			      u16 regaddr,
 			      u8 *buf,
 			      u16 count)

HW_Q8_CTRL_SPARE is the module and is a u32. regaddr is the offset within the module. 
They used to be both u16 but after the change, module becomes u32.

> Could you to split your patches into multiple steps to make them easier to
> reivew?

I will split the change into ptp and mfd respectively.