Arnd Bergmann writes: > On Monday 02 March 2015 12:54:39 Eric Anholt wrote: >> +/* >> + * Submits a set of concatenated tags to the VPU firmware through the >> + * mailbox power interface. >> + * >> + * The buffer header and the ending tag are added by this function and >> + * don't need to be supplied, just the actual tags for your operation. >> + * See struct bcm_mbox_power_tag_header for the per-tag structure. >> + */ >> +static int bcm_mbox_set_power(uint32_t power_enables) >> +{ >> + int ret; >> + >> + reinit_completion(&mbox_power->c); >> + ret = mbox_send_message(mbox_power->chan, >> + (void *)(power_enables << BCM_MBOX_DATA_SHIFT)); >> + if (ret >= 0) >> + wait_for_completion(&mbox_power->c); >> + >> + return ret; >> > > Please don't abuse the pointer argument to send an integer. > > Instead, pass a pointer to the data argument as intended. As this > is a recurring problem, we may want to add a different interface > to pass fixed-length inline data, maybe > > mbox_send_message_u32(struct mbox_chan *chan, u32 msg); > > and possibly add a length argument to the existing mbox_send_message. Good point. I've adjusted this code to do this (and to use "msg" instead of "mssg" -- is "mssg" supposed to mean something besides an unusual abbreviation of "message"?), and since it was the same number of lines of code, I don't think it makes sense to define a new API. I suspect this awkward style just arose from following the lead of the omap driver.