All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: Jarod Wilson <jarod@redhat.com>
Cc: linux-media@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH 3/3] ir-core: add imon driver
Date: Tue, 20 Apr 2010 18:22:36 -0300	[thread overview]
Message-ID: <20100420182236.2e5a1325@pedra> (raw)
In-Reply-To: <20100416212902.GD2427@redhat.com>

Em Fri, 16 Apr 2010 17:29:02 -0400
Jarod Wilson <jarod@redhat.com> escreveu:

> 
> This is a new driver for the SoundGraph iMON and Antec Veris IR/display
> devices commonly found in many home theater pc cases and as after-market
> case additions.


> +/* IR protocol: native iMON, Windows MCE (RC-6), or iMON w/o PAD stabilize */
> +static int ir_protocol;
> +module_param(ir_protocol, int, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(ir_protocol, "Which IR protocol to use. 0=auto-detect, "
> +		 "1=Windows Media Center Ed. (RC-6), 2=iMON native, "
> +		 "4=iMON w/o PAD stabilize (default: auto-detect)");
> +

You don't need this. Let's the protocol to be adjustable via sysfs. All you need to do is
to use the set_protocol callbacks with something like:

        props->allowed_protos = IR_TYPE_RC6 | IR_TYPE_<imon protocol>;
        props->change_protocol = imon_ir_change_protocol;

You can see an example of such implementation at drivers/media/video/em28xx-em28xx-input.c.
Look for em28xx_ir_change_protocol() function.

That's said, I'm not sure what would be better way to map IR_TYPE_<imon protocol>. Maybe we
can just use IR_TYPE_OTHER.

So, basically, we'll have:

	IR_TYPE_OTHER | IR_TYPE_RC6	- auto-detected between RC-6 and iMON
	IR_TYPE_OTHER			- iMON proprietary protocol
	IR_TYPE_RC6			- RC-6 protocol


By doing this, the userspace application ir-keycode will already be able to handle the
IR protocol.

I'm not sure how to map the "PAD stablilize" case, but it seems that the better would be to
add a sysfs node for it, at sys/class/rc/rc0. There are other cases where some protocols
may require some adjustments, so I'm thinking on having some protocol-specific properties there.

Except for that, the patch looked sane to my eyes. So, I'll add it on my tree and wait for a
latter patch from you addressing the protocol control.

-- 

Cheers,
Mauro

  reply	other threads:[~2010-04-20 21:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-16 21:26 [PATCH 0/3] ir-core: add imon device driver Jarod Wilson
2010-04-16 21:27 ` [PATCH 1/3] ir-core: make ir_g_keycode_from_table a public function Jarod Wilson
2010-04-16 21:28 ` [PATCH 2/3] ir-core: add imon pad and mce keymaps Jarod Wilson
2010-04-24  8:53   ` David Härdeman
2010-04-24 20:53     ` Jarod Wilson
2010-04-24 20:53       ` Jarod Wilson
2010-04-16 21:29 ` [PATCH 3/3] ir-core: add imon driver Jarod Wilson
2010-04-20 21:22   ` Mauro Carvalho Chehab [this message]
2010-04-22  1:55     ` Jarod Wilson
2010-04-22 13:36       ` Mauro Carvalho Chehab
2010-04-23  5:23         ` Jarod Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100420182236.2e5a1325@pedra \
    --to=mchehab@redhat.com \
    --cc=jarod@redhat.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.