Re: [PATCH v4 08/11] rtc: isl1208: Add support for the built-in RTC on the PMIC RAA215300

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux