linux-kernel - Re: [PATCH linux-next v6 03/13] peci: Add support for PECI bus driver core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 6 Jul 2018 08:03:32 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Jae Hyun Yoo <jae.hyun.yoo@...ux.intel.com>
Cc:     Jason M Biils <jason.m.bills@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Philippe Ombredanne <pombredanne@...b.com>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Vinod Koul <vkoul@...nel.org>, Takashi Iwai <tiwai@...e.de>,
        Peter Rosin <peda@...ntia.se>,
        David Kershner <david.kershner@...sys.com>,
        Sagar Dharia <sdharia@...eaurora.org>,
        Uwe Kleine-Konig <u.kleine-koenig@...gutronix.de>,
        Randy Dunlap <rdunlap@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Sanyog Kale <sanyog.r.kale@...el.com>,
        Cyrille Pitchen <cyrille.pitchen@...e-electrons.com>,
        Juergen Gross <jgross@...e.com>, linux-kernel@...r.kernel.org,
        openbmc@...ts.ozlabs.org, Fengguang Wu <fengguang.wu@...el.com>,
        Alan Cox <alan@...ux.intel.com>, Andrew Lunn <andrew@...n.ch>,
        Andy Shevchenko <andriy.shevchenko@...el.com>,
        Arnd Bergmann <arnd@...db.de>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Julia Cartwright <juliac@....teric.us>
Subject: Re: [PATCH linux-next v6 03/13] peci: Add support for PECI bus
 driver core

On Thu, 21 Jun 2018, Jae Hyun Yoo wrote:

> This commit adds driver implementation for PECI bus core into linux
> driver framework.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@...ux.intel.com>
> Signed-off-by: Fengguang Wu <fengguang.wu@...el.com>
> Reviewed-by: Haiyue Wang <haiyue.wang@...ux.intel.com>
> Reviewed-by: James Feist <james.feist@...ux.intel.com>
> Reviewed-by: Vernon Mauery <vernon.mauery@...ux.intel.com>
> Cc: Alan Cox <alan@...ux.intel.com>
> Cc: Andrew Lunn <andrew@...n.ch>
> Cc: Andy Shevchenko <andriy.shevchenko@...el.com>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>
> Cc: Fengguang Wu <fengguang.wu@...el.com>
> Cc: Greg KH <gregkh@...uxfoundation.org>
> Cc: Jason M Biils <jason.m.bills@...ux.intel.com>
> Cc: Julia Cartwright <juliac@....teric.us>
> ---
>  drivers/Kconfig                 |    2 +
>  drivers/Makefile                |    1 +
>  drivers/peci/Kconfig            |   12 +
>  drivers/peci/Makefile           |    6 +
>  drivers/peci/peci-core.c        | 1438 +++++++++++++++++++++++++++++++
>  include/linux/peci.h            |  104 +++
>  include/uapi/linux/peci-ioctl.h |  265 ++++++
>  7 files changed, 1828 insertions(+)
>  create mode 100644 drivers/peci/Kconfig
>  create mode 100644 drivers/peci/Makefile
>  create mode 100644 drivers/peci/peci-core.c
>  create mode 100644 include/linux/peci.h
>  create mode 100644 include/uapi/linux/peci-ioctl.h

I'm struggling to see the justification for adding an entirely new
subsystem for what looks like a bespoke, and perhaps more damning,
*proprietary* 1-wire interface.  Especially one which has such
limited use.  Between yourself and the other silicon chip vendors
there must be 100s of these knocking about.  What makes this one
special?  Or even useful?  Will there ever be more than a single
source file in this directory?

Don't get me wrong, I'm all for upstreaming code, but to create a new
subsystem and bus for this kind of device seems very over the top.

Since PECI's main purpose in life is Thermal Management, perhaps the
whole thing should live in drivers/thermal or drivers/hwmon.  I've
also seen you reference this as a kind of BMC too, so maybe
drivers/platform/x86 would also be a nice place for it to reside.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog