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