All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: "David Härdeman" <david@hardeman.nu>
Cc: jarod@wilsonet.com, jarod@redhat.com,
	linux-media@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
Date: Mon, 28 Jun 2010 13:56:16 -0300	[thread overview]
Message-ID: <4C28D430.9010305@redhat.com> (raw)
In-Reply-To: <20100613202930.6044.97940.stgit@localhost.localdomain>

Em 13-06-2010 17:29, David Härdeman escreveu:
> With the current logic, each raw decoder needs to add a copy of the exact
> same sysfs code. This is both unnecessary and also means that (re)loading
> an IR driver after raw decoder modules have been loaded won't work as
> expected.
> 
> This patch moves that logic into ir-raw-event and adds a single sysfs
> file per device.
> 
> Reading that file returns something like:
> 
> 	"rc5 [rc6] nec jvc [sony]"
> 
> (with enabled protocols in [] brackets)
> 
> Writing either "+protocol" or "-protocol" to that file will
> enable or disable the according protocol decoder.
> 
> An additional benefit is that the disabling of a decoder will be
> remembered across module removal/insertion so a previously
> disabled decoder won't suddenly be activated again. The default
> setting is to enable all decoders.
> 
> This is also necessary for the next patch which moves even more decoder
> state into the central raw decoding structs.
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>

> +	if (!strncasecmp(tmp, "unknown", 7)) {
> +		tmp += 7;
> +		mask = IR_TYPE_UNKNOWN;
> +	} else if (!strncasecmp(tmp, "rc5", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_RC5;
> +	} else if (!strncasecmp(tmp, "nec", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_NEC;
> +	} else if (!strncasecmp(tmp, "rc6", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_RC6;
> +	} else if (!strncasecmp(tmp, "jvc", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_JVC;
> +	} else if (!strncasecmp(tmp, "sony", 4)) {
> +		tmp += 4;
> +		mask = IR_TYPE_SONY;
> +	} else {
>  		IR_dprintk(1, "Unknown protocol\n");
>  		return -EINVAL;
>  	}

Those "magic" sizes at the strcasecmp are ugly. Also, you didn't send
a patch for the ir-keytable userspace tool (part of v4l-utils.git tree).

Also, this allows some undocumented (and problematic) ways to set protocols. 
For example, if someone writes: something like "rc5necjvcsony"
(on this exact order and without spaces), it will accept it as just "sony",
without complaining.

As I found some time during the weekend to add support for the new /protocols way at
the userspace tool, I'll be applying this patch as-is, plus a few patches after it 
fixing the problems I found and adding some additional features that will be needed,
in order to make userspace stuff easier, like supporting multiple protocols specs
without needing to close/open the sysfs node for each protocol.

Btw, we should start writing some docs about the sysfs interface at the media
specs (at Documentation/DocBoook).

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: "David Härdeman" <david@hardeman.nu>
Cc: jarod@wilsonet.com, jarod@redhat.com,
	linux-media@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling
Date: Mon, 28 Jun 2010 13:56:16 -0300	[thread overview]
Message-ID: <4C28D430.9010305@redhat.com> (raw)
In-Reply-To: <20100613202930.6044.97940.stgit@localhost.localdomain>

Em 13-06-2010 17:29, David Härdeman escreveu:
> With the current logic, each raw decoder needs to add a copy of the exact
> same sysfs code. This is both unnecessary and also means that (re)loading
> an IR driver after raw decoder modules have been loaded won't work as
> expected.
> 
> This patch moves that logic into ir-raw-event and adds a single sysfs
> file per device.
> 
> Reading that file returns something like:
> 
> 	"rc5 [rc6] nec jvc [sony]"
> 
> (with enabled protocols in [] brackets)
> 
> Writing either "+protocol" or "-protocol" to that file will
> enable or disable the according protocol decoder.
> 
> An additional benefit is that the disabling of a decoder will be
> remembered across module removal/insertion so a previously
> disabled decoder won't suddenly be activated again. The default
> setting is to enable all decoders.
> 
> This is also necessary for the next patch which moves even more decoder
> state into the central raw decoding structs.
> 
> Signed-off-by: David Härdeman <david@hardeman.nu>

> +	if (!strncasecmp(tmp, "unknown", 7)) {
> +		tmp += 7;
> +		mask = IR_TYPE_UNKNOWN;
> +	} else if (!strncasecmp(tmp, "rc5", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_RC5;
> +	} else if (!strncasecmp(tmp, "nec", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_NEC;
> +	} else if (!strncasecmp(tmp, "rc6", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_RC6;
> +	} else if (!strncasecmp(tmp, "jvc", 3)) {
> +		tmp += 3;
> +		mask = IR_TYPE_JVC;
> +	} else if (!strncasecmp(tmp, "sony", 4)) {
> +		tmp += 4;
> +		mask = IR_TYPE_SONY;
> +	} else {
>  		IR_dprintk(1, "Unknown protocol\n");
>  		return -EINVAL;
>  	}

Those "magic" sizes at the strcasecmp are ugly. Also, you didn't send
a patch for the ir-keytable userspace tool (part of v4l-utils.git tree).

Also, this allows some undocumented (and problematic) ways to set protocols. 
For example, if someone writes: something like "rc5necjvcsony"
(on this exact order and without spaces), it will accept it as just "sony",
without complaining.

As I found some time during the weekend to add support for the new /protocols way at
the userspace tool, I'll be applying this patch as-is, plus a few patches after it 
fixing the problems I found and adding some additional features that will be needed,
in order to make userspace stuff easier, like supporting multiple protocols specs
without needing to close/open the sysfs node for each protocol.

Btw, we should start writing some docs about the sysfs interface at the media
specs (at Documentation/DocBoook).

Cheers,
Mauro

  parent reply	other threads:[~2010-06-28 16:56 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-13 20:29 [PATCH 0/2] ir-core: raw decoder framework changes David Härdeman
2010-06-13 20:29 ` David Härdeman
2010-06-13 20:29 ` [PATCH 1/2] ir-core: centralize sysfs raw decoder enabling/disabling David Härdeman
2010-06-16 20:05   ` Jarod Wilson
2010-06-16 20:05     ` Jarod Wilson
2010-06-16 20:39     ` Jarod Wilson
2010-06-16 20:39       ` Jarod Wilson
2010-06-28 16:56   ` Mauro Carvalho Chehab [this message]
2010-06-28 16:56     ` Mauro Carvalho Chehab
2010-09-08 14:04   ` Brian Rogers
2010-09-08 14:16     ` Jarod Wilson
2010-09-08 21:22       ` David Härdeman
2010-09-08 21:22         ` David Härdeman
2010-09-15 12:57       ` [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface Brian Rogers
2010-09-15 14:41         ` Jarod Wilson
2010-06-13 20:29 ` [PATCH 2/2] ir-core: move decoding state to ir_raw_event_ctrl David Härdeman
2010-06-13 20:29   ` David Härdeman
2010-06-16 20:06   ` Jarod Wilson
2010-06-16 20:06     ` Jarod Wilson
2010-06-16 20:39     ` Jarod Wilson
2010-06-16 20:39       ` 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=4C28D430.9010305@redhat.com \
    --to=mchehab@redhat.com \
    --cc=david@hardeman.nu \
    --cc=jarod@redhat.com \
    --cc=jarod@wilsonet.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.