Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH arm/aspeed/ast2500 v1] ipmi: add an Aspeed KCS IPMI BMC
 driver
To:     minyard@acm.org, 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>
 <a716bfaf-d41e-7865-8b9d-61e6abed2035@acm.org>
 <54c6562b-f35a-c616-b6c2-a2eadf6937da@acm.org>
 <e462c75e-8d93-cddf-06f9-f40643e5b703@linux.intel.com>
 <cc88c8bf-65ec-c38c-0fdc-32e0436ebc7e@acm.org>
 <2fd6d4d7-39a1-f1b5-752e-e43ae2dd4cdd@linux.intel.com>
 <a6391cfa-0ea2-f8a6-5ac2-961de59ad3cc@acm.org>
From:   "Wang, Haiyue" <haiyue.wang@linux.intel.com>
Message-ID: <e2f682ad-e9e7-3d83-dbe5-952655bea1a0@linux.intel.com>
Date:   Thu, 18 Jan 2018 11:10:17 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.2
MIME-Version: 1.0
In-Reply-To: <a6391cfa-0ea2-f8a6-5ac2-961de59ad3cc@acm.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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
>
>