Re: [PATCH/RFC 2.6.17-rc4 1/1] ACPI: Atlas ACPI driver v3

[Jaya Kumar]

On 5/22/06, Pavel Machek <pavel@xxxxxx> wrote:
> ACK, but I guess you should cc Dmitry (input maintainer) and possibly
> Andrew Morton to get it in... Ok, few more nits.

Will do.

> > +#else
> > +#define atlas_free_input(...)
> > +#define atlas_setup_input(...) 0
> > +#define atlas_input_report(...)
> > +#endif
>
> Does the driver actually do anything useful in this case?

Depends on the application I guess. If CONFIG_INPUT is disabled, then
only applications that read /proc/acpi/event will see the ASIM events.

> > +     }
> > +
> > +     return status ;
>
> Extra " " before ;.

Sorry. Corrected.

> > +static struct acpi_driver atlas_acpi_driver = {
> > +     .name = ACPI_ATLAS_NAME,
> > +     .class = ACPI_ATLAS_CLASS,
> > +     .ids = ACPI_ATLAS_BUTTON_HID,
> > +     .ops = {
> > +             .add = atlas_acpi_button_add,
> > +             .remove = atlas_acpi_button_remove,
> > +             },
>
> Extra tab before }.

I hope I understood you correctly, the } on .ops after .remove should
align with the . ops right?. I tried to conform with button.c and
others. I will make the change I think you are suggesting anyway since
it seems more natural.

74 static struct acpi_driver acpi_button_driver = {
75         .name = ACPI_BUTTON_DRIVER_NAME,
76         .class = ACPI_BUTTON_CLASS,
77         .ids = "ACPI_FPB,ACPI_FSB,PNP0C0D,PNP0C0C,PNP0C0E",
78         .ops = {
79                 .add = acpi_button_add,
80                 .remove = acpi_button_remove,
81                 },
82 };

71 static struct acpi_driver acpi_video_bus = {
72         .name = ACPI_VIDEO_DRIVER_NAME,
73         .class = ACPI_VIDEO_CLASS,
74         .ops = {
75                 .add = acpi_video_bus_add,
76                 .remove = acpi_video_bus_remove,
77                 .match = acpi_video_bus_match,
78                 },
79 };

> > +MODULE_SUPPORTED_DEVICE("Atlas ACPI");
>
> Are you sure this si good idea?

You are right. It's a bad idea, none of the other drivers do it. I'll
take it out.

Thanks,
jayakumar
-
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/