Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

cpuidle: psci: Fix error path via converting to a platform driver

The current error paths for the cpuidle-psci driver, may leak memory or
possibly leave CPU devices attached to their PM domains. These are quite
harmless issues, but still deserves to be taken care of.

Although, rather than fixing them by keeping track of allocations that
needs to be freed, which tends to become a bit messy, let's convert into a
platform driver. In this way, it gets easier to fix the memory leaks as we
can rely on the devm_* functions.

Moreover, converting to a platform driver also enables support for deferred
probe, which subsequent changes takes benefit from.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

authored by

Ulf Hansson and committed by
Rafael J. Wysocki
166bf835 4b072cd6

+95 -65
+9 -1
drivers/cpuidle/cpuidle-psci-domain.c
··· 287 287 } 288 288 subsys_initcall(psci_idle_init_domains); 289 289 290 - struct device __init *psci_dt_attach_cpu(int cpu) 290 + struct device *psci_dt_attach_cpu(int cpu) 291 291 { 292 292 struct device *dev; 293 293 ··· 300 300 pm_runtime_get_sync(dev); 301 301 302 302 return dev; 303 + } 304 + 305 + void psci_dt_detach_cpu(struct device *dev) 306 + { 307 + if (IS_ERR_OR_NULL(dev)) 308 + return; 309 + 310 + dev_pm_domain_detach(dev, false); 303 311 }
+80 -61
drivers/cpuidle/cpuidle-psci.c
··· 17 17 #include <linux/module.h> 18 18 #include <linux/of.h> 19 19 #include <linux/of_device.h> 20 + #include <linux/platform_device.h> 20 21 #include <linux/psci.h> 21 22 #include <linux/pm_runtime.h> 22 23 #include <linux/slab.h> 24 + #include <linux/string.h> 23 25 24 26 #include <asm/cpuidle.h> 25 27 ··· 35 33 36 34 static DEFINE_PER_CPU_READ_MOSTLY(struct psci_cpuidle_data, psci_cpuidle_data); 37 35 static DEFINE_PER_CPU(u32, domain_state); 38 - static bool psci_cpuidle_use_cpuhp __initdata; 36 + static bool psci_cpuidle_use_cpuhp; 39 37 40 38 void psci_set_domain_state(u32 state) 41 39 { ··· 106 104 return 0; 107 105 } 108 106 109 - static void __init psci_idle_init_cpuhp(void) 107 + static void psci_idle_init_cpuhp(void) 110 108 { 111 109 int err; 112 110 ··· 129 127 return psci_enter_state(idx, state[idx]); 130 128 } 131 129 132 - static struct cpuidle_driver psci_idle_driver __initdata = { 133 - .name = "psci_idle", 134 - .owner = THIS_MODULE, 135 - /* 136 - * PSCI idle states relies on architectural WFI to 137 - * be represented as state index 0. 138 - */ 139 - .states[0] = { 140 - .enter = psci_enter_idle_state, 141 - .exit_latency = 1, 142 - .target_residency = 1, 143 - .power_usage = UINT_MAX, 144 - .name = "WFI", 145 - .desc = "ARM WFI", 146 - } 147 - }; 148 - 149 - static const struct of_device_id psci_idle_state_match[] __initconst = { 130 + static const struct of_device_id psci_idle_state_match[] = { 150 131 { .compatible = "arm,idle-state", 151 132 .data = psci_enter_idle_state }, 152 133 { }, 153 134 }; 154 135 155 - int __init psci_dt_parse_state_node(struct device_node *np, u32 *state) 136 + int psci_dt_parse_state_node(struct device_node *np, u32 *state) 156 137 { 157 138 int err = of_property_read_u32(np, "arm,psci-suspend-param", state); 158 139 ··· 152 167 return 0; 153 168 } 154 169 155 - static int __init psci_dt_cpu_init_topology(struct cpuidle_driver *drv, 156 - struct psci_cpuidle_data *data, 157 - unsigned int state_count, int cpu) 170 + static int psci_dt_cpu_init_topology(struct cpuidle_driver *drv, 171 + struct psci_cpuidle_data *data, 172 + unsigned int state_count, int cpu) 158 173 { 159 174 /* Currently limit the hierarchical topology to be used in OSI mode. */ 160 175 if (!psci_has_osi_support()) ··· 175 190 return 0; 176 191 } 177 192 178 - static int __init psci_dt_cpu_init_idle(struct cpuidle_driver *drv, 179 - struct device_node *cpu_node, 180 - unsigned int state_count, int cpu) 193 + static int psci_dt_cpu_init_idle(struct device *dev, struct cpuidle_driver *drv, 194 + struct device_node *cpu_node, 195 + unsigned int state_count, int cpu) 181 196 { 182 197 int i, ret = 0; 183 198 u32 *psci_states; ··· 185 200 struct psci_cpuidle_data *data = per_cpu_ptr(&psci_cpuidle_data, cpu); 186 201 187 202 state_count++; /* Add WFI state too */ 188 - psci_states = kcalloc(state_count, sizeof(*psci_states), GFP_KERNEL); 203 + psci_states = devm_kcalloc(dev, state_count, sizeof(*psci_states), 204 + GFP_KERNEL); 189 205 if (!psci_states) 190 206 return -ENOMEM; 191 207 ··· 199 213 of_node_put(state_node); 200 214 201 215 if (ret) 202 - goto free_mem; 216 + return ret; 203 217 204 218 pr_debug("psci-power-state %#x index %d\n", psci_states[i], i); 205 219 } 206 220 207 - if (i != state_count) { 208 - ret = -ENODEV; 209 - goto free_mem; 210 - } 221 + if (i != state_count) 222 + return -ENODEV; 211 223 212 224 /* Initialize optional data, used for the hierarchical topology. */ 213 225 ret = psci_dt_cpu_init_topology(drv, data, state_count, cpu); 214 226 if (ret < 0) 215 - goto free_mem; 227 + return ret; 216 228 217 229 /* Idle states parsed correctly, store them in the per-cpu struct. */ 218 230 data->psci_states = psci_states; 219 231 return 0; 220 - 221 - free_mem: 222 - kfree(psci_states); 223 - return ret; 224 232 } 225 233 226 - static __init int psci_cpu_init_idle(struct cpuidle_driver *drv, 227 - unsigned int cpu, unsigned int state_count) 234 + static int psci_cpu_init_idle(struct device *dev, struct cpuidle_driver *drv, 235 + unsigned int cpu, unsigned int state_count) 228 236 { 229 237 struct device_node *cpu_node; 230 238 int ret; ··· 234 254 if (!cpu_node) 235 255 return -ENODEV; 236 256 237 - ret = psci_dt_cpu_init_idle(drv, cpu_node, state_count, cpu); 257 + ret = psci_dt_cpu_init_idle(dev, drv, cpu_node, state_count, cpu); 238 258 239 259 of_node_put(cpu_node); 240 260 241 261 return ret; 242 262 } 243 263 244 - static int __init psci_idle_init_cpu(int cpu) 264 + static void psci_cpu_deinit_idle(int cpu) 265 + { 266 + struct psci_cpuidle_data *data = per_cpu_ptr(&psci_cpuidle_data, cpu); 267 + 268 + psci_dt_detach_cpu(data->dev); 269 + psci_cpuidle_use_cpuhp = false; 270 + } 271 + 272 + static int psci_idle_init_cpu(struct device *dev, int cpu) 245 273 { 246 274 struct cpuidle_driver *drv; 247 275 struct device_node *cpu_node; ··· 272 284 if (ret) 273 285 return ret; 274 286 275 - drv = kmemdup(&psci_idle_driver, sizeof(*drv), GFP_KERNEL); 287 + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); 276 288 if (!drv) 277 289 return -ENOMEM; 278 290 291 + drv->name = "psci_idle"; 292 + drv->owner = THIS_MODULE; 279 293 drv->cpumask = (struct cpumask *)cpumask_of(cpu); 280 294 281 295 /* 282 - * Initialize idle states data, starting at index 1, since 283 - * by default idle state 0 is the quiescent state reached 284 - * by the cpu by executing the wfi instruction. 285 - * 296 + * PSCI idle states relies on architectural WFI to be represented as 297 + * state index 0. 298 + */ 299 + drv->states[0].enter = psci_enter_idle_state; 300 + drv->states[0].exit_latency = 1; 301 + drv->states[0].target_residency = 1; 302 + drv->states[0].power_usage = UINT_MAX; 303 + strcpy(drv->states[0].name, "WFI"); 304 + strcpy(drv->states[0].desc, "ARM WFI"); 305 + 306 + /* 286 307 * If no DT idle states are detected (ret == 0) let the driver 287 308 * initialization fail accordingly since there is no reason to 288 309 * initialize the idle driver if only wfi is supported, the ··· 299 302 * on idle entry. 300 303 */ 301 304 ret = dt_init_idle_driver(drv, psci_idle_state_match, 1); 302 - if (ret <= 0) { 303 - ret = ret ? : -ENODEV; 304 - goto out_kfree_drv; 305 - } 305 + if (ret <= 0) 306 + return ret ? : -ENODEV; 306 307 307 308 /* 308 309 * Initialize PSCI idle states. 309 310 */ 310 - ret = psci_cpu_init_idle(drv, cpu, ret); 311 + ret = psci_cpu_init_idle(dev, drv, cpu, ret); 311 312 if (ret) { 312 313 pr_err("CPU %d failed to PSCI idle\n", cpu); 313 - goto out_kfree_drv; 314 + return ret; 314 315 } 315 316 316 317 ret = cpuidle_register(drv, NULL); 317 318 if (ret) 318 - goto out_kfree_drv; 319 + goto deinit; 319 320 320 321 cpuidle_cooling_register(drv); 321 322 322 323 return 0; 323 - 324 - out_kfree_drv: 325 - kfree(drv); 324 + deinit: 325 + psci_cpu_deinit_idle(cpu); 326 326 return ret; 327 327 } 328 328 329 329 /* 330 - * psci_idle_init - Initializes PSCI cpuidle driver 330 + * psci_idle_probe - Initializes PSCI cpuidle driver 331 331 * 332 332 * Initializes PSCI cpuidle driver for all CPUs, if any CPU fails 333 333 * to register cpuidle driver then rollback to cancel all CPUs 334 334 * registration. 335 335 */ 336 - static int __init psci_idle_init(void) 336 + static int psci_cpuidle_probe(struct platform_device *pdev) 337 337 { 338 338 int cpu, ret; 339 339 struct cpuidle_driver *drv; 340 340 struct cpuidle_device *dev; 341 341 342 342 for_each_possible_cpu(cpu) { 343 - ret = psci_idle_init_cpu(cpu); 343 + ret = psci_idle_init_cpu(&pdev->dev, cpu); 344 344 if (ret) 345 345 goto out_fail; 346 346 } ··· 350 356 dev = per_cpu(cpuidle_devices, cpu); 351 357 drv = cpuidle_get_cpu_driver(dev); 352 358 cpuidle_unregister(drv); 353 - kfree(drv); 359 + psci_cpu_deinit_idle(cpu); 354 360 } 355 361 356 362 return ret; 363 + } 364 + 365 + static struct platform_driver psci_cpuidle_driver = { 366 + .probe = psci_cpuidle_probe, 367 + .driver = { 368 + .name = "psci-cpuidle", 369 + }, 370 + }; 371 + 372 + static int __init psci_idle_init(void) 373 + { 374 + struct platform_device *pdev; 375 + int ret; 376 + 377 + ret = platform_driver_register(&psci_cpuidle_driver); 378 + if (ret) 379 + return ret; 380 + 381 + pdev = platform_device_register_simple("psci-cpuidle", -1, NULL, 0); 382 + if (IS_ERR(pdev)) { 383 + platform_driver_unregister(&psci_cpuidle_driver); 384 + return PTR_ERR(pdev); 385 + } 386 + 387 + return 0; 357 388 } 358 389 device_initcall(psci_idle_init);
+6 -3
drivers/cpuidle/cpuidle-psci.h
··· 3 3 #ifndef __CPUIDLE_PSCI_H 4 4 #define __CPUIDLE_PSCI_H 5 5 6 + struct device; 6 7 struct device_node; 7 8 8 9 void psci_set_domain_state(u32 state); 9 - int __init psci_dt_parse_state_node(struct device_node *np, u32 *state); 10 + int psci_dt_parse_state_node(struct device_node *np, u32 *state); 10 11 11 12 #ifdef CONFIG_ARM_PSCI_CPUIDLE_DOMAIN 12 - struct device __init *psci_dt_attach_cpu(int cpu); 13 + struct device *psci_dt_attach_cpu(int cpu); 14 + void psci_dt_detach_cpu(struct device *dev); 13 15 #else 14 - static inline struct device __init *psci_dt_attach_cpu(int cpu) { return NULL; } 16 + static inline struct device *psci_dt_attach_cpu(int cpu) { return NULL; } 17 + static inline void psci_dt_detach_cpu(struct device *dev) { } 15 18 #endif 16 19 17 20 #endif /* __CPUIDLE_PSCI_H */