On 5/24/24 10:59, Dan Carpenter wrote: > On Mon, May 20, 2024 at 10:54:01AM -0400, Sean Anderson wrote: >> +static int nwl_pcie_phy_enable(struct nwl_pcie *pcie) >> +{ >> + int i, ret; >> + >> + for (i = 0; i < ARRAY_SIZE(pcie->phy); i++) { >> + ret = phy_init(pcie->phy[i]); >> + if (ret) >> + goto err; >> + >> + ret = phy_power_on(pcie->phy[i]); >> + if (ret) { >> + WARN_ON(phy_exit(pcie->phy[i])); >> + goto err; >> + } >> + } >> + >> + return 0; >> + >> +err: >> + while (--i) { > > This doesn't work. If we fail on the first iteration, then it will > lead to an array underflow. It should be while (--i >= 0) or > while (i--). I prefer the first, but the second format works for people > who use unsigned iterators. Thanks, will fix. --Sean >> + WARN_ON(phy_power_off(pcie->phy[i])); >> + WARN_ON(phy_exit(pcie->phy[i])); >> + } >> + >> + return ret; >> +} > > regards, > dan carpenter >