2011/11/2 Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx>:
> On Wed, Nov 02, 2011 at 10:39:04AM +0000, Jamie Iles wrote:
>> > + clk = clk_get(&pdev->dev, NULL);
>> > + if (IS_ERR(clk)) {
>> > + err = PTR_ERR(clk);
>> > + dev_err(&pdev->dev, "Clock get failed\n");
>> > + goto out;
>> > + }
>> > +
>> > + clk_enable(clk);
>>
>> The return value of clk_enable() should really be checked.
>
> Now that the clk_prepare() patch has been enabled, new drivers should be
> written assuming that clk_prepare() will be necessary before clk_enable().
>
done for this as well.
diff --git a/drivers/i2c/busses/i2c-sirfsoc.c b/drivers/i2c/busses/i2c-sirfsoc.c
index 9c11458..cf4f61f 100644
--- a/drivers/i2c/busses/i2c-sirfsoc.c
+++ b/drivers/i2c/busses/i2c-sirfsoc.c
@@ -244,17 +244,27 @@ static int __devinit i2c_sirfsoc_probe(struct
platform_device *pdev)
if (pdata == NULL) {
err = -ENODEV;
dev_err(&pdev->dev, "No platform data!\n");
- goto out;
+ goto err_pdata;
}
clk = clk_get(&pdev->dev, NULL);
if (IS_ERR(clk)) {
err = PTR_ERR(clk);
dev_err(&pdev->dev, "Clock get failed\n");
- goto out;
+ goto err_get_clk;
}
- clk_enable(clk);
+ err = clk_prepare(clk);
+ if (err) {
+ dev_err(&pdev->dev, "Clock prepare failed\n");
+ goto err_clk_prep;
+ }
+
+ err = clk_enable(clk);
+ if (err) {
+ dev_err(&pdev->dev, "Clock enable failed\n");
+ goto err_clk_en;
+ }
ctrl_speed = clk_get_rate(clk);
@@ -263,14 +273,14 @@ static int __devinit i2c_sirfsoc_probe(struct
platform_device *pdev)
dev_err(&pdev->dev,
"Can't allocate new i2c adapter!\n");
err = -ENOMEM;
- goto clk_out;
+ goto out;
}
siic = devm_kzalloc(&pdev->dev, sizeof(*siic), GFP_KERNEL);
if (!siic) {
dev_err(&pdev->dev, "Can't allocate driver data\n");
err = -ENOMEM;
- goto clk_out;
+ goto out;
}
new_adapter->class = I2C_CLASS_HWMON | I2C_CLASS_DDC | I2C_CLASS_SPD;
siic->adapter = new_adapter;
@@ -279,7 +289,7 @@ static int __devinit i2c_sirfsoc_probe(struct
platform_device *pdev)
if (mem_res == NULL) {
dev_err(&pdev->dev, "Unable to get MEM resource\n");
err = -EINVAL;
- goto clk_out;
+ goto out;
}
siic->base =
@@ -287,18 +297,18 @@ static int __devinit i2c_sirfsoc_probe(struct
platform_device *pdev)
if (siic->base == NULL) {
dev_err(&pdev->dev, "IO remap failed!\n");
err = -ENOMEM;
- goto clk_out;
+ goto out;
}
siic->irq = platform_get_irq(pdev, 0);
if (!siic->irq) {
err = -EINVAL;
- goto clk_out;
+ goto out;
}
err = devm_request_irq(&pdev->dev, siic->irq, i2c_sirfsoc_irq, 0,
dev_name(&pdev->dev), siic);
if (err)
- goto clk_out;
+ goto out;
new_adapter->algo = &i2c_sirfsoc_algo;
new_adapter->algo_data = siic;
@@ -338,7 +348,7 @@ static int __devinit i2c_sirfsoc_probe(struct
platform_device *pdev)
err = i2c_add_numbered_adapter(new_adapter);
if (err < 0) {
dev_err(&pdev->dev, "Can't add new i2c adapter\n");
- goto clk_out;
+ goto out;
}
clk_disable(clk);
@@ -347,10 +357,14 @@ static int __devinit i2c_sirfsoc_probe(struct
platform_device *pdev)
return 0;
-clk_out:
+out:
clk_disable(clk);
+err_clk_en:
+ clk_unprepare(clk);
+err_clk_prep:
clk_put(clk);
-out:
+err_get_clk:
+err_pdata:
return err;
}
@@ -362,6 +376,7 @@ static int __devexit i2c_sirfsoc_remove(struct
platform_device *pdev)
writel(SIRFSOC_I2C_RESET, siic->base + SIRFSOC_I2C_CTRL);
i2c_del_adapter(adapter);
clk_disable(siic->clk);
+ clk_unprepare(siic->clk);
clk_put(siic->clk);
return 0;
}
> And one may query why it's not possible to use clk_enable()...clk_disable()
> around the transfer itself, so the clock can be turned off while the device
> is idle. Obviously if its expecting to be operated in slave mode as well
> then you may need to keep the clock enabled.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
