On Mon, Feb 26, 2024 at 10:07 PM Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > 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. ack > > > otherwise it might ask > > for a similar change at apb_pclk. > > If you need so, yes. Send a patch. Noted > > > 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? No problem here > > > 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. I was looking for a reason that the author has encountered. Error condition on an optional clock might also fail `p->uartclk = clk_get_rate` and result "clock rate not defined\n" error. > > -- > With Best Regards, > Andy Shevchenko > >
