Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1543144

RE: [PATCH v3 3/9] bus: fsl-mc: dpio: add APIs for DPIO objects

Show all headers | View raw

> > +#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

Back to linux.kernel | Previous | Next | Find similar | Unroll thread

Thread

RE: [PATCH v3 3/9] bus: fsl-mc: dpio: add APIs for DPIO objects Stuart Yoder <stuart.yoder@nxp.com> - 2016-12-16 01:20 +0100

csiph-web