Messages in this thread | | | Subject | Re: [PATCH arm/aspeed/ast2500 v1] ipmi: add an Aspeed KCS IPMI BMC driver | From | "Wang, Haiyue" <> | Date | Wed, 17 Jan 2018 14:32:36 +0800 |
| |
On 2018-01-17 07:06, Joel Stanley wrote: > On Tue, Jan 16, 2018 at 2:59 PM, Corey Minyard <minyard@acm.org> wrote: >> On 01/16/2018 05:43 AM, Haiyue Wang wrote: >>> The KCS (Keyboard Controller Style) interface is used to perform in-band >>> IPMI communication between a server host and its BMC (BaseBoard Management >>> Controllers). >>> >>> This driver exposes the KCS interface on ASpeed SOCs (AST2400 and AST2500) >>> as a character device. Such SOCs are commonly used as BMCs and this driver >>> implements the BMC side of the KCS interface. >> >> I thought we were going to unify the BMC ioctl interface? My preference >> would be to >> create a file named include/uapi/linux/ipmi-bmc.h and add the following: >> >> #define __IPMI_BMC_IOCTL_MAGIC 0xb1 >> #define IPMI_BMC_IOCTL_SMS_SET_ATN _IO(__IPMI_BMC_IOCTL_MAGIC, 0x00) >> >> to make it the same as BT. Then in bt-bmc.h, set BT_BMC_IOCTL_SMS_ATN to >> IPMI_BMC_IOCTL_SMS_SET_ATN. Then add the KCS ioctls in ipmi-bmc.h and >> use that. That way we stay backward compatible but we are unified. >> >> Since more KCS interfaces may come around, can you make the name more >> specific? (I made this same error on bt-bmc,c, it should probably be >> renamed.) > Yes, we had a group of openbmc people get together recently and spoke > about this. Unfortunately Haiyue wasn't there (but other Intel BMC > people were). > > We've got code coming from another BMC vendor who will use the same > userspace API. The intention is to unify ASPEED's KCS and BT, along > with Nuvoton's KCS and BT, as you outlined above. Great design for common BMC code, thanks Joel & Corey.
Then we need to divide the kcs-bmc.c into two files, one (still kcs-bmc.c) is for handling IPMI KCS state according to IPMI 2.0 specification, and also handing device misc operations; another file is called such as aspeed-kcs-bmc.c which handles ast2500 kcs controller. So that Nuvoton can define nvvoton-kcs-bmc.c low level API, and call the IPMI KCS function in kcs-bmc.c ?
How about this idea ? If it makes sense, I will change the code for review in patch v2.
> Cheers, > > Joel
| |