On Thu, May 26, 2016 at 9:00 PM, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > On Thu, May 26, 2016 at 11:00:55AM +0200, Linus Walleij wrote: >> On Thu, May 12, 2016 at 8:42 PM, Uwe Kleine-König >> <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: >> >> > [added Linus Walleij to Cc, there is a question for you/him below] >> (...) >> >> +void mdio_device_reset(struct mdio_device *mdiodev, int value) >> >> +{ >> >> + if (mdiodev->reset) >> >> + gpiod_set_value(mdiodev->reset, value); >> > >> > Before v4.6-rc1~108^2~91 it was not necessary to check for the first >> > parameter being non-NULL before calling gpiod_set_value. Linus, did you >> > change this on purpose? >> >> Not really. And AFAICT it is still not necessary: what changed is that >> an error message will be printed by VALIDATE_DESC() if you do that. >> And that is proper I guess? I think it's sloppy code to randomly pass in >> NULL to a call and just expect it to bail out, it seems more like >> exercising the error path than something you'd normally rely on. >> >> Or am I getting things wrong? > > is the following sloppy?: > > somegpio = gpiod_get_optional(dev, "some", GPIOD_OUT_LOW); > if (IS_ERR(somegpio)) > return PTR_ERR(somegpio); > gpiod_set_value(somegpio, 1); Grrr OK I see, it's explicit from the _optional() call that it may be NULL and then it should be ignored. So subsequent functions should ignore that and bail out. My bad, sorry. > If not (as I assume) you really changed something as this might trigger > the warning. Making a patch. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html