On Mon, 03 Dec 2018, Enric Balletbo i Serra wrote: > On 3/12/18 11:36, Lee Jones wrote: > > On Tue, 27 Nov 2018, Enric Balletbo i Serra wrote: > > > >> The entire way how cros sysfs attibutes are created is broken. > >> cros_ec_lightbar should be its own driver and its attributes should be > >> associated with a lightbar driver not the mfd driver. In order to retain > >> the path, the lightbar attributes are attached to the cros_class. > > > > I'm not exactly clear on what a lightbar is, but shouldn't it live in > > the appropriate subsystem. Like LED for example? > > > > The lightbar is a four-color indicator available on some Chromebook, but the > fact that can you can program this lightbar with different sequences, > including > user defined sequences makes the device a bit special and very chrome platform > specific. The same happens with the VBC driver.
Being Chrome specific doesn't necessarily mean that these drivers shouldn't reside in a proper subsystem. A lot of drivers in the kernel are only relevant to very specific hardware/platforms. IMHO code in drivers/platform should pertain only to the core platform itself. Any drivers for leaf hardware/functionality should be split out into the subsystems, however niche you think they are. I also understand the convenience factor of not having to go through a !Google Maintainer, but this is not a loophole I'm prepared to support. ;) > Other subdevices like, rtc, keyboard, usbpd charger,etc. are already in their > subsystems. > > >> The patch also adds the sysfs documentation. > >> > >> Signed-off-by: Enric Balletbo i Serra <enric.balle...@collabora.com> > >> --- > >> > >> Changes in v3: > >> - Removed unneded check for ec_dev. > >> > >> Changes in v2: > >> - Removed the two exported functions to attach/detach to the cros_class. > >> - Use dev_warn instead of dev_err when adding the lightbar. > >> > >> ...sfs-class-chromeos-driver-cros-ec-lightbar | 74 +++++++++++++++ > >> drivers/mfd/cros_ec_dev.c | 24 ++--- > >> drivers/mfd/cros_ec_dev.h | 6 -- > >> drivers/platform/chrome/Kconfig | 10 ++ > >> drivers/platform/chrome/Makefile | 3 +- > >> drivers/platform/chrome/cros_ec_lightbar.c | 95 ++++++++++++++----- > >> include/linux/mfd/cros_ec.h | 1 - > >> 7 files changed, 172 insertions(+), 41 deletions(-) > >> create mode 100644 > >> Documentation/ABI/testing/sysfs-class-chromeos-driver-cros-ec-lightbar > > -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
