On Mon, 11 Nov 2024 at 20:57, Andreas Kemnade <andreas@xxxxxxxxxxxx> wrote: > > 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 Thanks, I will send in a new patch then. I can also take a look at updating device tree binding. Best regards, Karol
