On Tue, Apr 02, 2024 at 02:14:13PM +0300, Shahar Avidar wrote: > pi433_init had "unregister_chrdev" called twice. > Remove it using goto statements. > > Signed-off-by: Shahar Avidar <ikobh7@xxxxxxxxx> Your commit message needs to mention all the other changes you made, especially the bug fix. I feel like you're doing too many changes really. Just use the gotos to clean up but don't re-order the calls. You need to add a Fixes tag. > --- > v2->v1: > Followed by Dan Carpenter's <dan.carpenter@xxxxxxxxxx> comments: > - Remove empty "fail" goto tag. > - Reorder pi433 init & exit calls so they have reverse order. Keep the init calls the same. Re-order the exit calls. It's a bit of a gray area if you can re-order the exit calls in this same patch. Do it in a separate patch just to be safe. > - Add "unreg_spi_drv" goto tag. > - Check "debugfs_create_dir" return value. No. This is wrong. Debugfs function are not supposed to be checked. https://staticthinking.wordpress.com/2023/07/24/debugfs-functions-are-not-supposed-to-be-checked/ > - Update "if" statments for consistency. I like this cleanup, but it's not worth the effort. Just leave it as-is unless you really really need to change it. Do it in a separate patch. What qualifies as One Thing Per Patch is a bit of gray area. Greg tends to want things split up into tiny patches. You have to know your maintainer. regards, dan carpenter
