On Wed, Jan 04, 2017 at 09:11:34AM -0800, Srinivas Pandruvada wrote: > On Wed, 2017-01-04 at 14:03 +0100, Greg KH wrote: > > On Fri, Dec 23, 2016 at 09:22:29AM +0800, Even Xu wrote: > > > > > > Intel ISHFW supports many different clients, in > > > hid/intel-ish-hid/ishtp bus driver, it creates following client > > > devices: > > > HID client: > > > interface of sensor configure and sensor event report. > > > SMHI client: > > > interface of sensor calibration, ISHFW debug, ISHFW performance > > > analysis and manufacture support. > > > Trace client: > > > interface of ISHFW debug log output. > > > Trace configure client: > > > interface of ISHFW debug log configuration, such as output > > > port, > > > log level, filter. > > > ISHFW loader client: > > > interface of customized ISHFW loader. > > > HID client has been handle by hid/intel-ish-hid/intel-ishtp-hid > > > client > > > driver, and rest of the clients export interface using > > > miscellaneous > > > drivers. This interface is used by user space tools for debugging > > > and > > > calibration of sensors. > > > > > > Signed-off-by: Even Xu <even.xu@xxxxxxxxx> > > > Reviewed-by: Andriy Shevchenko <andriy.shevchenko@xxxxxxxxx> > > > Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.c > > > om> > > > --- > > > drivers/misc/Kconfig | 1 + > > > drivers/misc/Makefile | 1 + > > > drivers/misc/intel-ish-client/Kconfig | 15 + > > > drivers/misc/intel-ish-client/Makefile | 8 + > > > .../misc/intel-ish-client/intel-ishtp-clients.c | 884 > > > +++++++++++++++++++++ > > > include/uapi/linux/intel-ishtp-clients.h | 73 ++ > > > > > > Why create a whole new subdirectory for just one .c file? Is that > > really needed? > The other option is to move this .c file to drivers/hid/intel-ish-hid/. > I think the folders inside drivers/hid/ is mostly for just implementing > transport layer for hid devices. > > > > > > And I'm not quite sure why you need a misc driver, what exactly is > > this > > code doing? > As described in the description, this driver is a companion driver for > ISH user space tools for calibration, production and debug. debug should not require a char device node, use debugfs, that is what it is there for. For "calibration", why not use configfs or even sysfs? > Basically the ISH provided a standalone low power processor to > developers and manufacturers to do download some custom algorithms for > sensors, which may not be compliant to USB HID sensor specifications > (mostly for IOT space). In that case the user space for those can > communicate using misc driver interface, without adding new kernel > drivers. So you hide it behind a char device node? That's not very descriptive or easy to understand :) Again, use the interfaces that the kernel gives you for this type of stuff, and don't create new one-off ones if at all possible. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
