linux-kernel - Re: [PATCH v3] i2c: ocores: use devm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 25 May 2023 08:02:02 +0300
From:   Dan Carpenter <dan.carpenter@...aro.org>
To:     Wang Zhang <silver_code@...t.edu.cn>
Cc:     Peter Korsgaard <peter@...sgaard.com>,
        Andrew Lunn <andrew@...n.ch>,
        hust-os-kernel-patches@...glegroups.com, linux-i2c@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] i2c: ocores: use devm_ managed clks

On Wed, May 24, 2023 at 11:43:18PM +0800, Wang Zhang wrote:
> @@ -780,19 +768,18 @@ static int ocores_i2c_resume(struct device *dev)
>  {
>  	struct ocores_i2c *i2c = dev_get_drvdata(dev);
>  
> -	if (!IS_ERR(i2c->clk)) {
> -		unsigned long rate;
> -		int ret = clk_prepare_enable(i2c->clk);
> +	unsigned long rate;
> +	int ret = clk_prepare_enable(i2c->clk);

Don't put functions which can fail in the declaration block.  Generally
the declaration block is for preliminary stuff, and the important
actions should be in the code block.  There should not be a blank line
before the function call and the error checking.

>  
> -		if (ret) {
> -			dev_err(dev,
> -				"clk_prepare_enable failed: %d\n", ret);
> -			return ret;
> -		}
> -		rate = clk_get_rate(i2c->clk) / 1000;
> -		if (rate)
> -			i2c->ip_clock_khz = rate;
> +	if (ret) {
> +		dev_err(dev,
> +			"clk_prepare_enable failed: %d\n", ret);

This can fit on one line now.

	int ret;

	ret = clk_prepare_enable(i2c->clk);
	if (ret) {
		dev_err(dev, "clk_prepare_enable failed: %d\n", ret);
		return ret;
	}

regards,
dan carpenter

> +		return ret;
>  	}
> +	rate = clk_get_rate(i2c->clk) / 1000;
> +	if (rate)
> +		i2c->ip_clock_khz = rate;
> +
>  	return ocores_init(dev, i2c);