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

[Dmitry Torokhov]

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.

-- 
Dmitry
--
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/