Messages in this thread | | | Date | Wed, 22 Mar 2023 12:56:19 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH mfd 1/1] mfd/ptp: clockmatrix: support 32-bit address space |
| |
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.
> + buf[2] = (u8)((page >> 16) & 0xFF); > + buf[3] = (u8)((page >> 24) & 0xFF); > + bytes = 4; > break;
> static inline int idtcm_read(struct idtcm *idtcm, > - u16 module, > + u32 module, > u16 regaddr, > u8 *buf, > u16 count) > @@ -50,7 +50,7 @@ static inline int idtcm_read(struct idtcm *idtcm, > } > > static inline int idtcm_write(struct idtcm *idtcm, > - u16 module, > + u32 module, > u16 regaddr, > u8 *buf, > u16 count)
> /* PLL5 can have OUT8 as second additional output. */ > if (pll == 5 && qn_plus_1 != 0) { > - err = idtcm_read(idtcm, 0, HW_Q8_CTRL_SPARE, > + err = idtcm_read(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; > > 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?
Could you to split your patches into multiple steps to make them easier to reivew?
| |