On 04/05/2017 11:41 AM, Dan Williams wrote:
On Wed, Apr 5, 2017 at 6:13 AM, Mauricio Faria de Oliveira
<mauricfo@xxxxxxxxxxxxxxxxxx> wrote:
1) imagine .get_power_status couldn't update the 'power_status' field
(it's a bit unlikely with the in-tree ses driver, but in the case
that ses_get_page2_descriptor() returns NULL, ses_get_power_status()
doesn't update 'power_status').
Ok, in that case we should probably return -EIO, if get_power_status
is non-NULL, but the power_status is still -1.
2) an out-of-tree module employs another enclosure_component_callbacks
structure, which doesn't implement a .get_power_status hook.
If get_power_status is null and power_status is -1 I think we should
return -ENOTTY to indicate the failure.
3) or it does, but that failed, and didn't update 'power_status' (e.g.,
like case 1)
I think we're back -EIO for this.
Absolutely. I see those are better values to convey the failure/reason.
However, for an in-tree usage, it's clear that just dropping that line
seemed to be sufficient -- the rest in the patch is just consideration
for whoever might be using it in out-of-tree ways.
(and if such consideration is deemed not to be required in this case,
I'd be fine with dropping the related changes and making this patch
a one-line. :- )
Let's not carry module parameters that only theoretically help an out
of tree module. If such a driver exists its owner can just holler if
this policy change breaks their usage, and then we can have a
discussion about why their driver isn't upstream already.
Okay, got it.
For the record, the patch author has been in To:/Cc:, for such cases.
I'll submit a v2.
Thanks,
--
Mauricio Faria de Oliveira
IBM Linux Technology Center
