On Mon, May 17, 2021 at 12:02:24PM +0200, Christophe JAILLET wrote: > Le 17/05/2021 à 10:01, Andy Shevchenko a écrit : > > On Sun, May 16, 2021 at 04:36:55PM +0200, Christophe JAILLET wrote: > > > There is no error handling path in the probe function. > > > Switch to managed resource so that errors in the probe are handled easily > > > and simplify the remove function accordingly. > > > > Yes, that's what I suggested earlier to another contributor. > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > > > Thanks! > > > > P.S. You may consider the following things as well: > > 1) converting to use pci_set_drvdata() / pci_get_drvdata() > > I can send a patch for that if you want. > But it looks really low value for a driver that is already very short and > clean. Yep, that's why 2) below came to my mind (then you will remove drvdata call). > > 2) providing devm_pvpanic_probe() [via devm_add_action() / > > devm_add_action_or_reset()] > > I don't follow you here. > The goal would be to avoid the remove function and "record" the needed > action directly in the probe? > > If this is it, I would only see an unusual pattern and a harder to follow > logic. > Did I miss something? > What would be the benefit? First of all it's a usual pattern when one, often used in ->probe(), function gains its devm variant. See, for example, `return devm_gpiochip_add_data(...);` used in the code. Benefit is to have everything under managed resources and yes, no ->remove() will be needed in the individual drivers. But it's up to you. It was just a proposal that you may simply refuse to follow, it's fine. -- With Best Regards, Andy Shevchenko
