> +static void oa_tc6_prepare_ctrl_buf(struct oa_tc6 *tc6, u32 addr, u32 val[], > + u8 len, bool wnr, u8 *buf, bool prote) One of the comments i made last time was that wnr is not obvious. I assume it means write-not-read. So why not just write? Since it a boolean, i assume war is never needed, so !wnr cal always be considered rnw. And prote could well be protect, the two extra characters make it a lot more obvious. Or better still. > +{ > + u32 hdr; > + > + /* Prepare the control header with the required details */ > + hdr = FIELD_PREP(CTRL_HDR_DNC, 0) | > + FIELD_PREP(CTRL_HDR_WNR, wnr) | > + FIELD_PREP(CTRL_HDR_AID, 0) | > + FIELD_PREP(CTRL_HDR_MMS, addr >> 16) | > + FIELD_PREP(CTRL_HDR_ADDR, addr) | > + FIELD_PREP(CTRL_HDR_LEN, len - 1); > + hdr |= FIELD_PREP(CTRL_HDR_P, oa_tc6_get_parity(hdr)); > + *(__be32 *)buf = cpu_to_be32(hdr); > + > + if (wnr) { > + for (u8 i = 0; i < len; i++) { > + u16 pos; > + > + if (!prote) { nitpick, but please use positive logic. Do the protected case first. Andrew