Re: [PATCH v4 3/4] PCI: st: Provide support for the sti PCIe controller

[Pratyush Anand]

Hi Gabriel,

Looks good to me.

On Thu, Aug 27, 2015 at 6:04 PM, Gabriel Fernandez
<gabriel.fernandez@xxxxxxxxxx> wrote:
> sti pcie is built around a Synopsis Designware PCIe IP.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxx>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@xxxxxxxxxx>


> +static int st_pcie_link_up(struct pcie_port *pp)
> +{
> +       u32 status;
> +       int link_up;

nit: why not bool

> +       int count = 0;

[...]

> +static void st_pcie_board_reset(struct pcie_port *pp)
> +{
> +       struct st_pcie *pcie = to_st_pcie(pp);
> +
> +       if (!gpio_is_valid(pcie->reset_gpio))
> +               return;
> +
> +       if (gpio_direction_output(pcie->reset_gpio, 0)) {
> +               dev_err(pp->dev, "Cannot set PERST# (gpio %u) to output\n",
> +                       pcie->reset_gpio);
> +               return;
> +       }
> +
> +       /* From PCIe spec */
> +       msleep(2);
> +       gpio_direction_output(pcie->reset_gpio, 1);
> +
> +       /*
> +        * PCIe specification states that you should not issue any config
> +        * requests until 100ms after asserting reset, so we enforce that here
> +        */
> +       msleep(100);

IIRC, specification says to wait after link training completes. So
shouldn't it be after st_pcie_enable_ltssm. Moreover, I wonder why
others do not need it.

Reviewed-by: Pratyush Anand <pratyush.anand@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/