From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936586AbdADRSi (ORCPT ); Wed, 4 Jan 2017 12:18:38 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:33888 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S936476AbdADRSd (ORCPT ); Wed, 4 Jan 2017 12:18:33 -0500 Date: Wed, 4 Jan 2017 18:18:53 +0100 From: Greg KH To: Srinivas Pandruvada Cc: Even Xu , jikos@kernel.org, benjamin.tissoires@redhat.com, arnd@arndb.de, andriy.shevchenko@intel.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 7/7] misc: intel-ish-client: add intel ishtp clients driver Message-ID: <20170104171853.GA23715@kroah.com> References: <1482456149-4841-1-git-send-email-even.xu@intel.com> <1482456149-4841-7-git-send-email-even.xu@intel.com> <20170104130316.GA8378@kroah.com> <1483549894.4497.55.camel@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1483549894.4497.55.camel@linux.intel.com> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > > Reviewed-by: Andriy Shevchenko > > > Reviewed-by: Srinivas Pandruvada > > 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