Hi, Thanks for your patch. There're some comments bellow. On 2022/1/19 14:57, Chen Baozi wrote:

The system would hang up when the Phytium S2500 communicates with some BMCs after several rounds of transactions, unless we reset the controller timeout counter manually by calling firmware through SMC.

Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn> --- drivers/char/ipmi/ipmi_si_mem_io.c | 117 ++++++++++++++++++++++++++++- 1 file changed, 113 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_si_mem_io.c b/drivers/char/ipmi/ipmi_si_mem_io.c index 75583612ab10..73a3d6979eaa 100644 --- a/drivers/char/ipmi/ipmi_si_mem_io.c +++ b/drivers/char/ipmi/ipmi_si_mem_io.c @@ -3,6 +3,105 @@ #include <linux/io.h> #include "ipmi_si.h"

+#ifdef CONFIG_ARM_GIC_PHYTIUM_2500 +#include <linux/arm-smccc.h> + +#define CTL_RST_FUNC_ID 0xC2000011 + +static void ctl_smc(unsigned long arg0, unsigned long arg1, + unsigned long arg2, unsigned long arg3) +{ + struct arm_smccc_res res; + + arm_smccc_smc(arg0, arg1, arg2, arg3, 0, 0, 0, 0, &res); + if (res.a0 != 0) + pr_err("Error: Firmware call SMC reset Failed: %d, addr: 0x%lx\n", + (int)res.a0, arg2); +} + +static void ctl_timeout_reset(void) +{ + ctl_smc(CTL_RST_FUNC_ID, 0x1, 0x28100208, 0x1); + ctl_smc(CTL_RST_FUNC_ID, 0x1, 0x2810020C, 0x1); +} + +static unsigned char intf_mem_inb_2500(const struct si_sm_io *io, + unsigned int offset) +{ + ctl_timeout_reset(); + + return readb((io->addr)+(offset * io->regspacing));

与 intf_mem_inb（） 代码重复，下同。

+} + +static unsigned char intf_mem_inw_2500(const struct si_sm_io *io, + unsigned int offset) +{ + ctl_timeout_reset(); + + return (readw((io->addr)+(offset * io->regspacing)) >> io->regshift) + & 0xff; +} + +static unsigned char intf_mem_inl_2500(const struct si_sm_io *io, + unsigned int offset) +{ + ctl_timeout_reset(); + + return (readl((io->addr)+(offset * io->regspacing)) >> io->regshift) + & 0xff; +} + +static unsigned char mem_inq_2500(const struct si_sm_io *io, + unsigned int offset) +{ + ctl_timeout_reset(); + + return (readq((io->addr)+(offset * io->regspacing)) >> io->regshift) + & 0xff; +} +#else +#define intf_mem_inb_2500 intf_mem_inb +#define intf_mem_inw_2500 intf_mem_inw +#define intf_mem_inl_2500 intf_mem_inl +#define mem_inq_2500 mem_inq +#endif + +static bool apply_phytium2500_workaround;

这个变量全局存在的，其实只在 phytium2500 平台上才有意义。

+ +#ifdef CONFIG_ACPI +struct ipmi_workaround_oem_info { + char oem_id[ACPI_OEM_ID_SIZE + 1]; +}; + +static struct ipmi_workaround_oem_info wa_info[] = { + { + .oem_id = "KPSVVJ", + } +}; + +static void ipmi_check_phytium_workaround(void) +{ + struct acpi_table_header tbl; + int i; + + if (ACPI_FAILURE(acpi_get_table_header(ACPI_SIG_DSDT, 0, &tbl))) + return; + + for (i = 0; i < ARRAY_SIZE(wa_info); i++) { + if (strncmp(wa_info[i].oem_id, tbl.oem_id, ACPI_OEM_ID_SIZE)) + continue; + + apply_phytium2500_workaround = true; + break; + } +}

这个 CONFIG_ACPI 和 PHYTIUM 没有依赖，即便不是 PHYTIUM 平台，仍然会做这些没必要的检查。 而且在非 ARM64 架构上，这个 apply_phytium2500_workaround 仍然会全局存在。 是不是类似这样比较好： #ifdef CONFIG_ARM_GIC_PHYTIUM_2500 static bool apply_phytium2500_workaround; static void ipmi_check_phytium_workaround(void) { #ifdef CONFIG_ACPI ... #endif } ... static inline void ipmi_phytium2500_workaround（void） { if (apply_phytium2500_workaround) ctl_timeout_reset(); } static unsigned char intf_mem_inb(const struct si_sm_io *io, unsigned int offset) { + ipmi_phytium2500_workaround(); return readb((io->addr)+(offset * io->regspacing)); } static unsigned char intf_mem_inw(const struct si_sm_io *io, unsigned int offset) { + ipmi_phytium2500_workaround(); return (readw((io->addr)+(offset * io->regspacing)) >> io->regshift) & 0xff; } ... #else static inline void ipmi_phytium2500_workaround（） {} static inline void ipmi_check_phytium_workaround(void) {} #endif

+#else +static void ipmi_check_phytium_workaround(void) +{ + apply_phytium2500_workaround = false; +} +#endif + static unsigned char intf_mem_inb(const struct si_sm_io *io, unsigned int offset) { @@ -81,26 +180,36 @@ int ipmi_si_mem_setup(struct si_sm_io *io) if (!addr) return -ENODEV;

+ ipmi_check_phytium_workaround(); + /* * Figure out the actual readb/readw/readl/etc routine to use based * upon the register size. */ switch (io->regsize) { case 1: - io->inputb = intf_mem_inb; + io->inputb = apply_phytium2500_workaround ? + intf_mem_inb_2500 : + intf_mem_inb; io->outputb = intf_mem_outb; break; case 2: - io->inputb = intf_mem_inw; + io->inputb = apply_phytium2500_workaround ? + intf_mem_inw_2500 : + intf_mem_inw; io->outputb = intf_mem_outw; break; case 4: - io->inputb = intf_mem_inl; + io->inputb = apply_phytium2500_workaround ? + intf_mem_inl_2500 : + intf_mem_inl; io->outputb = intf_mem_outl; break; #ifdef readq case 8: - io->inputb = mem_inq; + io->inputb = apply_phytium2500_workaround ? + mem_inq_2500 : + mem_inq; io->outputb = mem_outq; break; #endif