[PATCH] regulator: hi655x: Add DT bindings so module autoloads

[PATCH] regulator: hi655x: Add DT bindings so module autoloads

[Jeremy Linton]

The hi655x driver is required for mmc/sd functionality on the
96boards hikey, and likely other platforms. When built as
a standalone module it doesn't get automatically loaded because
it is missing the module probe hooks.

Adding that boilerplate so it gets demand loaded.

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 aca1846..924ed60 100644
--- a/drivers/regulator/hi655x-regulator.c
+++ b/drivers/regulator/hi655x-regulator.c
@@ -214,9 +214,16 @@ static int hi655x_regulator_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id hi655x_dt_ids[] = {
+	{ .compatible = "hisilicon,hi655x-pmic",  },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hi655x_dt_ids);
+
 static struct platform_driver hi655x_regulator_driver = {
 	.driver = {
 		.name	= "hi655x-regulator",
+		.of_match_table = hi655x_dt_ids,
 	},
 	.probe	= hi655x_regulator_probe,
 };
-- 
2.10.2

Re: [PATCH] regulator: hi655x: Add DT bindings so module autoloads

[Mark Brown]

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

On Tue, Feb 28, 2017 at 07:07:06PM -0600, Jeremy Linton wrote:
> The hi655x driver is required for mmc/sd functionality on the
> 96boards hikey, and likely other platforms. When built as
> a standalone module it doesn't get automatically loaded because
> it is missing the module probe hooks.

> +static const struct of_device_id hi655x_dt_ids[] = {
> +	{ .compatible = "hisilicon,hi655x-pmic",  },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, hi655x_dt_ids);

This is an MFD subdevice, I'd expect it to instantiate as a platform
device from the MFD - there shouldn't be a compatible string in the DT
and indeed one is not documented (I see something slipped into the
example but it's not in the actual binding).  It's not like this device
can exist independently of the parent device and that compatible string
is not specific enough anyway.

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

Re: [PATCH] regulator: hi655x: Add DT bindings so module autoloads

[Jeremy Linton]

Hi,

On Wed, Mar 1, 2017 at 5:37 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Feb 28, 2017 at 07:07:06PM -0600, Jeremy Linton wrote:
>> The hi655x driver is required for mmc/sd functionality on the
>> 96boards hikey, and likely other platforms. When built as
>> a standalone module it doesn't get automatically loaded because
>> it is missing the module probe hooks.
>
>> +static const struct of_device_id hi655x_dt_ids[] = {
>> +     { .compatible = "hisilicon,hi655x-pmic",  },
>> +     {},
>> +};
>> +MODULE_DEVICE_TABLE(of, hi655x_dt_ids);
>
> This is an MFD subdevice, I'd expect it to instantiate as a platform
> device from the MFD - there shouldn't be a compatible string in the DT
> and indeed one is not documented (I see something slipped into the
> example but it's not in the actual binding).  It's not like this device
> can exist independently of the parent device and that compatible string
> is not specific enough anyway.

Right, there also appears to be dependency issues. Its like the
regulator should be pulling in the parent pmic rather than the current
situation where the pmic loads, the mmc subsystem loads, but then
triggers hung task timeouts until the regulator is manually loaded.
Worse, because the pmic isn't being held by the regulator you get this
if the pmic is unloaded.

[31695.158053] ------------[ cut here ]------------
[31695.162713] WARNING: CPU: 4 PID: 1729 at
drivers/regulator/core.c:4136 regulator_unregister+0xfc/0x104
[31695.172042] Modules linked in: xt_CHECKSUM ipt_MASQUERADE
nf_nat_masquerade_ipv4 tun ip6t_rpfilter ip6t_REJECT nf_reject_ipv6
xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp
llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6
ip6table_mangle ip6table_raw ip6table_security iptable_nat
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle iptable_raw iptable_security ebtable_filter ebtables
ip6table_filter ip6_tables vfat fat kirin_drm drm_kms_helper
fb_sys_fops syscopyarea crc32_arm64 crc32_ce crct10dif_ce ghash_ce
dw_drm_dsi sysfillrect drm mmc_block sysimgblt hisi_thermal
hi6220_mailbox i2c_designware_platform i2c_designware_core
hi655x_pmic(-) cpufreq_dt nfsd auth_rpcgss nfs_acl lockd grace sunrpc
xfs libcrc32c uas usb_storage r8152 mii dw_mmc_k3
[31695.243511]  dw_mmc_pltfm phy_hi6220_usb dwc2 dw_mmc mmc_core
udc_core hi655x_regulator
[31695.251544]
[31695.253040] CPU: 4 PID: 1729 Comm: rmmod Tainted: P
4.10.0-rc8JL64k+ #10
[31695.261069] Hardware name: HiKey Development Board (DT)
[31695.266305] task: ffff80002449ee00 task.stack: ffff800024504000
[31695.272243] PC is at regulator_unregister+0xfc/0x104
[31695.277220] LR is at regulator_unregister+0x80/0x104
[31695.282196] pc : [<ffff0000085fea28>] lr : [<ffff0000085fe9ac>]
pstate: 60000145
[31695.289611] sp : ffff800024507a60
[31695.292932] x29: ffff800024507a60 x28: 0000000000000000
[31695.298259] x27: ffff000008663948 x26: ffff80006ecc7c18
[31695.303586] x25: ffff00000a07b458 x24: 000000000000000b
[31695.308913] x23: ffff80006ecc7810 x22: ffff800024507b20
[31695.314239] x21: ffff0000090a8000 x20: ffff80006e268480
[31695.319565] x19: ffff80006ecca000 x18: ffff8000753afd40
[31695.324891] x17: ffff000008a87188 x16: 000000000000000e
[31695.330217] x15: 0000000000000007 x14: ffff80007fe4fcfd
[31695.335543] x13: 0000000000000000 x12: 0000000000000020
[31695.340870] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
[31695.346197] x9 : ffff0000089f9c00 x8 : 0000000000000000
[31695.351523] x7 : ffff0000080f1d98 x6 : 0000000000000000
[31695.356849] x5 : 0000000000000000 x4 : 0000000000000000
[31695.362175] x3 : ffff80002449ee00 x2 : 000000007fffffff
[31695.367501] x1 : 0000000000000000 x0 : 0000000000000001
[31695.372827]
[31695.374320] ---[ end trace 77e1c94118fa8ce0 ]---
[31695.378947] Call trace:
[31695.381402] Exception stack(0xffff800024507880 to 0xffff8000245079b0)
[31695.387858] 7880: ffff80006ecca000 0001000000000000
ffff800024507a60 ffff0000085fea28
[31695.395710] 78a0: 0000000060000145 000000000000003d
ffff8000245078e0 ffff00000813aa74
[31695.403564] 78c0: ffff00000903d6a8 ffff00000903d718
ffff00000903d708 ffff0000089a4a8c
[31695.411417] 78e0: ffff800000000000 ffff000000000000
ffff800024507970 ffff0000080f1dc0
[31695.419270] 7900: ffff800024507970 ffff0000080f1dd4
ffff800024507940 ffff0000081041e4
[31695.427123] 7920: ffff000008c30760 0000000000000ae7
0000000000000001 0000000000000000
[31695.434976] 7940: 000000007fffffff ffff80002449ee00
0000000000000000 0000000000000000
[31695.442830] 7960: 0000000000000000 ffff0000080f1d98
0000000000000000 ffff0000089f9c00
[31695.450683] 7980: 7f7f7f7f7f7f7f7f 0101010101010101
0000000000000020 0000000000000000
[31695.458535] 79a0: ffff80007fe4fcfd 0000000000000007
[31695.463426] [<ffff0000085fea28>] regulator_unregister+0xfc/0x104
[31695.469447] [<ffff0000085ffa8c>] devm_rdev_release+0x20/0x28
[31695.475122] [<ffff000008664160>] release_nodes+0x1a4/0x2c8
[31695.480619] [<ffff000008664df4>] devres_release_all+0x3c/0x54
[31695.486381] [<ffff00000865f7f4>] device_release_driver_internal+0x16c/0x1d4
[31695.493359] [<ffff00000865f884>] device_release_driver+0x28/0x34
[31695.499380] [<ffff00000865e140>] bus_remove_device+0xf4/0x15c
[31695.505138] [<ffff00000865a0ec>] device_del+0x1d0/0x2e0
[31695.510377] [<ffff000008661ed0>] platform_device_del+0x30/0x9c
[31695.516224] [<ffff000008661f5c>] platform_device_unregister+0x20/0x38
[31695.522682] [<ffff000008696748>] mfd_remove_devices_fn+0x80/0x94
[31695.528703] [<ffff0000086591d8>] device_for_each_child_reverse+0x50/0x80
[31695.535420] [<ffff000008695df0>] mfd_remove_devices+0x30/0x44
[31695.541186] [<ffff000001960038>] hi655x_pmic_remove+0x38/0x48 [hi655x_pmic]
[31695.548165] [<ffff00000866200c>] platform_drv_remove+0x2c/0x6c
[31695.554013] [<ffff00000865f7e4>] device_release_driver_internal+0x15c/0x1d4
[31695.560990] [<ffff00000865f8d8>] driver_detach+0x48/0x84
[31695.566314] [<ffff00000865e4b8>] bus_remove_driver+0x74/0xe8
[31695.571986] [<ffff000008660870>] driver_unregister+0x34/0x54
[31695.577660] [<ffff000008662130>] platform_driver_unregister+0x20/0x2c
[31695.584117] [<ffff00000196031c>]
hi655x_pmic_driver_exit+0x14/0xfcf8 [hi655x_pmic]
[31695.591712] [<ffff00000818b370>] SyS_delete_module+0x204/0x268
[31695.597562] [<ffff0000080833f0>] el0_svc_naked+0x24/0x28
[31695.609063] ------------[ cut here ]------------

Re: [PATCH] regulator: hi655x: Add DT bindings so module autoloads

[Mark Brown]

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

On Wed, Mar 01, 2017 at 10:44:16AM -0600, Jeremy Linton wrote:

> Right, there also appears to be dependency issues. Its like the
> regulator should be pulling in the parent pmic rather than the current

No, why would it do that?  The regulator can't be instantiated without
the parent so it can't do anything until the parent appears.

> situation where the pmic loads, the mmc subsystem loads, but then
> triggers hung task timeouts until the regulator is manually loaded.

If MMC needs a regulator it should be deferring probe until the
regulator appears.

> Worse, because the pmic isn't being held by the regulator you get this
> if the pmic is unloaded.
> 
> [31695.158053] ------------[ cut here ]------------
> [31695.162713] WARNING: CPU: 4 PID: 1729 at
> drivers/regulator/core.c:4136 regulator_unregister+0xfc/0x104

This is nothing to do with the parent, this is telling you that you're
removing a device which is currently in use which is obviously not going
to go well.

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