On Mon, Nov 18, 2019 at 01:10:58PM +0000, Robin Murphy wrote: > On 18/11/2019 12:39 pm, Andrew Murray wrote: > > On Mon, Nov 18, 2019 at 12:20:10PM +0000, Robin Murphy wrote: > > > On 18/11/2019 11:59 am, Mark Brown wrote: > > > > On Sat, Nov 16, 2019 at 12:54:20PM +0000, Robin Murphy wrote: > > > > > Null checks are both cheaper and more readable than having !IS_ERR() > > > > > splattered everywhere. > > > > > > > > > - if (IS_ERR(rockchip->vpcie3v3)) > > > > > + if (!rockchip->vpcie3v3) > > > > > return; > > > > > /* > > > > > @@ -611,6 +611,7 @@ static int rockchip_pcie_parse_host_dt(struct rockchip_pcie *rockchip) > > > > > if (PTR_ERR(rockchip->vpcie12v) != -ENODEV) > > > > > return PTR_ERR(rockchip->vpcie12v); > > > > > dev_info(dev, "no vpcie12v regulator found\n"); > > > > > + rockchip->vpcie12v = NULL; > > > > > > > > According to the API NULL is a valid regulator. We don't currently > > > > actually do this but it's storing up surprises if you treat it as > > > > invalid. > > > > > > Ah, OK - I'd assumed NULL wasn't valid based on regulator_enable() > > > immediately dereferencing its argument without any checks. If we'd rather > > > not bake in that assumption then this patch can happily be ignored. > > > > I'd suggest we drop this patch. > > > > "IS_ERR(ptr)" is not the same as "!ptr", for values of ptr between 0 and > > -4095 inclusive. > > Hence the explicit initial "if (IS_ERR(ptr)) ptr = NULL;" condition quoted > above ;) Doh. Andrew Murray > > But yeah, it was merely an attempt at a minor cosmetic cleanup, so let's > just forget about it to avoid any possible confusion. > > Cheers, > Robin.
