> > +#define DPIO_CMD(id) ((id << DPIO_CMD_ID_OFFSET) | DPIO_CMD_BASE_VERSION) > > Paranthesis around 'id'? In all cases these are opcode values and will never be an expression. If we really need to future proof these definitions, we should do it for all objects not just DPIO. I'd like to see consistency across objects and don't want to see DPIO gratuitously diverge. So, my suggestion is to have an offline discussion and if we think the change is needed, submit a patch for all objects currently supported. > > + /* prepare command */ > > + cmd.header = mc_encode_cmd_header(DPIO_CMDID_OPEN, > > + cmd_flags, > > + 0); > > + dpio_cmd = (struct dpio_cmd_open *)cmd.params; > > + dpio_cmd->dpio_id = cpu_to_le32(dpio_id); > > + > > + /* send command to mc*/ > > + err = mc_send_command(mc_io, &cmd); > > + if (err) > > + return err; > > + > > + /* retrieve response parameters */ > > + *token = mc_cmd_hdr_read_token(&cmd); > > Nit: maybe we should drop these repetitive "prepare / send / retrieve" comments > as the code is pretty self explanatory. The 'send' comment certainly isn't needed given that the function is 'mc_send_command()'. For the others, I think having some comment is helpful, even though a bit repetitive. Stuart _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
