Re: [PATCH][ACPI] Panasonic Hotkey Driver

[Hiroshi Miura <miura@da-cha.org>]

Hi,

At 18 Aug 2004 01:04:34 -0400,
Len Brown wrote:
> > 
> > The event is for example 'HKEY HKEY 00000080 0000001' for Fn+F1.
> 
> Would be good if you had a comment that listed what hot keys are
> available etc.

ok, I will do. 

> > # This is a BitKeeper generated diff -Nru style patch.
> 
> Since you're using bitkeeper, one option is to preserve your check-in
> comments by sending me the patch via bjorn's bkexport script (attached).
> Though I reserve the right to edit all comments;-)

It's good. I want to try it.
 
> >  
> > +config ACPI_PCC
> > +       tristate "Panasonic Laptop Extras"
> 
> how about  calling it a hot key driver instead of an "extras driver",
> unless there is something other than hot keys coming.

now I implemented controller for lcd brightness in this driver.
I will post it soon.

> > + *
> > + *      Jul.17, 2004   Hiroshi Miura <miura@da-cha.org>
> > + *             - v0.1  based on acpi video driver
> > + *
> 
> would be polite to credit the exact files that you leveraged.

Sorry, I will fix it as another post.
 
> > +/****************************************************
> > + * Define ACPI PATHs 
> > + ****************************************************/
> > +/* crt/lcd hot key definitions */
> > +#define DEVICE_NAME_VGA                "\\_SB_.PCI0.GRFX"
> > +#define DEVICE_NAME_CRT                "CRT1"
> > +#define DEVICE_NAME_LCD                "LCD1"
> 
> These three are unused and should be deleted.
> (they should be part of the video driver)

Yes these are part of video driver.
 
> > +#define METHOD_CHGD            "\\_SB_.CHGD"

I will remove it as same as above.

> > +#if 0
> > +static int
> > +write_acpi_int(const char* methodName, int val)
> > +{
> > +       struct acpi_object_list params;
> > +       union acpi_object in_objs[1];
> > +       acpi_status status;
> > +
> > +       params.count = sizeof(in_objs)/sizeof(in_objs[0]);
> > +       params.pointer = in_objs;
> > +       in_objs[0].type = ACPI_TYPE_INTEGER;
> > +       in_objs[0].integer.value = val;
> > +
> > +       status = acpi_evaluate_object(0, (char*)methodName, &params,
> > 0);
> > +       return (status == AE_OK);
> > +}
> > +#endif
> 
> hmm, this dead code above looks familiar:-)
> 
> we'll want to clean out the #if 0's, or at least
> put them under a descriptive #ifdef, yes?

will remove.

 

> > +/*
> > + * proc file handlers
> > + */
> 
> Of course we're trying to get away from /proc at this point and head for
> sysfs...

Uuum, I must study...  will rewrite.

> > +/* device(HKEY) definitions */
> > +#define HKEY_HID               "MAT0019"
> 
> I'm extremely pleased to see this keys off a HID.

> > +/* init funcs. */
> > +static int __init
> > +acpi_pcc_init(void)
> > +{
> > +       acpi_status result = AE_OK;
> > + 
> > +       if (acpi_disabled)
> > +               return -ENODEV;
> > +
> > +       /* simple device detection: look forI method */
> > +       if (!(is_valid_acpi_path(METHOD_CHGD)))
> > +               return -ENODEV;
> 
> Why is this necessary if you key off the HID with
> acpi_bus_register_driver below?

It's my first acpi driver. My understanding of acpi functions is 
not enough at that time. I want to rewrite it.

-- 
Hiroshi Miura  --- http://www.da-cha.org/  --- miura@da-cha.org
NTTDATA Corp. OpenSource Software Center. --- miurahr@nttdata.co.jp 
NTTDATA Intellilink Corp. OpenSource Engineering Dev. -- miurahr@intellilink.co.jp
Key fingerprint = 9117 9407 5684 FBF1 4063  15B4 401D D077 04AB 8617