Found by Linux Driver Verification project (linuxtesting.org).
Fixes: 142168eba9dc spi: bcm63xx-hsspi: add bcm63xx HSSPI driver
Signed-off-by: Stefan Potyra <[email protected]>
---
drivers/spi/spi-bcm63xx-hsspi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index cbcba614b253..46cd9b683d22 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -352,6 +352,10 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
if (IS_ERR(clk))
return PTR_ERR(clk);
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
rate = clk_get_rate(clk);
if (!rate) {
struct clk *pll_clk = devm_clk_get(dev, "pll");
@@ -364,10 +368,6 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
return -EINVAL;
}
- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
master = spi_alloc_master(&pdev->dev, sizeof(*bs));
if (!master) {
ret = -ENOMEM;
--
2.17.0
+Jonas,
On 04/19/2018 06:03 AM, Stefan Potyra wrote:
> Found by Linux Driver Verification project (linuxtesting.org).
Please use a commit subject which matches what was used before:
git log --no-merges --oneline drivers/spi/spi-bcm63xx-hsspi.c
378da4a65f3a spi/bcm63xx-hspi: fix error return code in
bcm63xx_hsspi_probe()
c3c25ea712c9 spi/bcm63xx-hspi: Fix checkpatch warnings
0b85a8421790 spi: bcm63xx-hsspi: Export OF device ID table as module aliases
7ab2463550e2 spi/bcm63xx-hsspi: allow for probing through devicetree
ff18e1ef04e2 spi/bcm63xx-hsspi: allow providing clock rate through a
second clock
f4d862237715 spi/bcm63xx-hsspi: add support for dual spi read/write
14ac00e033c5 spi: drop owner assignment from platform_drivers
e4745fef5595 spi: Remove unneeded include of linux/workqueue.h
1480916ebd6f spi: bcm63xx-hsspi: Use SIMPLE_DEV_PM_OPS macro
aa0fe82629f1 spi: Use reinit_completion at appropriate places
937ebf9cd34a spi/bcm63xx-hsspi: fix pm sleep support
7d255695804f spi/bcm63xx-hsspi: use devm_register_master()
dea5de1b37c0 spi/bcm63xx-hsspi: check result of clk_prepare_enable
b1bdd4f883e1 spi: bcm63xx-hsspi: Use devm_clk_get()
87917528cc92 spi: bcm63xx-hsspi: checking for ERR_PTR instead of NULL
142168eba9dc spi: bcm63xx-hsspi: add bcm63xx HSSPI driver
>
> Fixes: 142168eba9dc spi: bcm63xx-hsspi: add bcm63xx HSSPI driver
Please include the commit title in ("<title>") as is indicated in the
SubmittingPatches document.
Thank you
> Signed-off-by: Stefan Potyra <[email protected]>
> ---
> drivers/spi/spi-bcm63xx-hsspi.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
> index cbcba614b253..46cd9b683d22 100644
> --- a/drivers/spi/spi-bcm63xx-hsspi.c
> +++ b/drivers/spi/spi-bcm63xx-hsspi.c
> @@ -352,6 +352,10 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
> if (IS_ERR(clk))
> return PTR_ERR(clk);
>
> + ret = clk_prepare_enable(clk);
> + if (ret)
> + return ret;
> +
> rate = clk_get_rate(clk);
> if (!rate) {
> struct clk *pll_clk = devm_clk_get(dev, "pll");
> @@ -364,10 +368,6 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - ret = clk_prepare_enable(clk);
> - if (ret)
> - return ret;
> -
> master = spi_alloc_master(&pdev->dev, sizeof(*bs));
> if (!master) {
> ret = -ENOMEM;
>
--
Florian
Enable the clock prior to calling clk_get_rate(), because clk_get_rate()
should only be called if the clock is enabled.
Found by Linux Driver Verification project (linuxtesting.org).
Fixes: 142168eba9dc (spi: "bcm63xx-hsspi: add bcm63xx HSSPI driver")
Signed-off-by: Stefan Potyra <[email protected]>
---
drivers/spi/spi-bcm63xx-hsspi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index cbcba614b253..46cd9b683d22 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -352,6 +352,10 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
if (IS_ERR(clk))
return PTR_ERR(clk);
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
rate = clk_get_rate(clk);
if (!rate) {
struct clk *pll_clk = devm_clk_get(dev, "pll");
@@ -364,10 +368,6 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
return -EINVAL;
}
- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
master = spi_alloc_master(&pdev->dev, sizeof(*bs));
if (!master) {
ret = -ENOMEM;
--
2.17.0
On Tue, Apr 24, 2018 at 06:16:05PM +0200, Stefan Potyra wrote:
> + ret = clk_prepare_enable(clk);
> + if (ret)
> + return ret;
> +
> rate = clk_get_rate(clk);
> if (!rate) {
> struct clk *pll_clk = devm_clk_get(dev, "pll");
> @@ -364,10 +368,6 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - ret = clk_prepare_enable(clk);
> - if (ret)
> - return ret;
> -
We can return an error after the clock was enabled, this will leake the
clock if that happens.
Enable the clock prior to calling clk_get_rate(), because clk_get_rate()
should only be called if the clock is enabled.
Found by Linux Driver Verification project (linuxtesting.org).
Fixes: 142168eba9dc ("spi: bcm63xx-hsspi: add bcm63xx HSSPI driver")
Signed-off-by: Stefan Potyra <[email protected]>
---
drivers/spi/spi-bcm63xx-hsspi.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index cbcba614b253..8475703543e5 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -352,22 +352,27 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
if (IS_ERR(clk))
return PTR_ERR(clk);
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
rate = clk_get_rate(clk);
if (!rate) {
struct clk *pll_clk = devm_clk_get(dev, "pll");
- if (IS_ERR(pll_clk))
- return PTR_ERR(pll_clk);
+ if (IS_ERR(pll_clk)) {
+ ret = PTR_ERR(pll_clk);
+ goto out_disable_clk;
+ }
+
rate = clk_get_rate(pll_clk);
- if (!rate)
- return -EINVAL;
+ if (!rate) {
+ ret = -EINVAL;
+ goto out_disable_clk;
+ }
}
- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
master = spi_alloc_master(&pdev->dev, sizeof(*bs));
if (!master) {
ret = -ENOMEM;
--
2.17.0
On Wed, Apr 25, 2018 at 03:47:30PM +0200, Stefan Potyra wrote:
> + ret = clk_prepare_enable(clk);
> + if (ret)
> + return ret;
> +
> rate = clk_get_rate(clk);
> if (!rate) {
> struct clk *pll_clk = devm_clk_get(dev, "pll");
>
> - if (IS_ERR(pll_clk))
> - return PTR_ERR(pll_clk);
> + if (IS_ERR(pll_clk)) {
> + ret = PTR_ERR(pll_clk);
> + goto out_disable_clk;
> + }
> +
>
> rate = clk_get_rate(pll_clk);
Isn't this just showing the same problem with not enabling the pll_clk
before getting the rate that you're trying to fix for the main clk?
Enable the clock prior to calling clk_get_rate(), because clk_get_rate()
should only be called if the clock is enabled.
Additionally, prepare/enable the pll_clk before calling clk_get_rate()
for the same reason.
Found by Linux Driver Verification project (linuxtesting.org).
Fixes: 142168eba9dc ("spi: bcm63xx-hsspi: add bcm63xx HSSPI driver")
Signed-off-by: Stefan Potyra <[email protected]>
---
drivers/spi/spi-bcm63xx-hsspi.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index cbcba614b253..8475703543e5 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -352,22 +352,27 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
if (IS_ERR(clk))
return PTR_ERR(clk);
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
rate = clk_get_rate(clk);
if (!rate) {
struct clk *pll_clk = devm_clk_get(dev, "pll");
- if (IS_ERR(pll_clk))
- return PTR_ERR(pll_clk);
+ if (IS_ERR(pll_clk)) {
+ ret = PTR_ERR(pll_clk);
+ goto out_disable_clk;
+ }
+
rate = clk_get_rate(pll_clk);
- if (!rate)
- return -EINVAL;
+ if (!rate) {
+ ret = -EINVAL;
+ goto out_disable_clk;
+ }
}
- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
master = spi_alloc_master(&pdev->dev, sizeof(*bs));
if (!master) {
ret = -ENOMEM;
--
2.17.0
On Wed, Apr 25, 2018 at 06:49:06PM +0200, Stefan Potyra wrote:
> Additionally, prepare/enable the pll_clk before calling clk_get_rate()
> for the same reason.
I don't see this change in the code:
> struct clk *pll_clk = devm_clk_get(dev, "pll");
>
> - if (IS_ERR(pll_clk))
> - return PTR_ERR(pll_clk);
> + if (IS_ERR(pll_clk)) {
> + ret = PTR_ERR(pll_clk);
> + goto out_disable_clk;
> + }
> +
>
> rate = clk_get_rate(pll_clk);
> - if (!rate)
> - return -EINVAL;
Did you forget a git add?
Enable the clock prior to calling clk_get_rate(), because clk_get_rate()
should only be called if the clock is enabled.
Additionally, prepare/enable the pll_clk before calling clk_get_rate()
for the same reason.
Found by Linux Driver Verification project (linuxtesting.org).
Fixes: 142168eba9dc ("spi: bcm63xx-hsspi: add bcm63xx HSSPI driver")
Signed-off-by: Stefan Potyra <[email protected]>
---
drivers/spi/spi-bcm63xx-hsspi.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index cbcba614b253..c23849f7aa7b 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -352,22 +352,31 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
if (IS_ERR(clk))
return PTR_ERR(clk);
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
rate = clk_get_rate(clk);
if (!rate) {
struct clk *pll_clk = devm_clk_get(dev, "pll");
- if (IS_ERR(pll_clk))
- return PTR_ERR(pll_clk);
+ if (IS_ERR(pll_clk)) {
+ ret = PTR_ERR(pll_clk);
+ goto out_disable_clk;
+ }
+
+ ret = clk_prepare_enable(pll_clk);
+ if (ret)
+ goto out_disable_clk;
rate = clk_get_rate(pll_clk);
- if (!rate)
- return -EINVAL;
+ clk_disable_unprepare(pll_clk);
+ if (!rate) {
+ ret = -EINVAL;
+ goto out_disable_clk;
+ }
}
- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
master = spi_alloc_master(&pdev->dev, sizeof(*bs));
if (!master) {
ret = -ENOMEM;
--
2.17.0
The patch
spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate().
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 0d7412ed1f5dc0858eb4f29650a8c9c5cce8b285 Mon Sep 17 00:00:00 2001
From: Stefan Potyra <[email protected]>
Date: Thu, 26 Apr 2018 09:28:02 +0200
Subject: [PATCH] spi/bcm63xx-hspi: Enable the clock before calling
clk_get_rate().
Enable the clock prior to calling clk_get_rate(), because clk_get_rate()
should only be called if the clock is enabled.
Additionally, prepare/enable the pll_clk before calling clk_get_rate()
for the same reason.
Found by Linux Driver Verification project (linuxtesting.org).
Fixes: 142168eba9dc ("spi: bcm63xx-hsspi: add bcm63xx HSSPI driver")
Signed-off-by: Stefan Potyra <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/spi/spi-bcm63xx-hsspi.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index cbcba614b253..c23849f7aa7b 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -352,22 +352,31 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
if (IS_ERR(clk))
return PTR_ERR(clk);
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
rate = clk_get_rate(clk);
if (!rate) {
struct clk *pll_clk = devm_clk_get(dev, "pll");
- if (IS_ERR(pll_clk))
- return PTR_ERR(pll_clk);
+ if (IS_ERR(pll_clk)) {
+ ret = PTR_ERR(pll_clk);
+ goto out_disable_clk;
+ }
+
+ ret = clk_prepare_enable(pll_clk);
+ if (ret)
+ goto out_disable_clk;
rate = clk_get_rate(pll_clk);
- if (!rate)
- return -EINVAL;
+ clk_disable_unprepare(pll_clk);
+ if (!rate) {
+ ret = -EINVAL;
+ goto out_disable_clk;
+ }
}
- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
master = spi_alloc_master(&pdev->dev, sizeof(*bs));
if (!master) {
ret = -ENOMEM;
--
2.17.0