On 23.08.2022 03:22, Joel Stanley wrote: > «Внимание! Данное письмо от внешнего адресата!» > > On Mon, 22 Aug 2022 at 06:36, Igor Kononenko <[email protected]> wrote: >> >> The previos v1 [PATCH 1/3] have error of getting `struct kcs_bmc_device` >> pointer from `struct platform_device *`. Replaced to retriveing pointer by >> `platform_get_drvdata()` > > Can we get a v3 with your original commit message added? You had a > good write up in v1 but it was dropped in v2. >
Thanks for the review. Ok, I'll include the origin commit message to a v3. > This change looks like the right thing to do. We should get Andrew to > review too, as he spends more time with the KCS controllers. > > The missed irq issue was seen with the other LPC sub drivers because > the clock wasn't enabled. We ended up with this patch: > > https://lore.kernel.org/all/[email protected]/ >> Do we need to do something similar for KCS? As far as I see by the LPC 'nobody cared irq' issue there had the feature of enabling LCLK individually earlier, there is patch about: https://lore.kernel.org/all/[email protected]/ Originally I found the bug on the linux-dev.v5.4 that includes the 'LCLK individually enabling' feature. It seems to me the issue is that lpc-snoop and the lpc-kcs has same IRQ#35 that is used in separated drivers(by the IRQF_SHARED flag). The IRQ handler determinate request purpose(for kcs or snoop) by LPC interrupt registers state, and if such interrupt is not for any one of them, the irq-handler passthrough request to a next handler by returning `IRQ_NONE`. So, even if lpc-kcs will be having adjusted own individual LCLK, that is doesn't solve issue, because when lpc-snoop will had configured irq-handler the irq-manager will know that for IRQ#35 already registered a good handler, but such handler will skip all requests by `IRQ_NONE` because such irqs are intended for lpc-kcs. I guess that is the point of bug. > >> >> Reported-by: kernel test robot <[email protected]> >> Signed-off-by: Igor Kononenko <[email protected]> >> --- >> drivers/char/ipmi/kcs_bmc_aspeed.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c >> b/drivers/char/ipmi/kcs_bmc_aspeed.c >> index cdc88cde1e9a..8437f3cfe3f4 100644 >> --- a/drivers/char/ipmi/kcs_bmc_aspeed.c >> +++ b/drivers/char/ipmi/kcs_bmc_aspeed.c >> @@ -636,6 +636,14 @@ static int aspeed_kcs_remove(struct platform_device >> *pdev) >> return 0; >> } >> >> +static void aspeed_kcs_shutdown(struct platform_device *pdev) >> +{ >> + struct aspeed_kcs_bmc *priv = platform_get_drvdata(pdev); >> + struct kcs_bmc_device *kcs_bmc = &priv->kcs_bmc; >> + >> + aspeed_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF), 0); >> +} >> + >> static const struct of_device_id ast_kcs_bmc_match[] = { >> { .compatible = "aspeed,ast2400-kcs-bmc-v2" }, >> { .compatible = "aspeed,ast2500-kcs-bmc-v2" }, >> @@ -651,6 +659,7 @@ static struct platform_driver ast_kcs_bmc_driver = { >> }, >> .probe = aspeed_kcs_probe, >> .remove = aspeed_kcs_remove, >> + .shutdown = aspeed_kcs_shutdown, >> }; >> module_platform_driver(ast_kcs_bmc_driver); >> >> -- >> 2.25.1 >> -- Best regards, Igor Kononenko _______________________________________________ Openipmi-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openipmi-developer
