RE: [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver




If you look closely you'll see that the patch already adds a new
entry to MAINTAINERS.

Excellent.

I put the the driver in drivers/acpi/ because it relies heavily on the
ACPI EC stuff. But OK, what is a better place for the driver?

It is possible that parts of it should live in different places,
but I agree that it is also desireable to keep the compnents of a
platform
specific driver together in as few files as possible.

drivers/char/
It doesn't even register any character device.

I believe that is okay, and that creating a /dev file isn't a
requirement.

drivers/video/backlight/
It doesn't just do backlight control.

Perhaps the backlight part should live there.

drivers/misc/
Seems to be the last resort for everything that doesn't fit it
otherwise.

Unless anyone has a better idea I will move it to drivers/misc/, then.

Yeah, there is probably a better place than misc.

The only thing I can say for sure is that it implements
platform-dependent
features rather than standard features in the ACPI spec,
and thus it must live outside of drivers/acpi/.

lcd brightness platform support should talk to the existing
backlight stuff under sysfs.

It already does exactly that, you can find the backlight class driver
in /sys/class/backlight/s270/.

Excellent.

I cannot map the "automatic brightness control" feature to the
backlight class driver, that's why I duplicated the brightness stuff
in /proc/acpi/s270/.

Better to extend the backlight code so that it can handle the
special features of this platform than to duplicate brightness
control in multiple places.

wlan and bluetooth indicators/controls need to appear under
generic places under sysfs -- not under platform specific
files under /proc/acpi.

What are those "generic" places? I cannot think of any besides a
"platform" device.

They should appear as properties under their associated
devices in /sys/devices tree rather than invening a new place.

Any ideas on that ec_transaction() patch I sent earlier?

I agree 100% that ec.c needs to be whacked. Unfortunately
there seem to be 3 people doing it at the same time,
so we'll have to sort that out.

thanks,
-Len

ps. Lennart, please understand that I didn't intend to be gruff.
I figured that by the quality of your work -- which is better than most
--
that I'd go for an immediate reply, even though I was still holding a
sharp pointed stick at the end of a 6-hour bug scrub marathon.
-
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/



Relevant Pages

  • Re: [patch 2.6.20-rc3 1/3] rtc-cmos driver
    ... I believe that splitting this driver into multiple files ... do recall that at least ia64 uses ACPI too. ... ARMs tend to integrate some other RTC on-chip. ... on whatever non-PC platform is involved in such sanity testing, ...
    (Linux-Kernel)
  • [GIT PULL] ACPI patches for Linux-2.6.28-rc4
    ... ACPI: EC: make kernel messages more useful when GPE storm is detected ... add driver component definitions to sysfs debug_layers ... fingers off backlight if video.ko is serving this functionality ... commit d1876ba4dead6ace7e9fbf16f83397e6486d0dfe ...
    (Linux-Kernel)
  • Re: [PATCH] cmpc_acpi: Added support for Classmate PC ACPI devices.
    ... a nice and clean ACPI interface to work with. ... Perhaps this driver should be called "classmate-laptop". ... Please confirm that the brightness keys do not directly affect the ... backlight, and these key events are a signal for userspace to adjust ...
    (Linux-Kernel)
  • Re: sonypc with Sony Vaio VGN-SZ1VP
    ... as if acpi were not involved in providing them. ... it is a platform specific driver. ... We have to use generic code to create unified interface for the ...
    (Linux-Kernel)
  • [PATCH 1/1] LCD backlight driver using Atmel PWM driver
    ... This patch adds a platform driver using the ATMEL PWM driver to control a ... The driver is configurable by supplying a struct with the platform data. ... tristate "Generic Backlight Driver" ... * Backlight driver using Atmel PWM peripheral. ...
    (Linux-Kernel)