linux-kernel - Re: [PATCH v2 2/2] cros_ec: Move cros_ec

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:   Wed, 20 Jun 2018 16:05:03 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     Benson Leung <bleung@...omium.org>,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        Gwendal Grignou <gwendal@...omium.org>,
        Guenter Roeck <groeck@...gle.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Thierry Escande <thierry.escande@...labora.com>
Subject: Re: [PATCH v2 2/2] cros_ec: Move cros_ec_dev module to drivers/mfd

On Mon, Nov 20, 2017 at 8:18 AM Thierry Escande
<thierry.escande@...labora.com> wrote:
>
> The cros_ec_dev module is responsible for registering the MFD devices
> attached to the ChromeOS EC. This patch moves this module to drivers/mfd
> so calls to mfd_add_devices() are not done from outside the MFD subtree
> anymore.

I am quite a bit late to the party, but what's the rationale for not
using mfd_add_devices() from outside of MFD tree? We do allow
registering i2c clients from outside of i2c core, and spi from outside
of spi core, etc, etc.

Right now I see cros_ec being split, quite haphazardly, between
drivers/platform/chrome and drivers/mfd, with some transport drivers
(i2C, SPI) and some interfaces living in MFD, while LPC transport and
host of other stuff living in drivers/platform. On top of that we have
cros_ec_keyb in input, RTC drivers, CEC, and god knows what else
spread across various subsystems:

dtor@...r-ws:~/kernel/linus $ find -name 'cros_ec*.c'
./drivers/iio/light/cros_ec_light_prox.c
./drivers/iio/accel/cros_ec_accel_legacy.c
./drivers/iio/pressure/cros_ec_baro.c
./drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
./drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
./drivers/mfd/cros_ec_spi.c
./drivers/mfd/cros_ec.c
./drivers/mfd/cros_ec_i2c.c
./drivers/mfd/cros_ec_dev.c
./drivers/input/keyboard/cros_ec_keyb.c
./drivers/platform/chrome/cros_ec_vbc.c
./drivers/platform/chrome/cros_ec_debugfs.c
./drivers/platform/chrome/cros_ec_lpc.c
./drivers/platform/chrome/cros_ec_lightbar.c
./drivers/platform/chrome/cros_ec_lpc_reg.c
./drivers/platform/chrome/cros_ec_proto.c
./drivers/platform/chrome/cros_ec_lpc_mec.c
./drivers/platform/chrome/cros_ec_sysfs.c

The fact that sysfs/debugfs code is in platform but we instantiate it
from MFD is pure madness (it's driver private data, there is no reason
why it should be exported to nclude/linux/mfd/cros_ec.h). This all
creates unnecessary friction and I would love to move most of the code
into drivers/platform/chrome.

I see the wisdom of having code that could potentially be used in
several systems in respective subsystems code (pretty much majority of
drivers/mfd/ drivers are for chips/IP blocks that are used and reused
by different systems and boards), but I think cros ec is quite
different in that regard as it is only used by ChromeOS devices and
has little to no chance to be useful anywhere else.

Thanks.

-- 
Dmitry