ARM: mvebu: fix PCIe/mvebu-soc-id issue

The mvebu-soc-id code in mach-mvebu/ needs to enable a clock to read the SoC device ID and revision number. To do so, it does a clk_get(), then a clk_prepare_enable(), reads the value, and disables the clock with clk_disable_unprepare(). However, it forgets to clk_put() the clock. This commit fixes this issue.

Signed-off-by Thomas Petazzoni <[email protected]>

Cc <[email protected]> # 3.14+

Fixes af8d1c63afcbf36eea06789c92e22d4af118d2fb ('ARM: mvebu: Add support to get the ID and the revision of a SoC')

---
 arch/arm/mach-mvebu/mvebu-soc-id.c | 1 +
 1 file changed, 1 insertion(+)

arch/arm/mach-mvebu/mvebu-soc-id.c +1 -0

index f3d4cf5..b52af6f 100644
@@ -109,6 +109,7 @@ static int __init mvebu_soc_id_init(void)
 
 res_ioremap:
 	clk_disable_unprepare(clk);
+	clk_put(clk);
 
 clk_err:
 	of_node_put(child);

Since the mvebu-soc-id code in mach-mvebu/ was introduced, several users have noticed a regression: the PCIe card connected in the first PCIe interface is not detected properly.

This is due to the fact that the mvebu-soc-id code enables the PCIe clock of the first PCIe interface, reads the SoC device ID and revision number (yes this information is made available as part of PCIe registers), and then disables the clock. However, by doing this, we gate the clock and therefore loose the complex PCIe configuration that was done by the bootloader.

Unfortunately, as of today, the kernel is not capable of doing this complex configuration by itself, so we really need to keep the PCIe clock enabled. However, we don't want to keep it enabled
unconditionally: if the PCIe interface is not enabled or PCI support is not compiled into the kernel, there is no reason to keep the PCIe clock running.

This issue was discussed with Kevin Hilman, and the suggested solution was to make the mvebu-soc-id code keep the clock enabled in case it will be needed for PCIe. This is therefore the solution implemented in this patch.

Long term, we hope to make the kernel more capable in terms of PCIe configuration for this platform, which will anyway be needed to support the compilation of the PCIe host controller driver as a module. In the mean time however, we don't have much other choice than to implement the currently proposed solution.

Reported-by Neil Greatorex <[email protected]>

Cc Neil Greatorex <[email protected]>

Cc Jason Gunthorpe <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Thomas Petazzoni <[email protected]>

Cc <[email protected]> # 3.14+

Fixes af8d1c63afcbf36eea06789c92e22d4af118d2fb ('ARM: mvebu: Add support to get the ID and the revision of a SoC')

---
 arch/arm/mach-mvebu/mvebu-soc-id.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

arch/arm/mach-mvebu/mvebu-soc-id.c +12 -2

index b52af6f..09520e1 100644
@@ -108,8 +108,18 @@ static int __init mvebu_soc_id_init(void)
 	iounmap(pci_base);
 
 res_ioremap:
-	clk_disable_unprepare(clk);
-	clk_put(clk);
+	/*
+	 * If the PCIe unit is actually enabled and we have PCI
+	 * support in the kernel, we intentionally do not release the
+	 * reference to the clock. We want to keep it running since
+	 * the bootloader does some PCIe link configuration that the
+	 * kernel is for now unable to do, and gating the clock would
+	 * make us loose this precious configuration.
+	 */
+	if (!of_device_is_available(child) || !IS_ENABLED(CONFIG_PCI_MVEBU)) {
+		clk_disable_unprepare(clk);
+		clk_put(clk);
+	}
 
 clk_err:
 	of_node_put(child);

This issue was discussed with Kevin Hilman, and the suggested solution was to make the mvebu-soc-id code keep the clock enabled in case it will be needed for PCIe. This is therefore the solution implemented in this patch.

Hello,

This set of two patches (and more specifically the second patch) fix a PCIe detection problem caused by the introduction of the mvebu-soc-id and a bad interaction between the clock manipulation done by the mvebu-soc-id code and the PCIe driver. The details are all in the commit log of the second patch.

This second patch fixes a PCIe detection problem noticed by several persons.

Hello,

This set of two patches (and more specifically the second patch) fix a PCIe detection problem caused by the introduction of the mvebu-soc-id and a bad interaction between the clock manipulation done by the mvebu-soc-id code and the PCIe driver. The details are all in the commit log of the second patch.

This second patch fixes a PCIe detection problem noticed by several persons. The first patch is just a related preparation patch.

Both patches are meant for 3.15-rc, and should be backported to 3.14 as part of the stable process.

Hello,

This set of two patches (and more specifically the second patch) fix a PCIe detection problem caused by the introduction of the mvebu-soc-id and a bad interaction between the clock manipulation done by the mvebu-soc-id code and the PCIe driver. The details are all in the commit log of the second patch.

This second patch fixes a PCIe detection problem noticed by several persons. The first patch is just a related preparation patch.

Both patches are meant for 3.15-rc, and should be backported to 3.14 as part of the stable process.

Best regards,

Thomas

Thomas Petazzoni (2):
ARM: mvebu: mvebu-soc-id: add missing clk_put() call
ARM: mvebu: mvebu-soc-id: keep clock enabled if PCIe unit is enabled

 arch/arm/mach-mvebu/mvebu-soc-id.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Series applied to mvebu/fixes with Gregory's Ack, and Tested-by's from Gregory, Andrew, and Willy.

Also, as Rob noted, if you are going to add 'Fixes' tags, please do so in the following format:

Fixes af8d1c63afcb ("ARM: mvebu: Add support to get the ID and the revision of a SoC")

Note that it uses double quotes, and only 12 characters for the hash.

Also, the second patch depends on the first, so I've amended the tags as such for the second patch:

Fixes af8d1c63afcb ("ARM: mvebu: Add support to get the ID and the revision of a SoC")

Cc <[email protected]> # 3.14+: 42a18d1cf484: ARM: mvebu: mvebu-soc-id: add missing clk_put() call

Cc <[email protected]> # 3.14+

Keep in mind, I don't require any of this. I /ask/ for the commit hash of the commit introducing the regression, just to make my job easier. If you are going to put it in the Fixes format, great.

As for the stable tags, you can't very well add the dependency info since you don't have the commit hash of the first patch yet. :)

Thanks for all the effort diagnosing this problem and finding the fix!