Re: [PATCH v6 0/3] input: Add support for ECI (multimedia)accessories

[Tapio Vihuri]

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@xxxxxxxxx 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.

Thanks,
Tapio

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