Hi Geert,
Thanks for the feedback.
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Friday, May 26, 2023 8:21 AM
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Cc: Alessandro Zummo <a.zummo@xxxxxxxxxxxx>; Alexandre Belloni
> <alexandre.belloni@xxxxxxxxxxx>; linux-rtc@xxxxxxxxxxxxxxx; Fabrizio
> Castro <fabrizio.castro.jz@xxxxxxxxxxx>; linux-renesas-
> soc@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxx>
>
> 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
