> From: Jae Hyun Yoo <jae.hyun.yoo@xxxxxxxxxxxxxxx> > Sent: Wednesday, November 3, 2021 12:36 AM > > 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@sw > > 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@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 for your feedback and the information shared. We will keep tracking these changes and construct a compatible patch for the SIO clocks. Regards, Chiawei
