Re: [PATCH v3] i2c: ocores: use devm_ managed clks

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

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);