From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753872AbeARC6N (ORCPT ); Wed, 17 Jan 2018 21:58:13 -0500 Received: from mail-pg0-f44.google.com ([74.125.83.44]:41958 "EHLO mail-pg0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753412AbeARC6M (ORCPT ); Wed, 17 Jan 2018 21:58:12 -0500 X-Google-Smtp-Source: ACJfBos+s+Y6oXtRu5sjZeV42eS3IqWM2AAoqKK99DdufN9DNnZJeCQK8QgXOPSwxtPclfqmNUkPAQ== Reply-To: minyard@acm.org Subject: Re: [PATCH arm/aspeed/ast2500 v1] ipmi: add an Aspeed KCS IPMI BMC driver To: "Wang, Haiyue" , joel@jms.id.au, openbmc@lists.ozlabs.org, openipmi-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org Cc: andriy.shevchenko@intel.com References: <1516103023-19244-1-git-send-email-haiyue.wang@linux.intel.com> <54c6562b-f35a-c616-b6c2-a2eadf6937da@acm.org> <2fd6d4d7-39a1-f1b5-752e-e43ae2dd4cdd@linux.intel.com> From: Corey Minyard Message-ID: Date: Wed, 17 Jan 2018 20:58:07 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <2fd6d4d7-39a1-f1b5-752e-e43ae2dd4cdd@linux.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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