Re: [PATCH v6 2/5] firmware: add extensible driver data API

[Luca Coelho]

On Thu, 2017-04-27 at 05:16 +0200, Luis R. Rodriguez wrote:
> > > @@ -1460,6 +1471,128 @@ void release_firmware(const struct firmware *fw)
> > > Â }
> > > Â EXPORT_SYMBOL(release_firmware);
> > > Â 
> > > +static int _driver_data_request_api(struct driver_data_params *params,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device *device,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const char *name)
> > > +{
> > > +ÂÂÂstruct driver_data_priv_params *priv_params = &params->priv_params;
> > > +ÂÂÂconst struct driver_data_req_params *req_params = &params->req_params;
> > > +ÂÂÂint ret;
> > > +ÂÂÂchar *try_name;
> > > +ÂÂÂu8 api_max;
> > > +
> > > +ÂÂÂif (priv_params->retry_api) {
> > > +ÂÂÂÂÂÂÂÂÂÂÂif (!priv_params->api)
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn -ENOENT;
> > > +ÂÂÂÂÂÂÂÂÂÂÂapi_max = priv_params->api - 1;
> > > +ÂÂÂ} else
> > > +ÂÂÂÂÂÂÂÂÂÂÂapi_max = req_params->api_max;
> > 
> > Braces.
> 
> Not sure what you mean here, the else is a 1 liner so I skip the brace.

It's really a nitpick, but the CodingStyle[1] says:

"Do not unnecessarily use braces where a single statement will do.
[...]
This does not apply if only one branch of a conditional statement is a
single statement; in the latter case use braces in both branches:

	if (condition) {
		do_this();
		do_that();
	} else {
		otherwise();
	}"

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n175

--
Cheers,
Luca.