diff mbox series

PCI: mediatek-gen3: Assert resets to ensure expected init state

Message ID 20220404144858.92390-1-angelogioacchino.delregno@collabora.com (mailing list archive)
State New, archived
Headers show
Series PCI: mediatek-gen3: Assert resets to ensure expected init state | expand

Commit Message

AngeloGioacchino Del Regno April 4, 2022, 2:48 p.m. UTC
The controller may have been left out of reset by the bootloader,
in which case, before the powerup sequence, the controller will be
found preconfigured with values that were set before booting the
kernel: this produces a controller failure, with the result of
a failure during the mtk_pcie_startup_port() sequence as the PCIe
link never gets up.

To ensure that we get a clean start in an expected state, assert
both the PHY and MAC resets before executing the controller
power-up sequence.

Fixes: d3bf75b579b9 ("PCI: mediatek-gen3: Add MediaTek Gen3 driver for MT8192")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

AngeloGioacchino Del Regno May 9, 2022, 12:18 p.m. UTC | #1
Il 04/04/22 16:48, AngeloGioacchino Del Regno ha scritto:
> The controller may have been left out of reset by the bootloader,
> in which case, before the powerup sequence, the controller will be
> found preconfigured with values that were set before booting the
> kernel: this produces a controller failure, with the result of
> a failure during the mtk_pcie_startup_port() sequence as the PCIe
> link never gets up.
> 
> To ensure that we get a clean start in an expected state, assert
> both the PHY and MAC resets before executing the controller
> power-up sequence.
> 
> Fixes: d3bf75b579b9 ("PCI: mediatek-gen3: Add MediaTek Gen3 driver for MT8192")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Friendly ping for an important fix :-)

Regards,
Angelo

> ---
>   drivers/pci/controller/pcie-mediatek-gen3.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index f7048ea4c020..dccdfce12b1c 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -838,6 +838,14 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)
>   	if (err)
>   		return err;
>   
> +	/*
> +	 * The controller may have been left out of reset by the bootloader
> +	 * so make sure that we get a clean start by asserting resets here.
> +	 */
> +	reset_control_assert(pcie->phy_reset);
> +	reset_control_assert(pcie->mac_reset);
> +	usleep_range(10, 20);
> +
>   	/* Don't touch the hardware registers before power up */
>   	err = mtk_pcie_power_up(pcie);
>   	if (err)
Bjorn Helgaas May 9, 2022, 5:16 p.m. UTC | #2
On Mon, May 09, 2022 at 02:18:34PM +0200, AngeloGioacchino Del Regno wrote:
> Il 04/04/22 16:48, AngeloGioacchino Del Regno ha scritto:
> > The controller may have been left out of reset by the bootloader,
> > in which case, before the powerup sequence, the controller will be
> > found preconfigured with values that were set before booting the
> > kernel: this produces a controller failure, with the result of
> > a failure during the mtk_pcie_startup_port() sequence as the PCIe
> > link never gets up.
> > 
> > To ensure that we get a clean start in an expected state, assert
> > both the PHY and MAC resets before executing the controller
> > power-up sequence.
> > 
> > Fixes: d3bf75b579b9 ("PCI: mediatek-gen3: Add MediaTek Gen3 driver for MT8192")
> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> Friendly ping for an important fix :-)

d3bf75b579b9 appeared in v5.13-rc1 (May 2021, a year ago), so I assume
this is important for v5.19, but not urgent enough to require being in
v5.18.  Right?

> > ---
> >   drivers/pci/controller/pcie-mediatek-gen3.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> > index f7048ea4c020..dccdfce12b1c 100644
> > --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> > @@ -838,6 +838,14 @@ static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)
> >   	if (err)
> >   		return err;
> > +	/*
> > +	 * The controller may have been left out of reset by the bootloader
> > +	 * so make sure that we get a clean start by asserting resets here.
> > +	 */
> > +	reset_control_assert(pcie->phy_reset);
> > +	reset_control_assert(pcie->mac_reset);
> > +	usleep_range(10, 20);
> > +
> >   	/* Don't touch the hardware registers before power up */
> >   	err = mtk_pcie_power_up(pcie);
> >   	if (err)
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
AngeloGioacchino Del Regno May 10, 2022, 7:59 a.m. UTC | #3
Il 09/05/22 19:16, Bjorn Helgaas ha scritto:
> On Mon, May 09, 2022 at 02:18:34PM +0200, AngeloGioacchino Del Regno wrote:
>> Il 04/04/22 16:48, AngeloGioacchino Del Regno ha scritto:
>>> The controller may have been left out of reset by the bootloader,
>>> in which case, before the powerup sequence, the controller will be
>>> found preconfigured with values that were set before booting the
>>> kernel: this produces a controller failure, with the result of
>>> a failure during the mtk_pcie_startup_port() sequence as the PCIe
>>> link never gets up.
>>>
>>> To ensure that we get a clean start in an expected state, assert
>>> both the PHY and MAC resets before executing the controller
>>> power-up sequence.
>>>
>>> Fixes: d3bf75b579b9 ("PCI: mediatek-gen3: Add MediaTek Gen3 driver for MT8192")
>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>
>> Friendly ping for an important fix :-)
> 
> d3bf75b579b9 appeared in v5.13-rc1 (May 2021, a year ago), so I assume
> this is important for v5.19, but not urgent enough to require being in
> v5.18.  Right?
> 

Hello Bjorn,
no it's not *very* urgent, as MediaTek devices that do require this will be
in v5.19 anyway...

Thanks,
Angelo
Lorenzo Pieralisi May 11, 2022, 2:26 p.m. UTC | #4
On Mon, 4 Apr 2022 16:48:58 +0200, AngeloGioacchino Del Regno wrote:
> The controller may have been left out of reset by the bootloader,
> in which case, before the powerup sequence, the controller will be
> found preconfigured with values that were set before booting the
> kernel: this produces a controller failure, with the result of
> a failure during the mtk_pcie_startup_port() sequence as the PCIe
> link never gets up.
> 
> [...]

Applied to pci/mediatek, thanks!

[1/1] PCI: mediatek-gen3: Assert resets to ensure expected init state
      https://git.kernel.org/lpieralisi/pci/c/1d565935e3

Thanks,
Lorenzo
diff mbox series

Patch

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index f7048ea4c020..dccdfce12b1c 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -838,6 +838,14 @@  static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)
 	if (err)
 		return err;
 
+	/*
+	 * The controller may have been left out of reset by the bootloader
+	 * so make sure that we get a clean start by asserting resets here.
+	 */
+	reset_control_assert(pcie->phy_reset);
+	reset_control_assert(pcie->mac_reset);
+	usleep_range(10, 20);
+
 	/* Don't touch the hardware registers before power up */
 	err = mtk_pcie_power_up(pcie);
 	if (err)