Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1235930

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

From Gabriel Fernandez <gabriel.fernandez@linaro.org>
Newsgroups linux.kernel
Subject Re: [PATCH v4 3/4] PCI: st: Provide support for the sti PCIe controller
Date 2015-09-30 11:30 +0200
Message-ID <qelS1-Ol-1@gated-at.bofh.it> (permalink)
References <q24Df-4x6-11@gated-at.bofh.it> <q24Df-4x6-9@gated-at.bofh.it> <q29th-330-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hi Pratyush,

Thanks for the review.

Best regards

Gabriel

On 27 August 2015 at 19:31, Pratyush Anand <pratyush.anand@gmail.com> wrote:
> Hi Gabriel,
>
> Looks good to me.
>
> On Thu, Aug 27, 2015 at 6:04 PM, Gabriel Fernandez
> <gabriel.fernandez@linaro.org> wrote:
>> sti pcie is built around a Synopsis Designware PCIe IP.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
>
>
>> +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'.

>
>> +       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.
>
Ok i will fix it.

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

Back to linux.kernel | Previous | Next | Find similar | Unroll thread


Thread

Re: [PATCH v4 3/4] PCI: st: Provide support for the sti PCIe controller Gabriel Fernandez <gabriel.fernandez@linaro.org> - 2015-09-30 11:30 +0200

csiph-web