Re: [PATCH arm/aspeed/ast2500 v1] ipmi: add an Aspeed KCS IPMI BMC driver

[Wang, Haiyue]

Thanks Avi, wait for your response when new patch is ready. :-)

BR,
Haiyue


On 2018-01-17 20:54, Avi Fishman wrote:
> Sounds great for us (Nuvoton).
>
> Avi.
> On Wed, Jan 17, 2018 at 8:32 AM, Wang, Haiyue
> <haiyue.wang@xxxxxxxxxxxxxxx> wrote:
> > On 2018-01-17 07:06, Joel Stanley wrote:
> > > On Tue, Jan 16, 2018 at 2:59 PM, Corey Minyard <minyard@xxxxxxx> 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