Skip to contentSkip to conversation

mfd: ssbi: Add MODULE_DEVICE_TABLE

PATCH Superseded Stephen Boyd 2 participants 4 messages Jun 2, 2013 – Jun 3, 2013

Subsystems (2)
Arm Qualcomm mailing list Multifunction devices (MFD)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Stephen Boyd

author
#0 lore raw

PATCH 1/2 mfd: ssbi: Add MODULE_DEVICE_TABLE

This allows the ssbi module to be autoloaded on boot.

Cc David Brown <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>
---
 drivers/mfd/ssbi.c | 1 +
 1 file changed, 1 insertion(+)
drivers/mfd/ssbi.c +1 -0
index f32da02..e561d3b 100644
@@ -350,6 +350,7 @@ static struct of_device_id ssbi_match_table[] = {
 	{ .compatible = "qcom,ssbi" },
 	{}
 };
+MODULE_DEVICE_TABLE(of, ssbi_match_table);
 
 static struct platform_driver ssbi_driver = {
 	.probe		= ssbi_probe,
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
author
#1 lore raw

PATCH 2/2 mfd: ssbi: Use devm_* to simplify code

Use devm_ioremap_resource and devm_kzalloc to simplify error paths and reduce lines of code.

Cc David Brown <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>
---
 drivers/mfd/ssbi.c | 69 +++++++++---------------------------------------------
 1 file changed, 11 insertions(+), 58 deletions(-)
drivers/mfd/ssbi.c +11 -58
index e561d3b..102a2284 100644
@@ -268,35 +268,23 @@ static int ssbi_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct resource *mem_res;
 	struct ssbi *ssbi;
-	int ret = 0;
 	const char *type;
 
-	ssbi = kzalloc(sizeof(struct ssbi), GFP_KERNEL);
-	if (!ssbi) {
-		pr_err("can not allocate ssbi_data\n");
+	ssbi = devm_kzalloc(&pdev->dev, sizeof(*ssbi), GFP_KERNEL);
+	if (!ssbi)
 		return -ENOMEM;
-	}
 
 	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem_res) {
-		pr_err("missing mem resource\n");
-		ret = -EINVAL;
-		goto err_get_mem_res;
-	}
+	ssbi->base = devm_ioremap_resource(&pdev->dev, mem_res);
+	if (IS_ERR(ssbi->base))
+		return PTR_ERR(ssbi->base);
 
-	ssbi->base = ioremap(mem_res->start, resource_size(mem_res));
-	if (!ssbi->base) {
-		pr_err("ioremap of 0x%p failed\n", (void *)mem_res->start);
-		ret = -EINVAL;
-		goto err_ioremap;
-	}
 	platform_set_drvdata(pdev, ssbi);
 
 	type = of_get_property(np, "qcom,controller-type", NULL);
 	if (type == NULL) {
-		pr_err("Missing qcom,controller-type property\n");
-		ret = -EINVAL;
-		goto err_ssbi_controller;
+		dev_err(&pdev->dev, "Missing qcom,controller-type property\n");
+		return -EINVAL;
 	}
 	dev_info(&pdev->dev, "SSBI controller type: '%s'\n", type);
 	if (strcmp(type, "ssbi") == 0)

@@ -306,9 +294,8 @@ static int ssbi_probe(struct platform_device *pdev)
 	else if (strcmp(type, "pmic-arbiter") == 0)
 		ssbi->controller_type = MSM_SBI_CTRL_PMIC_ARBITER;
 	else {
-		pr_err("Unknown qcom,controller-type\n");
-		ret = -EINVAL;
-		goto err_ssbi_controller;
+		dev_err(&pdev->dev, "Unknown qcom,controller-type\n");
+		return -EINVAL;
 	}
 
 	if (ssbi->controller_type == MSM_SBI_CTRL_PMIC_ARBITER) {

@@ -321,29 +308,7 @@ static int ssbi_probe(struct platform_device *pdev)
 
 	spin_lock_init(&ssbi->lock);
 
-	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
-	if (ret)
-		goto err_ssbi_controller;
-
-	return 0;
-
-err_ssbi_controller:
-	platform_set_drvdata(pdev, NULL);
-	iounmap(ssbi->base);
-err_ioremap:
-err_get_mem_res:
-	kfree(ssbi);
-	return ret;
-}
-
-static int ssbi_remove(struct platform_device *pdev)
-{
-	struct ssbi *ssbi = platform_get_drvdata(pdev);
-
-	platform_set_drvdata(pdev, NULL);
-	iounmap(ssbi->base);
-	kfree(ssbi);
-	return 0;
+	return of_platform_populate(np, NULL, NULL, &pdev->dev);
 }
 
 static struct of_device_id ssbi_match_table[] = {

@@ -354,25 +319,13 @@ MODULE_DEVICE_TABLE(of, ssbi_match_table);
 
 static struct platform_driver ssbi_driver = {
 	.probe		= ssbi_probe,
-	.remove		= ssbi_remove,
 	.driver		= {
 		.name	= "ssbi",
 		.owner	= THIS_MODULE,
 		.of_match_table = ssbi_match_table,
 	},
 };
-
-static int __init ssbi_init(void)
-{
-	return platform_driver_register(&ssbi_driver);
-}
-module_init(ssbi_init);
-
-static void __exit ssbi_exit(void)
-{
-	platform_driver_unregister(&ssbi_driver);
-}
-module_exit(ssbi_exit)
+module_platform_driver(ssbi_driver);
 
 MODULE_LICENSE("GPL v2");
 MODULE_VERSION("1.0");
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#2 lore raw

Re: [PATCH 2/2] mfd: ssbi: Use devm_* to simplify code

Quoted 2 lines

Use devm_ioremap_resource and devm_kzalloc to simplify error paths and reduce lines of code.

The code looks fine, but you're actually doing a great deal more than you describe in the commit message. Would you mind expanding?

I.e:

drivers/mfd/ssbi.c static int ssbi_probe(struct platform_device *pdev) · line 297

Quoted 1 line
drivers/mfd/ssbi.c +1 -0
+		dev_err(&pdev->dev, "Unknown qcom,controller-type\n");

Switching to these 'cos they're more informative.

Review

Quoted 1 line
(quoted diff) +0 -1
-	platform_set_drvdata(pdev, NULL);

Removing these because they're obsolete.

drivers/mfd/ssbi.c MODULE_DEVICE_TABLE(of, ssbi_match_table); · line 357

Quoted 1 line
drivers/mfd/ssbi.c +0 -1
-	.remove		= ssbi_remove,

Removing this 'cos it no longer does anything useful.

Quoted 1 line

+module_platform_driver(ssbi_driver);

Moving these because it's more succinct when init() and exit() routines are baron.

None of the above have anything to do with devm_* managed resources.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#3 lore raw

Re: [PATCH 2/2] mfd: ssbi: Use devm_* to simplify code

Quoted 13 lines · Lee Jones
Quoted 2 lines

Use devm_ioremap_resource and devm_kzalloc to simplify error paths and reduce lines of code.

The code looks fine, but you're actually doing a great deal more than you describe in the commit message. Would you mind expanding?

I.e:

drivers/mfd/ssbi.c static int ssbi_probe(struct platform_device *pdev) · line 297

Quoted 1 line
drivers/mfd/ssbi.c +1 -0
+		dev_err(&pdev->dev, "Unknown qcom,controller-type\n");

Switching to these 'cos they're more informative.

Review

Quoted 1 line
(quoted diff) +0 -1
-	platform_set_drvdata(pdev, NULL);

Removing these because they're obsolete.

Quoted 9 lines

drivers/mfd/ssbi.c MODULE_DEVICE_TABLE(of, ssbi_match_table); · line 357

Quoted 1 line
drivers/mfd/ssbi.c +0 -1
-	.remove		= ssbi_remove,

Removing this 'cos it no longer does anything useful.

Quoted 1 line

+module_platform_driver(ssbi_driver);

Moving these because it's more succinct when init() and exit() routines are baron.

None of the above have anything to do with devm_* managed resources.

Ok I'll update the commit text and resend. Thanks for the review.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation