From: Jae Hyun Yoo <redacted>
Sent: Wednesday, November 3, 2021 12:36 AM

On 11/1/2021 8:28 PM, Joel Stanley wrote:

quoted

On Tue, 2 Nov 2021 at 03:16, ChiaWei Wang

[off-list ref] wrote:

quoted

quoted

Hi Jae,

quoted

From: linux-arm-kernel
[off-list ref] On

From: Jae Hyun Yoo <redacted>

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).

quoted

quoted

The LPC devices such as SIO can be used before kernel booting, even

without any BMC firmware.

quoted

quoted

Thereby, we recommend to make LCLK critical or guarded by protected

clock instead of having all LPC sub drivers hold the LCLK control.

quoted

quoted

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
+

quoted

5aneUMHZw@mail.gmail.com/

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@sw

quoted

boyd.mtv.corp.google.com/

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@sholland.org/ (local)

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 for your feedback and the information shared.
We will keep tracking these changes and construct a compatible patch for the SIO clocks.

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