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

Configure Feed

Select the types of activity you want to include in your feed.

usb: gadget: aspeed_udc: Convert to platform remove callback returning void

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

ast_udc_remove() is one of these functions that return an error code
after doing only a partial cleanup. Replace the core's error message by
a more drastic one and still convert the driver to .remove_new().
Note the only semantic change here is the changed error message.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20231026221701.2521483-2-u.kleine-koenig@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Uwe Kleine-König and committed by
Greg Kroah-Hartman
3d56e5aa c3097719

+13 -6
+13 -6
drivers/usb/gadget/udc/aspeed_udc.c
··· 1432 1432 ast_udc_write(udc, 0, AST_UDC_EP0_CTRL); 1433 1433 } 1434 1434 1435 - static int ast_udc_remove(struct platform_device *pdev) 1435 + static void ast_udc_remove(struct platform_device *pdev) 1436 1436 { 1437 1437 struct ast_udc_dev *udc = platform_get_drvdata(pdev); 1438 1438 unsigned long flags; 1439 1439 u32 ctrl; 1440 1440 1441 1441 usb_del_gadget_udc(&udc->gadget); 1442 - if (udc->driver) 1443 - return -EBUSY; 1442 + if (udc->driver) { 1443 + /* 1444 + * This is broken as only some cleanup is skipped, *udev is 1445 + * freed and the register mapping goes away. Any further usage 1446 + * probably crashes. Also the device is unbound, so the skipped 1447 + * cleanup is never catched up later. 1448 + */ 1449 + dev_alert(&pdev->dev, 1450 + "Driver is busy and still going away. Fasten your seat belts!\n"); 1451 + return; 1452 + } 1444 1453 1445 1454 spin_lock_irqsave(&udc->lock, flags); 1446 1455 ··· 1468 1459 udc->ep0_buf_dma); 1469 1460 1470 1461 udc->ep0_buf = NULL; 1471 - 1472 - return 0; 1473 1462 } 1474 1463 1475 1464 static int ast_udc_probe(struct platform_device *pdev) ··· 1588 1581 1589 1582 static struct platform_driver ast_udc_driver = { 1590 1583 .probe = ast_udc_probe, 1591 - .remove = ast_udc_remove, 1584 + .remove_new = ast_udc_remove, 1592 1585 .driver = { 1593 1586 .name = KBUILD_MODNAME, 1594 1587 .of_match_table = ast_udc_of_dt_ids,