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