Re: [RFC] [PATCH] panasonic-laptop.c: add support for CD power management

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Martin,

thanks for your patch, here some comments:

* please provide Signed-off-by line
* please invert the logic (as discussed earlier in the thread)
* please don't use the goto construct in pcc_hotkey_add(), since it
  makes it easy to introduce future bugs while adding more code,
  just check for the ODD drive presence and put the entire block
  in an 'if ()  { }' construct.
* I agree, checking for _SB.FBAY and _SB.STAT is probably the better
  solution.  I would accept both versions, though.
* Please run your patch through checkpatch.pl, I think there were some
  indentation errors in it (just spotted them with my eye, didn't run
  the script)

> 3) Please advise on the correct way to propagate an error result from
> the functions that call the ACPI methods {get,set}optd_power_state() to
> userspace in the sysfs interface.  I couldn't find a straightforward
> example anywhere.

I cannot help with that either, sorry.  I'm not the ACPI expert here, just
happening to have merged the panasonic driver ;)

Oh, and with regard to 'who in userspace is responsible': This is actually a
good question.  I've had many of this kind of cases in embedded development,
where you had to explicitly have to enable the power to a certain peripheral
before using.  I personally believe this doesn't belong into userspace, and
the kernel should provide hooks for it, i.e. issue some kind of
event/notifier/... to call any power-up hanlers before using, and then doing
the inverse after use.

Regards,
-- 
- Harald Welte <laforge@xxxxxxxxxxxx>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux