All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Enable the clock before calling clk_get_rate().
@ 2018-04-19 13:03 ` Stefan Potyra
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Potyra @ 2018-04-19 13:03 UTC (permalink / raw)
  To: Mark Brown, Florian Fainelli, bcm-kernel-feedback-list,
	linux-spi, linux-arm-kernel, linux-kernel
  Cc: ldv-project, sil2review

Found by Linux Driver Verification project (linuxtesting.org).

Fixes: 142168eba9dc spi: bcm63xx-hsspi: add bcm63xx HSSPI driver
Signed-off-by: Stefan Potyra <Stefan.Potyra@elektrobit.com>
---
 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] Enable the clock before calling clk_get_rate().
@ 2018-04-19 13:03 ` Stefan Potyra
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Potyra @ 2018-04-19 13:03 UTC (permalink / raw)
  To: linux-arm-kernel

Found by Linux Driver Verification project (linuxtesting.org).

Fixes: 142168eba9dc spi: bcm63xx-hsspi: add bcm63xx HSSPI driver
Signed-off-by: Stefan Potyra <Stefan.Potyra@elektrobit.com>
---
 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] Enable the clock before calling clk_get_rate().
  2018-04-19 13:03 ` Stefan Potyra
@ 2018-04-21 23:10   ` Florian Fainelli
  -1 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2018-04-21 23:10 UTC (permalink / raw)
  To: Stefan Potyra, Mark Brown, Florian Fainelli,
	bcm-kernel-feedback-list, linux-spi, linux-arm-kernel,
	linux-kernel, Jonas Gorski
  Cc: ldv-project, sil2review

+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 <Stefan.Potyra@elektrobit.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] Enable the clock before calling clk_get_rate().
@ 2018-04-21 23:10   ` Florian Fainelli
  0 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2018-04-21 23:10 UTC (permalink / raw)
  To: linux-arm-kernel

+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 <Stefan.Potyra@elektrobit.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate().
  2018-04-21 23:10   ` Florian Fainelli
  (?)
@ 2018-04-24 16:16   ` Stefan Potyra
  2018-04-24 17:32       ` Mark Brown
  -1 siblings, 1 reply; 17+ messages in thread
From: Stefan Potyra @ 2018-04-24 16:16 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Mark Brown, Florian Fainelli, bcm-kernel-feedback-list,
	linux-spi, linux-arm-kernel, linux-kernel, Jonas Gorski,
	ldv-project, sil2review

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 <Stefan.Potyra@elektrobit.com>
---
 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate().
  2018-04-24 16:16   ` [PATCH v2] spi/bcm63xx-hspi: " Stefan Potyra
@ 2018-04-24 17:32       ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-04-24 17:32 UTC (permalink / raw)
  To: Stefan Potyra
  Cc: Florian Fainelli, Florian Fainelli, bcm-kernel-feedback-list,
	linux-spi, linux-arm-kernel, linux-kernel, Jonas Gorski,
	ldv-project, sil2review

[-- Attachment #1: Type: text/plain, Size: 534 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate().
@ 2018-04-24 17:32       ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-04-24 17:32 UTC (permalink / raw)
  To: linux-arm-kernel

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180424/092def40/attachment.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3] spi/bcm63xx-hspi: Enable the clock before calling
  2018-04-24 17:32       ` Mark Brown
  (?)
@ 2018-04-25 13:47       ` Stefan Potyra
  2018-04-25 15:50           ` Mark Brown
  -1 siblings, 1 reply; 17+ messages in thread
From: Stefan Potyra @ 2018-04-25 13:47 UTC (permalink / raw)
  To: Mark Brown
  Cc: Florian Fainelli, Florian Fainelli, bcm-kernel-feedback-list,
	linux-spi, linux-arm-kernel, linux-kernel, Jonas Gorski,
	ldv-project, sil2review

[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]

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 <Stefan.Potyra@elektrobit.com>
---
 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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] spi/bcm63xx-hspi: Enable the clock before calling
  2018-04-25 13:47       ` [PATCH v3] spi/bcm63xx-hspi: Enable the clock before calling Stefan Potyra
  2018-04-25 15:50           ` Mark Brown
@ 2018-04-25 15:50           ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-04-25 15:50 UTC (permalink / raw)
  To: Stefan Potyra
  Cc: Florian Fainelli, Florian Fainelli, bcm-kernel-feedback-list,
	linux-spi, linux-arm-kernel, linux-kernel, Jonas Gorski,
	ldv-project, sil2review

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] spi/bcm63xx-hspi: Enable the clock before calling
@ 2018-04-25 15:50           ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-04-25 15:50 UTC (permalink / raw)
  To: Stefan Potyra
  Cc: ldv-project, sil2review, Florian Fainelli, Florian Fainelli,
	linux-kernel, linux-spi, bcm-kernel-feedback-list, Jonas Gorski,
	linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 592 bytes --]

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?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3] spi/bcm63xx-hspi: Enable the clock before calling
@ 2018-04-25 15:50           ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-04-25 15:50 UTC (permalink / raw)
  To: linux-arm-kernel

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?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180425/88181dcb/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate().
  2018-04-25 15:50           ` Mark Brown
  (?)
  (?)
@ 2018-04-25 16:49           ` Stefan Potyra
  2018-04-25 17:28               ` Mark Brown
  -1 siblings, 1 reply; 17+ messages in thread
From: Stefan Potyra @ 2018-04-25 16:49 UTC (permalink / raw)
  To: Mark Brown
  Cc: Florian Fainelli, Florian Fainelli, bcm-kernel-feedback-list,
	linux-spi, linux-arm-kernel, linux-kernel, Jonas Gorski,
	ldv-project, sil2review

[-- Attachment #1: Type: text/plain, Size: 1502 bytes --]

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 <Stefan.Potyra@elektrobit.com>
---
 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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate().
  2018-04-25 16:49           ` [PATCH v4] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate() Stefan Potyra
@ 2018-04-25 17:28               ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-04-25 17:28 UTC (permalink / raw)
  To: Stefan Potyra
  Cc: Florian Fainelli, Florian Fainelli, bcm-kernel-feedback-list,
	linux-spi, linux-arm-kernel, linux-kernel, Jonas Gorski,
	ldv-project, sil2review

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate().
@ 2018-04-25 17:28               ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-04-25 17:28 UTC (permalink / raw)
  To: linux-arm-kernel

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?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180425/2a164cf9/attachment.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v5] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate().
  2018-04-25 17:28               ` Mark Brown
  (?)
@ 2018-04-26  7:28               ` Stefan Potyra
  2018-04-26 11:53                   ` Mark Brown
  -1 siblings, 1 reply; 17+ messages in thread
From: Stefan Potyra @ 2018-04-26  7:28 UTC (permalink / raw)
  To: Mark Brown
  Cc: Florian Fainelli, Florian Fainelli, bcm-kernel-feedback-list,
	linux-spi, linux-arm-kernel, linux-kernel, Jonas Gorski,
	ldv-project, sil2review

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

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 <Stefan.Potyra@elektrobit.com>
---
 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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Applied "spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate()." to the spi tree
  2018-04-26  7:28               ` [PATCH v5] " Stefan Potyra
@ 2018-04-26 11:53                   ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-04-26 11:53 UTC (permalink / raw)
  To: Stefan Potyra
  Cc: Mark Brown, Mark Brown, Florian Fainelli, Florian Fainelli,
	bcm-kernel-feedback-list, linux-spi, linux-arm-kernel,
	linux-kernel, Jonas Gorski, ldv-project, sil2review, linux-spi

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 <Stefan.Potyra@elektrobit.com>
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 <Stefan.Potyra@elektrobit.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Applied "spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate()." to the spi tree
@ 2018-04-26 11:53                   ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-04-26 11:53 UTC (permalink / raw)
  To: linux-arm-kernel

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 <Stefan.Potyra@elektrobit.com>
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 <Stefan.Potyra@elektrobit.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-04-26 11:54 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19 13:03 [PATCH] Enable the clock before calling clk_get_rate() Stefan Potyra
2018-04-19 13:03 ` Stefan Potyra
2018-04-21 23:10 ` Florian Fainelli
2018-04-21 23:10   ` Florian Fainelli
2018-04-24 16:16   ` [PATCH v2] spi/bcm63xx-hspi: " Stefan Potyra
2018-04-24 17:32     ` Mark Brown
2018-04-24 17:32       ` Mark Brown
2018-04-25 13:47       ` [PATCH v3] spi/bcm63xx-hspi: Enable the clock before calling Stefan Potyra
2018-04-25 15:50         ` Mark Brown
2018-04-25 15:50           ` Mark Brown
2018-04-25 15:50           ` Mark Brown
2018-04-25 16:49           ` [PATCH v4] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate() Stefan Potyra
2018-04-25 17:28             ` Mark Brown
2018-04-25 17:28               ` Mark Brown
2018-04-26  7:28               ` [PATCH v5] " Stefan Potyra
2018-04-26 11:53                 ` Applied "spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate()." to the spi tree Mark Brown
2018-04-26 11:53                   ` Mark Brown

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.