Skip to content

Commit 6740de9

Browse files
Dan Carpentergregkh
authored andcommitted
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: 6a0953c ("coresight: cti: Add CPU idle pm notifer to CTI devices") Signed-off-by: Dan Carpenter <[email protected]> Reviewed-by: Mike Leach <[email protected]> Reviewed-by: Mathieu Poirier <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 853eab6 commit 6740de9

File tree

1 file changed

+54
-42
lines changed

1 file changed

+54
-42
lines changed

drivers/hwtracing/coresight/coresight-cti.c

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -747,17 +747,50 @@ static int cti_dying_cpu(unsigned int cpu)
747747
return 0;
748748
}
749749

750+
static int cti_pm_setup(struct cti_drvdata *drvdata)
751+
{
752+
int ret;
753+
754+
if (drvdata->ctidev.cpu == -1)
755+
return 0;
756+
757+
if (nr_cti_cpu)
758+
goto done;
759+
760+
cpus_read_lock();
761+
ret = cpuhp_setup_state_nocalls_cpuslocked(
762+
CPUHP_AP_ARM_CORESIGHT_CTI_STARTING,
763+
"arm/coresight_cti:starting",
764+
cti_starting_cpu, cti_dying_cpu);
765+
if (ret) {
766+
cpus_read_unlock();
767+
return ret;
768+
}
769+
770+
ret = cpu_pm_register_notifier(&cti_cpu_pm_nb);
771+
cpus_read_unlock();
772+
if (ret) {
773+
cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STARTING);
774+
return ret;
775+
}
776+
777+
done:
778+
nr_cti_cpu++;
779+
cti_cpu_drvdata[drvdata->ctidev.cpu] = drvdata;
780+
781+
return 0;
782+
}
783+
750784
/* release PM registrations */
751785
static void cti_pm_release(struct cti_drvdata *drvdata)
752786
{
753-
if (drvdata->ctidev.cpu >= 0) {
754-
if (--nr_cti_cpu == 0) {
755-
cpu_pm_unregister_notifier(&cti_cpu_pm_nb);
787+
if (drvdata->ctidev.cpu == -1)
788+
return;
756789

757-
cpuhp_remove_state_nocalls(
758-
CPUHP_AP_ARM_CORESIGHT_CTI_STARTING);
759-
}
760-
cti_cpu_drvdata[drvdata->ctidev.cpu] = NULL;
790+
cti_cpu_drvdata[drvdata->ctidev.cpu] = NULL;
791+
if (--nr_cti_cpu == 0) {
792+
cpu_pm_unregister_notifier(&cti_cpu_pm_nb);
793+
cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STARTING);
761794
}
762795
}
763796

@@ -823,19 +856,14 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
823856

824857
/* driver data*/
825858
drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
826-
if (!drvdata) {
827-
ret = -ENOMEM;
828-
dev_info(dev, "%s, mem err\n", __func__);
829-
goto err_out;
830-
}
859+
if (!drvdata)
860+
return -ENOMEM;
831861

832862
/* Validity for the resource is already checked by the AMBA core */
833863
base = devm_ioremap_resource(dev, res);
834-
if (IS_ERR(base)) {
835-
ret = PTR_ERR(base);
836-
dev_err(dev, "%s, remap err\n", __func__);
837-
goto err_out;
838-
}
864+
if (IS_ERR(base))
865+
return PTR_ERR(base);
866+
839867
drvdata->base = base;
840868

841869
dev_set_drvdata(dev, drvdata);
@@ -854,8 +882,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
854882
pdata = coresight_cti_get_platform_data(dev);
855883
if (IS_ERR(pdata)) {
856884
dev_err(dev, "coresight_cti_get_platform_data err\n");
857-
ret = PTR_ERR(pdata);
858-
goto err_out;
885+
return PTR_ERR(pdata);
859886
}
860887

861888
/* 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)
867894
drvdata->ctidev.cpu);
868895
else
869896
cti_desc.name = coresight_alloc_device_name(&cti_sys_devs, dev);
870-
if (!cti_desc.name) {
871-
ret = -ENOMEM;
872-
goto err_out;
873-
}
897+
if (!cti_desc.name)
898+
return -ENOMEM;
874899

875900
/* setup CPU power management handling for CPU bound CTI devices. */
876-
if (drvdata->ctidev.cpu >= 0) {
877-
cti_cpu_drvdata[drvdata->ctidev.cpu] = drvdata;
878-
if (!nr_cti_cpu++) {
879-
cpus_read_lock();
880-
ret = cpuhp_setup_state_nocalls_cpuslocked(
881-
CPUHP_AP_ARM_CORESIGHT_CTI_STARTING,
882-
"arm/coresight_cti:starting",
883-
cti_starting_cpu, cti_dying_cpu);
884-
885-
if (!ret)
886-
ret = cpu_pm_register_notifier(&cti_cpu_pm_nb);
887-
cpus_read_unlock();
888-
if (ret)
889-
goto err_out;
890-
}
891-
}
901+
ret = cti_pm_setup(drvdata);
902+
if (ret)
903+
return ret;
892904

893905
/* create dynamic attributes for connections */
894906
ret = cti_create_cons_sysfs(dev, drvdata);
895907
if (ret) {
896908
dev_err(dev, "%s: create dynamic sysfs entries failed\n",
897909
cti_desc.name);
898-
goto err_out;
910+
goto pm_release;
899911
}
900912

901913
/* set up coresight component description */
@@ -908,7 +920,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
908920
drvdata->csdev = coresight_register(&cti_desc);
909921
if (IS_ERR(drvdata->csdev)) {
910922
ret = PTR_ERR(drvdata->csdev);
911-
goto err_out;
923+
goto pm_release;
912924
}
913925

914926
/* add to list of CTI devices */
@@ -927,7 +939,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
927939
dev_info(&drvdata->csdev->dev, "CTI initialized\n");
928940
return 0;
929941

930-
err_out:
942+
pm_release:
931943
cti_pm_release(drvdata);
932944
return ret;
933945
}

0 commit comments

Comments
 (0)