On 14-Jul-21 12:09 AM, Mark Brown wrote: > On Tue, Jul 13, 2021 at 12:57:42PM +0000, Apurva Nandan wrote: > >> + f_pdata->dtr = op->cmd.dtr && >> + (op->addr.dtr || !op->addr.nbytes) && >> + (op->data.dtr || !op->data.nbytes); > > I'm not sure anyone reading this code is going to figure out what it's > doing without thinking about it, the combination of writing the bytes > check with a !, putting it after the check for .dtr and not having any > comments is a bit obscure. Something like > > (op->addr.nbytes && op.addr.dtr) > > might be a bit clearer, or a comment explicitly spelling it out. > Okay, I will add a comment explaining it, as other logic (with &&) won't deliver the behavior we expect. The logic it implements is: for an op to be dtr, if any phase has non-zero bytes, it must have dtr field set as true. So, it does p implies q: p->q = (!p || q) i.e. if phase has non-zero bytes (p) then check its dtr field (q). If all phases are empty, then follow what op.cmd.dtr says. Regards, Apurva Nandan