| Message ID | 20200109032851.13377-3-shawn.guo@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve pcie-histb GPIO reset implementation | expand |
On Thu, Jan 09, 2020 at 11:28:51AM +0800, Shawn Guo wrote: > The PCIe reset via GPIO in the driver never worked as expected. Per > "Power Sequencing and Reset Signal Timings" table in > PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, the PERST# should be > deasserted after minimum of 100us once REFCLK is stable. > > The assertion has been done when the GPIO is being requested, and > deassertion should be done in host enabling rather than disabling. Also > a bit wait is added to ensure device get ready after reset. > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > --- > drivers/pci/controller/dwc/pcie-histb.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) Shawn, this looks like a fix, please tag it as such and let me know if it has to be backported, in which case also the previous patch should I assume. Thanks, Lorenzo > diff --git a/drivers/pci/controller/dwc/pcie-histb.c b/drivers/pci/controller/dwc/pcie-histb.c > index 112254619ed0..67c27a8036c7 100644 > --- a/drivers/pci/controller/dwc/pcie-histb.c > +++ b/drivers/pci/controller/dwc/pcie-histb.c > @@ -219,9 +219,6 @@ static void histb_pcie_host_disable(struct histb_pcie *hipcie) > clk_disable_unprepare(hipcie->sys_clk); > clk_disable_unprepare(hipcie->bus_clk); > > - if (hipcie->reset_gpio) > - gpiod_set_value_cansleep(hipcie->reset_gpio, 0); > - > if (hipcie->vpcie) > regulator_disable(hipcie->vpcie); > } > @@ -242,9 +239,6 @@ static int histb_pcie_host_enable(struct pcie_port *pp) > } > } > > - if (hipcie->reset_gpio) > - gpiod_set_value_cansleep(hipcie->reset_gpio, 1); > - > ret = clk_prepare_enable(hipcie->bus_clk); > if (ret) { > dev_err(dev, "cannot prepare/enable bus clk\n"); > @@ -278,6 +272,20 @@ static int histb_pcie_host_enable(struct pcie_port *pp) > reset_control_assert(hipcie->bus_reset); > reset_control_deassert(hipcie->bus_reset); > > + if (hipcie->reset_gpio) { > + /* > + * "Power Sequencing and Reset Signal Timings" table in > + * PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, indicates > + * PERST# should be deasserted after minimum of 100us > + * once REFCLK is stable. > + */ > + usleep_range(100, 200); > + gpiod_set_value_cansleep(hipcie->reset_gpio, 0); > + > + /* wait 1ms for device to be ready */ > + usleep_range(1000, 2000); > + } > + > return 0; > > err_aux_clk: > -- > 2.17.1 >
On Wed, Feb 26, 2020 at 7:31 PM Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > On Thu, Jan 09, 2020 at 11:28:51AM +0800, Shawn Guo wrote: > > The PCIe reset via GPIO in the driver never worked as expected. Per > > "Power Sequencing and Reset Signal Timings" table in > > PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, the PERST# should be > > deasserted after minimum of 100us once REFCLK is stable. > > > > The assertion has been done when the GPIO is being requested, and > > deassertion should be done in host enabling rather than disabling. Also > > a bit wait is added to ensure device get ready after reset. > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > --- > > drivers/pci/controller/dwc/pcie-histb.c | 20 ++++++++++++++------ > > 1 file changed, 14 insertions(+), 6 deletions(-) > > Shawn, > > this looks like a fix, please tag it as such and let me know if > it has to be backported, in which case also the previous patch > should I assume. Hi Lorenzo, It is a fix, but we recently realized that the problem needs to be fixed in a way that does not break existing DTB. So please ignore the series for now, and we will try to work out a better one. Sorry that we did not update the thread in time. Shawn
diff --git a/drivers/pci/controller/dwc/pcie-histb.c b/drivers/pci/controller/dwc/pcie-histb.c index 112254619ed0..67c27a8036c7 100644 --- a/drivers/pci/controller/dwc/pcie-histb.c +++ b/drivers/pci/controller/dwc/pcie-histb.c @@ -219,9 +219,6 @@ static void histb_pcie_host_disable(struct histb_pcie *hipcie) clk_disable_unprepare(hipcie->sys_clk); clk_disable_unprepare(hipcie->bus_clk); - if (hipcie->reset_gpio) - gpiod_set_value_cansleep(hipcie->reset_gpio, 0); - if (hipcie->vpcie) regulator_disable(hipcie->vpcie); } @@ -242,9 +239,6 @@ static int histb_pcie_host_enable(struct pcie_port *pp) } } - if (hipcie->reset_gpio) - gpiod_set_value_cansleep(hipcie->reset_gpio, 1); - ret = clk_prepare_enable(hipcie->bus_clk); if (ret) { dev_err(dev, "cannot prepare/enable bus clk\n"); @@ -278,6 +272,20 @@ static int histb_pcie_host_enable(struct pcie_port *pp) reset_control_assert(hipcie->bus_reset); reset_control_deassert(hipcie->bus_reset); + if (hipcie->reset_gpio) { + /* + * "Power Sequencing and Reset Signal Timings" table in + * PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, indicates + * PERST# should be deasserted after minimum of 100us + * once REFCLK is stable. + */ + usleep_range(100, 200); + gpiod_set_value_cansleep(hipcie->reset_gpio, 0); + + /* wait 1ms for device to be ready */ + usleep_range(1000, 2000); + } + return 0; err_aux_clk:
The PCIe reset via GPIO in the driver never worked as expected. Per "Power Sequencing and Reset Signal Timings" table in PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, the PERST# should be deasserted after minimum of 100us once REFCLK is stable. The assertion has been done when the GPIO is being requested, and deassertion should be done in host enabling rather than disabling. Also a bit wait is added to ensure device get ready after reset. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- drivers/pci/controller/dwc/pcie-histb.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)