linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: "David Härdeman" <david@hardeman.nu>
Cc: linux-media@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH 2/4] ir-core: centralize sysfs raw decoder enabling/disabling
Date: Mon, 03 May 2010 16:49:20 -0300	[thread overview]
Message-ID: <4BDF28C0.4060102@redhat.com> (raw)
In-Reply-To: <20100424211406.11570.96241.stgit@localhost.localdomain>

Hi David,

David Härdeman wrote:
> 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.

I liked the idea of your redesign, but I didn't like the removal of
a per-decoder sysfs entry. As already discussed, there are cases where
we'll need a per-decoder sysfs entry (lirc_dev is probably one of those
cases - also Jarod's imon driver is currently implementing a modprobe
parameter that needs to be moved to the driver).

So, while we can implement some core support at the raw event core, we should
keep the decoder attributes internally inside the driver. So, each decoder
may have his own code like:

static struct attribute *decoder_attributes[] = {
       &dev_attr_enabled.attr,
       &dev_attr_bar1.attr,
       &dev_attr_bar2.attr,
       &dev_attr_bar3.attr,
       NULL
};

static struct attribute_group decoder_attribute_group = {
       .name   = "foo_decoder",
       .attrs  = decoder_attributes,
};

As the attr_enabled is common to all, maybe we can have a default enable
code at the .h or inside the core.
 
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

  reply	other threads:[~2010-05-03 19:49 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-24 21:13 [PATCH 0/4] ir-core sysfs protocol selection simplification David Härdeman
2010-04-24 21:14 ` [PATCH 1/4] ir-core: remove IR_TYPE_PD David Härdeman
2010-04-24 21:14 ` [PATCH 2/4] ir-core: centralize sysfs raw decoder enabling/disabling David Härdeman
2010-05-03 19:49   ` Mauro Carvalho Chehab [this message]
2010-06-07 18:48     ` David Härdeman
2010-04-24 21:14 ` [PATCH 3/4] ir-core: move decoding state to ir_raw_event_ctrl David Härdeman
2010-05-03 20:00   ` Mauro Carvalho Chehab
2010-06-07 19:00     ` David Härdeman
2010-06-07 20:15       ` Jarod Wilson
2010-06-08 17:50         ` David Härdeman
2010-06-09  3:46           ` Jarod Wilson
2010-06-09 13:29             ` Jarod Wilson
2010-06-09 17:56               ` David Härdeman
2010-06-09 18:15                 ` Jarod Wilson
2010-06-10  1:25                   ` Jarod Wilson
2010-06-13 20:29                     ` David Härdeman
2010-06-16 20:04                       ` Jarod Wilson
2010-06-16 20:41                         ` Jarod Wilson
2010-06-17 12:14                           ` Andy Walls
2010-06-17 15:11                             ` Jarod Wilson
2010-06-21  0:47                               ` Andy Walls
2010-06-21  3:51                                 ` Jarod Wilson
2010-06-21 11:04                                   ` Andy Walls
2010-07-06 17:12                                     ` Jarod Wilson
2010-04-24 21:14 ` [PATCH 4/4] ir-core: remove ir-functions usage from cx231xx David Härdeman

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=4BDF28C0.4060102@redhat.com \
    --to=mchehab@redhat.com \
    --cc=david@hardeman.nu \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).