Commit 6740de94 authored by Dan Carpenter's avatar Dan Carpenter Committed by Greg Kroah-Hartman

coresight: cti: Fix error handling in probe

There were a couple problems with error handling in the probe function:
1)  If the "drvdata" allocation failed then it lead to a NULL
    dereference.
2)  On several error paths we decremented "nr_cti_cpu" before it was
    incremented which lead to a reference counting bug.

There were also some parts of the error handling which were not bugs but
were messy.  The error handling was confusing to read.  It printed some
unnecessary error messages.

The simplest way to fix these problems was to create a cti_pm_setup()
function that did all the power management setup in one go.  That way
when we call cti_pm_release() we don't have to deal with the
complications of a partially configured power management config.

I reversed the "if (drvdata->ctidev.cpu >= 0)" condition in
cti_pm_release() so that it mirros the new cti_pm_setup() function.

Fixes: 6a0953ce ("coresight: cti: Add CPU idle pm notifer to CTI devices")
Signed-off-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: default avatarMike Leach <mike.leach@linaro.org>
Reviewed-by: default avatarMathieu Poirier <mathieu.poirier@linaro.org>
Link: https://lore.kernel.org/r/20200701160852.2782823-2-mathieu.poirier@linaro.orgSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 853eab68
...@@ -747,17 +747,50 @@ static int cti_dying_cpu(unsigned int cpu) ...@@ -747,17 +747,50 @@ static int cti_dying_cpu(unsigned int cpu)
return 0; return 0;
} }
static int cti_pm_setup(struct cti_drvdata *drvdata)
{
int ret;
if (drvdata->ctidev.cpu == -1)
return 0;
if (nr_cti_cpu)
goto done;
cpus_read_lock();
ret = cpuhp_setup_state_nocalls_cpuslocked(
CPUHP_AP_ARM_CORESIGHT_CTI_STARTING,
"arm/coresight_cti:starting",
cti_starting_cpu, cti_dying_cpu);
if (ret) {
cpus_read_unlock();
return ret;
}
ret = cpu_pm_register_notifier(&cti_cpu_pm_nb);
cpus_read_unlock();
if (ret) {
cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STARTING);
return ret;
}
done:
nr_cti_cpu++;
cti_cpu_drvdata[drvdata->ctidev.cpu] = drvdata;
return 0;
}
/* release PM registrations */ /* release PM registrations */
static void cti_pm_release(struct cti_drvdata *drvdata) static void cti_pm_release(struct cti_drvdata *drvdata)
{ {
if (drvdata->ctidev.cpu >= 0) { if (drvdata->ctidev.cpu == -1)
if (--nr_cti_cpu == 0) { return;
cpu_pm_unregister_notifier(&cti_cpu_pm_nb);
cpuhp_remove_state_nocalls(
CPUHP_AP_ARM_CORESIGHT_CTI_STARTING);
}
cti_cpu_drvdata[drvdata->ctidev.cpu] = NULL; cti_cpu_drvdata[drvdata->ctidev.cpu] = NULL;
if (--nr_cti_cpu == 0) {
cpu_pm_unregister_notifier(&cti_cpu_pm_nb);
cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STARTING);
} }
} }
...@@ -823,19 +856,14 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) ...@@ -823,19 +856,14 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
/* driver data*/ /* driver data*/
drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
if (!drvdata) { if (!drvdata)
ret = -ENOMEM; return -ENOMEM;
dev_info(dev, "%s, mem err\n", __func__);
goto err_out;
}
/* Validity for the resource is already checked by the AMBA core */ /* Validity for the resource is already checked by the AMBA core */
base = devm_ioremap_resource(dev, res); base = devm_ioremap_resource(dev, res);
if (IS_ERR(base)) { if (IS_ERR(base))
ret = PTR_ERR(base); return PTR_ERR(base);
dev_err(dev, "%s, remap err\n", __func__);
goto err_out;
}
drvdata->base = base; drvdata->base = base;
dev_set_drvdata(dev, drvdata); dev_set_drvdata(dev, drvdata);
...@@ -854,8 +882,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) ...@@ -854,8 +882,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
pdata = coresight_cti_get_platform_data(dev); pdata = coresight_cti_get_platform_data(dev);
if (IS_ERR(pdata)) { if (IS_ERR(pdata)) {
dev_err(dev, "coresight_cti_get_platform_data err\n"); dev_err(dev, "coresight_cti_get_platform_data err\n");
ret = PTR_ERR(pdata); return PTR_ERR(pdata);
goto err_out;
} }
/* default to powered - could change on PM notifications */ /* default to powered - could change on PM notifications */
...@@ -867,35 +894,20 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) ...@@ -867,35 +894,20 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
drvdata->ctidev.cpu); drvdata->ctidev.cpu);
else else
cti_desc.name = coresight_alloc_device_name(&cti_sys_devs, dev); cti_desc.name = coresight_alloc_device_name(&cti_sys_devs, dev);
if (!cti_desc.name) { if (!cti_desc.name)
ret = -ENOMEM; return -ENOMEM;
goto err_out;
}
/* setup CPU power management handling for CPU bound CTI devices. */ /* setup CPU power management handling for CPU bound CTI devices. */
if (drvdata->ctidev.cpu >= 0) { ret = cti_pm_setup(drvdata);
cti_cpu_drvdata[drvdata->ctidev.cpu] = drvdata;
if (!nr_cti_cpu++) {
cpus_read_lock();
ret = cpuhp_setup_state_nocalls_cpuslocked(
CPUHP_AP_ARM_CORESIGHT_CTI_STARTING,
"arm/coresight_cti:starting",
cti_starting_cpu, cti_dying_cpu);
if (!ret)
ret = cpu_pm_register_notifier(&cti_cpu_pm_nb);
cpus_read_unlock();
if (ret) if (ret)
goto err_out; return ret;
}
}
/* create dynamic attributes for connections */ /* create dynamic attributes for connections */
ret = cti_create_cons_sysfs(dev, drvdata); ret = cti_create_cons_sysfs(dev, drvdata);
if (ret) { if (ret) {
dev_err(dev, "%s: create dynamic sysfs entries failed\n", dev_err(dev, "%s: create dynamic sysfs entries failed\n",
cti_desc.name); cti_desc.name);
goto err_out; goto pm_release;
} }
/* set up coresight component description */ /* set up coresight component description */
...@@ -908,7 +920,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) ...@@ -908,7 +920,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
drvdata->csdev = coresight_register(&cti_desc); drvdata->csdev = coresight_register(&cti_desc);
if (IS_ERR(drvdata->csdev)) { if (IS_ERR(drvdata->csdev)) {
ret = PTR_ERR(drvdata->csdev); ret = PTR_ERR(drvdata->csdev);
goto err_out; goto pm_release;
} }
/* add to list of CTI devices */ /* add to list of CTI devices */
...@@ -927,7 +939,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) ...@@ -927,7 +939,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
dev_info(&drvdata->csdev->dev, "CTI initialized\n"); dev_info(&drvdata->csdev->dev, "CTI initialized\n");
return 0; return 0;
err_out: pm_release:
cti_pm_release(drvdata); cti_pm_release(drvdata);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment