Hi Pratyush,

Thanks for the review.

Best regards

Gabriel

On 27 August 2015 at 19:31, Pratyush Anand [off-list ref] wrote:

Hi Gabriel,

Looks good to me.

On Thu, Aug 27, 2015 at 6:04 PM, Gabriel Fernandez
[off-list ref] wrote:

quoted

sti pcie is built around a Synopsis Designware PCIe IP.

Signed-off-by: Fabrice Gasnier <redacted>
Signed-off-by: Gabriel Fernandez <redacted>

quoted

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

nit: why not bool

i prefer to keep it as 'int' because the prototype of link_up callback
is an 'int'.

quoted

+       int count = 0;

[...]

quoted

+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.

Ok i will fix it.

Reviewed-by: Pratyush Anand <pratyush.anand@gmail.com>