| Message ID | 20211117140044.193865-1-marcan@marcan.st (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: apple: Fix REFCLK1 enable/poll logic | expand |
On Wed, 17 Nov 2021 14:00:44 +0000, Hector Martin <marcan@marcan.st> wrote: > > REFCLK1 has req/ack bits just like REFCLK0 > > Signed-off-by: Hector Martin <marcan@marcan.st> > --- > drivers/pci/controller/pcie-apple.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c > index b665d29af77a..420c291a5c68 100644 > --- a/drivers/pci/controller/pcie-apple.c > +++ b/drivers/pci/controller/pcie-apple.c > @@ -42,8 +42,9 @@ > #define CORE_FABRIC_STAT_MASK 0x001F001F > #define CORE_LANE_CFG(port) (0x84000 + 0x4000 * (port)) > #define CORE_LANE_CFG_REFCLK0REQ BIT(0) > -#define CORE_LANE_CFG_REFCLK1 BIT(1) > +#define CORE_LANE_CFG_REFCLK1REQ BIT(1) > #define CORE_LANE_CFG_REFCLK0ACK BIT(2) > +#define CORE_LANE_CFG_REFCLK1ACK BIT(3) > #define CORE_LANE_CFG_REFCLKEN (BIT(9) | BIT(10)) > #define CORE_LANE_CTL(port) (0x84004 + 0x4000 * (port)) > #define CORE_LANE_CTL_CFGACC BIT(15) > @@ -481,9 +482,9 @@ static int apple_pcie_setup_refclk(struct apple_pcie *pcie, > if (res < 0) > return res; > > - rmw_set(CORE_LANE_CFG_REFCLK1, pcie->base + CORE_LANE_CFG(port->idx)); > + rmw_set(CORE_LANE_CFG_REFCLK1REQ, pcie->base + CORE_LANE_CFG(port->idx)); > res = readl_relaxed_poll_timeout(pcie->base + CORE_LANE_CFG(port->idx), > - stat, stat & CORE_LANE_CFG_REFCLK1, > + stat, stat & CORE_LANE_CFG_REFCLK1ACK, > 100, 50000); > > if (res < 0) > -- > 2.33.0 > > Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") Acked-by: Marc Zyngier <maz@kernel.org> M.
On Wed, Nov 17, 2021 at 05:56:12PM +0000, Marc Zyngier wrote: > On Wed, 17 Nov 2021 14:00:44 +0000, > Hector Martin <marcan@marcan.st> wrote: > > > > REFCLK1 has req/ack bits just like REFCLK0 > > > > Signed-off-by: Hector Martin <marcan@marcan.st> > > --- > > drivers/pci/controller/pcie-apple.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c > > index b665d29af77a..420c291a5c68 100644 > > --- a/drivers/pci/controller/pcie-apple.c > > +++ b/drivers/pci/controller/pcie-apple.c > > @@ -42,8 +42,9 @@ > > #define CORE_FABRIC_STAT_MASK 0x001F001F > > #define CORE_LANE_CFG(port) (0x84000 + 0x4000 * (port)) > > #define CORE_LANE_CFG_REFCLK0REQ BIT(0) > > -#define CORE_LANE_CFG_REFCLK1 BIT(1) > > +#define CORE_LANE_CFG_REFCLK1REQ BIT(1) > > #define CORE_LANE_CFG_REFCLK0ACK BIT(2) > > +#define CORE_LANE_CFG_REFCLK1ACK BIT(3) > > #define CORE_LANE_CFG_REFCLKEN (BIT(9) | BIT(10)) > > #define CORE_LANE_CTL(port) (0x84004 + 0x4000 * (port)) > > #define CORE_LANE_CTL_CFGACC BIT(15) > > @@ -481,9 +482,9 @@ static int apple_pcie_setup_refclk(struct apple_pcie *pcie, > > if (res < 0) > > return res; > > > > - rmw_set(CORE_LANE_CFG_REFCLK1, pcie->base + CORE_LANE_CFG(port->idx)); > > + rmw_set(CORE_LANE_CFG_REFCLK1REQ, pcie->base + CORE_LANE_CFG(port->idx)); > > res = readl_relaxed_poll_timeout(pcie->base + CORE_LANE_CFG(port->idx), > > - stat, stat & CORE_LANE_CFG_REFCLK1, > > + stat, stat & CORE_LANE_CFG_REFCLK1ACK, > > 100, 50000); > > > > if (res < 0) > > -- > > 2.33.0 > > > > > > Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") > Acked-by: Marc Zyngier <maz@kernel.org> Hi Hector, Bjorn, if this is a fix we can aim at one of the upcoming -rcX. It would be nicer though to explain a bit better what it is fixing in the commit log (and what's broken if we don't merge it), as it stands it is a bit terse. Thanks, Lorenzo
On 01/12/2021 01.49, Lorenzo Pieralisi wrote: > On Wed, Nov 17, 2021 at 05:56:12PM +0000, Marc Zyngier wrote: >> On Wed, 17 Nov 2021 14:00:44 +0000, >> Hector Martin <marcan@marcan.st> wrote: >>> >>> REFCLK1 has req/ack bits just like REFCLK0 >>> >>> Signed-off-by: Hector Martin <marcan@marcan.st> >>> --- >>> drivers/pci/controller/pcie-apple.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c >>> index b665d29af77a..420c291a5c68 100644 >>> --- a/drivers/pci/controller/pcie-apple.c >>> +++ b/drivers/pci/controller/pcie-apple.c >>> @@ -42,8 +42,9 @@ >>> #define CORE_FABRIC_STAT_MASK 0x001F001F >>> #define CORE_LANE_CFG(port) (0x84000 + 0x4000 * (port)) >>> #define CORE_LANE_CFG_REFCLK0REQ BIT(0) >>> -#define CORE_LANE_CFG_REFCLK1 BIT(1) >>> +#define CORE_LANE_CFG_REFCLK1REQ BIT(1) >>> #define CORE_LANE_CFG_REFCLK0ACK BIT(2) >>> +#define CORE_LANE_CFG_REFCLK1ACK BIT(3) >>> #define CORE_LANE_CFG_REFCLKEN (BIT(9) | BIT(10)) >>> #define CORE_LANE_CTL(port) (0x84004 + 0x4000 * (port)) >>> #define CORE_LANE_CTL_CFGACC BIT(15) >>> @@ -481,9 +482,9 @@ static int apple_pcie_setup_refclk(struct apple_pcie *pcie, >>> if (res < 0) >>> return res; >>> >>> - rmw_set(CORE_LANE_CFG_REFCLK1, pcie->base + CORE_LANE_CFG(port->idx)); >>> + rmw_set(CORE_LANE_CFG_REFCLK1REQ, pcie->base + CORE_LANE_CFG(port->idx)); >>> res = readl_relaxed_poll_timeout(pcie->base + CORE_LANE_CFG(port->idx), >>> - stat, stat & CORE_LANE_CFG_REFCLK1, >>> + stat, stat & CORE_LANE_CFG_REFCLK1ACK, >>> 100, 50000); >>> >>> if (res < 0) >>> -- >>> 2.33.0 >>> >>> >> >> Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") >> Acked-by: Marc Zyngier <maz@kernel.org> > > Hi Hector, Bjorn, > > if this is a fix we can aim at one of the upcoming -rcX. > > It would be nicer though to explain a bit better what it is > fixing in the commit log (and what's broken if we don't merge it), > as it stands it is a bit terse. I don't think anything is broken per se, it still works without this patch (probably because the refclk gets enabled fast enough that we don't have to wait for it); it's just that I think this is the correct logic. This is all reverse engineered anyway, so ultimately there is no hardware documentation to point to to say what's right and what's wrong...
On Wed, 17 Nov 2021 23:00:44 +0900, Hector Martin wrote: > REFCLK1 has req/ack bits just like REFCLK0 > > Applied to pci/apple, thanks! [1/1] PCI: apple: Fix REFCLK1 enable/poll logic https://git.kernel.org/lpieralisi/pci/c/75d36df680 Thanks, Lorenzo
diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c index b665d29af77a..420c291a5c68 100644 --- a/drivers/pci/controller/pcie-apple.c +++ b/drivers/pci/controller/pcie-apple.c @@ -42,8 +42,9 @@ #define CORE_FABRIC_STAT_MASK 0x001F001F #define CORE_LANE_CFG(port) (0x84000 + 0x4000 * (port)) #define CORE_LANE_CFG_REFCLK0REQ BIT(0) -#define CORE_LANE_CFG_REFCLK1 BIT(1) +#define CORE_LANE_CFG_REFCLK1REQ BIT(1) #define CORE_LANE_CFG_REFCLK0ACK BIT(2) +#define CORE_LANE_CFG_REFCLK1ACK BIT(3) #define CORE_LANE_CFG_REFCLKEN (BIT(9) | BIT(10)) #define CORE_LANE_CTL(port) (0x84004 + 0x4000 * (port)) #define CORE_LANE_CTL_CFGACC BIT(15) @@ -481,9 +482,9 @@ static int apple_pcie_setup_refclk(struct apple_pcie *pcie, if (res < 0) return res; - rmw_set(CORE_LANE_CFG_REFCLK1, pcie->base + CORE_LANE_CFG(port->idx)); + rmw_set(CORE_LANE_CFG_REFCLK1REQ, pcie->base + CORE_LANE_CFG(port->idx)); res = readl_relaxed_poll_timeout(pcie->base + CORE_LANE_CFG(port->idx), - stat, stat & CORE_LANE_CFG_REFCLK1, + stat, stat & CORE_LANE_CFG_REFCLK1ACK, 100, 50000); if (res < 0)
REFCLK1 has req/ack bits just like REFCLK0 Signed-off-by: Hector Martin <marcan@marcan.st> --- drivers/pci/controller/pcie-apple.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)