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
