On 11/1/2021 8:28 PM, Joel Stanley wrote:
On Tue, 2 Nov 2021 at 03:16, ChiaWei Wang <chiawei_wang@xxxxxxxxxxxxxx> wrote:
Hi Jae,
From: linux-arm-kernel <linux-arm-kernel-bounces@xxxxxxxxxxxxxxxxxxx> On
From: Jae Hyun Yoo <jae.hyun.yoo@xxxxxxxxxxxxxxx>
If LPC KCS driver is registered ahead of lpc-ctrl module, LPC KCS 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 KCS driver.
Have all LPC sub drivers could result in entire LPC block down if any of them disables the clock (e.g. driver unload).
The LPC devices such as SIO can be used before kernel booting, even without any BMC firmware.
Thereby, we recommend to make LCLK critical or guarded by protected clock instead of having all LPC sub drivers hold the LCLK control.
The previous discussion for your reference:
https://lkml.org/lkml/2020/9/28/153
Please read the entire thread. The conclusion:
https://lore.kernel.org/all/CACPK8XdBmkhZ8mcSFmDAFV8k7Qj7ajBL8TVKfK8c+5aneUMHZw@xxxxxxxxxxxxxx/
That is, for the devices that have a driver loaded can enable the
clock. When they are unloaded, they will reduce the reference count
until the last driver is unloaded. eg:
https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk.c#L945
There was another fork to the thread, where we suggested that a
protected clocks binding could be added:
https://lore.kernel.org/all/160269577311.884498.8429245140509326318@xxxxxxxxxxxxxxxxxxxxxxxxxx/
If you wish to use this mechanism for eg. SIO clocks, then I encourage
Aspeed to submit a patch to do that.
We are revisiting the aged discussion. Thanks for bringing it back.
I agree with Joel that a clock should be enabled only on systems that
need the clock actually so it should be configurable by a device driver
or through device tree setting, not by the static setting in
clk-ast2600.c code. So that's the reason why I stopped upstreaming below
change for making BCLK as a critical clock.
https://www.spinics.net/lists/linux-clk/msg44836.html
Instead, I submitted these two changes to make it configurable through
device tree setting.
https://lists.ozlabs.org/pipermail/linux-aspeed/2020-January/003394.html
https://lists.ozlabs.org/pipermail/linux-aspeed/2020-January/003339.html
But these were not accepted too.
And recently, Samuel introduced a better and more generic way.
https://lore.kernel.org/all/20200903040015.5627-2-samuel@xxxxxxxxxxxx/
But it's not accepted yet either.
Chiawei,
Please refine the mechanism and submit a change to make SIO clocks
configurable through device tree setting. I believe that we can keep
this patch series even with the change, or it can be modified and
adjusted if needed after the SIO clocks fix is accepted.
Thanks,
Jae
