| Message ID | 20230522101849.297499-8-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add Renesas PMIC RAA215300 and built-in RTC support | expand |
Hi Biju, On Mon, May 22, 2023 at 12:19 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > As per the HW manual, 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 the status register > in probe() as it is reused for writing. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v4->v5: > * Fixed the typo in commit description. > * Replaced the variable int_osc_en->xtosb_val for isl1208_set_xtoscb() and > changed the data type from bool->u8. You might as well just use int. > * Replaced devm_clk_get->devm_clk_get_optional() in probe. > * IS_ERR() related error is propagated and check for NULL to find out > if a clock is present. Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: Friday, May 26, 2023 8:21 AM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; linux-rtc@vger.kernel.org; Fabrizio > Castro <fabrizio.castro.jz@renesas.com>; linux-renesas- > soc@vger.kernel.org > Subject: Re: [PATCH v5 07/11] rtc: isl1208: Add isl1208_set_xtoscb() > > Hi Biju, > > On Mon, May 22, 2023 at 12:19 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > As per the HW manual, 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 the status register > > in probe() as it is reused for writing. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v4->v5: > > * Fixed the typo in commit description. > > * Replaced the variable int_osc_en->xtosb_val for > isl1208_set_xtoscb() and > > changed the data type from bool->u8. > > You might as well just use int. OK, Will change it to int. Cheers, Biju > > > * Replaced devm_clk_get->devm_clk_get_optional() in probe. > > * IS_ERR() related error is propagated and check for NULL to find out > > if a clock is present. > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. > But when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds
diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c index d42615fcdd9f..6ca977595977 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, u8 xtosb_val) +{ + if (xtosb_val) + 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; int evdet_irq = -1; + struct clk *clkin; + u8 xtosb_val = 1; + struct clk *xin; + int rc = 0; + int sr; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -ENODEV; @@ -837,6 +852,19 @@ isl1208_probe(struct i2c_client *client) isl1208->config = (struct isl1208_config *)id->driver_data; } + xin = devm_clk_get_optional(&client->dev, "xin"); + if (IS_ERR(xin)) + return PTR_ERR(xin); + + if (!xin) { + clkin = devm_clk_get_optional(&client->dev, "clkin"); + if (IS_ERR(clkin)) + return PTR_ERR(clkin); + + if (clkin) + xtosb_val = 0; + } + isl1208->rtc = devm_rtc_allocate_device(&client->dev); if (IS_ERR(isl1208->rtc)) return PTR_ERR(isl1208->rtc); @@ -848,13 +876,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, xtosb_val); + 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, 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 the status register in probe() as it is reused for writing. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v4->v5: * Fixed the typo in commit description. * Replaced the variable int_osc_en->xtosb_val for isl1208_set_xtoscb() and changed the data type from bool->u8. * Replaced devm_clk_get->devm_clk_get_optional() in probe. * IS_ERR() related error is propagated and check for NULL to find out if a clock is present. v4: * New patch. --- drivers/rtc/rtc-isl1208.c | 42 ++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-)