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

Groups > linux.kernel > #1672433

Re: [PATCH 5/7] staging: ccree: add clock management support

Show all headers | View raw

On Thu, Jun 22, 2017 at 10:07:51AM +0300, Gilad Ben-Yossef wrote:
> +int cc_clk_on(struct ssi_drvdata *drvdata)
> +{
> +	int rc = 0;
> +	struct clk *clk = drvdata->clk;
> +
> +	if (IS_ERR(clk))
> +	/* No all devices have a clock associated with CCREE */
> +		goto out;

Ugh...  I hate this.  The "goto out;" here is a waste of time
do-nothing-goto that returns diretly.  It's equivalent to "return 0;".
Is that intended?  Even with the comment, it's not clear...

People think do nothing gotos are a great idea but from reviewing tons
and tons of real life errors, I can assure you that in real life (as
opposed to theory) they don't prevent any future bugs and only introduce
"forgot to set the error code" bugs.

The indenting is messed up and multi-line indents get curly braces.

> +
> +	rc = clk_prepare_enable(clk);
> +	if (rc) {
> +		SSI_LOG_ERR("error enabling clock\n");
> +		clk_disable_unprepare(clk);

Don't unprepare something that hasn't been prepared.

> +	}
> +
> +out:
> +	return rc;
> +}

int cc_clk_on(struct ssi_drvdata *drvdata)
{
	struct clk *clk = drvdata->clk;
	int rc;

	if (IS_ERR(clk)) {
		/* Not all devices have a clock associated with CCREE */
		return 0;
	}

	rc = clk_prepare_enable(clk);
	if (rc)
		return rc;

	return 0;
}

regards,
dan carpenter

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

Thread

[PATCH 5/7] staging: ccree: add clock management support Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-22 09:10 +0200
  Re: [PATCH 5/7] staging: ccree: add clock management support Dan Carpenter <dan.carpenter@oracle.com> - 2017-06-22 11:00 +0200
    Re: [PATCH 5/7] staging: ccree: add clock management support Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-22 15:30 +0200

csiph-web