On Fri, 2021-03-12 at 07:47 +0100, Mike Looijmans wrote: > Met vriendelijke groet / kind regards, > > Mike Looijmans > System Expert > > > TOPIC Embedded Products B.V. > Materiaalweg 4, 5681 RJ Best > The Netherlands > > T: +31 (0) 499 33 69 69 > E: mike.looijmans@xxxxxxxxxxxxxxxxx > W: > https://urldefense.com/v3/__http://www.topicproducts.com__;!!IOGos0k!wiWtHGRxxa459e4cDke1e1LNom9k55h5ayHt6t0CboF-tMxY8bgYWiRsplbzxz6sN94$ > > > Please consider the environment before printing this e-mail > On 11-03-2021 23:24, Robert Hancock wrote: > > After initializing the device, allow sufficient time for the PLL to lock > > (if we reconfigured it) and verify that the input clock is present and the > > PLL has locked before declaring success. > > > > Fixes: 3044a860fd ("clk: Add Si5341/Si5340 driver") > > Signed-off-by: Robert Hancock <robert.hancock@xxxxxxxxxx> > > --- > > drivers/clk/clk-si5341.c | 46 ++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 46 insertions(+) > > > > diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c > > index 2d69b2144acf..5221e431f6cb 100644 > > --- a/drivers/clk/clk-si5341.c > > +++ b/drivers/clk/clk-si5341.c > > @@ -92,6 +92,9 @@ struct clk_si5341_output_config { > > #define SI5341_PN_BASE 0x0002 > > #define SI5341_DEVICE_REV 0x0005 > > #define SI5341_STATUS 0x000C > > +#define SI5341_LOS 0x000D > > +#define SI5341_STATUS_STICKY 0x0011 > > +#define SI5341_LOS_STICKY 0x0012 > > #define SI5341_SOFT_RST 0x001C > > #define SI5341_IN_SEL 0x0021 > > #define SI5341_DEVICE_READY 0x00FE > > @@ -99,6 +102,12 @@ struct clk_si5341_output_config { > > #define SI5341_IN_EN 0x0949 > > #define SI5341_INX_TO_PFD_EN 0x094A > > > > +/* Status bits */ > > +#define SI5341_STATUS_SYSINCAL BIT(0) > > +#define SI5341_STATUS_LOSXAXB BIT(1) > > +#define SI5341_STATUS_LOSREF BIT(2) > > +#define SI5341_STATUS_LOL BIT(3) > > + > > /* Input selection */ > > #define SI5341_IN_SEL_MASK 0x06 > > #define SI5341_IN_SEL_SHIFT 1 > > @@ -1403,6 +1412,29 @@ static int si5341_clk_select_active_input(struct > > clk_si5341 *data) > > return res; > > } > > > > +static int si5341_check_healthy(struct clk_si5341 *data) > > +{ > > + u32 status; > > + int res = regmap_read(data->regmap, SI5341_STATUS, &status); > > + > > + if (res < 0) { > > + dev_err(&data->i2c_client->dev, "failed to read status\n"); > > + return res; > > + } > > + > > + if ((status & SI5341_STATUS_LOSREF)) { > > + dev_err(&data->i2c_client->dev, "input clock not present\n"); > > + return -EIO; > > + } > > + > > + if ((status & SI5341_STATUS_LOL)) { > > + dev_err(&data->i2c_client->dev, "PLL not locked\n"); > > + return -EIO; > > + } > > + > > + return 0; > > +} > > + > > static int si5341_probe(struct i2c_client *client, > > const struct i2c_device_id *id) > > { > > @@ -1580,6 +1612,20 @@ static int si5341_probe(struct i2c_client *client, > > err = si5341_finalize_defaults(data); > > if (err < 0) > > return err; > > + > > + /* allow time for PLL to lock */ > > + msleep(250); > > Can't this be a poll loop with timeout? Seems rather harsh to just sleep > here. Indeed, regmap_read_poll_timeout would likely work here. Will address in v2. > > > + } > > + > > + err = si5341_check_healthy(data); > > + if (err) > > + return err; > > + > > + /* clear sticky alarm bits from initialization */ > > + err = regmap_write(data->regmap, SI5341_STATUS_STICKY, 0); > > + if (err) { > > + dev_err(&client->dev, "unable to clear sticky status\n"); > > + return err; > > } > > > > /* Free the names, clk framework makes copies */ > >
