On Mon, May 03, 2010 at 05:00:05PM -0300, Mauro Carvalho Chehab wrote:
> David Härdeman wrote:
> > This patch moves the state from each raw decoder into the
> > ir_raw_event_ctrl struct.
> >
> > This allows the removal of code like this:
> >
> > spin_lock(&decoder_lock);
> > list_for_each_entry(data, &decoder_list, list) {
> > if (data->ir_dev == ir_dev)
> > break;
> > }
> > spin_unlock(&decoder_lock);
> > return data;
> >
> > which is currently run for each decoder on each event in order
> > to get the client-specific decoding state data.
> >
> > In addition, ir decoding modules and ir driver module load
> > order is now independent. Centralizing the data also allows
> > for a nice code reduction of about 30% per raw decoder as
> > client lists and client registration callbacks are no longer
> > necessary.
>
> The registration callbacks will likely still be needed by lirc,
> as you need to create/delete lirc_dev interfaces, when the module
> is registered, but I might be wrong. It would be interesting to
> add lirc_dev first, in order to be sure about the better interfaces
> for it.
Or the lirc_dev patch can add whatever interfaces it needs. Anyway, the
current interfaces are not good enough since it'll break if lirc_dev is
loaded after the hardware modules.
> Also, from one side, you reduced the code size, but, on the other hand,
> you've increased the memory usage, as now the protocol data will be
> stored even for protocols that weren't compiled/loaded.
In <4BBF3309.6020909@xxxxxxxxxxxxx>, Mauro Carvalho Chehab wrote:
>> Andy Walls wrote:
>>> Encoding pulse vs space with a negative sign, even if now hidden
>>> with macros, is still just using a sign instead of a boolean.
>>> Memory in modern computers (and now microcontrollers) is cheap and
>>> only getting cheaper. Don't give up readability, flexibility, or
>>> mainatainability, for the sake of saving memory.
>
> That was my point since the beginning: the amount of saved memory
> doesn't justify the lack of readability.
Are you worried about memory usage now?
> Probably, the code size savings are big enough to justify the runtime
> memory footprint, at least with the current decoders. Not sure how big
> this will become when we add lirc_dev and other decoders that might be
> missing.
Right now, the "reasonable default" is a user machine with one hardware
decoder and with all of the rc-core decoders loaded (cause that's how
his/her distro will set it up). For that machine, the patch will save a
lot of memory, not waste it (~380 lines less code...I'll assure you it's
a net gain).
In addition, random module load order is currently broken (try loading
decoders first and hardware later and you'll see). With this patch, it
works again.
Anyway, I'll post a new patch series this evening and then we can go
back to our regular arguing :)
--
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
