diff mbox series

[-next,2/4] ipmi: bt: add clock control logic

Message ID 20211101233751.49222-3-jae.hyun.yoo@intel.com
State New
Headers show
Series Add LCLK control into Aspeed LPC sub drivers | expand

Commit Message

jae.hyun.yoo@intel.com Nov. 1, 2021, 11:37 p.m. UTC
From: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

If LPC BT driver is registered ahead of lpc-ctrl module, LPC BT
hardware block will be enabled without heart beating of LCLK until
lpc-ctrl enables the LCLK. This issue causes improper handling on
host interrupts when the host sends interrupts in that time frame.
Then kernel eventually forcibly disables the interrupt with
dumping stack and printing a 'nobody cared this irq' message out.

To prevent this issue, all LPC sub drivers should enable LCLK
individually so this patch adds clock control logic into the LPC
BT driver.

Fixes: 54f9c4d0778b ("ipmi: add an Aspeed BT IPMI BMC driver")
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/char/ipmi/bt-bmc.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

Comments

Cédric Le Goater Nov. 2, 2021, 9:35 a.m. UTC | #1
On 11/2/21 00:37, jae.hyun.yoo@intel.com wrote:
> From: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> 
> If LPC BT driver is registered ahead of lpc-ctrl module, LPC BT
> hardware block will be enabled without heart beating of LCLK until
> lpc-ctrl enables the LCLK. This issue causes improper handling on
> host interrupts when the host sends interrupts in that time frame.
> Then kernel eventually forcibly disables the interrupt with
> dumping stack and printing a 'nobody cared this irq' message out.
> 
> To prevent this issue, all LPC sub drivers should enable LCLK
> individually so this patch adds clock control logic into the LPC
> BT driver.
> 
> Fixes: 54f9c4d0778b ("ipmi: add an Aspeed BT IPMI BMC driver")
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

LGTM,

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   drivers/char/ipmi/bt-bmc.c | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
> index 7450904e330a..a20f92cc7b18 100644
> --- a/drivers/char/ipmi/bt-bmc.c
> +++ b/drivers/char/ipmi/bt-bmc.c
> @@ -5,6 +5,7 @@
>   
>   #include <linux/atomic.h>
>   #include <linux/bt-bmc.h>
> +#include <linux/clk.h>
>   #include <linux/errno.h>
>   #include <linux/interrupt.h>
>   #include <linux/io.h>
> @@ -62,6 +63,7 @@ struct bt_bmc {
>   	wait_queue_head_t	queue;
>   	struct timer_list	poll_timer;
>   	struct mutex		mutex;
> +	struct clk		*clk;
>   };
>   
>   static atomic_t open_count = ATOMIC_INIT(0);
> @@ -423,6 +425,19 @@ static int bt_bmc_probe(struct platform_device *pdev)
>   	if (IS_ERR(bt_bmc->base))
>   		return PTR_ERR(bt_bmc->base);
>   
> +	bt_bmc->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(bt_bmc->clk)) {
> +		rc = PTR_ERR(bt_bmc->clk);
> +		if (rc != -EPROBE_DEFER)
> +			dev_err(dev, "Unable to get clock\n");
> +		return rc;
> +	}
> +	rc = clk_prepare_enable(bt_bmc->clk);
> +	if (rc) {
> +		dev_err(dev, "Unable to enable clock\n");
> +		return rc;
> +	}
> +
>   	mutex_init(&bt_bmc->mutex);
>   	init_waitqueue_head(&bt_bmc->queue);
>   
> @@ -433,7 +448,7 @@ static int bt_bmc_probe(struct platform_device *pdev)
>   	rc = misc_register(&bt_bmc->miscdev);
>   	if (rc) {
>   		dev_err(dev, "Unable to register misc device\n");
> -		return rc;
> +		goto err;
>   	}
>   
>   	bt_bmc_config_irq(bt_bmc, pdev);
> @@ -457,6 +472,11 @@ static int bt_bmc_probe(struct platform_device *pdev)
>   	clr_b_busy(bt_bmc);
>   
>   	return 0;
> +
> +err:
> +	clk_disable_unprepare(bt_bmc->clk);
> +
> +	return rc;
>   }
>   
>   static int bt_bmc_remove(struct platform_device *pdev)
> @@ -466,6 +486,8 @@ static int bt_bmc_remove(struct platform_device *pdev)
>   	misc_deregister(&bt_bmc->miscdev);
>   	if (bt_bmc->irq < 0)
>   		del_timer_sync(&bt_bmc->poll_timer);
> +	clk_disable_unprepare(bt_bmc->clk);
> +
>   	return 0;
>   }
>   
>
Jae Hyun Yoo Nov. 2, 2021, 4:36 p.m. UTC | #2
> Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks a lot Cédric!
Andrew Jeffery Nov. 2, 2021, 10:14 p.m. UTC | #3
On Tue, 2 Nov 2021, at 10:07, jae.hyun.yoo@intel.com wrote:
> From: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>
> If LPC BT driver is registered ahead of lpc-ctrl module, LPC BT
> hardware block will be enabled without heart beating of LCLK until
> lpc-ctrl enables the LCLK. This issue causes improper handling on
> host interrupts when the host sends interrupts in that time frame.
> Then kernel eventually forcibly disables the interrupt with
> dumping stack and printing a 'nobody cared this irq' message out.
>
> To prevent this issue, all LPC sub drivers should enable LCLK
> individually so this patch adds clock control logic into the LPC
> BT driver.
>
> Fixes: 54f9c4d0778b ("ipmi: add an Aspeed BT IPMI BMC driver")
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
diff mbox series

Patch

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index 7450904e330a..a20f92cc7b18 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -5,6 +5,7 @@ 
 
 #include <linux/atomic.h>
 #include <linux/bt-bmc.h>
+#include <linux/clk.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -62,6 +63,7 @@  struct bt_bmc {
 	wait_queue_head_t	queue;
 	struct timer_list	poll_timer;
 	struct mutex		mutex;
+	struct clk		*clk;
 };
 
 static atomic_t open_count = ATOMIC_INIT(0);
@@ -423,6 +425,19 @@  static int bt_bmc_probe(struct platform_device *pdev)
 	if (IS_ERR(bt_bmc->base))
 		return PTR_ERR(bt_bmc->base);
 
+	bt_bmc->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(bt_bmc->clk)) {
+		rc = PTR_ERR(bt_bmc->clk);
+		if (rc != -EPROBE_DEFER)
+			dev_err(dev, "Unable to get clock\n");
+		return rc;
+	}
+	rc = clk_prepare_enable(bt_bmc->clk);
+	if (rc) {
+		dev_err(dev, "Unable to enable clock\n");
+		return rc;
+	}
+
 	mutex_init(&bt_bmc->mutex);
 	init_waitqueue_head(&bt_bmc->queue);
 
@@ -433,7 +448,7 @@  static int bt_bmc_probe(struct platform_device *pdev)
 	rc = misc_register(&bt_bmc->miscdev);
 	if (rc) {
 		dev_err(dev, "Unable to register misc device\n");
-		return rc;
+		goto err;
 	}
 
 	bt_bmc_config_irq(bt_bmc, pdev);
@@ -457,6 +472,11 @@  static int bt_bmc_probe(struct platform_device *pdev)
 	clr_b_busy(bt_bmc);
 
 	return 0;
+
+err:
+	clk_disable_unprepare(bt_bmc->clk);
+
+	return rc;
 }
 
 static int bt_bmc_remove(struct platform_device *pdev)
@@ -466,6 +486,8 @@  static int bt_bmc_remove(struct platform_device *pdev)
 	misc_deregister(&bt_bmc->miscdev);
 	if (bt_bmc->irq < 0)
 		del_timer_sync(&bt_bmc->poll_timer);
+	clk_disable_unprepare(bt_bmc->clk);
+
 	return 0;
 }