On 10/08/2015 11:35 AM, Stephen Boyd wrote:
> On 10/08, Rajendra Nayak wrote:
>> @@ -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.
>
>> +
>> + 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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
