[PATCH -next 0/4] Add LCLK control into Aspeed LPC sub drivers
Jae Hyun Yoo
jae.hyun.yoo at linux.intel.com
Wed Nov 3 11:54:30 AEDT 2021
On 11/2/2021 5:30 PM, Zev Weiss wrote:
> On Tue, Nov 02, 2021 at 05:17:30PM PDT, Jae Hyun Yoo wrote:
>> Hi Zev,
>>
>> On 11/2/2021 5:04 PM, Zev Weiss wrote:
>>> On Mon, Nov 01, 2021 at 04:36:38PM PDT, Joel Stanley wrote:
>>>> On Mon, 1 Nov 2021 at 23:18, <jae.hyun.yoo at intel.com> wrote:
>>>>>
>>>>> From: Jae Hyun Yoo <jae.hyun.yoo at linux.intel.com>
>>>>>
>>>>> Hello all,
>>>>>
>>>>> This series is for appliying below fix to all Aspped LPC sub drivers.
>>>>> https://lore.kernel.org/all/20201208091748.1920-1-wangzhiqiang.bj@bytedance.com/
>>>>>
>>>>>
>>>>>
>>>>> An LPC sub driver can be enabled without using the lpc-ctrl driver
>>>>> or it
>>>>> can be registered ahead of lpc-ctrl depends on each system
>>>>> configuration and
>>>>> this difference introduces that LPC can be enabled without heart
>>>>> beating of
>>>>> LCLK so it 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 remaining Aspeed
>>>>> LPC sub
>>>>> drivers.
>>>>
>>>> Thanks for sending this out!
>>>>
>>>> This will resolve a few of the issues we have in the issue tracker:
>>>>
>>>> https://github.com/openbmc/linux/issues/210
>>>> https://github.com/openbmc/linux/issues/130
>>>>
>>>> The patches look good to me. I think you've just missed Corey's PR for
>>>> v5.16, but I will stick them in the openbmc tree once they've had a
>>>> review.
>>>>
>>>
>>> Hi Jae,
>>>
>>> I tried this series out on the same in-progress OpenBMC port from
>>> issue number 210 linked above and am still seeing problems (dmesg
>>> pasted below).
>>>
>>> I cherry-picked commit f9241fe8b9652 ("ARM: dts: aspeed: Add uart
>>> routing to device tree") from linux-next to allow the first patch to
>>> apply cleanly; is there anything else I might be missing that'd be
>>> needed to test the series properly?
>>
>> Looks like below dmesg shows an error from 'aspeed_lpc_snoop_probe'
>> which this series doesn't touch. Do you have below fix in your code
>> tree?
>>
>> https://lore.kernel.org/all/20201208091748.1920-1-wangzhiqiang.bj@bytedance.com/
>>
>>
>> Thanks,
>> Jae
>>
>
> Yes, I've got that patch (commit 3f94cf1558), and the accompanying dts
> update to add the clocks property to the lpc-snoop device (commit
> d050d049f8).
>
> However, while there is an aspeed_lpc_snoop_probe() backtrace there,
> note that there's *also* one from aspeed_kcs_probe() further on
> (starting at timestamp 3.263306).
>
>
> Zev
>
Can you please test additional changes below?
Thanks,
Jae
diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c
b/drivers/char/ipmi/kcs_bmc_aspeed.c
index 00706472cc4d..af03aea0f3ce 100644
--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
@@ -644,6 +644,17 @@ static int aspeed_kcs_probe(struct platform_device
*pdev)
if (rc)
goto err;
+ platform_set_drvdata(pdev, priv);
+
+ aspeed_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF |
KCS_BMC_EVENT_TYPE_OBE), 0);
+ aspeed_kcs_enable_channel(kcs_bmc, true);
+
+ rc = kcs_bmc_add_device(&priv->kcs_bmc);
+ if (rc) {
+ dev_warn(&pdev->dev, "Failed to register channel %d:
%d\n", kcs_bmc->channel, rc);
+ goto err;
+ }
+
/* Host to BMC IRQ */
rc = aspeed_kcs_config_downstream_irq(kcs_bmc, pdev);
if (rc)
@@ -658,17 +669,6 @@ static int aspeed_kcs_probe(struct platform_device
*pdev)
priv->upstream_irq.mode = aspeed_kcs_irq_none;
}
- platform_set_drvdata(pdev, priv);
-
- aspeed_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF |
KCS_BMC_EVENT_TYPE_OBE), 0);
- aspeed_kcs_enable_channel(kcs_bmc, true);
-
- rc = kcs_bmc_add_device(&priv->kcs_bmc);
- if (rc) {
- dev_warn(&pdev->dev, "Failed to register channel %d:
%d\n", kcs_bmc->channel, rc);
- goto err;
- }
-
dev_info(&pdev->dev, "Initialised channel %d at 0x%x\n",
kcs_bmc->channel, addrs[0]);
diff --git a/drivers/soc/aspeed/aspeed-lpc-snoop.c
b/drivers/soc/aspeed/aspeed-lpc-snoop.c
index eceeaf8dfbeb..044c8f6665b7 100644
--- a/drivers/soc/aspeed/aspeed-lpc-snoop.c
+++ b/drivers/soc/aspeed/aspeed-lpc-snoop.c
@@ -306,10 +306,6 @@ static int aspeed_lpc_snoop_probe(struct
platform_device *pdev)
return rc;
}
- rc = aspeed_lpc_snoop_config_irq(lpc_snoop, pdev);
- if (rc)
- goto err;
-
rc = aspeed_lpc_enable_snoop(lpc_snoop, dev, 0, port);
if (rc)
goto err;
@@ -324,6 +320,10 @@ static int aspeed_lpc_snoop_probe(struct
platform_device *pdev)
}
}
+ rc = aspeed_lpc_snoop_config_irq(lpc_snoop, pdev);
+ if (rc)
+ goto err;
+
return 0;
err:
More information about the Linux-aspeed
mailing list