linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled()
@ 2021-11-07 19:10 marek.vasut
  2021-11-07 20:11 ` Krzysztof Wilczyński
  2021-11-08  9:53 ` Geert Uytterhoeven
  0 siblings, 2 replies; 7+ messages in thread
From: marek.vasut @ 2021-11-07 19:10 UTC (permalink / raw)
  To: linux-pci
  Cc: Marek Vasut, Arnd Bergmann, Bjorn Helgaas, Geert Uytterhoeven,
	Lorenzo Pieralisi, Stephen Boyd, Wolfram Sang, Yoshihiro Shimoda,
	linux-renesas-soc

From: Marek Vasut <marek.vasut+renesas@gmail.com>

Replace __clk_is_enabled() with pm_runtime_suspended(),
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'

Fixes: a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-renesas-soc@vger.kernel.org
---
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")
V3: Use pm_runtime_suspended() instead of __clk_is_enabled()
---
 drivers/pci/controller/pcie-rcar-host.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
index e12c2d8be05a..138592e22600 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -50,10 +50,10 @@ struct rcar_msi {
  */
 static void __iomem *pcie_base;
 /*
- * Static copy of bus clock pointer, so we can check whether the clock
- * is enabled or not.
+ * Static copy of pcie device pointer, so we can check whether the
+ * device is runtime suspended or not.
  */
-static struct clk *pcie_bus_clk;
+static struct device *pcie_dev;
 #endif
 
 /* Structure representing the PCIe interface */
@@ -792,7 +792,7 @@ 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;
+	pcie_dev = pcie->dev;
 #endif
 
 	return 0;
@@ -1062,7 +1062,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 || pm_runtime_suspended(pcie_dev)) {
 		ret = 1;
 		goto unlock_exit;
 	}
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled()
  2021-11-07 19:10 [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled() marek.vasut
@ 2021-11-07 20:11 ` Krzysztof Wilczyński
  2021-11-07 21:01   ` Randy Dunlap
  2021-11-08  9:53 ` Geert Uytterhoeven
  1 sibling, 1 reply; 7+ messages in thread
From: Krzysztof Wilczyński @ 2021-11-07 20:11 UTC (permalink / raw)
  To: marek.vasut
  Cc: linux-pci, Marek Vasut, Arnd Bergmann, Bjorn Helgaas,
	Geert Uytterhoeven, Lorenzo Pieralisi, Stephen Boyd,
	Wolfram Sang, Yoshihiro Shimoda, linux-renesas-soc, Randy Dunlap

[+CC Randy as he sent a patch to fix the same thing]

Hi Marek,

> Replace __clk_is_enabled() with pm_runtime_suspended(),
> 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'
> 
> Fixes: a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")

Randy sent a patch which aims to fix the same issue, albeit in a slightly
different way.  I wonder if it would make sense for the two of you to look
at which approach should we pick, or even whether we should combine the
two into one patch?

  https://lore.kernel.org/linux-pci/20211107013703.19995-1-rdunlap@infradead.org/

	Krzysztof

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled()
  2021-11-07 20:11 ` Krzysztof Wilczyński
@ 2021-11-07 21:01   ` Randy Dunlap
  2021-11-07 22:33     ` Krzysztof Wilczyński
  0 siblings, 1 reply; 7+ messages in thread
From: Randy Dunlap @ 2021-11-07 21:01 UTC (permalink / raw)
  To: Krzysztof Wilczyński, marek.vasut
  Cc: linux-pci, Marek Vasut, Arnd Bergmann, Bjorn Helgaas,
	Geert Uytterhoeven, Lorenzo Pieralisi, Stephen Boyd,
	Wolfram Sang, Yoshihiro Shimoda, linux-renesas-soc

On 11/7/21 12:11 PM, Krzysztof Wilczyński wrote:
> [+CC Randy as he sent a patch to fix the same thing]
> 
> Hi Marek,
> 
>> Replace __clk_is_enabled() with pm_runtime_suspended(),
>> 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'
>>
>> Fixes: a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")
> 
> Randy sent a patch which aims to fix the same issue, albeit in a slightly
> different way.  I wonder if it would make sense for the two of you to look
> at which approach should we pick, or even whether we should combine the
> two into one patch?
> 
>    https://lore.kernel.org/linux-pci/20211107013703.19995-1-rdunlap@infradead.org/

Hi,
I saw Marek's patch a couple of hours ago.

As long as pm_runtime_suspended() is always available,
either live or as a stub (and it looks like it is),
I don't see any reason for my patch at all.

thanks.
-- 
~Randy

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled()
  2021-11-07 21:01   ` Randy Dunlap
@ 2021-11-07 22:33     ` Krzysztof Wilczyński
  2021-11-07 22:34       ` Randy Dunlap
  0 siblings, 1 reply; 7+ messages in thread
From: Krzysztof Wilczyński @ 2021-11-07 22:33 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: marek.vasut, linux-pci, Marek Vasut, Arnd Bergmann,
	Bjorn Helgaas, Geert Uytterhoeven, Lorenzo Pieralisi,
	Stephen Boyd, Wolfram Sang, Yoshihiro Shimoda, linux-renesas-soc

Hi Randy,

[...]
> > Randy sent a patch which aims to fix the same issue, albeit in a slightly
> > different way.  I wonder if it would make sense for the two of you to look
> > at which approach should we pick, or even whether we should combine the
> > two into one patch?
> > 
> >    https://lore.kernel.org/linux-pci/20211107013703.19995-1-rdunlap@infradead.org/
> 
> Hi,
> I saw Marek's patch a couple of hours ago.
> 
> As long as pm_runtime_suspended() is always available,
> either live or as a stub (and it looks like it is),
> I don't see any reason for my patch at all.

Understood!  I marked it accordingly in Patchwok.  Thank you both!

By the way, can I count on your Acked-by or Reviewed-by here for Marek?  If
you agree with the premise of the changes here, etc., of course.

	Krzysztof

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled()
  2021-11-07 22:33     ` Krzysztof Wilczyński
@ 2021-11-07 22:34       ` Randy Dunlap
  0 siblings, 0 replies; 7+ messages in thread
From: Randy Dunlap @ 2021-11-07 22:34 UTC (permalink / raw)
  To: Krzysztof Wilczyński
  Cc: marek.vasut, linux-pci, Marek Vasut, Arnd Bergmann,
	Bjorn Helgaas, Geert Uytterhoeven, Lorenzo Pieralisi,
	Stephen Boyd, Wolfram Sang, Yoshihiro Shimoda, linux-renesas-soc

On 11/7/21 2:33 PM, Krzysztof Wilczyński wrote:
> Hi Randy,
> 
> [...]
>>> Randy sent a patch which aims to fix the same issue, albeit in a slightly
>>> different way.  I wonder if it would make sense for the two of you to look
>>> at which approach should we pick, or even whether we should combine the
>>> two into one patch?
>>>
>>>     https://lore.kernel.org/linux-pci/20211107013703.19995-1-rdunlap@infradead.org/
>>
>> Hi,
>> I saw Marek's patch a couple of hours ago.
>>
>> As long as pm_runtime_suspended() is always available,
>> either live or as a stub (and it looks like it is),
>> I don't see any reason for my patch at all.
> 
> Understood!  I marked it accordingly in Patchwok.  Thank you both!
> 
> By the way, can I count on your Acked-by or Reviewed-by here for Marek?  If
> you agree with the premise of the changes here, etc., of course.

Sure.
Acked-by: Randy Dunlap <rdunlap@infradead.org>

thanks.
-- 
~Randy

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled()
  2021-11-07 19:10 [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled() marek.vasut
  2021-11-07 20:11 ` Krzysztof Wilczyński
@ 2021-11-08  9:53 ` Geert Uytterhoeven
  2021-11-08 11:14   ` Krzysztof Wilczyński
  1 sibling, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2021-11-08  9:53 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-pci, Marek Vasut, Arnd Bergmann, Bjorn Helgaas,
	Geert Uytterhoeven, Lorenzo Pieralisi, Stephen Boyd,
	Wolfram Sang, Yoshihiro Shimoda, Linux-Renesas

Hi Marek,

Thanks for your patch!

On Sun, Nov 7, 2021 at 8:11 PM <marek.vasut@gmail.com> wrote:
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> Replace __clk_is_enabled() with pm_runtime_suspended(),
> 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'

While this describes what is done and why, it misses one important
semantic change: the old __clk_is_enabled() actually checked the wrong
clock (bus clock instead of module clock), while pm_runtime_suspended()
reflects (a.o.) the state of the module clock.

>
> Fixes: a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled()
  2021-11-08  9:53 ` Geert Uytterhoeven
@ 2021-11-08 11:14   ` Krzysztof Wilczyński
  0 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Wilczyński @ 2021-11-08 11:14 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Vasut, linux-pci, Marek Vasut, Arnd Bergmann,
	Bjorn Helgaas, Geert Uytterhoeven, Lorenzo Pieralisi,
	Stephen Boyd, Wolfram Sang, Yoshihiro Shimoda, Linux-Renesas,
	Sasha Levin

[+CC Adding Sasha for visibiity]

Hi Geert,

[...]
> > Replace __clk_is_enabled() with pm_runtime_suspended(),
> > 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'
> 
> While this describes what is done and why, it misses one important
> semantic change: the old __clk_is_enabled() actually checked the wrong
> clock (bus clock instead of module clock), while pm_runtime_suspended()
> reflects (a.o.) the state of the module clock.

This looks like it was a latent bug then, something that we might need to
port back to the stable and long-term kernels, I believe.

Thank you, Geert!

	Krzysztof

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-11-08 11:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-07 19:10 [PATCH v3] PCI: rcar: Check if device is runtime suspended instead of __clk_is_enabled() marek.vasut
2021-11-07 20:11 ` Krzysztof Wilczyński
2021-11-07 21:01   ` Randy Dunlap
2021-11-07 22:33     ` Krzysztof Wilczyński
2021-11-07 22:34       ` Randy Dunlap
2021-11-08  9:53 ` Geert Uytterhoeven
2021-11-08 11:14   ` Krzysztof Wilczyński

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).