On Fri, Aug 20, 2021 at 4:03 PM Florian Fainelli <f.fainelli@xxxxxxxxx> wrote: > > > > On 8/18/2021 9:32 PM, Christophe JAILLET wrote: > > If an error occurs after a successful 'clk_prepare_enable()' call, it must > > be undone by a corresponding 'clk_disable_unprepare()' call. > > This call is already present in the remove function. > > > > Add this call in the error handling path and reorder the code so that the > > 'clk_prepare_enable()' call happens later in the function. > > The goal is to have as much managed resources functions as possible > > before the 'clk_prepare_enable()' call in order to keep the error handling > > path simple. > > > > While at it, remove the now unneeded 'clk' variable. > > > > Fixes: c87dca047849 ("usb: bdc: Add clock enable for new chips with a separate BDC clock") > > Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> > > Acked-by: Florian Fainelli <f.fainelli@xxxxxxxxx> > > > --- > > Review with care. > > I don't like shuffling code like that because of possible side effect. > > Moving the code related to this clk looks fine to me, but who knows... > > There are no register accesses until bdc_phy_init() gets called, so this > looks fine to me. Al knows this code better than I do though, so it > would be better to wait for his Acked-by tag. This is safe. Acked-by: Al Cooper <alcooperx@xxxxxxxxx> > -- > Florian