Re: [RFC PATCH 2/2] PCI: apple: Add driver for the Apple M1

From: Alyssa Rosenzweig
Date: Sun Aug 15 2021 - 23:21:05 EST


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