Skip to contentSkip to conversation

soc: mediatek: SCPSYS: use module_platform_driver

PATCH New Sascha Hauer 5 participants 6 messages Dec 16, 2015 – Dec 18, 2015 Arm MediaTek

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 soc: mediatek: SCPSYS: use module_platform_driver

With regulator support to the scpsys driver needs to be able to defer probe. Probe deferral is incompatible with module_platform_driver_probe, so use module_platform_driver. module_platform_driver_probe sets the suppress_bind_attrs. To prevent unbinding the device with module_platform_driver we have to set the flag explicitly in the driver now.

---

Changes since v1:
- Add suppress_bind_attrs attribute

 drivers/soc/mediatek/mtk-scpsys.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/soc/mediatek/mtk-scpsys.c +3 -1
index e425619..ae2d177 100644
@@ -542,11 +542,13 @@ static const struct of_device_id of_scpsys_match_tbl[] = {
 };
 
 static struct platform_driver scpsys_drv = {
+	.probe = scpsys_probe,
 	.driver = {
 		.name = "mtk-scpsys",
+		.suppress_bind_attrs = true,
 		.owner = THIS_MODULE,
 		.of_match_table = of_match_ptr(of_scpsys_match_tbl),
 	},
 };
 
-module_platform_driver_probe(scpsys_drv, scpsys_probe);
+module_platform_driver(scpsys_drv);
Signature · 1 line
2.6.2
#1 lore raw

PATCH v2 1/1 soc: mediatek: SCPSYS: use module_platform_driver

Quoted 10 lines · On Wednesday 16 December 2015 12:00:56 Sascha Hauer wrote:

With regulator support to the scpsys driver needs to be able to defer probe. Probe deferral is incompatible with module_platform_driver_probe, so use module_platform_driver. module_platform_driver_probe sets the suppress_bind_attrs. To prevent unbinding the device with module_platform_driver we have to set the flag explicitly in the driver now.

---

Looks good now,

maintainer Applied
#2 lore raw

PATCH v2 1/1 soc: mediatek: SCPSYS: use module_platform_driver

Quoted 8 lines · Arnd Bergmann
Quoted 7 lines · On Wednesday 16 December 2015 12:00:56 Sascha Hauer wrote:

With regulator support to the scpsys driver needs to be able to defer probe. Probe deferral is incompatible with module_platform_driver_probe, so use module_platform_driver. module_platform_driver_probe sets the suppress_bind_attrs. To prevent unbinding the device with module_platform_driver we have to set the flag explicitly in the driver now.

Quoted 8 lines
Quoted 3 lines

---

Looks good now,

Applied to v4.4-next/soc

Sascha next time please don't forget to send Mediatek related patches to the corresponding mailinglist as well.

Thanks,
Matthias

#3 lore raw

PATCH v2 1/1 soc: mediatek: SCPSYS: use module_platform_driver

Hi Sasha, Matthias,

Quoted 17 lines · Matthias Brugger
Quoted 14 lines · Arnd Bergmann
Quoted 11 lines · On Wednesday 16 December 2015 12:00:56 Sascha Hauer wrote:

With regulator support to the scpsys driver needs to be able to defer probe. Probe deferral is incompatible with module_platform_driver_probe, so use module_platform_driver. module_platform_driver_probe sets the suppress_bind_attrs. To prevent unbinding the device with module_platform_driver we have to set the flag explicitly in the driver now.

---

Quoted 6 lines
Quoted 4 lines

Looks good now,

Applied to v4.4-next/soc

This patch does not compile on my system.

With "CONFIG_DEBUG_SECTION_MISMATCH=y" I see:

WARNING: vmlinux.o(.data+0x387c8): Section mismatch in reference from the variable scpsys_drv to the function .init.text:scpsys_probe() The variable scpsys_drv references
the function __init scpsys_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console

FATAL: modpost: Section mismatches detected. Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.

The problem is that scpsys_probe() is still __init, but it references scp_domain_data[] which is __initconst.
However, scpsys_drv is not __initdata, so it cannot contain a pointer to an __init function.

The only way I know to fix this is by dropping the __init & __initconst. This is rather unfortunate.
Is there another way where we can use -EPROBE_DEFER and still preserve __init / __initconst ?

Also, since scpsys' Kconfig is "bool" not "tristate" it isn't a real module so I think we should use:
builtin_platform_driver not module_platform_driver

CC'ing Paul Gortmaker who seems to be on a mass builtin_platform_driver conversion at the moment, see:
https://lkml.org/lkml/2015/12/9/952

-Dan

#4 lore raw

PATCH v2 1/1 soc: mediatek: SCPSYS: use module_platform_driver

[Re: [PATCH v2] soc: mediatek: SCPSYS: use module_platform_driver] On 17/12/2015 (Thu 10:52) Daniel Kurtz wrote:

[...]

Quoted 9 lines

Also, since scpsys' Kconfig is "bool" not "tristate" it isn't a real module so I think we should use:
builtin_platform_driver not module_platform_driver

CC'ing Paul Gortmaker who seems to be on a mass builtin_platform_driver conversion at the moment, see:
https://lkml.org/lkml/2015/12/9/952

Good guess. In my giant queue I have that change. I just haven't got around to the requisite triple check and multi arch rebuild tests and put out a series for soc yet. Here is the short list for soc:

drivers-soc-make-mediatek-mtk-scpsys.c-explicitly-no.patch drivers-soc-make-qcom-spm.c-explicitly-non-modular.patch drivers-soc-make-samsung-exynos-srom.c.c-explicitly-.patch drivers-soc-make-sunxi_sram.c-explicitly-non-modular.patch drivers-soc-tegra-make-fuse-tegra.c-explicitly-non-m.patch

...and here is the patch you predicted. If someone wants to fold the change into their work then great; I'll be happy to drop mine and be with one less patch to herd along.

Paul.
--

From eb32cc2c1db8192c08dd64af69cb172b3be4c98b Mon Sep 17 00:00:00 2001
From: Paul Gortmaker <[email protected]>
Date: Fri, 4 Sep 2015 19:33:54 -0400
Subject: [PATCH] drivers/soc: make mediatek/mtk-scpsys.c explicitly
non-modular

The Kconfig currently controlling compilation of this code is:

drivers/soc/mediatek/Kconfig:config MTK_SCPSYS
drivers/soc/mediatek/Kconfig: bool "MediaTek SCPSYS Support"

...meaning that it currently is not being built as a module by anyone.

Lets remove the couple traces of modularity so that when reading the driver there is no doubt it is builtin-only.

Since module_platform_driver() uses the same init level priority as builtin_platform_driver() the init ordering remains unchanged with this commit.

Cc Matthias Brugger <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Daniel Kurtz <[email protected]>
Cc Paul Gortmaker <[email protected]>
Cc Olof Johansson <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
Cc linux-mediatek at lists.infradead.org
drivers/soc/mediatek/mtk-scpsys.c +2 -3
index 164a7d8439b1..ae0ca34e64dd 100644
@@ -15,7 +15,7 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
@@ -484,5 +484,4 @@ static struct platform_driver scpsys_drv = {
 		.of_match_table = of_match_ptr(of_scpsys_match_tbl),
 	},
 };
-
-module_platform_driver_probe(scpsys_drv, scpsys_probe);
+builtin_platform_driver_probe(scpsys_drv, scpsys_probe);
Signature · 1 line
2.5.0
maintainer
#5 lore raw

PATCH v2 1/1 soc: mediatek: SCPSYS: use module_platform_driver

Hi Paul,

Quoted 24 lines · Paul Gortmaker

[Re: [PATCH v2] soc: mediatek: SCPSYS: use module_platform_driver] On 17/12/2015 (Thu 10:52) Daniel Kurtz wrote:

[...]

Quoted 9 lines

Also, since scpsys' Kconfig is "bool" not "tristate" it isn't a real module so I think we should use:
builtin_platform_driver not module_platform_driver

CC'ing Paul Gortmaker who seems to be on a mass builtin_platform_driver conversion at the moment, see:
https://lkml.org/lkml/2015/12/9/952

Good guess. In my giant queue I have that change. I just haven't got around to the requisite triple check and multi arch rebuild tests and put out a series for soc yet. Here is the short list for soc:

drivers-soc-make-mediatek-mtk-scpsys.c-explicitly-no.patch drivers-soc-make-qcom-spm.c-explicitly-non-modular.patch drivers-soc-make-samsung-exynos-srom.c.c-explicitly-.patch drivers-soc-make-sunxi_sram.c-explicitly-non-modular.patch drivers-soc-tegra-make-fuse-tegra.c-explicitly-non-m.patch

Quoted 4 lines

...and here is the patch you predicted. If someone wants to fold the change into their work then great; I'll be happy to drop mine and be with one less patch to herd along.

I take this through my tree.

Thanks a lot.
Matthias

Quoted 53 lines

Paul.
--

From eb32cc2c1db8192c08dd64af69cb172b3be4c98b Mon Sep 17 00:00:00 2001
From: Paul Gortmaker <[email protected]>
Date: Fri, 4 Sep 2015 19:33:54 -0400
Subject: [PATCH] drivers/soc: make mediatek/mtk-scpsys.c explicitly
non-modular

The Kconfig currently controlling compilation of this code is:

drivers/soc/mediatek/Kconfig:config MTK_SCPSYS
drivers/soc/mediatek/Kconfig: bool "MediaTek SCPSYS Support"

...meaning that it currently is not being built as a module by anyone.

Lets remove the couple traces of modularity so that when reading the driver there is no doubt it is builtin-only.

Since module_platform_driver() uses the same init level priority as builtin_platform_driver() the init ordering remains unchanged with this commit.

Cc Matthias Brugger <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Daniel Kurtz <[email protected]>
Cc Paul Gortmaker <[email protected]>
Cc Olof Johansson <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
Cc linux-mediatek at lists.infradead.org
drivers/soc/mediatek/mtk-scpsys.c +2 -3
index 164a7d8439b1..ae0ca34e64dd 100644
@@ -15,7 +15,7 @@
  #include <linux/io.h>
  #include <linux/kernel.h>
  #include <linux/mfd/syscon.h>
-#include <linux/module.h>
+#include <linux/init.h>
  #include <linux/of_device.h>
  #include <linux/platform_device.h>
  #include <linux/pm_domain.h>
@@ -484,5 +484,4 @@ static struct platform_driver scpsys_drv = {
  		.of_match_table = of_match_ptr(of_scpsys_match_tbl),
  	},
  };
-
-module_platform_driver_probe(scpsys_drv, scpsys_probe);
+builtin_platform_driver_probe(scpsys_drv, scpsys_probe);