On Mon, 29 Apr 2024 21:00:41 +0200 Vasileios Amoiridis <vassilisamir@xxxxxxxxx> wrote: > Throughout the driver there are quite a few places were return > values are treated as errors if they are negative or not-zero. > This commit tries to make the return values of those functions > consistent and treat them as errors in case there is a negative > value since the vast majority of the functions are returning > erorrs coming from regmap_*() functions. The changes are fine, but that argument isn't correct. regmap_*() functions never (that I can recall) return positive values, so if (ret) would be valid for those and I'd have expected the exact opposite outcome if you are looking at regmap*() return values to make the decision. The if (ret) pattern is sometimes used throughout because it makes return function() consistent without needing to do ret = function(); if (ret < 0) return ret; return 0; That pattern isn't particularly common in this driver (there are few cases). We also tend not to worry too much about that slight inconsistency though in a few cases it has lead to compilers failing to detect that some paths are not possible and reporting false warnings. However, all arguments about which is 'better' aside, key is that consistency (either choice) is better than a mix. So I'm fine with ret < 0 on basis it's the most common in this driver being your justification. Just don't blame regmap*() return values! > > While at it, add error messages that were not implemented before. > > Finally, remove any extra error checks that are dead code. Ideally this would be broken up a little more as, whilst all error code related, these aren't all the same thing. I'd have preferred: 1) Dead code removal. 2) Message updates. 3) Switch to consistent ret handling. However it isn't that bad as a single patch, so just address the question above and I think this will be fine as one patch. > > Signed-off-by: Vasileios Amoiridis <vassilisamir@xxxxxxxxx> Jonathan
