Hi Biju,
On Fri, Jun 2, 2023 at 4:25 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 instantiates the
> RTC device based on i2c_device_id.
>
> The internal oscillator is enabled or not is determined by the
> parent clock name.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> ---
> v5->v6:
> * Added Rb tag from Geert.
> * Parsing of parent node is moved from probe->isl1208_clk_present()
> * Added comment for parsing parent node for getting clock resource.
> * Replaced XOR->NOT to make the operation more clear for the inverted case.
Thanks for the update!
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -822,14 +831,32 @@ isl1208_clk_present(struct i2c_client *client, const char *name)
> struct clk *clk;
> int ret;
>
> - clk = devm_clk_get_optional(&client->dev, name);
> - if (IS_ERR(clk)) {
> - ret = PTR_ERR(clk);
> - } else {
> - if (!clk)
> - ret = 0;
> - else
> + /*
> + * For the built-in RTC found on PMIC RAA21530, enabling of the
> + * internal oscillator is based on the parent clock. So parse the
> + * parent node to get the details.
> + */
> + if (client->dev.parent->type == &i2c_client_type) {
> + clk = of_clk_get_by_name(client->dev.parent->of_node, name);
> + if (IS_ERR(clk)) {
> + if (PTR_ERR(clk) != -ENOENT)
> + ret = PTR_ERR(clk);
> + else
> + ret = 0;
> + } else {
> + clk_put(clk);
> ret = 1;
> + }
Perhaps reshuffle the conditions to decrease indentation?
And return early?
if (!IS_ERR(clk)) {
clk_put(clk);
return 1;
}
if (PTR_ERR(clk) != -ENOENT)
return PTR_ERR(clk);
return 0;
> + } else {
> + clk = devm_clk_get_optional(&client->dev, name);
> + if (IS_ERR(clk)) {
> + ret = PTR_ERR(clk);
> + } else {
> + if (!clk)
> + ret = 0;
> + else
> + ret = 1;
> + }
Same comments as [PATCH v6 10/11].
> }
>
> return ret;
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
