Hi Biju,
On Thu, May 18, 2023 at 1:37 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> The built-in RTC found on PMIC RAA215300 is the same as ISL1208.
> However, the external oscillator bit is inverted on PMIC version
> 0x11. The PMIC driver detects PMIC version and instantiate appropriate
instantiates the
> RTC device based on i2c_device_id.
>
> Enhance isl1208_set_xtoscb() to handle inverted bit and internal oscillator
> is enabled or not is determined by the parent clock name.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v3->v4:
> * Added support for internal oscillator enable/disable.
Thanks for the update!
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -852,11 +865,25 @@ 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))
> + if (client->dev.parent->type == &i2c_client_type) {
> + xin = of_clk_get_by_name(client->dev.parent->of_node, "xin");
> + if (IS_ERR(xin)) {
-ENOENT means clock not present, so continue below.
Any other error codes are to be propagated upstream.
> + clkin = of_clk_get_by_name(client->dev.parent->of_node, "clkin");
> + if (IS_ERR(clkin))
Likewise.
> + return -ENODEV;
Clock not present is not an error, as the clock is optional for DT
backwards compatibility.
> +
> int_osc_en = false;
> + clk_put(clkin);
> + } else {
> + clk_put(xin);
> + }
> + } else {
> + 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);
> @@ -876,7 +903,8 @@ isl1208_probe(struct i2c_client *client)
> return sr;
> }
>
> - rc = isl1208_set_xtoscb(client, sr, int_osc_en);
> + rc = isl1208_set_xtoscb(client, sr, int_osc_en,
> + isl1208->config->has_inverted_osc_bit);
No need to change isl1208_set_xtoscb() (but perhaps rename
the int_osc_en parameter?) if you would pass
"int_osc_en ^ isl1208->config->has_inverted_osc_bit".
(beware: C has no logical ^^, only bitwise ^).
> if (rc)
> return rc;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
