Messages in this thread | | | From | <> | Subject | RE: [PATCH 3/3] misc: microchip: pci1xxxx: Fix the error handling paths of gp_aux_bus_probe() | Date | Mon, 19 Sep 2022 17:47:32 +0000 |
| |
> -----Original Message----- > From: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > Sent: Sunday, September 18, 2022 11:58 AM > To: Kumaravel Thiagarajan - I21417 > <Kumaravel.Thiagarajan@microchip.com>; Arnd Bergmann > <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org; > Christophe JAILLET <christophe.jaillet@wanadoo.fr>; linux- > gpio@vger.kernel.org > Subject: [PATCH 3/3] misc: microchip: pci1xxxx: Fix the error handling paths > of gp_aux_bus_probe() > > There are several issues related to the error handling paths of > gp_aux_bus_probe(): > - some resources may be released twice. Once explicitly in the error > handling path, and once via the release() function > - auxiliary_device_delete() should be called after the first successful > auxiliary_device_add() Thanks for your patch. I had noticed this after one of the reviewers had only partially fixed them up. I need some time to review and test your patch with the hardware before approving them.
> > To fix them, reorder the code: > - move the place where we get the irq for the 2nd wrapper. > - call kfree() and ida_free() after error checks, rather then in the > error handling path. > - have the error handling path look like the remove function > > Fixes: 393fc2f5948f ("misc: microchip: pci1xxxx: load auxiliary bus driver for > the PIO function in the multi-function endpoint of pci1xxxx device.") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > This patch is speculative and untested, review with care. > > Other solutions are possible. > For example, we could use devm_kzalloc() to simplify the error handling path > and the release function. I thought about this but had some doubts whether it will work with the auxiliary bus architecture as the memory allocated will get tagged with the same device pointer. I need to do some experiments before taking up this path.
Thank You.
Regards, Kumar
| |