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@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
> > ---
> > 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
