Mark Brown <broo...@kernel.org> writes:
> On Tue, Jun 17, 2014 at 09:16:52PM +0200, Robert Jarzmik wrote:
>> Mark Brown <broo...@kernel.org> writes:
>> > On Sat, Jun 14, 2014 at 04:54:24PM +0200, Robert Jarzmik wrote:
>
>> >> + matched = of_regulator_match(dev, np, rmatch, ARRAY_SIZE(rmatch));
>> >> + of_node_put(np);
>> >> + if (matched <= 0)
>> >> + return matched;
>
>> > Why is this treating zero as an error? We should be able to at least
>> > report the current state of regulators even if none are configured in
>> > the device tree.
>
>> Euh how so an error ?
>
>> If 0 is returned, this means no regulators are found in device-tree. It's
>> not an
>> error, it's a lack of regulators (ie. no Output_V3 and no Output_V6), and no
>> more handling is necessary in this function, while returning "ok", ie 0 ...
>
> OK, so there's just nothing to do in that case. That's fine, but it's
> just not at all clear from the code. A comment would help.
OK, no problem.
>
>> As for the "state report", this max1586 doesn't report anything, it cannot
>> even
>> be queried about the current voltage, sic ...
>
> It can't? That's unfortunate, though I was able to turn up a datasheet
> which appears to support that.
Oh really ? Well, tell me where you read it.
My personal reading from the Max1586 specs is (page 21, chapter "Serial
Interface") :
The LSB of the address word is the read/write (R/W) bit.
R/W indicates whether the master is writing or reading
(RD/W 0 = write, RD/W 1 = read). The MAX1586/
MAX1587 only support the SEND BYTE format; there-
fore, RD/W is required to be 0.
I'm wondering if you have this sentence in your datasheet too.
>> If you want me to modify this bit I need a bit more of an explanation to
>> understand.
>
> Where the driver is doing unusual things if they are actually sensible
> then the change needs to be clearer about why.
So would a comment like this address your comment ?
/* Either matched < 0 and return the error. Or matched is 0 which means
* no init data was found, ie. no regulator is configured, and return 0
* to caller, stating neither error nor any matched regulator.
*/
if (matched <= 0)
return matched;
Cheers.
--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
