* [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
@ 2016-02-17 10:29 Andy Shevchenko
2016-02-17 10:29 ` [PATCH v1 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device * Andy Shevchenko
2016-02-19 2:45 ` [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Stephen Boyd
0 siblings, 2 replies; 4+ messages in thread
From: Andy Shevchenko @ 2016-02-17 10:29 UTC (permalink / raw)
To: Stephen Boyd, Lee Jones, linux-kernel; +Cc: Andy Shevchenko
There is a potential resource leak in case when ->probe() fails. We have to
unregister and remove clock tree which is done here.
This is a follow up to previously pushed commit c4726abce63b ("mfd:
intel_quark_i2c_gpio: Use clkdev_create()") that prevents double free() when
clkdev_drop() followed by kfree() in devm_kcalloc() release stage.
I leave Fixes tag here, but the backporting will require to backport the commit
c4726abce63b ("mfd: intel_quark_i2c_gpio: Use clkdev_create()") first.
Fixes: 60ae5b9f5cdd (mfd: intel_quark_i2c_gpio: Add Intel Quark X1000 I2C-GPIO MFD Driver)
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/mfd/intel_quark_i2c_gpio.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index bdc5e27..43d8066 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -150,11 +150,10 @@ static void intel_quark_unregister_i2c_clk(struct pci_dev *pdev)
{
struct intel_quark_mfd *quark_mfd = dev_get_drvdata(&pdev->dev);
- if (!quark_mfd->i2c_clk || !quark_mfd->i2c_clk_lookup)
+ clk_unregister(quark_mfd->i2c_clk);
+ if (!quark_mfd->i2c_clk_lookup)
return;
-
clkdev_drop(quark_mfd->i2c_clk_lookup);
- clk_unregister(quark_mfd->i2c_clk);
}
static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
@@ -246,25 +245,33 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
quark_mfd = devm_kzalloc(&pdev->dev, sizeof(*quark_mfd), GFP_KERNEL);
if (!quark_mfd)
return -ENOMEM;
+
quark_mfd->pdev = pdev;
+ dev_set_drvdata(&pdev->dev, quark_mfd);
ret = intel_quark_register_i2c_clk(quark_mfd);
if (ret)
- return ret;
-
- dev_set_drvdata(&pdev->dev, quark_mfd);
+ goto err_unregister_i2c_clk;
ret = intel_quark_i2c_setup(pdev, &intel_quark_mfd_cells[1]);
if (ret)
- return ret;
+ goto err_unregister_i2c_clk;
ret = intel_quark_gpio_setup(pdev, &intel_quark_mfd_cells[0]);
if (ret)
- return ret;
+ goto err_unregister_i2c_clk;
+
+ ret = mfd_add_devices(&pdev->dev, 0, intel_quark_mfd_cells,
+ ARRAY_SIZE(intel_quark_mfd_cells), NULL, 0,
+ NULL);
+ if (ret)
+ goto err_unregister_i2c_clk;
- return mfd_add_devices(&pdev->dev, 0, intel_quark_mfd_cells,
- ARRAY_SIZE(intel_quark_mfd_cells), NULL, 0,
- NULL);
+ return 0;
+
+err_unregister_i2c_clk:
+ intel_quark_unregister_i2c_clk(pdev);
+ return ret;
}
static void intel_quark_mfd_remove(struct pci_dev *pdev)
--
2.7.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v1 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device *
2016-02-17 10:29 [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Andy Shevchenko
@ 2016-02-17 10:29 ` Andy Shevchenko
2016-02-19 2:45 ` [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Stephen Boyd
1 sibling, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2016-02-17 10:29 UTC (permalink / raw)
To: Stephen Boyd, Lee Jones, linux-kernel; +Cc: Andy Shevchenko
There is no need to pass struct pci_dev * to intel_quark_register_i2c_clk() and
intel_quark_unregister_i2c_clk(). Change the parameter to struct device *. As a
result store it in the private struct instead of struct pci_dev *.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/mfd/intel_quark_i2c_gpio.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 43d8066..ca8751e 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -53,7 +53,7 @@
#define INTEL_QUARK_I2C_CLK_HZ 33000000
struct intel_quark_mfd {
- struct pci_dev *pdev;
+ struct device *dev;
struct clk *i2c_clk;
struct clk_lookup *i2c_clk_lookup;
};
@@ -123,12 +123,12 @@ static const struct pci_device_id intel_quark_mfd_ids[] = {
};
MODULE_DEVICE_TABLE(pci, intel_quark_mfd_ids);
-static int intel_quark_register_i2c_clk(struct intel_quark_mfd *quark_mfd)
+static int intel_quark_register_i2c_clk(struct device *dev)
{
- struct pci_dev *pdev = quark_mfd->pdev;
+ struct intel_quark_mfd *quark_mfd = dev_get_drvdata(dev);
struct clk *i2c_clk;
- i2c_clk = clk_register_fixed_rate(&pdev->dev,
+ i2c_clk = clk_register_fixed_rate(dev,
INTEL_QUARK_I2C_CONTROLLER_CLK, NULL,
CLK_IS_ROOT, INTEL_QUARK_I2C_CLK_HZ);
if (IS_ERR(i2c_clk))
@@ -139,16 +139,16 @@ static int intel_quark_register_i2c_clk(struct intel_quark_mfd *quark_mfd)
INTEL_QUARK_I2C_CONTROLLER_CLK);
if (!quark_mfd->i2c_clk_lookup) {
- dev_err(&pdev->dev, "Fixed clk register failed\n");
+ dev_err(dev, "Fixed clk register failed\n");
return -ENOMEM;
}
return 0;
}
-static void intel_quark_unregister_i2c_clk(struct pci_dev *pdev)
+static void intel_quark_unregister_i2c_clk(struct device *dev)
{
- struct intel_quark_mfd *quark_mfd = dev_get_drvdata(&pdev->dev);
+ struct intel_quark_mfd *quark_mfd = dev_get_drvdata(dev);
clk_unregister(quark_mfd->i2c_clk);
if (!quark_mfd->i2c_clk_lookup)
@@ -246,10 +246,10 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
if (!quark_mfd)
return -ENOMEM;
- quark_mfd->pdev = pdev;
+ quark_mfd->dev = &pdev->dev;
dev_set_drvdata(&pdev->dev, quark_mfd);
- ret = intel_quark_register_i2c_clk(quark_mfd);
+ ret = intel_quark_register_i2c_clk(&pdev->dev);
if (ret)
goto err_unregister_i2c_clk;
@@ -270,13 +270,13 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
return 0;
err_unregister_i2c_clk:
- intel_quark_unregister_i2c_clk(pdev);
+ intel_quark_unregister_i2c_clk(&pdev->dev);
return ret;
}
static void intel_quark_mfd_remove(struct pci_dev *pdev)
{
- intel_quark_unregister_i2c_clk(pdev);
+ intel_quark_unregister_i2c_clk(&pdev->dev);
mfd_remove_devices(&pdev->dev);
}
--
2.7.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
2016-02-17 10:29 [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Andy Shevchenko
2016-02-17 10:29 ` [PATCH v1 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device * Andy Shevchenko
@ 2016-02-19 2:45 ` Stephen Boyd
2016-02-19 8:12 ` Andy Shevchenko
1 sibling, 1 reply; 4+ messages in thread
From: Stephen Boyd @ 2016-02-19 2:45 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Lee Jones, linux-kernel
On 02/17, Andy Shevchenko wrote:
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index bdc5e27..43d8066 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -150,11 +150,10 @@ static void intel_quark_unregister_i2c_clk(struct pci_dev *pdev)
> {
> struct intel_quark_mfd *quark_mfd = dev_get_drvdata(&pdev->dev);
>
> - if (!quark_mfd->i2c_clk || !quark_mfd->i2c_clk_lookup)
> + clk_unregister(quark_mfd->i2c_clk);
> + if (!quark_mfd->i2c_clk_lookup)
> return;
> -
> clkdev_drop(quark_mfd->i2c_clk_lookup);
It's probably not a great idea to unregister the clk before the
lookup is dropped. I suppose nothing too bad will happen though
because we handle this case in the framework and substitute dummy
ops in place of the real ones.
> - clk_unregister(quark_mfd->i2c_clk);
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
2016-02-19 2:45 ` [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Stephen Boyd
@ 2016-02-19 8:12 ` Andy Shevchenko
0 siblings, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2016-02-19 8:12 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Andy Shevchenko, Lee Jones, linux-kernel@vger.kernel.org
On Fri, Feb 19, 2016 at 4:45 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 02/17, Andy Shevchenko wrote:
>> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
>> index bdc5e27..43d8066 100644
>> --- a/drivers/mfd/intel_quark_i2c_gpio.c
>> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
>> @@ -150,11 +150,10 @@ static void intel_quark_unregister_i2c_clk(struct pci_dev *pdev)
>> {
>> struct intel_quark_mfd *quark_mfd = dev_get_drvdata(&pdev->dev);
>>
>> - if (!quark_mfd->i2c_clk || !quark_mfd->i2c_clk_lookup)
>> + clk_unregister(quark_mfd->i2c_clk);
>> + if (!quark_mfd->i2c_clk_lookup)
>> return;
>> -
>> clkdev_drop(quark_mfd->i2c_clk_lookup);
>
> It's probably not a great idea to unregister the clk before the
> lookup is dropped. I suppose nothing too bad will happen though
> because we handle this case in the framework and substitute dummy
> ops in place of the real ones.
Thanks for pointing this out.
Okay, I will re-do this. I think the correct one is to add
clk_unregister() call to the _register_i2c_clk().
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-19 8:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-17 10:29 [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Andy Shevchenko
2016-02-17 10:29 ` [PATCH v1 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device * Andy Shevchenko
2016-02-19 2:45 ` [PATCH v1 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Stephen Boyd
2016-02-19 8:12 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).