Messages in this thread | | | Subject | Re: [PATCH arm/aspeed/ast2500 v1] ipmi: add an Aspeed KCS IPMI BMC driver | From | Corey Minyard <> | Date | Wed, 17 Jan 2018 20:58:07 -0600 |
| |
On 01/17/2018 06:16 PM, Wang, Haiyue wrote: > > > On 2018-01-17 23:59, Corey Minyard wrote: >> On 01/17/2018 08:31 AM, Wang, Haiyue wrote: >>> >>> >>> On 2018-01-17 06:12, Corey Minyard wrote: >>>> On 01/16/2018 02:59 PM, Corey Minyard 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.) >>>>> >>> How about these IOCTL definitions ? Is it more specific ? >>> >>> #define IPMI_BMC_IOCTL_SET_SMS_ATN _IO(__IPMI_BMC_IOCTL_MAGIC, 0x00) >>> #define IPMI_BMC_IOCTL_CLEAR_SMS_ATN _IO(__IPMI_BMC_IOCTL_MAGIC, 0x01) >>> #define IPMI_BMC_IOCTL_FORCE_ABORT _IO(__IPMI_BMC_IOCTL_MAGIC, 0x02) >>> >> >> Those look good to me. If you could do the switchover to ipmi-bmc.h >> in a separate >> patch, that would be cleaner. Then add the clear atn and force abort >> ioctls in the >> patch to add the new driver. >> >> Sound good? >> >> -corey >> > If I understood correctly, still use KCS_BMC_IOCTL_xxx in kcs_bmc.h > currently, then add a > patch for ipmi-bmc.h, and modify the bt_bmc.h together. Right ? >
No, not exactly. Just add ipmi-bmc.h and put the ioctls you define above in it. No need for kcs_bmc.h at all. We can then switch bt-bmc.c over to use the new ioctls later and remove bt_bmc.h when all the software gets switched over.
-corey
> Haiyue
| |