Messages in this thread | | | Subject | Re: [PATCH] iio:potentiostat:lmp91000: solve codestyle WARNINGs and CHECKs | From | LSO <> | Date | Fri, 1 Feb 2019 12:29:11 -0200 |
| |
Thanks for the review!
On 29/01/2019 20:48, Joe Perches wrote: > On Tue, 2019-01-29 at 16:36 -0200, Lucas Oshiro wrote: >> Solve most of the checkpatch.pl WARNINGs and CHECKs on lmp9100.c. They >> are the following: >> >> lmp91000.c:116: CHECK: Unnecessary parentheses around 'state != channel' >> lmp91000.c:116: CHECK: Unnecessary parentheses around 'channel == LMP91000_REG_MODECN_TEMP' >> lmp91000.c:214: CHECK: braces {} should be used on all arms of this statement >> lmp91000.c:216: CHECK: Unbalanced braces around else statement >> lmp91000.c:258: WARNING: line over 80 characters >> lmp91000.c:279: CHECK: Please don't use multiple blank lines > > Some will say this is too many things to do at once. > I think it's mostly fine, but there are a few nits > that also could use fixing. > >> diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c > [] >> @@ -211,9 +211,9 @@ static int lmp91000_read_config(struct lmp91000_data *data) >> >> ret = of_property_read_u32(np, "ti,tia-gain-ohm", &val); >> if (ret) { >> - if (of_property_read_bool(np, "ti,external-tia-resistor")) >> + if (of_property_read_bool(np, "ti,external-tia-resistor")) { >> val = 0; >> - else { >> + } else { >> dev_err(dev, "no ti,tia-gain-ohm defined"); >> return ret; >> } > > This could use inverting the test > > if (ret) { > if (!of_property_read_bool(...)) { > dev_err(dev, "no ti,ti-gain-ohm defined\n"); > return ret; > } > val = 0; > } > Thanks for the suggestion, I'll do that in the next version.
> Also the dev_err is missing a '\n' termination
My aim in this patch was only solve style problems, but I can put that missing '\n' too. Do you think it could be done in the same commit or it's a better idea do it in another commit and send both as a patchset? > >
| |