Message-ID: <20110202062122.GA22251@core.coreip.homeip.net>
Date:	Tue, 1 Feb 2011 22:21:22 -0800
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Tapio Vihuri <tapio.vihuri@...ia.com>
Cc:	peter.ujfalusi@...ia.com, randy.dunlap@...cle.com,
	linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
	ilkka.koskinen@...ia.com, samu.p.onkalo@...ia.com
Subject: Re: [PATCH v6 0/3] input: Add support for ECI (multimedia)
 accessories

On Tue, Feb 01, 2011 at 11:32:41AM +0200, Tapio Vihuri wrote:
> On Tue, 2011-02-01 at 00:58 -0800, ext Dmitry Torokhov wrote:
> > Hi Tapio,
> > 
> > On Mon, Jan 31, 2011 at 04:03:51PM +0200, tapio.vihuri@...ia.com wrote:
> > > 
> > > This patch set introduce Multimedia Headset Accessory support for
> > > Nokia phones. Technically those are known as ECI (Enhancement Control Interface)
> > > 	
> > > If headset has many buttons, like play, vol+, vol- etc. then it is propably ECI
> > > accessory.
> > > 
> > > Among several buttons ECI accessories contains memory for storing several
> > > parameters.
> > > 
> > > This ECI input driver provides the following features:
> > >  - reading ECI configuration memory
> > >  - ECI buttons as input events
> > > 
> > > Drive is constructed as follows:
> > >  - ECI accessory input driver deals with headset accessory
> > >  - ECI bus control driver deals the HW transfering data to/from headset
> > >  - platform data match used HW
> > 
> > I finally had a chance to look though the patches more closely and I do
> > not understand why you decided to introduce the platform device in
> > addition to I2C device. You end up with 2 artificially separated bodies of
> > code that are not viable on their own. The ECI module with it's platform
> > device is not usable without the controller; the controller can not be
> > registered without the ECI device initialized; there are ordering
> > issues, both initialization and PM-wise and you are forced to support
> > only one device.
> > 
> > Is there going to be an SPI interface as well? If not then fold it all
> > together and have I2C device as the only device involved. If SPI is a
> > possibility then look in drivers such as adxl34x, ad714x and others that
> > are split into core module and bus interface implementations.
> > 
> > Thanks.
> > 
> 
> Hi Dmitry
> 
> Thank you for reviewing.
> 
> The reasoning for separating driver to the two parts is having both
> common and different hardware.
> 
> The ECI input, as platform device is common part and only dealing with
> the accessory HW (meaning the headset accessory).
> This is standardiced part of system, where specification guarantee HW
> behaving same way accross several headset accessories.
> The accessory is plugged to the terminal using four pin AV-cable, much
> same way as USB mouse.
> I just didn't see need to introduce new bus type for one wire ECI data
> link, hence platform device.
> 
> The ECI controller, as I2C device is the variable part of the system.
> In this case it's microcontroller in I2C bus, but there are other
> versions too.
> There is no specification about these controller's internals, and those
> are quite different.
> It is also possible to use other buses too for connecting controller to
> terminal, like SPI.
> 
> The idea was that if ECI is used in device, just take ECI input driver
> and select the ECI controller which is used in the platform.
> 
> And as long as the ECI controller driver use the same interface to ECI
> input driver, evetrything works OK.
> This allows also dynamic selection of used control device. I mean we can
> compile several ECI controllers as modules and detect the HW at
> initialization.
> 
> I use the ad714x as example, but I can take second look on that too.
> 
> Please let me know if this is making sense.

Tapio,

I understand that ECI accessory is the standard part that can be attached
to several controllers, it still does not explain the need for the
platform device.

If ECI accessory is the only device that uses the interface you
specified then write a module containing that core functionality, with a
function to instantiate an instance of the accessory and have controller
drivers call this function as part of their bind procedure (i.e. do what
adxl34x and company are doing).

If there could be other accessories utilizing the same interface then
creating a new bus is the only viable option.

Thank you.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists