Messages in this thread | | | From | Parshuram Raju Thombare <> | Subject | RE: [PATCH v10 2/7] i3c: master: use i3c_master_register only for main master | Date | Thu, 17 Jun 2021 08:42:28 +0000 |
| |
Hi Alexandre,
Thank you for review comments.
>On 08/12/2020 05:47:51+0000, Parshuram Raju Thombare wrote: >> >This looks a bit confusing. Here you're rolling back detailss in >> >i3c_primary_master_register() that were factored out in >> >i3c_master_init(). If i3c_master_init() is successful, then you >> >shouldn't be undoing its things openly in i3c_primary_master_register(). >> >Instead, there should be another function that does the reverse of >> >i3c_master_init() here. >> >> Every function do its cleanup in case of failures. >> And if any failure occur after successful i3c_master_init(), we have >> function i3c_master_bus_cleanup() which does the major cleanup. >> > >The point from Nicolas here was that the workqueue is allocated in >i3c_master_init so you should have a function to destroy it instead of >having to do that separately in i3c_primary_master_register. The same is >true for the put_device. Or you have to ensure i3c_masterdev_release >is called when i3c_primary_master_register fails.
Ok, IIUC, it is just that we need separate clean up function. i3c_master_bus_cleanup is used here to call driver cleanup function and detach + release devices from bus, and I am not sure i3c_masterdev_release can be used for that. Better alternative is i3c_master_unregister(). However, it doesn't handle the case where device_add fails, as this function unconditionally calls device_unregister which call both device_del and put_device.
| |