On Thu, 2007-02-22 at 12:56 -0200, Henrique de Moraes Holschuh wrote:

On Thu, 22 Feb 2007, Richard Purdie wrote:

quoted

quoted

it to Len Brown for merging into 2.6.21, or NACK it if you'd rather do it in
the backlight class core.

We can't change the backlight class code since some hardware can't read
from the device, only write to it. Initialisation in that case is a bit
different.

Initializing stuff after registering is also racy as the device is not
locked but we are going to clobber data in its properties struct.  I don't
particularly care about that race, but...

If you really care, add a a call to backlight_update_status() after you
set the brightness attribute like some of the other drivers have. The
only data you're changing are single numbers and as long as
update_status is called afterwards, a consistent state is pushed to the
hardware so there is no race problem.

Anyway, you have the 2.6.21-rc patch now, to ACK or NACK.  I still think the
class should be handling this.  If a device is write-only, it should have no
_get ops handler, which means that the class can easily differentiate the
two cases and do the right thing for both.  There's less code duplication
that way.

Have a look at what corgi_bl does. It can know what state it set the
hardware too as it keeps track itself, it just can't read that state
from the hardware. Note how there is extra code in it to handle a power
limit on the backlight under certain conditions and how this is fed back
through the class through the get_brightness method.

Adding one line of code (admittedly slight more due to error handling),
is hardly that much code duplication.

Howerver, I *do* strongly wish for a way to combine various drivers into a
single backlight device, where radeon/intelfb takes care of some stuff,
ibm-acpi/asus-laptop/sony-laptop takes care of other stuff, etc.  Also, a
standard naming for the builtin screen(s) would help, calling it "ibm",
"asus", "sony" is not good IMHO.

I wasn't aware of this problem. If some devices need bits from both
raedon/whatever and acpi, the current implementations are just plain
wrong. Its not really a backlight class problem and more of an
implementation and interaction problem between acpi and the framebuffer
drivers. They should be presenting and registering *one* backlight class
device, not two. Without knowing more about the circumstances and
how/when to combine which drivers its hard for me to help further...

Richard



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV