lkml.org 
[lkml]   [2009]   [Jun]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC/PATCH] Winbond CIR driver for the WPCD376I chip (ACPI/PNP id WEC1022)
From
On Thu, June 25, 2009 00:13, Jesse Barnes wrote:
> On Wed, 24 Jun 2009 14:36:45 -0700
> David Härdeman <david@hardeman.nu> wrote:
>
>> I've written a driver for the
...
>> Winbond WPCD376I chipset
>
> Yay, glad I could get these released for you. I just did a quick scan
> of the driver (notes below)

Two more things that Intel could provide:

a) Publish the datasheet (I know you mentioned doing this but
I can't find it on the Intel website)

b) Make the hardware needed to actually use the CIR functionality
available for purchase. http://www.easy-cir.com seems to be more
or less dead (which is curious since an ad for the website
seems to be included with every CIR-enabled Intel motherboard).
I had to solder my own IR receiver in order to write the driver.

>> I'd appreciate having the driver reviewed...and in addition I have
>> some questions for the list:
>>
>> 1) SuperI/O concurrency
>> ...
>
> Yeah, often multifunction devices like this have higher level "bus
> drivers" that take care of managing the global parts, and drivers that
> attach to it to manage individual functions. If you were feeling
> really ambitious you could do that for the superio chip and port any
> sub-drivers... :)

My ambitions are more directed towards some kind of IR-subsystem into the
kernel at the moment :) Besides, the Intel mainboards doesn't actually
seem to use any of the other logical devices (which are mostly supported
by existing drivers anyway).

>> Where should this driver go in the tree? drivers/platform/x86/?
>
> drivers/char is probably fine.

I'm leaning towards drivers/input/misc now...

>> #define dprintk(fmt, arg...) \
>> do { \
>> if (debug) \
>> printk(KERN_DEBUG DRVNAME fmt , ## arg); \
>> } while (0)
>
> Maybe you could use the generic debug functions instead (pr_debug iirc)?

Yes

> ...
> There are a few magic numbers above here you could possibly make into
> #defines just to make things more readable.

I'll try

> The key up/down timeout handling seems like a pretty general problem,
> maybe the input layer has some helpers for it? Dunno.

drivers/media/common/ir-functions.c is the closest thing I could find
while writing the driver. The functions there aren't usable because they
do not properly implement the toggle/repeat handling and it forces the use
of a small, fixed-size keymap. The same problem existed when I improved
the IR functionality in drivers/media/dvb/ttpci/budget-ci.c by the way, so
a generic version could probably be added to ir-functions in the future.

>> static ssize_t
>> wbcir_show_last_scancode(struct device *dev,
>> struct device_attribute *attr, char *buf)
>> {
>> struct acpi_device *device = container_of(dev, struct
>> acpi_device, dev); struct wbcir_data *data = acpi_driver_data(device);
>> return sprintf(buf, "0x%08X\n", data->last_scancode);
>> }
>>
>> static struct device_attribute dev_attr_last_scancode = {
>> .attr = {
>> .name = "last_scancode",
>> .mode = 0444,
>> },
>> .show = wbcir_show_last_scancode,
>> .store = NULL,
>>
>> };
>>
>> static struct attribute *wbcir_attributes[] = {
>> &dev_attr_last_scancode.attr,
>> NULL,
>> };
>>
>> static struct attribute_group wbcir_attribute_group = {
>> .attrs = wbcir_attributes,
>> };
>
> Are these just for debugging? If so, you could put them in debugfs
> instead...

No, they are there to help the user when generating a keymap for an
unknown remote. Press key on remote, read value from
/sys/.../last_scancode, add line saying "0x12345678 = KEY_EXPLODE" to
keymap file, repeat...there aren't any user-friendly tools for this yet
though.

(Dropped Terry from the CC, I just saw that he had requested a driver for
this chip earlier but I'm not sure he's that interested in the rest of the
discussion)

--
David Härdeman

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-06-25 13:49    [W:0.073 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site