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.
> + WARN_ON(phy_power_off(pcie->phy[i]));
> + WARN_ON(phy_exit(pcie->phy[i]));
> + }
> +
> + return ret;
> +}
regards,
dan carpenter
