Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

[Jaya Kumar]

On 2/20/06, Matthew Garrett <mjg59@xxxxxxxxxxxxx> wrote:
> On Mon, Feb 20, 2006 at 10:13:53AM +0800, jayakumar.acpi@xxxxxxxxx wrote:
>
> > +     /* setup proc entry to set and get lcd brightness */
> > +     proc = create_proc_read_entry("lcd", S_IFREG | S_IRUGO | S_IWUSR,
> > +                     atlas_proc_dir, atlas_read_proc_lcd, atlas_dev);
>
> For basic sanity, could this please be a standard backlight driver
> rather than sticking yet another backlight control under yet another
> directory in /proc? It makes userspace much, much easier.

I'm not sure how standard that is. For example, I looked at the asus
and toshiba drivers. These ACPI board drivers use
/proc/acpi/somedevice/lcd. For example,

asus_acpi.c
894                 asus_proc_add(PROC_LCD, &proc_write_lcd,
&proc_read_lcd, mode,
895                               device);

toshiba_acpi.c
472         {"lcd", read_lcd, write_lcd},

So, that's why I chose to do the same in my implementation. I'd have
much rather used a generic sysfs entry but that's not what any ACPI
drivers appear to do. Further, I see that Patrick Mochel is rewriting
the whole acpi driver model (and incorporating sysfs) anyway so I
figured I'd go with the flow of existing drivers. Perhaps someone
could clarify what the consensus is. I'd be happy to make any desired
adjustments.

> drivers/video/backlight/corgi_bl.c is an example, but also see my posts
> to acpi-devel with patches that add it to existing acpi drivers.

I'll go take a look at that. I didn't look for an acpi driver outside
of the drivers/acpi directory. But if that's the consensus, shouldn't
someone also mod the toshiba and asus drivers?

>
> > +             return atlas_acpi_button_add(device);
>
> What buttons does the hardware have? Would it make more sense for it to

Standard wallmount stuff. There's 8 buttons on the one I'm using for
testing. Vol up/down. Brightness up/down. Then several buttons for
miscellaneous usage by people who customize the chassis. Most apps for
this type of board are custom written and tend to just select on
/proc/acpi/event.

> be an input driver rather than (or as well as) just dropping stuff in
> acpi/events?

I would have loved to make it an input driver. But looking at the
mailing list archives, that seems to be a bone of contention and hence
I chose to go with the flow. I'll be happy to switch it over to an
input driver if there is consensus around that. Please do let me know.

Thanks,
jayakumar

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