| Message ID | 20230518113643.420806-8-biju.das.jz@bp.renesas.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add Renesas PMIC RAA215300 and built-in RTC support | expand |
Hi Biju, On Thu, May 18, 2023 at 1:37 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > As per the HW manual, setting of XTOSCB bit as follws ... set the XTOSCB bit as follows: > If using an external clock signal, set the XTOSCB bit as 1 to > disable the crystal oscillator. > > If using an external crystal, the XTOSCB bit needs to be set at 0 > to enable the crystal oscillator. > > Add isl1208_set_xtoscb() to set XTOSCB bit based on the clock-names > property. Fallback is enabling the internal crystal oscillator. > > While at it, introduce a variable "sr" for reading status register the status register > in probe() as it is reused for writing. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v4: > * New patch. Thanks for your patch! > --- a/drivers/rtc/rtc-isl1208.c > +++ b/drivers/rtc/rtc-isl1208.c > @@ -837,6 +852,13 @@ isl1208_probe(struct i2c_client *client) > isl1208->config = (struct isl1208_config *)id->driver_data; > } > > + xin = devm_clk_get(&client->dev, "xin"); > + if (IS_ERR(xin)) { > + clkin = devm_clk_get(&client->dev, "clkin"); > + if (!IS_ERR(clkin)) > + int_osc_en = false; > + } devm_clk_get_optional() (doesn't devm_clk_get() print an error message?) And check for NULL to find out if a clock is present. IS_ERR() is a real error condition to be propagated. Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v4 07/11] rtc: isl1208: Add isl1208_set_xtoscb() > > Hi Biju, > > On Thu, May 18, 2023 at 1:37 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > As per the HW manual, setting of XTOSCB bit as follws > > ... set the XTOSCB bit as follows: Oops. Will fix. > > > If using an external clock signal, set the XTOSCB bit as 1 to disable > > the crystal oscillator. > > > > If using an external crystal, the XTOSCB bit needs to be set at 0 to > > enable the crystal oscillator. > > > > Add isl1208_set_xtoscb() to set XTOSCB bit based on the clock-names > > property. Fallback is enabling the internal crystal oscillator. > > > > While at it, introduce a variable "sr" for reading status register > > the status register Ok. > > > in probe() as it is reused for writing. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v4: > > * New patch. > > Thanks for your patch! > > > --- a/drivers/rtc/rtc-isl1208.c > > +++ b/drivers/rtc/rtc-isl1208.c > > > @@ -837,6 +852,13 @@ isl1208_probe(struct i2c_client *client) > > isl1208->config = (struct isl1208_config *)id- > >driver_data; > > } > > > > + xin = devm_clk_get(&client->dev, "xin"); > > + if (IS_ERR(xin)) { > > + clkin = devm_clk_get(&client->dev, "clkin"); > > + if (!IS_ERR(clkin)) > > + int_osc_en = false; > > + } > > devm_clk_get_optional() OK, Will use optional variant. > (doesn't devm_clk_get() print an error message?) I will double check. If I remember correctly it didn't throw any prtints. > > And check for NULL to find out if a clock is present. OK. > IS_ERR() is a real error condition to be propagated. Agreed. Cheers, Biju
diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c index 916e7d44c96f..5f91a3ca5920 100644 --- a/drivers/rtc/rtc-isl1208.c +++ b/drivers/rtc/rtc-isl1208.c @@ -6,6 +6,7 @@ */ #include <linux/bcd.h> +#include <linux/clk.h> #include <linux/i2c.h> #include <linux/module.h> #include <linux/of_device.h> @@ -175,6 +176,16 @@ isl1208_i2c_validate_client(struct i2c_client *client) return 0; } +static int isl1208_set_xtoscb(struct i2c_client *client, int sr, bool int_osc_en) +{ + if (int_osc_en) + sr &= ~ISL1208_REG_SR_XTOSCB; + else + sr |= ISL1208_REG_SR_XTOSCB; + + return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, sr); +} + static int isl1208_i2c_get_sr(struct i2c_client *client) { @@ -808,9 +819,13 @@ static int isl1208_setup_irq(struct i2c_client *client, int irq) static int isl1208_probe(struct i2c_client *client) { - int rc = 0; struct isl1208_state *isl1208; + bool int_osc_en = true; int evdet_irq = -1; + struct clk *clkin; + struct clk *xin; + int rc = 0; + int sr; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -ENODEV; @@ -837,6 +852,13 @@ isl1208_probe(struct i2c_client *client) isl1208->config = (struct isl1208_config *)id->driver_data; } + xin = devm_clk_get(&client->dev, "xin"); + if (IS_ERR(xin)) { + clkin = devm_clk_get(&client->dev, "clkin"); + if (!IS_ERR(clkin)) + int_osc_en = false; + } + isl1208->rtc = devm_rtc_allocate_device(&client->dev); if (IS_ERR(isl1208->rtc)) return PTR_ERR(isl1208->rtc); @@ -848,13 +870,17 @@ isl1208_probe(struct i2c_client *client) isl1208->nvmem_config.size = isl1208->config->nvmem_length; isl1208->nvmem_config.priv = isl1208; - rc = isl1208_i2c_get_sr(client); - if (rc < 0) { + sr = isl1208_i2c_get_sr(client); + if (sr < 0) { dev_err(&client->dev, "reading status failed\n"); - return rc; + return sr; } - if (rc & ISL1208_REG_SR_RTCF) + rc = isl1208_set_xtoscb(client, sr, int_osc_en); + if (rc) + return rc; + + if (sr & ISL1208_REG_SR_RTCF) dev_warn(&client->dev, "rtc power failure detected, " "please set clock.\n");
As per the HW manual, setting of XTOSCB bit as follws If using an external clock signal, set the XTOSCB bit as 1 to disable the crystal oscillator. If using an external crystal, the XTOSCB bit needs to be set at 0 to enable the crystal oscillator. Add isl1208_set_xtoscb() to set XTOSCB bit based on the clock-names property. Fallback is enabling the internal crystal oscillator. While at it, introduce a variable "sr" for reading status register in probe() as it is reused for writing. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v4: * New patch. --- drivers/rtc/rtc-isl1208.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-)