Re: [PATCH arm/aspeed/ast2500 v4 1/2] ipmi: add a KCS IPMI BMC driver

From: Wang, Haiyue
Date: Thu Feb 01 2018 - 20:34:01 EST




On 2018-02-02 09:10, Corey Minyard wrote:

I loaded this in, tried a compile on x86_64, and got the following:

In file included from ../drivers/char/ipmi/kcs_bmc.c:15:0:
../drivers/char/ipmi/kcs_bmc.h: In function âkcs_bmc_privâ:
../drivers/char/ipmi/kcs_bmc.h:100:9: warning: return discards âconstâ qualifier from pointer target type [-Wdiscarded-qualifiers]
 return kcs_bmc->priv;
ÂÂÂÂÂÂÂÂ ^
-static inline void *kcs_bmc_priv(const struct kcs_bmc *kcs_bmc)
+static inline void *kcs_bmc_priv(struct kcs_bmc *kcs_bmc)Â <-- Can this fix error on x86_64 ?
{
ÂÂÂ return kcs_bmc->priv;
}
In file included from ../include/linux/printk.h:7:0,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ from ../include/linux/kernel.h:14,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ from ../include/asm-generic/bug.h:18,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ from ../arch/x86/include/asm/bug.h:82,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ from ../include/linux/bug.h:5,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ from ../include/linux/io.h:23,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ from ../drivers/char/ipmi/kcs_bmc.c:7:
../drivers/char/ipmi/kcs_bmc.c: In function âkcs_bmc_readâ:
../include/linux/kern_levels.h:5:18: warning: format â%uâ expects argument of type âunsigned intâ, but argument 3 has type âsize_t {aka long unsigned int}â [-Wformat=]
Â#define KERN_SOH "\001"Â /* ASCII Start Of Header */
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ^
../include/linux/kern_levels.h:11:18: note: in expansion of macro âKERN_SOHâ
Â#define KERN_ERR KERN_SOH "3" /* error conditions */
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ^
../include/linux/printk.h:301:9: note: in expansion of macro âKERN_ERRâ
 printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
ÂÂÂÂÂÂÂÂ ^
../drivers/char/ipmi/kcs_bmc.c:307:3: note: in expansion of macro âpr_errâ
ÂÂ pr_err("channel=%u with too large data : %u\n",
ÂÂ ^
https://elinux.org/Debugging_by_printing
However please*note*: always use/%zu/,/%zd/or/%zx/for printing/size_t/and/ssize_t/values. ssize_t and size_t are quite common values in the kernel, so please use the/%z/to avoid annoying compile warnings.
So I change it from "%u" to "%zu", it is passed on my arm-32 compile, is it OK on your X64 ?
pr_err("channel=%u with too large data : %zu\n",
In file included from ../drivers/char/ipmi/kcs_bmc_aspeed.c:20:0:
../drivers/char/ipmi/kcs_bmc.h: In function âkcs_bmc_privâ:
../drivers/char/ipmi/kcs_bmc.h:100:9: warning: return discards âconstâ qualifier from pointer target type [-Wdiscarded-qualifiers]
 return kcs_bmc->priv;
ÂÂÂÂÂÂÂÂ ^

So that needs to be fixed before it goes in.

Also, since you are respinning, can you make ASPEED_KCS_IPMI_BMC select IPMI_KCS_BMC, like:

diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index bc2568a..d34f40e 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -99,16 +99,11 @@ config IPMI_POWEROFF
Âendif # IPMI_HANDLER

Âconfig IPMI_KCS_BMC
-ÂÂÂÂÂÂ tristate 'IPMI KCS BMC Interface'
-ÂÂÂÂÂÂ help
-ÂÂÂÂÂÂÂÂ Provides a device driver for the KCS (Keyboard Controller Style)
-ÂÂÂÂÂÂÂÂ IPMI interface which meets the requirement of the BMC (Baseboard
-ÂÂÂÂÂÂÂÂ Management Controllers) side for handling the IPMI request from
-ÂÂÂÂÂÂÂÂ host system software.
+ÂÂÂÂÂÂ tristate

Âconfig ASPEED_KCS_IPMI_BMC
ÂÂÂÂÂÂÂ depends on ARCH_ASPEED || COMPILE_TEST
-ÂÂÂÂÂÂ depends on IPMI_KCS_BMC
+ÂÂÂÂÂÂ select IPMI_KCS_BMC
ÂÂÂÂÂÂÂ select REGMAP_MMIO
ÂÂÂÂÂÂÂ tristate "Aspeed KCS IPMI BMC driver"
ÂÂÂÂÂÂÂ help

It doesn't make much sense to have IPMI_KCS_BMC on its own. I was going to do this till I saw the compiler error.

Got it, will change it to 'select'
-corey