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()? While at that, maybe the device tree binding could also be updated and converted to yaml. > > BTW: If you do this kind of work, you could also use W=1 or > CONFIG_WERROR during compiling to catch easy things. At least I see new > compile warnings with your patch. > > Regards, > Andreas -- cheers, -roger
