Messages in this thread | | | Subject | Re: [PATCH] rtc: rv3028: add clkout support | From | Parthiban Nallathambi <> | Date | Fri, 18 Oct 2019 12:02:28 +0200 |
| |
Hi,
Thanks for the comments. Will send v2 with changes.
On 10/11/19 11:17 AM, Alexandre Belloni wrote: > Hi, > > thanks for the patch, minor comments below. > > On 27/09/2019 16:15:05+0200, Parthiban Nallathambi wrote: >> + ret = regmap_update_bits(rv3028->regmap, RV3028_STATUS, >> + RV3028_STATUS_CLKF, 0); >> + if (ret < 0) >> + return ret; >> + > > This is already done earlier and this will not be enabled again unless > CLKIE is set which should not happen. So I don't think it is necessary > to do it once again here.
Agree, removed in v2.
> >> + for (i = 0; i < ARRAY_SIZE(clkout_rates); i++) { >> + if (clkout_rates[i] == rate) { >> + ret = regmap_update_bits(rv3028->regmap, >> + RV3028_CLKOUT, >> + RV3028_CLKOUT_FD_MASK, i); >> + if (ret < 0) >> + return ret; >> + >> + return regmap_write(rv3028->regmap, RV3028_CLKOUT, >> + RV3028_CLKOUT_CLKSY | RV3028_CLKOUT_CLKOE); >> + } >> + } >> + >> + return -EINVAL; >> +} > > >> +static int rv3028_clkout_register_clk(struct rv3028_data *rv3028, >> + struct i2c_client *client) >> +{ >> + int ret; >> + struct clk *clk; >> + struct clk_init_data init; >> + struct device_node *node = client->dev.of_node; >> + >> + /* disable the clkout output */ >> + ret = regmap_write(rv3028->regmap, RV3028_CLKOUT, 0x0); >> + if (ret < 0) >> + return ret; >> + > > This is not what the user would expect and could introduce a glitch in > the clock output every time the platform is booted. If there are no > users of the clock, then you should probably let the core disable it > once the boot has ended.
Thanks, removed in v2.
> >
-- Thanks, Parthiban N
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-22 Fax: (+49)-8142-66989-80 Email: pn@denx.de
| |