Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933470AbeAKNWw (ORCPT + 1 other); Thu, 11 Jan 2018 08:22:52 -0500 Received: from mail-oi0-f67.google.com ([209.85.218.67]:37156 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932869AbeAKNWt (ORCPT ); Thu, 11 Jan 2018 08:22:49 -0500 X-Google-Smtp-Source: ACJfBosiRQbAE8zj+VvSM+aYPzifXJBR0VOZBv0dIj6A6Z9dUcQSn8cnQ69VJMQNW4MORmyGScCzVczJfl+gzQ9qqS4= MIME-Version: 1.0 In-Reply-To: References: <20180109223126.13093-1-jae.hyun.yoo@linux.intel.com> <20180109223126.13093-7-jae.hyun.yoo@linux.intel.com> From: Arnd Bergmann Date: Thu, 11 Jan 2018 14:22:48 +0100 X-Google-Sender-Auth: CCPY_O_hdmY59sdcIJdR1ttx6Zs Message-ID: Subject: Re: [PATCH linux dev-4.10 6/6] drivers/hwmon: Add a driver for a generic PECI hwmon To: Jae Hyun Yoo Cc: Joel Stanley , Andrew Jeffery , gregkh , Jean Delvare , Guenter Roeck , Linux Kernel Mailing List , linux-doc@vger.kernel.org, DTML , linux-hwmon@vger.kernel.org, Linux ARM , OpenBMC Maillist Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Thu, Jan 11, 2018 at 12:45 AM, Jae Hyun Yoo wrote: > On 1/10/2018 4:29 AM, Arnd Bergmann wrote: >> >> On Tue, Jan 9, 2018 at 11:31 PM, Jae Hyun Yoo >> wrote: >>> >>> This commit adds driver implementation for a generic PECI hwmon. >>> >>> Signed-off-by: Jae Hyun Yoo >> >> >>> +static int xfer_peci_msg(int cmd, void *pmsg) >>> +{ >>> + int rc; >>> + >>> + mutex_lock(&peci_hwmon_lock); >>> + rc = peci_ioctl(NULL, cmd, (unsigned long)pmsg); >>> + mutex_unlock(&peci_hwmon_lock); >>> + >>> + return rc; >>> +} >> >> >> I said earlier that peci_ioctl() looked unused, that was obviously >> wrong, but what you have here >> is not a proper way to abstract a bus. >> >> Maybe this can be done more like an i2c bus: make the peci controller >> a bus device >> and register all known target/index pairs as devices with the peci bus >> type, and have >> them probed from DT. The driver can then bind to each of those >> individually. >> Not sure if that is getting to granular at that point, I'd have to >> understand better >> how it is expected to get used, and what the variances are between >> implementations. >> > > Thanks for sharing your opinion. In fact, this was also suggested by openbmc > community so I should consider of redesigning it. I'm currently thinking > about adding a new PECI device class as an abstract layer and any BMC > chipset specific driver could be attached to the PECI class driver. Then, > each CPU client could be registered as an individual device as you > suggested. Will consider your suggestion. Another idea might be to pretend that PECI was I2C. We already have a few drivers for hardware that is not I2C but whose software interface looks similar enough that it just works. No idea if that is the case for PECI, but xfer_peci_msg might be close enough to i2c_xfer to make it work. If you are able to do that, then the PECI controller would just register itself as an i2c controller and it can be accessed using /dev/i2c from user space or a high-level i2c_driver. Arnd