Re: [PATCH] ACPI: Support Fujitsu Lifebook S6410 in fujitsu-laptop

Jonathan Woithe
2008-04-15T03:39:13Z

Hi

> IMO there should be two fujitsu drivers, one for:
> ACPI_FUJITSUB1_HID "FUJ02B1"
> e.g. acpi_fujitsu_brightness
> or
> acpi_fujitsu_02b1
> and one for:
> ACPI_FUJITSUE3_HID "FUJ02E3"
> e.g. acpi_fujitsu_keys
> or
> acpi_fujitsu_02e3
> not sure what the perfect name could be...
>
> There probably are machines out there which only have one of above
> devices, then only the relevant driver should be loaded.

I tend to agree with the idea of separate drivers since circumstantial
evidence suggests that Fujitsu do change the roles of these components
between models. It think it would be entirely possible that some laptops
have only one of these device functions. Given this, the modular approach
would seem to be more maintainable going forward. I haven't however looked
into this in any detail.

As for names, "acpi_fujitsu_brightness", "acpi_fujitsu_keys" makes sense but
are perhaps a little long. Going with just "fujitsu_brightness" and
"fujitsu_keys" isn't really any less clear unless of course the respective
devices end up controlling things other than those already identified. To
stay completely generic, "fujitsu_02b1" and "fujitsu_02e3" would be best
although clearly these don't suggest any idea of their functionality.

> You should also take people into CC who are already stated as authors in
> the driver, chances are high that they miss this post.

Thanks for raising this. I did indeed miss it. :-)

> Someone should review this who is deeper involved into fujitsu machines.

I wouldn't say I'm deepling involved in the fujitsu machines or all that
knowledgeable about ACPI, but I do have an S7020 which is what the
fujitsu-laptop module was developed on. :-)

Regards
jonathan