On Mon, Feb 26, 2024 at 09:30:53PM +0530, VAMSHI GAJJELA wrote: > On Thu, Feb 22, 2024 at 4:50 PM Uwe Kleine-König > <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: ... > > if (data->clk == NULL) > > data->clk = devm_clk_get_optional_enabled(dev, NULL); > > if (IS_ERR(data->clk)) > > - return PTR_ERR(data->clk); > > + return dev_err_probe(dev, PTR_ERR(data->clk), > > + "failed to get baudclk\n"); > > Not required IMO as the baudclk is optional, It adds verbosity to the cases when it's defined, but for some reason can't be retrieved. > otherwise it might ask > for a similar change at apb_pclk. If you need so, yes. Send a patch. > Could you please provide some insight into the circumstances that lead > to encountering this error case? > > Also the check is for IS_ERR(data->clk), data-clk could be NULL aswell. Yes, and that's the case of everything is fine as the clock is optional. Do you see any problem with that check? > I see any error should be caught at the following line, provided no > clock-frequency > ``` > /* If no clock rate is defined, fail. */ > if (!p->uartclk) > return dev_err_probe(dev, -EINVAL, "clock rate not defined\n"); > ``` Not sure what you meant by this. Even if we ask for an optional clock, the error condition still might happen with many other reasons besides the absence of the clock. -- With Best Regards, Andy Shevchenko
