* [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
@ 2016-02-19 8:42 Andy Shevchenko
2016-02-19 8:42 ` [PATCH v2 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device * Andy Shevchenko
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Andy Shevchenko @ 2016-02-19 8:42 UTC (permalink / raw)
To: Stephen Boyd, Lee Jones, linux-kernel; +Cc: Andy Shevchenko, stable
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)
Cc: stable@vger.kernel.org
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2:
- do not unregister clock before lookup (Stephen)
drivers/mfd/intel_quark_i2c_gpio.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index bdc5e27..7450f5d 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -139,6 +139,7 @@ static int intel_quark_register_i2c_clk(struct intel_quark_mfd *quark_mfd)
INTEL_QUARK_I2C_CONTROLLER_CLK);
if (!quark_mfd->i2c_clk_lookup) {
+ clk_unregister(quark_mfd->i2c_clk);
dev_err(&pdev->dev, "Fixed clk register failed\n");
return -ENOMEM;
}
@@ -150,7 +151,7 @@ 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)
+ if (!quark_mfd->i2c_clk_lookup)
return;
clkdev_drop(quark_mfd->i2c_clk_lookup);
@@ -246,25 +247,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);
-
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;
- return mfd_add_devices(&pdev->dev, 0, intel_quark_mfd_cells,
- ARRAY_SIZE(intel_quark_mfd_cells), NULL, 0,
- NULL);
+ 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 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] 10+ messages in thread
* [PATCH v2 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device *
2016-02-19 8:42 [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Andy Shevchenko
@ 2016-02-19 8:42 ` Andy Shevchenko
2016-03-21 11:54 ` Lee Jones
2016-02-27 0:58 ` [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Stephen Boyd
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2016-02-19 8:42 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 7450f5d..24c2d29 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))
@@ -140,16 +140,16 @@ static int intel_quark_register_i2c_clk(struct intel_quark_mfd *quark_mfd)
if (!quark_mfd->i2c_clk_lookup) {
clk_unregister(quark_mfd->i2c_clk);
- 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);
if (!quark_mfd->i2c_clk_lookup)
return;
@@ -248,10 +248,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)
return ret;
@@ -272,13 +272,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] 10+ messages in thread
* Re: [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
2016-02-19 8:42 [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Andy Shevchenko
2016-02-19 8:42 ` [PATCH v2 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device * Andy Shevchenko
@ 2016-02-27 0:58 ` Stephen Boyd
2016-02-29 10:16 ` Andy Shevchenko
2016-03-21 11:54 ` Lee Jones
2016-03-10 11:08 ` Andy Shevchenko
2016-03-21 11:54 ` Lee Jones
3 siblings, 2 replies; 10+ messages in thread
From: Stephen Boyd @ 2016-02-27 0:58 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Lee Jones, linux-kernel, stable
On 02/19, Andy Shevchenko wrote:
> 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)
> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
Both look fine to me. You may also want to use
devm_clk_register() instead so that cleanup is easier.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
2016-02-27 0:58 ` [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Stephen Boyd
@ 2016-02-29 10:16 ` Andy Shevchenko
2016-03-21 11:54 ` Lee Jones
1 sibling, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2016-02-29 10:16 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Lee Jones, linux-kernel, stable
On Fri, 2016-02-26 at 16:58 -0800, Stephen Boyd wrote:
> On 02/19, Andy Shevchenko wrote:
> > 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)
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
>
> Both look fine to me.
Thanks for review!
> You may also want to use
> devm_clk_register() instead so that cleanup is easier.
Perhaps in the future.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
2016-02-19 8:42 [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Andy Shevchenko
2016-02-19 8:42 ` [PATCH v2 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device * Andy Shevchenko
2016-02-27 0:58 ` [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Stephen Boyd
@ 2016-03-10 11:08 ` Andy Shevchenko
2016-03-16 8:46 ` Lee Jones
2016-03-21 11:54 ` Lee Jones
3 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2016-03-10 11:08 UTC (permalink / raw)
To: Stephen Boyd, Lee Jones, linux-kernel; +Cc: stable
On Fri, 2016-02-19 at 10:42 +0200, Andy Shevchenko wrote:
> 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.
Lee, ping?
>
> Fixes: 60ae5b9f5cdd (mfd: intel_quark_i2c_gpio: Add Intel Quark X1000
> I2C-GPIO MFD Driver)
> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2:
> - do not unregister clock before lookup (Stephen)
> drivers/mfd/intel_quark_i2c_gpio.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c
> b/drivers/mfd/intel_quark_i2c_gpio.c
> index bdc5e27..7450f5d 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -139,6 +139,7 @@ static int intel_quark_register_i2c_clk(struct
> intel_quark_mfd *quark_mfd)
> INTEL_QUARK_I2C_CONT
> ROLLER_CLK);
>
> if (!quark_mfd->i2c_clk_lookup) {
> + clk_unregister(quark_mfd->i2c_clk);
> dev_err(&pdev->dev, "Fixed clk register failed\n");
> return -ENOMEM;
> }
> @@ -150,7 +151,7 @@ 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)
> + if (!quark_mfd->i2c_clk_lookup)
> return;
>
> clkdev_drop(quark_mfd->i2c_clk_lookup);
> @@ -246,25 +247,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);
> -
> 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;
>
> - return mfd_add_devices(&pdev->dev, 0, intel_quark_mfd_cells,
> - ARRAY_SIZE(intel_quark_mfd_cells),
> NULL, 0,
> - NULL);
> + 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 0;
> +
> +err_unregister_i2c_clk:
> + intel_quark_unregister_i2c_clk(pdev);
> + return ret;
> }
>
> static void intel_quark_mfd_remove(struct pci_dev *pdev)
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
2016-03-10 11:08 ` Andy Shevchenko
@ 2016-03-16 8:46 ` Lee Jones
0 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2016-03-16 8:46 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Stephen Boyd, linux-kernel, stable
On Thu, 10 Mar 2016, Andy Shevchenko wrote:
> On Fri, 2016-02-19 at 10:42 +0200, Andy Shevchenko wrote:
> > 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.
>
> Lee, ping?
Pings don't serve any purpose but to irritate. This patch hasn't
been forgotten, it's in the pile.
The last few weeks have been pretty crazy. Please bear with me while I
play catch-up.
> > Fixes: 60ae5b9f5cdd (mfd: intel_quark_i2c_gpio: Add Intel Quark X1000
> > I2C-GPIO MFD Driver)
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > v2:
> > - do not unregister clock before lookup (Stephen)
> > drivers/mfd/intel_quark_i2c_gpio.c | 25 +++++++++++++++++--------
> > 1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mfd/intel_quark_i2c_gpio.c
> > b/drivers/mfd/intel_quark_i2c_gpio.c
> > index bdc5e27..7450f5d 100644
> > --- a/drivers/mfd/intel_quark_i2c_gpio.c
> > +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> > @@ -139,6 +139,7 @@ static int intel_quark_register_i2c_clk(struct
> > intel_quark_mfd *quark_mfd)
> > INTEL_QUARK_I2C_CONT
> > ROLLER_CLK);
> >
> > if (!quark_mfd->i2c_clk_lookup) {
> > + clk_unregister(quark_mfd->i2c_clk);
> > dev_err(&pdev->dev, "Fixed clk register failed\n");
> > return -ENOMEM;
> > }
> > @@ -150,7 +151,7 @@ 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)
> > + if (!quark_mfd->i2c_clk_lookup)
> > return;
> >
> > clkdev_drop(quark_mfd->i2c_clk_lookup);
> > @@ -246,25 +247,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);
> > -
> > 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;
> >
> > - return mfd_add_devices(&pdev->dev, 0, intel_quark_mfd_cells,
> > - ARRAY_SIZE(intel_quark_mfd_cells),
> > NULL, 0,
> > - NULL);
> > + 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 0;
> > +
> > +err_unregister_i2c_clk:
> > + intel_quark_unregister_i2c_clk(pdev);
> > + return ret;
> > }
> >
> > static void intel_quark_mfd_remove(struct pci_dev *pdev)
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
2016-02-27 0:58 ` [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Stephen Boyd
2016-02-29 10:16 ` Andy Shevchenko
@ 2016-03-21 11:54 ` Lee Jones
2016-03-21 18:40 ` Stephen Boyd
1 sibling, 1 reply; 10+ messages in thread
From: Lee Jones @ 2016-03-21 11:54 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Andy Shevchenko, linux-kernel, stable
On Fri, 26 Feb 2016, Stephen Boyd wrote:
> On 02/19, Andy Shevchenko wrote:
> > 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)
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
>
> Both look fine to me. You may also want to use
> devm_clk_register() instead so that cleanup is easier.
I'm going to apply your Ack in order to show that you've seen and have
no problems with the patch.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
2016-02-19 8:42 [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Andy Shevchenko
` (2 preceding siblings ...)
2016-03-10 11:08 ` Andy Shevchenko
@ 2016-03-21 11:54 ` Lee Jones
3 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2016-03-21 11:54 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Stephen Boyd, linux-kernel, stable
On Fri, 19 Feb 2016, Andy Shevchenko wrote:
> 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)
> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2:
> - do not unregister clock before lookup (Stephen)
> drivers/mfd/intel_quark_i2c_gpio.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
Applied, thanks.
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index bdc5e27..7450f5d 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -139,6 +139,7 @@ static int intel_quark_register_i2c_clk(struct intel_quark_mfd *quark_mfd)
> INTEL_QUARK_I2C_CONTROLLER_CLK);
>
> if (!quark_mfd->i2c_clk_lookup) {
> + clk_unregister(quark_mfd->i2c_clk);
> dev_err(&pdev->dev, "Fixed clk register failed\n");
> return -ENOMEM;
> }
> @@ -150,7 +151,7 @@ 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)
> + if (!quark_mfd->i2c_clk_lookup)
> return;
>
> clkdev_drop(quark_mfd->i2c_clk_lookup);
> @@ -246,25 +247,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);
> -
> 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;
>
> - return mfd_add_devices(&pdev->dev, 0, intel_quark_mfd_cells,
> - ARRAY_SIZE(intel_quark_mfd_cells), NULL, 0,
> - NULL);
> + 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 0;
> +
> +err_unregister_i2c_clk:
> + intel_quark_unregister_i2c_clk(pdev);
> + return ret;
> }
>
> static void intel_quark_mfd_remove(struct pci_dev *pdev)
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device *
2016-02-19 8:42 ` [PATCH v2 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device * Andy Shevchenko
@ 2016-03-21 11:54 ` Lee Jones
0 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2016-03-21 11:54 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Stephen Boyd, linux-kernel
On Fri, 19 Feb 2016, Andy Shevchenko wrote:
> 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(-)
Applied, thanks.
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index 7450f5d..24c2d29 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))
> @@ -140,16 +140,16 @@ static int intel_quark_register_i2c_clk(struct intel_quark_mfd *quark_mfd)
>
> if (!quark_mfd->i2c_clk_lookup) {
> clk_unregister(quark_mfd->i2c_clk);
> - 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);
>
> if (!quark_mfd->i2c_clk_lookup)
> return;
> @@ -248,10 +248,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)
> return ret;
>
> @@ -272,13 +272,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);
> }
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path
2016-03-21 11:54 ` Lee Jones
@ 2016-03-21 18:40 ` Stephen Boyd
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2016-03-21 18:40 UTC (permalink / raw)
To: Lee Jones; +Cc: Andy Shevchenko, linux-kernel, stable
On 03/21, Lee Jones wrote:
> On Fri, 26 Feb 2016, Stephen Boyd wrote:
> >
> > Both look fine to me. You may also want to use
> > devm_clk_register() instead so that cleanup is easier.
>
> I'm going to apply your Ack in order to show that you've seen and have
> no problems with the patch.
Ok, thanks.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-03-21 19:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-19 8:42 [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Andy Shevchenko
2016-02-19 8:42 ` [PATCH v2 2/2] mfd: intel_quark_i2c_gpio: switch to use struct device * Andy Shevchenko
2016-03-21 11:54 ` Lee Jones
2016-02-27 0:58 ` [PATCH v2 1/2] mfd: intel_quark_i2c_gpio: remove clock tree on error path Stephen Boyd
2016-02-29 10:16 ` Andy Shevchenko
2016-03-21 11:54 ` Lee Jones
2016-03-21 18:40 ` Stephen Boyd
2016-03-10 11:08 ` Andy Shevchenko
2016-03-16 8:46 ` Lee Jones
2016-03-21 11:54 ` Lee Jones
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.