> On Thu, 2024-11-07 at 17:08 +0100, Lorenzo Bianconi wrote:
> > > On Thu, Nov 07, 2024 at 02:50:55PM +0100, Lorenzo Bianconi wrote:
> > > > In order to make the code more readable, move phy and mac reset
> > > > lines
> > > > assert/de-assert configuration in .power_up callback
> > > > (mtk_pcie_en7581_power_up/mtk_pcie_power_up).
> > > >
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> > > > ---
> > > > drivers/pci/controller/pcie-mediatek-gen3.c | 24
> > > > ++++++++++++++++--------
> > > > 1 file changed, 16 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c
> > > > b/drivers/pci/controller/pcie-mediatek-gen3.c
> > > > index
> > > > 8c8c733a145634cdbfefd339f4a692f25a6e24de..c0127d0fb4f059b9f9e8163
> > > > 60130e183e8f0e990 100644
> > > > --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> > > > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> > > > @@ -867,6 +867,13 @@ static int mtk_pcie_en7581_power_up(struct
> > > > mtk_gen3_pcie *pcie)
> > > > int err;
> > > > u32 val;
> > > >
> > > > + /*
> > > > + * 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_bulk_assert(pcie->soc->phy_resets.num_resets,
> > > > + pcie->phy_resets);
> > > > + reset_control_assert(pcie->mac_reset);
> > >
> > > Add blank line here.
> >
> > ack, I will fix it.
> >
> > >
> > > > /*
> > > > * Wait for the time needed to complete the bulk assert in
> > > > * mtk_pcie_setup for EN7581 SoC.
> > > > @@ -941,6 +948,15 @@ static int mtk_pcie_power_up(struct
> > > > mtk_gen3_pcie *pcie)
> > > > struct device *dev = pcie->dev;
> > > > int 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_bulk_assert(pcie->soc->phy_resets.num_resets,
> > > > + pcie->phy_resets);
> > > > + reset_control_assert(pcie->mac_reset);
> > > > + usleep_range(10, 20);
> > >
> > > Unrelated to this patch, but since you're moving it, do you know
> > > what
> > > this delay is for? Can we add a #define and a spec citation for
> > > it?
> >
> > I am not sure about it, this was already there.
> > @Jianjun Wang: any input on it?
>
> This delay is used to ensure the reset is effective. A delay of 10us
> should be sufficient in this scenario.
ack, so we can introduce a marco like:
#define PCIE_RESET_TIME_US 10
...
usleep_range(PCIE_RESET_TIME_US, 2 * PCIE_RESET_TIME_US);
what do you think?
Regards,
Lorenzo
>
> >
> > >
> > > Is there a requirement that the PHY and MAC reset ordering be
> > > different for EN7581 vs other chips?
> > >
> > > EN7581:
> > >
> > > assert PHY reset
> > > assert MAC reset
> > > power on PHY
> > > deassert PHY reset
> > > deassert MAC reset
> > >
> > > others:
> > >
> > > assert PHY reset
> > > assert MAC reset
> > > deassert PHY reset
> > > power on PHY
> > > deassert MAC reset
> > >
> > > Is there one order that would work for both?
> >
> > EN7581 requires to run phy_init()/phy_power_on() before deassert PHY
> > reset
> > lines.
> >
> > Regards,
> > Lorenzo
> >
> > >
> > > > /* PHY power on and enable pipe clock */
> > > > err = reset_control_bulk_deassert(pcie->soc-
> > > > >phy_resets.num_resets, pcie->phy_resets);
> > > > if (err) {
> > > > @@ -1013,14 +1029,6 @@ static int mtk_pcie_setup(struct
> > > > mtk_gen3_pcie *pcie)
> > > > * counter since the bulk is shared.
> > > > */
> > > > reset_control_bulk_deassert(pcie->soc->phy_resets.num_resets,
> > > > pcie->phy_resets);
> > > > - /*
> > > > - * 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_bulk_assert(pcie->soc->phy_resets.num_resets,
> > > > pcie->phy_resets);
> > > > -
> > > > - reset_control_assert(pcie->mac_reset);
> > > > - usleep_range(10, 20);
> > > >
> > > > /* Don't touch the hardware registers before power up */
> > > > err = pcie->soc->power_up(pcie);
> > > >
> > > > --
> > > > 2.47.0
> > > >
Attachment:
signature.asc
Description: PGP signature
