Hi Andy,
Thanks for the review.
> + depends on GPIOLIB
>
> It doesn’t seem to provide a GPIO.
I thought that was needed to consume GPIOs, but it looks like other
PCI drivers don't do it. Removed.
> + if (IS_ERR(port))
> + return -ENODEV;
> +
> + reset = devm_gpiod_get_index(pcie->dev, "reset", i, 0);
>
> Use appropriate flag.
>
>
> + if (IS_ERR(reset))
> + return PTR_ERR(reset);
> +
> + gpiod_direction_output(reset, 0);
>
> Ditto and remove this line.
Fixed in v2, thank you.
> + usleep_range(5000, 10000);
>
> Sleep of such length should be explained.
Removed in v2.
> +
>
> Redundant blank line
Presumably fixed in v2.
> + pcie->bitmap = devm_kcalloc(pcie->dev,
> BITS_TO_LONGS(pcie->nvecs),
> + sizeof(long), GFP_KERNEL);
>
> devm_bitmap_zalloc()
Done in v2.
> +static const struct of_device_id apple_pci_of_match[] = {
> + { .compatible = "apple,pcie", .data = &apple_m1_cfg_ecam_ops },
>
>
>
> + { },
>
> No comma for termination entry
Fixed in v2.
BR,
Alyssa
