[v2] PCI: rcar: Remove __clk_is_enabled() call

Commit Message

Marek Vasut Oct. 10, 2021, 10:06 p.m. UTC

From: Marek Vasut <[e-mail]>

Remove __clk_is_enabled() call, otherwise the following build error occurs:
  arm-linux-gnueabi-ld: drivers/pci/controller/pcie-rcar-host.o: in function `rcar_pcie_aarch32_abort_handler':
  pcie-rcar-host.c:(.text+0xdd0): undefined reference to `__clk_is_enabled'
This should be OK, since the bus clock are always enabled in probe() of the driver.

Fixes: a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")
Signed-off-by: Marek Vasut <[e-mail]>
Cc: Arnd Bergmann <[e-mail]>
Cc: Bjorn Helgaas <[e-mail]>
Cc: Geert Uytterhoeven <[e-mail]>
Cc: Lorenzo Pieralisi <[e-mail]>
Cc: Stephen Boyd <[e-mail]>
Cc: Wolfram Sang <[e-mail]>
Cc: Yoshihiro Shimoda <[e-mail]>
Cc: [e-mail]
---
V2: Drop the __clk_is_enabled(), like it was done already in V1 of
    a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")
---
 drivers/pci/controller/pcie-rcar-host.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Comments

Geert Uytterhoeven Oct. 11, 2021, 7:29 a.m. UTC | #1

Hi Marek,

Thanks for your patch!

On Mon, Oct 11, 2021 at 12:07 AM <[e-mail]> wrote:
> From: Marek Vasut <[e-mail]>
>
> Remove __clk_is_enabled() call, otherwise the following build error occurs:
>   arm-linux-gnueabi-ld: drivers/pci/controller/pcie-rcar-host.o: in function `rcar_pcie_aarch32_abort_handler':
>   pcie-rcar-host.c:(.text+0xdd0): undefined reference to `__clk_is_enabled'
> This should be OK, since the bus clock are always enabled in probe() of the driver.

As I said in [1], the bus clock is the wrong clock to check.  The
correct clock to check is the module clock (like in the ATF version
of your patch).  Unfortunately I missed this in the review of the
original patch, sorry for that.

As I said in [2], while the bus clock is always enabled, the module
clock is under control of Runtime PM.  I'm not sure it's never
disabled (e.g. during system suspend).  So you may have to check
pm_runtime_suspended().

If you ever get an imprecise external abort for some other reason,
while the PCIe module clock is disabled, accessing the PCIe module
registers will lead to another imprecise external abort, causing
infinite recursion.

> Fixes: a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")
> Signed-off-by: Marek Vasut <[e-mail]>

> V2: Drop the __clk_is_enabled(), like it was done already in V1 of
>     a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")

I couldn't find v1, not in my mailbox, and not in lore?

[1] https://lore.kernel.org/linux-pci/[e-mail]
[2] https://lore.kernel.org/linux-pci/[e-mail]

Gr{oetje,eeting}s,

                        Geert

Patch

diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
index 8f3131844e77..e3b7bf250e36 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -51,11 +51,6 @@  struct rcar_msi {
  * the L1 link state fixup function, called from the ARM fault handler.
  */
 static void __iomem *pcie_base;
-/*
- * Static copy of bus clock pointer, so we can check whether the clock
- * is enabled or not.
- */
-static struct clk *pcie_bus_clk;
 #endif
 
 /* Structure representing the PCIe interface */
@@ -794,7 +789,6 @@  static int rcar_pcie_get_resources(struct rcar_pcie_host *host)
 #ifdef CONFIG_ARM
 	/* Cache static copy for L1 link state fixup hook on aarch32 */
 	pcie_base = pcie->base;
-	pcie_bus_clk = host->bus_clk;
 #endif
 
 	return 0;
@@ -1064,7 +1058,7 @@  static int rcar_pcie_aarch32_abort_handler(unsigned long addr,
 
 	spin_lock_irqsave(&pmsr_lock, flags);
 
-	if (!pcie_base || !__clk_is_enabled(pcie_bus_clk)) {
+	if (!pcie_base) {
 		ret = 1;
 		goto unlock_exit;
 	}