Am Mon, 11 Nov 2024 16:41:47 +0200 schrieb Roger Quadros <rogerq@xxxxxxxxxx>: > Hi, > > On 10/11/2024 01:29, Andreas Kemnade wrote: > > Am Thu, 7 Nov 2024 12:12:52 +0100 > > schrieb Karol P <karprzy7@xxxxxxxxx>: > > > >> On Thu, 7 Nov 2024 at 00:15, Andreas Kemnade <andreas@xxxxxxxxxxxx> wrote: > >>> > >>> Am Wed, 6 Nov 2024 23:33:24 +0100 > >>> schrieb Karol Przybylski <karprzy7@xxxxxxxxx>: > >>> > >>>> clk_prepare() is called in usbtll_omap_probe to fill clk array. > >>>> Return code is not checked, leaving possible error condition unhandled. > >>>> > >>>> Added variable to hold return value from clk_prepare() and return statement > >>>> when it's not successful. > >>>> > >>>> Found in coverity scan, CID 1594680 > >>>> > >>>> Signed-off-by: Karol Przybylski <karprzy7@xxxxxxxxx> > >>>> --- > >>>> drivers/mfd/omap-usb-tll.c | 8 ++++++-- > >>>> 1 file changed, 6 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c > >>>> index 0f7fdb99c809..28446b082c85 100644 > >>>> --- a/drivers/mfd/omap-usb-tll.c > >>>> +++ b/drivers/mfd/omap-usb-tll.c > >>>> @@ -202,7 +202,7 @@ static int usbtll_omap_probe(struct platform_device *pdev) > >>>> struct device *dev = &pdev->dev; > >>>> struct usbtll_omap *tll; > >>>> void __iomem *base; > >>>> - int i, nch, ver; > >>>> + int i, nch, ver, err; > >>>> > >>>> dev_dbg(dev, "starting TI HSUSB TLL Controller\n"); > >>>> > >>>> @@ -251,7 +251,11 @@ static int usbtll_omap_probe(struct platform_device *pdev) > >>>> if (IS_ERR(tll->ch_clk[i])) > >>>> dev_dbg(dev, "can't get clock : %s\n", clkname); > >>> > >>> if you add more intensive error checking, then why is this error > >>> ignored and not returned? > >> > >> Thank you for the feedback. It does seem that elevated error checking > >> is not the way > >> to go in this case. > > > > As far as I can see everything checks ch_clk[i] for validity before > > usage. Also clk_enable() called later is checked which would catch > > clk_prepare() failures, if there were even possible here. > > > > So the only question which I am not 100% sure about is whether having > > ch_clk sparsly populated is normal operation. If that is the case, then > > more error checking is not useful. If not, then it might let us better > > sleep. As said as far as I can see errors are catched later. > > > > @Roger: what is your opintion towards this? > > I don't see usb_tll_hs_usb_ch?_clk in any of the OMAP device trees. > Could it be that they are optional? > If so then we could convert it to devm_clk_get_optional()? > They live in drivers/clk/ti/clk-[54]4xx.c But nothing about omap3. So apparently we can have valid use cases where these clocks are not available. So no real need more anything more than dev_dbg output here. Regards, Andreas
