On 10/08/2015 11:35 AM, Stephen Boyd wrote:

On 10/08, Rajendra Nayak wrote:

quoted

@@ -3520,11 +3522,26 @@ static int gcc_msm8960_probe(struct platform_device *pdev)
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
-	return qcom_cc_probe(pdev, match->data);
+	ret = qcom_cc_probe(pdev, match->data);
+	if (ret)
+		return ret;
+
+	tsens = platform_device_register_data(&pdev->dev, "qcom-tsens", -1,
+					      NULL, 0);
+	if (IS_ERR(tsens)) {
+		qcom_cc_remove(pdev);
+		return PTR_ERR(tsens);
+	}
+	platform_set_drvdata(pdev, tsens);

We just blew away the pointer that qcom_cc_probe() stores.

Oops, I seem to have completely overlooked that.

quoted

+
+	return 0;
 }
 
 static int gcc_msm8960_remove(struct platform_device *pdev)
 {
+	struct platform_device *tsens = platform_get_drvdata(pdev);
+
+	platform_device_unregister(tsens);
 	qcom_cc_remove(pdev);

So now we've leaked the reset controller.

I suppose the simplest solution is to get the drvdata pointer
after qcom_cc_probe(), allocate a container structure to hold
that as a void * plus the tsens device, i.e.

	struct container {
		void *data;
		struct platform_device *tsens;
	};
	
and then set the drvdata to that. And finally undo all that and
restore the drvdata pointer on remove.

I've also been considering putting some devm stuff inside
qcom_cc_probe() itself so that in the normal case you don't even
need to call qcom_cc_remove(), it just gets done automatically.
That would open up the possibility for drivers to use the drvdata
member as they wish.

I'll test with the patch you just sent.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation