On 2021-08-13 14:34, Rob Herring wrote:
On Fri, Aug 13, 2021 at 7:55 AM Robin Murphy <robin.murphy@xxxxxxx> wrote:
On 2021-08-13 12:33, Dan Carpenter wrote:
If devm_regulator_get_optional() returns an error pointer, then we
should return it to the user. The current code makes an exception
for -ENODEV that will result in an error pointer dereference on the
next line when it calls regulator_enable(). Remove the exception.
Doesn't this break the apparent intent of the regulator being optional,
though?
I'm pretty sure 'optional' means ENODEV is never returned. So there
wasn't any real problem, but the check was unnecessary.
In fact it's the other way round - "optional" in this case is for when
the supply may legitimately not exist so the driver may or may not need
to handle it, so it can return -ENODEV if a regulator isn't described by
firmware. A non-optional regulator is assumed to represent a necessary
supply, so if there's nothing described by firmware you get the (valid)
dummy regulator back.
Robin.
Fixes: e1229e884e19 ("PCI: rockchip-dwc: Add Rockchip RK356X host controller driver")
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 20cef2e06f66..2d0ffd3c4e16 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -225,9 +225,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
/* DON'T MOVE ME: must be enable before PHY init */
rockchip->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3");
if (IS_ERR(rockchip->vpcie3v3))
- if (PTR_ERR(rockchip->vpcie3v3) != -ENODEV)
- return dev_err_probe(dev, PTR_ERR(rockchip->vpcie3v3),
- "failed to get vpcie3v3 regulator\n");
+ return dev_err_probe(dev, PTR_ERR(rockchip->vpcie3v3),
+ "failed to get vpcie3v3 regulator\n");
ret = regulator_enable(rockchip->vpcie3v3);
if (ret) {
