[PATCH v2 0/2] Hi655x: mfd/pmic: regulator: Correct dependencies

[PATCH v2 0/2] Hi655x: mfd/pmic: regulator: Correct dependencies

[Jeremy Linton]

In an environment where the hi655x pmic is being built as a 
standalone module, it fails to automatically load resulting
in boot failures, machine hangs, etc. First we correct this
by setting an appropriate module dependency. Then we bump the
module use count on the mfd/pmic driver so that it cannot be
unloaded when in use. This avoids an unload warning and
crashes like:

 Unable to handle kernel paging request at virtual address ffff800078fc5c78
 pgd = ffff00000a150000
 *pgd=000000007ffc0003, *pud=000000007ffc0003, *pmd=00e8000060000711
 Internal error: Oops: 8600000e [#1] SMP
 ...
 [<ffff800078fc5c78>] 0xffff800078fc5c78
 [<ffff0000086248f8>] regulator_set_voltage_sel_regmap+0x50/0x98
 [<ffff00000861e830>] _regulator_do_set_voltage+0x4c8/0x7c0
 [<ffff000008620804>] regulator_set_voltage_unlocked+0xc4/0x258
 [<ffff0000086209d4>] regulator_set_voltage+0x3c/0x70
 [<ffff000000af1b30>] mmc_regulator_set_ocr+0x48/0x100 [mmc_core]
 [<ffff000000c93da8>] dw_mci_set_ios+0x1e8/0x280 [dw_mmc]
 [<ffff000000af483c>] mmc_set_initial_state+0x84/0xf8 [mmc_core]
 [<ffff000000af490c>] mmc_power_up.part.13+0x5c/0x1f8 [mmc_core]
 [<ffff000000af5c20>] mmc_rescan+0x270/0x3b0 [mmc_core]
 [<ffff0000080f3a14>] process_one_work+0x264/0x7c0
 [<ffff0000080f3fc4>] worker_thread+0x54/0x430
 [<ffff0000080fc5dc>] kthread+0x104/0x130
 [<ffff000008083380>] ret_from_fork+0x10/0x50

V1->V2:
  Change from a module post call to a module_device_table.

Jeremy Linton (2):
  regulator: hi655x: Describe consumed platform device
  regulator: hi655x: Bump parent pmic module use count

 drivers/regulator/hi655x-regulator.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

-- 
2.10.2

[PATCH v2 1/2] regulator: hi655x: Describe consumed platform device

[Jeremy Linton]

The hi655x-regulator driver consumes a similarly named platform device.
Adding that to the module device table, allows modprobe to locate this
driver once the device is created.

Signed-off-by: Jeremy Linton <lintonrjeremy@gmail.com>
---
 drivers/regulator/hi655x-regulator.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/regulator/hi655x-regulator.c b/drivers/regulator/hi655x-regulator.c
index 065c100..36ae54b 100644
--- a/drivers/regulator/hi655x-regulator.c
+++ b/drivers/regulator/hi655x-regulator.c
@@ -214,7 +214,14 @@ static int hi655x_regulator_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id hi655x_regulator_table[] = {
+	{ .name = "hi655x-regulator" },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, hi655x_regulator_table);
+
 static struct platform_driver hi655x_regulator_driver = {
+	.id_table = hi655x_regulator_table,
 	.driver = {
 		.name	= "hi655x-regulator",
 	},
-- 
2.10.2

[PATCH v2 2/2] regulator: hi655x: Bump parent pmic module use count

[Jeremy Linton]

The hi655x-regulator driver depends on the parent pmic/mfc
device driver but doesn't increase its use count. This results
in system crashes if the parent module is unloaded while the
regulators are still in use. Add explicit module get/put
calls to keep the parent from being unloaded.

example crash:

 Unable to handle kernel paging request at virtual address ffff800078fc5c78
 ...
 [<ffff800078fc5c78>] 0xffff800078fc5c78
 [<ffff0000086248f8>] regulator_set_voltage_sel_regmap+0x50/0x98
 [<ffff00000861e830>] _regulator_do_set_voltage+0x4c8/0x7c0
 [<ffff000008620804>] regulator_set_voltage_unlocked+0xc4/0x258
 [<ffff0000086209d4>] regulator_set_voltage+0x3c/0x70
 [<ffff000000af1b30>] mmc_regulator_set_ocr+0x48/0x100 [mmc_core]
 [<ffff000000c93da8>] dw_mci_set_ios+0x1e8/0x280 [dw_mmc]
 [<ffff000000af483c>] mmc_set_initial_state+0x84/0xf8 [mmc_core]
 [<ffff000000af490c>] mmc_power_up.part.13+0x5c/0x1f8 [mmc_core]
 [<ffff000000af5c20>] mmc_rescan+0x270/0x3b0 [mmc_core]
 [<ffff0000080f3a14>] process_one_work+0x264/0x7c0
 [<ffff0000080f3fc4>] worker_thread+0x54/0x430
 [<ffff0000080fc5dc>] kthread+0x104/0x130
 [<ffff000008083380>] ret_from_fork+0x10/0x50

Signed-off-by: Jeremy Linton <lintonrjeremy@gmail.com>
---
 drivers/regulator/hi655x-regulator.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/hi655x-regulator.c b/drivers/regulator/hi655x-regulator.c
index 36ae54b..336aaed 100644
--- a/drivers/regulator/hi655x-regulator.c
+++ b/drivers/regulator/hi655x-regulator.c
@@ -185,16 +185,29 @@ static int hi655x_regulator_probe(struct platform_device *pdev)
 	struct hi655x_pmic *pmic;
 	struct regulator_config config = { };
 	struct regulator_dev *rdev;
+	struct device *parent = pdev->dev.parent;
 
-	pmic = dev_get_drvdata(pdev->dev.parent);
+	if (!parent) {
+		dev_err(&pdev->dev, "no regulator parent node\n");
+		return -ENODEV;
+	}
+
+	pmic = dev_get_drvdata(parent);
 	if (!pmic) {
 		dev_err(&pdev->dev, "no pmic in the regulator parent node\n");
 		return -ENODEV;
 	}
 
+	if (!try_module_get(parent->driver->owner)) {
+		dev_err(&pdev->dev, "unable to get parent module\n");
+		return -ENODEV;
+	}
+
 	regulator = devm_kzalloc(&pdev->dev, sizeof(*regulator), GFP_KERNEL);
-	if (!regulator)
+	if (!regulator)	{
+		module_put(parent->driver->owner);
 		return -ENOMEM;
+	}
 
 	platform_set_drvdata(pdev, regulator);
 
@@ -214,6 +227,14 @@ static int hi655x_regulator_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int hi655x_regulator_remove(struct platform_device *pdev)
+{
+	struct device *parent = pdev->dev.parent;
+
+	module_put(parent->driver->owner);
+	return 0;
+}
+
 static const struct platform_device_id hi655x_regulator_table[] = {
 	{ .name = "hi655x-regulator" },
 	{},
@@ -226,6 +247,7 @@ static struct platform_driver hi655x_regulator_driver = {
 		.name	= "hi655x-regulator",
 	},
 	.probe	= hi655x_regulator_probe,
+	.remove = hi655x_regulator_remove
 };
 module_platform_driver(hi655x_regulator_driver);
 
-- 
2.10.2

Re: [PATCH v2 2/2] regulator: hi655x: Bump parent pmic module use count

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

On Mon, Apr 03, 2017 at 12:28:43AM -0500, Jeremy Linton wrote:
> The hi655x-regulator driver depends on the parent pmic/mfc
> device driver but doesn't increase its use count. This results
> in system crashes if the parent module is unloaded while the
> regulators are still in use. Add explicit module get/put
> calls to keep the parent from being unloaded.

Once again, it is a complete hack to do this at the individual driver
level.  Please don't ignore review feedback.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Applied "regulator: hi655x: Describe consumed platform device" to the regulator tree

[Mark Brown]

The patch

   regulator: hi655x: Describe consumed platform device

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 2f3c578fe2edf1de7ab981d1d7121e2eeee5b466 Mon Sep 17 00:00:00 2001
From: Jeremy Linton <lintonrjeremy@gmail.com>
Date: Mon, 3 Apr 2017 00:28:42 -0500
Subject: [PATCH] regulator: hi655x: Describe consumed platform device

The hi655x-regulator driver consumes a similarly named platform device.
Adding that to the module device table, allows modprobe to locate this
driver once the device is created.

Signed-off-by: Jeremy Linton <lintonrjeremy@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/hi655x-regulator.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/regulator/hi655x-regulator.c b/drivers/regulator/hi655x-regulator.c
index aca18466f522..11a6d27ec82f 100644
--- a/drivers/regulator/hi655x-regulator.c
+++ b/drivers/regulator/hi655x-regulator.c
@@ -214,7 +214,14 @@ static int hi655x_regulator_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id hi655x_regulator_table[] = {
+	{ .name = "hi655x-regulator" },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, hi655x_regulator_table);
+
 static struct platform_driver hi655x_regulator_driver = {
+	.id_table = hi655x_regulator_table,
 	.driver = {
 		.name	= "hi655x-regulator",
 	},
-- 
2.11.0