Messages in this thread | | | Subject | Re: [PATCH arm/aspeed/ast2500 v1] ipmi: add an Aspeed KCS IPMI BMC driver | From | "Wang, Haiyue" <> | Date | Thu, 18 Jan 2018 11:10:17 +0800 |
| |
On 2018-01-18 10:58, Corey Minyard wrote: > 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. > Understood. Will use the new ioctls for kcs_bmc firstly. > -corey > >> Haiyue > >
| |