Re: [PATCH 2/2] PCI: rockchip: Simplify optional regulator handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 ;)

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.



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux