On Mon 8 October 2012 22:06:29 Andrey Smirnov wrote:
> On 10/08/2012 01:56 AM, Hans Verkuil wrote:
> > On Sat October 6 2012 03:54:59 Andrey Smirnov wrote:
> >> This patch adds all the functions used for exchanging commands with
> >> the chip.
> >>
> >> Signed-off-by: Andrey Smirnov <andrey.smirnov@xxxxxxxxxxxxxxxxxxxx>
> >> ---
> >> drivers/mfd/si476x-cmd.c | 1493 ++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 1493 insertions(+)
> >> create mode 100644 drivers/mfd/si476x-cmd.c
> >>
> >> diff --git a/drivers/mfd/si476x-cmd.c b/drivers/mfd/si476x-cmd.c
> >> new file mode 100644
> >> index 0000000..f11cf58
> >> --- /dev/null
> >> +++ b/drivers/mfd/si476x-cmd.c
<snip>
> >> +/**
> >> + * si476x_cmd_am_rsq_status - send 'FM_TUNE_FREQ' command to the
> >> + * device
> >> + * @core - device to send the command to
> >> + * @rsqack - if set command clears RSQINT, SNRINT, SNRLINT, RSSIHINT,
> >> + * RSSSILINT, BLENDINT, MULTHINT and MULTLINT
> >> + * @attune - when set the values in the status report are the values
> >> + * that were calculated at tune
> >> + * @cancel - abort ongoing seek/tune opertation
> >> + * @stcack - clear the STCINT bin in status register
> >> + * @report - all signal quality information retured by the command
> >> + * (if NULL then the output of the command is ignored)
I've just noticed that this comment block does not correspond at all to
the code. It's a good idea to check the other comment blocks for similar
copy/paste errors.
> >> + *
> >> + * Function returns 0 on success and negative error code on failure
> >> + */
> >> +int si476x_core_cmd_am_rsq_status(struct si476x_core *core,
> >> + struct si476x_rsq_status_args *rsqargs,
> >> + struct si476x_rsq_status_report *report)
> >> +{
> >> + int err;
> >> + u8 resp[CMD_AM_RSQ_STATUS_NRESP];
> >> + const u8 args[CMD_AM_RSQ_STATUS_NARGS] = {
> >> + rsqargs->rsqack << 3 | rsqargs->attune << 2 |
> >> + rsqargs->cancel << 1 | rsqargs->stcack,
> >> + };
> >> +
> >> + err = CORE_SEND_COMMAND(core, CMD_AM_RSQ_STATUS,
> >> + args, resp,
> >> + atomic_read(&core->timeouts.command));
> >> +
> >> + if (report) {
> > Do you really need to test 'report'? Does it ever make sense if this is
> > called with a NULL report pointer?
>
> Unfortunately yes. This command is also used to acknowledge and
> STC(seek-tune completed)
> interrupt.
A comment would be welcome here, and in similar cases. It's not obvious.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
