Hi Andrew, On 24/10/23 4:13 am, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >> + ret = oa_tc6_perform_ctrl(tc6, RESET, ®val, 1, true, true); >> + ret = oa_tc6_perform_ctrl(tc6, RESET, ®val, 1, true, false); > > Just looking at this, it is not clear what these true/false mean. Maybe add some #defines > > #define TC6_READ true > #define TC6_WRITE false > #define TC6_PROTECTED true > #define TC6_UNPROTECTED false Sure will do this. > >> + if (ret) >> + return ret; >> + >> + /* The chip completes a reset in 3us, we might get here earlier than >> + * that, as an added margin we'll conditionally sleep 5us. >> + */ >> + udelay(5); >> + >> + ret = oa_tc6_perform_ctrl(tc6, STATUS0, ®val, 1, false, false); >> + if (ret) >> + return ret; >> + >> + /* Check for reset complete interrupt status */ >> + if (regval & RESETC) { >> + regval = RESETC; > > People don't always agree, but i found STATUS0_RESETC easier to see > you have the correct bit for the register you just read. Do you want me to define STATUS0_RESETC instead of RESETC or is my understanding wrong? Best Regards, Parthiban V > > Andrew