Hi Simon, Andy, On 17 July 2018 13:24, Andy Shevchenko wrote: > On Tue, 2018-07-17 at 14:07 +0200, Simon Horman wrote: > > On Mon, Jul 16, 2018 at 09:59:13AM +0100, Phil Edworthy wrote: > > > The Synopsys I2C Controller has a bus clock, but typically SoCs hide > > > this away. > > > However, on some SoCs you need to explicity enable the bus clock in > > > order to access the registers. > > > Therefore, enable an optional bus clock specified by DT. > > > > + /* Optional bus clock */ > > > + if (!IS_ERR(dev->busclk)) { > > > > I suspect that error values stored in dev->busclk, other than > > -ENOENT, should be treated as errors. IS_ERR catches all errors and is the correct way to check the value returned by devm_clk_get. > While your point sounds valid (don't remember how clk_get() is > implemented), NULL is also OK to have. Ok as in there is no bus clock, right? So it should be: if (!IS_ERR_OR_NULL (dev->busclk)) { Thanks Phil