Re: [PATCH] PCAP misc input driver (for 2.6.32)

2009-06-29 Thread Mark Brown
On Sat, Jun 27, 2009 at 02:09:52PM -0300, Daniel Ribeiro wrote:
 This is a driver for misc input events for the PCAP2 PMIC, it handles
 the power button, headphone insertion/removal and the headphone button.

Depending on the power management capabilities of the CODEC it may be
better to support at laest the headphone insert/removal via the audio
driver as a jack to allow automatic management of the power for the
relevant paths within the CODEC.



Re: [PATCH] PCAP misc input driver (for 2.6.32)

2009-06-29 Thread Ilya Petrov
2009/6/29 Mark Brown broo...@opensource.wolfsonmicro.com:

 Depending on the power management capabilities of the CODEC it may be
 better to support at laest the headphone insert/removal via the audio
 driver as a jack to allow automatic management of the power for the
 relevant paths within the CODEC.

we need to control this manually from userspace - for example, even with
headphone plugged in ringtone should be played via speaker.



-- 
 wbr, Ilya
 ICQ: none, Jabber: ilya.muro...@jabber.ru



Re: [PATCH] PCAP misc input driver (for 2.6.32)

2009-06-29 Thread Mark Brown
On Mon, Jun 29, 2009 at 07:27:24PM +0300, Ilya Petrov wrote:
 2009/6/29 Mark Brown broo...@opensource.wolfsonmicro.com:

  Depending on the power management capabilities of the CODEC it may be
  better to support at laest the headphone insert/removal via the audio
  driver as a jack to allow automatic management of the power for the
  relevant paths within the CODEC.

 we need to control this manually from userspace - for example, even with
 headphone plugged in ringtone should be played via speaker.

ALSA jacks are still visible to user space as input devices.  The
difference it will make here is that the kernel can autonomously control
power for paths where that makes sense.



Re: [PATCH] PCAP misc input driver (for 2.6.32)

2009-06-29 Thread Daniel Ribeiro
Em Dom, 2009-06-28 às 23:14 -0700, Dmitry Torokhov escreveu:
   drivers/input/keyboard/Kconfig |7 ++
   drivers/input/keyboard/Makefile|1 +
   drivers/input/keyboard/pcap_keys.c |  152 
  
   3 files changed, 160 insertions(+), 0 deletions(-)
  
 
 First of all I think the driver should live in misc, not in keyboard,
 since it is not a full-fledged keyboard.

Ok.

  +  Say Y here if you want to use power key and jack events
  +  on Motorola EZX 2nd generation phones
  +
 
 To compile this driver as a module...

Ok.

  +   case PCAP_IRQ_MIC:
  +   input_report_key(pcap_keys-input, KEY_HP, !pstat);
 
 Why not SW_MICROPHONE_INSERT?

Its actually a button.

The device has a single jack for headphone and microphone. And the
headset that we connect to it has a button that you use to answer calls,
or dial.

  +static int __init pcap_keys_probe(struct platform_device *pdev)
 
 __devinit, not __init should be used on driver's probe() methods.

Ok.

  +   pcap_keys-input-name = pdev-name;
  +   pcap_keys-input-phys = pcap-keys/input0;
  +   pcap_keys-input-dev.parent = pdev-dev;
 
 I do like a temp for input_dev, it usually makes code a bit smaller.
 Also it would be nice to have but type set (BUS_HOST I think).

Ok.

  +   set_bit(KEY_HP, pcap_keys-input-keybit);

 __set_bit() please, like Trolok said.

Ok.

  +static int pcap_keys_remove(struct platform_device *pdev)
 
 __devexit here.

Ok.

  +   .remove = pcap_keys_remove,
 
 __devexit_p()

Ok.

  +MODULE_DESCRIPTION(Motorola PCAP2 input events driver);
  +MODULE_AUTHOR(Ilya Petrov ilya.muro...@gmail.com);
  +MODULE_LICENSE(GPL);
 
 Do we need MODULE_ALIAS() here?

Do we? I think we don't, but well... It costs nothing. ;)

-- 
Daniel Ribeiro


signature.asc
Description: Esta é uma parte de mensagem assinada digitalmente


Re: [PATCH] PCAP misc input driver (for 2.6.32)

2009-06-28 Thread Trilok Soni
Hi Daniel,

 +
 +static int __init pcap_keys_probe(struct platform_device *pdev)
 +{
 +       int err = -ENOMEM;
 +       struct pcap_keys *pcap_keys;
 +
 +       pcap_keys = kmalloc(sizeof(struct pcap_keys), GFP_KERNEL);
 +       if (!pcap_keys)
 +               return err;
 +
 +       pcap_keys-pcap = platform_get_drvdata(pdev);
 +
 +       pcap_keys-input = input_allocate_device();
 +       if (!pcap_keys-input)
 +               goto fail;
 +
 +       platform_set_drvdata(pdev, pcap_keys);
 +       pcap_keys-input-name = pdev-name;
 +       pcap_keys-input-phys = pcap-keys/input0;
 +       pcap_keys-input-dev.parent = pdev-dev;
 +
 +       pcap_keys-input-evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_SW);
 +       set_bit(KEY_POWER, pcap_keys-input-keybit);
 +       set_bit(SW_HEADPHONE_INSERT, pcap_keys-input-swbit);
 +       set_bit(KEY_HP, pcap_keys-input-keybit);

__set_bit please.

 +
 +       err = request_irq(pcap_to_irq(pcap_keys-pcap, PCAP_IRQ_ONOFF),
 +                       pcap_keys_handler, 0, Power key, pcap_keys);
 +       if (err)
 +               goto fail_dev;
 +
 +       err = request_irq(pcap_to_irq(pcap_keys-pcap, PCAP_IRQ_HS),
 +                       pcap_keys_handler, 0, Headphone jack, pcap_keys);
 +       if (err)
 +               goto fail_pwrkey;
 +
 +       err = request_irq(pcap_to_irq(pcap_keys-pcap, PCAP_IRQ_MIC),
 +                       pcap_keys_handler, 0, MIC jack/button, pcap_keys);
 +       if (err)
 +               goto fail_jack;
 +
 +       err = input_register_device(pcap_keys-input);
 +       if (err)
 +               goto fail_mic;

Same comment as given in PCAP touchscreen driver.

 +
 +static int pcap_keys_remove(struct platform_device *pdev)
 +{
 +       struct pcap_keys *pcap_keys = platform_get_drvdata(pdev);
 +
 +       free_irq(pcap_to_irq(pcap_keys-pcap, PCAP_IRQ_ONOFF), pcap_keys);
 +       free_irq(pcap_to_irq(pcap_keys-pcap, PCAP_IRQ_HS), pcap_keys);
 +       free_irq(pcap_to_irq(pcap_keys-pcap, PCAP_IRQ_MIC), pcap_keys);
 +
 +       input_unregister_device(pcap_keys-input);
 +       kfree(pcap_keys);
 +
 +       return 0;
 +}
 +
 +static struct platform_driver pcap_keys_device_driver = {
 +       .probe          = pcap_keys_probe,
 +       .remove         = pcap_keys_remove,

__devexit_p ?

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni



Re: [PATCH] PCAP misc input driver (for 2.6.32)

2009-06-28 Thread Daniel Ribeiro
Em Dom, 2009-06-28 às 12:49 +0530, Trilok Soni escreveu:
  +   pcap_keys-input-evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_SW);
  +   set_bit(KEY_POWER, pcap_keys-input-keybit);
  +   set_bit(SW_HEADPHONE_INSERT, pcap_keys-input-swbit);
  +   set_bit(KEY_HP, pcap_keys-input-keybit);
 
 __set_bit please.

Ok.

  +
  +   err = request_irq(pcap_to_irq(pcap_keys-pcap, PCAP_IRQ_ONOFF),
  +   pcap_keys_handler, 0, Power key, pcap_keys);
  +   if (err)
  +   goto fail_dev;
  +
  +   err = request_irq(pcap_to_irq(pcap_keys-pcap, PCAP_IRQ_HS),
  +   pcap_keys_handler, 0, Headphone jack, pcap_keys);
  +   if (err)
  +   goto fail_pwrkey;
  +
  +   err = request_irq(pcap_to_irq(pcap_keys-pcap, PCAP_IRQ_MIC),
  +   pcap_keys_handler, 0, MIC jack/button, pcap_keys);
  +   if (err)
  +   goto fail_jack;
  +
  +   err = input_register_device(pcap_keys-input);
  +   if (err)
  +   goto fail_mic;
 
 Same comment as given in PCAP touchscreen driver.

Ok.

  +static struct platform_driver pcap_keys_device_driver = {
  +   .probe  = pcap_keys_probe,
  +   .remove = pcap_keys_remove,
 
 __devexit_p ?

Ok.

Thanks for the review, I will send new versions addressing your comments
as soon as possible.

-- 
Daniel Ribeiro


signature.asc
Description: Esta é uma parte de mensagem assinada digitalmente