diff mbox

[v4,4/7] rtc: ac100: Add clk output support

Message ID 1467302331-9663-5-git-send-email-wens@csie.org
State Superseded
Headers show

Commit Message

Chen-Yu Tsai June 30, 2016, 3:58 p.m. UTC
The AC100's RTC side has 3 clock outputs on external pins, which can
provide a clock signal to the SoC or other modules, such as WiFi or
GSM modules.

Support this with a custom clk driver integrated with the rtc driver.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
Changes since v3:

  - Renamed clk32k prefixes to clkout, except for the internal 32k clk
  - Changed default clk output names to "ac100-cko{1,2,3}-rtc"
  - Moved 4M ADDA clk to codec side

Changes since v2: none
Changes since v1: none
---
 drivers/rtc/rtc-ac100.c | 302 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 302 insertions(+)

Comments

Michael Turquette July 8, 2016, 6:36 p.m. UTC | #1
Quoting Chen-Yu Tsai (2016-06-30 08:58:48)
> +static long ac100_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
> +                                   unsigned long prate)
> +{
> +       unsigned long best_rate = 0, tmp_rate, tmp_prate;
> +       int i;
> +
> +       if (prate == AC100_RTC_32K_RATE)
> +               return divider_round_rate(hw, rate, &prate, NULL,
> +                                         AC100_CLKOUT_DIV_WIDTH,
> +                                         CLK_DIVIDER_POWER_OF_TWO);
> +
> +       for (i = 0; ac100_clkout_prediv[i].div; i++) {
> +               tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[i].val);
> +               tmp_rate = divider_round_rate(hw, rate, &tmp_prate, NULL,
> +                                             AC100_CLKOUT_DIV_WIDTH,
> +                                             CLK_DIVIDER_POWER_OF_TWO);
> +
> +               if (tmp_rate > rate)
> +                       continue;
> +               if (rate - tmp_rate < best_rate - tmp_rate)
> +                       best_rate = tmp_rate;
> +       }
> +
> +       return best_rate;
> +}
> +
> +static int ac100_clkout_determine_rate(struct clk_hw *hw,
> +                                      struct clk_rate_request *req)
> +{
> +       struct clk_hw *best_parent;
> +       unsigned long best = 0;
> +       int i, num_parents = clk_hw_get_num_parents(hw);
> +
> +       for (i = 0; i < num_parents; i++) {
> +               struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
> +               unsigned long tmp, prate = clk_hw_get_rate(parent);
> +
> +               tmp = ac100_clkout_round_rate(hw, req->rate, prate);
> +
> +               if (tmp > req->rate)
> +                       continue;
> +               if (req->rate - tmp < req->rate - best) {
> +                       best = tmp;
> +                       best_parent = parent;
> +               }
> +       }
> +
> +       if (!best)
> +               return -EINVAL;
> +
> +       req->best_parent_hw = best_parent;
> +       req->best_parent_rate = best;
> +       req->rate = best;
> +
> +       return 0;
> +}

You only need one of the two functions above. Keep the .determine_rate
callback and drop .round_rate.

Otherwise the rest of the clk support looks great to me. Feel free to
add:

Reviewed-by: Michael Turquette <mturquette@baylibre.com>

Regards,
Mike
Chen-Yu Tsai July 14, 2016, 3:34 a.m. UTC | #2
On Sat, Jul 9, 2016 at 2:36 AM, Michael Turquette
<mturquette@baylibre.com> wrote:
> Quoting Chen-Yu Tsai (2016-06-30 08:58:48)
>> +static long ac100_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
>> +                                   unsigned long prate)
>> +{
>> +       unsigned long best_rate = 0, tmp_rate, tmp_prate;
>> +       int i;
>> +
>> +       if (prate == AC100_RTC_32K_RATE)
>> +               return divider_round_rate(hw, rate, &prate, NULL,
>> +                                         AC100_CLKOUT_DIV_WIDTH,
>> +                                         CLK_DIVIDER_POWER_OF_TWO);
>> +
>> +       for (i = 0; ac100_clkout_prediv[i].div; i++) {
>> +               tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[i].val);
>> +               tmp_rate = divider_round_rate(hw, rate, &tmp_prate, NULL,
>> +                                             AC100_CLKOUT_DIV_WIDTH,
>> +                                             CLK_DIVIDER_POWER_OF_TWO);
>> +
>> +               if (tmp_rate > rate)
>> +                       continue;
>> +               if (rate - tmp_rate < best_rate - tmp_rate)
>> +                       best_rate = tmp_rate;
>> +       }
>> +
>> +       return best_rate;
>> +}
>> +
>> +static int ac100_clkout_determine_rate(struct clk_hw *hw,
>> +                                      struct clk_rate_request *req)
>> +{
>> +       struct clk_hw *best_parent;
>> +       unsigned long best = 0;
>> +       int i, num_parents = clk_hw_get_num_parents(hw);
>> +
>> +       for (i = 0; i < num_parents; i++) {
>> +               struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
>> +               unsigned long tmp, prate = clk_hw_get_rate(parent);
>> +
>> +               tmp = ac100_clkout_round_rate(hw, req->rate, prate);
>> +
>> +               if (tmp > req->rate)
>> +                       continue;
>> +               if (req->rate - tmp < req->rate - best) {
>> +                       best = tmp;
>> +                       best_parent = parent;
>> +               }
>> +       }
>> +
>> +       if (!best)
>> +               return -EINVAL;
>> +
>> +       req->best_parent_hw = best_parent;
>> +       req->best_parent_rate = best;
>> +       req->rate = best;
>> +
>> +       return 0;
>> +}
>
> You only need one of the two functions above. Keep the .determine_rate
> callback and drop .round_rate.

Only .round_rate is set in the clk_ops. ac100_clkout_round_rate just
serves to split out some of the calculations done per parent.

> Otherwise the rest of the clk support looks great to me. Feel free to
> add:
>
> Reviewed-by: Michael Turquette <mturquette@baylibre.com>

Thanks!

ChenYu
Michael Turquette July 22, 2016, 12:24 a.m. UTC | #3
Quoting Chen-Yu Tsai (2016-07-13 20:34:34)
> On Sat, Jul 9, 2016 at 2:36 AM, Michael Turquette
> <mturquette@baylibre.com> wrote:
> > Quoting Chen-Yu Tsai (2016-06-30 08:58:48)
> >> +static long ac100_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
> >> +                                   unsigned long prate)
> >> +{
> >> +       unsigned long best_rate = 0, tmp_rate, tmp_prate;
> >> +       int i;
> >> +
> >> +       if (prate == AC100_RTC_32K_RATE)
> >> +               return divider_round_rate(hw, rate, &prate, NULL,
> >> +                                         AC100_CLKOUT_DIV_WIDTH,
> >> +                                         CLK_DIVIDER_POWER_OF_TWO);
> >> +
> >> +       for (i = 0; ac100_clkout_prediv[i].div; i++) {
> >> +               tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[i].val);
> >> +               tmp_rate = divider_round_rate(hw, rate, &tmp_prate, NULL,
> >> +                                             AC100_CLKOUT_DIV_WIDTH,
> >> +                                             CLK_DIVIDER_POWER_OF_TWO);
> >> +
> >> +               if (tmp_rate > rate)
> >> +                       continue;
> >> +               if (rate - tmp_rate < best_rate - tmp_rate)
> >> +                       best_rate = tmp_rate;
> >> +       }
> >> +
> >> +       return best_rate;
> >> +}
> >> +
> >> +static int ac100_clkout_determine_rate(struct clk_hw *hw,
> >> +                                      struct clk_rate_request *req)
> >> +{
> >> +       struct clk_hw *best_parent;
> >> +       unsigned long best = 0;
> >> +       int i, num_parents = clk_hw_get_num_parents(hw);
> >> +
> >> +       for (i = 0; i < num_parents; i++) {
> >> +               struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
> >> +               unsigned long tmp, prate = clk_hw_get_rate(parent);
> >> +
> >> +               tmp = ac100_clkout_round_rate(hw, req->rate, prate);
> >> +
> >> +               if (tmp > req->rate)
> >> +                       continue;
> >> +               if (req->rate - tmp < req->rate - best) {
> >> +                       best = tmp;
> >> +                       best_parent = parent;
> >> +               }
> >> +       }
> >> +
> >> +       if (!best)
> >> +               return -EINVAL;
> >> +
> >> +       req->best_parent_hw = best_parent;
> >> +       req->best_parent_rate = best;
> >> +       req->rate = best;
> >> +
> >> +       return 0;
> >> +}
> >
> > You only need one of the two functions above. Keep the .determine_rate
> > callback and drop .round_rate.
> 
> Only .round_rate is set in the clk_ops. ac100_clkout_round_rate just
> serves to split out some of the calculations done per parent.

If you only use .round_rate, shouldn't you drop .determine_rate then?

Regards,
Mike

> 
> > Otherwise the rest of the clk support looks great to me. Feel free to
> > add:
> >
> > Reviewed-by: Michael Turquette <mturquette@baylibre.com>
> 
> Thanks!
> 
> ChenYu
Chen-Yu Tsai July 22, 2016, 1:22 a.m. UTC | #4
On Fri, Jul 22, 2016 at 8:24 AM, Michael Turquette
<mturquette@baylibre.com> wrote:
> Quoting Chen-Yu Tsai (2016-07-13 20:34:34)
>> On Sat, Jul 9, 2016 at 2:36 AM, Michael Turquette
>> <mturquette@baylibre.com> wrote:
>> > Quoting Chen-Yu Tsai (2016-06-30 08:58:48)
>> >> +static long ac100_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
>> >> +                                   unsigned long prate)
>> >> +{
>> >> +       unsigned long best_rate = 0, tmp_rate, tmp_prate;
>> >> +       int i;
>> >> +
>> >> +       if (prate == AC100_RTC_32K_RATE)
>> >> +               return divider_round_rate(hw, rate, &prate, NULL,
>> >> +                                         AC100_CLKOUT_DIV_WIDTH,
>> >> +                                         CLK_DIVIDER_POWER_OF_TWO);
>> >> +
>> >> +       for (i = 0; ac100_clkout_prediv[i].div; i++) {
>> >> +               tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[i].val);
>> >> +               tmp_rate = divider_round_rate(hw, rate, &tmp_prate, NULL,
>> >> +                                             AC100_CLKOUT_DIV_WIDTH,
>> >> +                                             CLK_DIVIDER_POWER_OF_TWO);
>> >> +
>> >> +               if (tmp_rate > rate)
>> >> +                       continue;
>> >> +               if (rate - tmp_rate < best_rate - tmp_rate)
>> >> +                       best_rate = tmp_rate;
>> >> +       }
>> >> +
>> >> +       return best_rate;
>> >> +}
>> >> +
>> >> +static int ac100_clkout_determine_rate(struct clk_hw *hw,
>> >> +                                      struct clk_rate_request *req)
>> >> +{
>> >> +       struct clk_hw *best_parent;
>> >> +       unsigned long best = 0;
>> >> +       int i, num_parents = clk_hw_get_num_parents(hw);
>> >> +
>> >> +       for (i = 0; i < num_parents; i++) {
>> >> +               struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
>> >> +               unsigned long tmp, prate = clk_hw_get_rate(parent);
>> >> +
>> >> +               tmp = ac100_clkout_round_rate(hw, req->rate, prate);
>> >> +
>> >> +               if (tmp > req->rate)
>> >> +                       continue;
>> >> +               if (req->rate - tmp < req->rate - best) {
>> >> +                       best = tmp;
>> >> +                       best_parent = parent;
>> >> +               }
>> >> +       }
>> >> +
>> >> +       if (!best)
>> >> +               return -EINVAL;
>> >> +
>> >> +       req->best_parent_hw = best_parent;
>> >> +       req->best_parent_rate = best;
>> >> +       req->rate = best;
>> >> +
>> >> +       return 0;
>> >> +}
>> >
>> > You only need one of the two functions above. Keep the .determine_rate
>> > callback and drop .round_rate.
>>
>> Only .round_rate is set in the clk_ops. ac100_clkout_round_rate just
>> serves to split out some of the calculations done per parent.
>
> If you only use .round_rate, shouldn't you drop .determine_rate then?

Oops! I meant we only set .determine_rate. ac100_clkout_round_rate is
called in the per parent loop in .determine_rate.

ChenYu

>
> Regards,
> Mike
>
>>
>> > Otherwise the rest of the clk support looks great to me. Feel free to
>> > add:
>> >
>> > Reviewed-by: Michael Turquette <mturquette@baylibre.com>
>>
>> Thanks!
>>
>> ChenYu
diff mbox

Patch

diff --git a/drivers/rtc/rtc-ac100.c b/drivers/rtc/rtc-ac100.c
index 5a9ca89d04c7..70b4fd0f6122 100644
--- a/drivers/rtc/rtc-ac100.c
+++ b/drivers/rtc/rtc-ac100.c
@@ -16,6 +16,7 @@ 
  */
 
 #include <linux/bcd.h>
+#include <linux/clk-provider.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -31,6 +32,15 @@ 
 /* Control register */
 #define AC100_RTC_CTRL_24HOUR	BIT(0)
 
+/* Clock output register bits */
+#define AC100_CLKOUT_PRE_DIV_SHIFT	5
+#define AC100_CLKOUT_PRE_DIV_WIDTH	3
+#define AC100_CLKOUT_MUX_SHIFT		4
+#define AC100_CLKOUT_MUX_WIDTH		1
+#define AC100_CLKOUT_DIV_SHIFT		1
+#define AC100_CLKOUT_DIV_WIDTH		3
+#define AC100_CLKOUT_EN			BIT(0)
+
 /* RTC */
 #define AC100_RTC_SEC_MASK	GENMASK(6, 0)
 #define AC100_RTC_MIN_MASK	GENMASK(6, 0)
@@ -67,14 +77,292 @@ 
 #define AC100_YEAR_MAX				2069
 #define AC100_YEAR_OFF				(AC100_YEAR_MIN - 1900)
 
+struct ac100_clkout {
+	struct clk_hw hw;
+	struct regmap *regmap;
+	u8 offset;
+};
+
+#define to_ac100_clkout(_hw) container_of(_hw, struct ac100_clkout, hw)
+
+#define AC100_RTC_32K_NAME	"ac100-rtc-32k"
+#define AC100_RTC_32K_RATE	32768
+#define AC100_CLKOUT_NUM	3
+
+static const char * const ac100_clkout_names[AC100_CLKOUT_NUM] = {
+	"ac100-cko1-rtc",
+	"ac100-cko2-rtc",
+	"ac100-cko3-rtc",
+};
+
 struct ac100_rtc_dev {
 	struct rtc_device *rtc;
 	struct device *dev;
 	struct regmap *regmap;
 	int irq;
 	unsigned long alarm;
+
+	struct clk_hw *rtc_32k_clk;
+	struct ac100_clkout clks[AC100_CLKOUT_NUM];
+	struct clk_hw_onecell_data *clk_data;
 };
 
+/**
+ * Clock controls for 3 clock output pins
+ */
+
+static const struct clk_div_table ac100_clkout_prediv[] = {
+	{ .val = 0, .div = 1 },
+	{ .val = 1, .div = 2 },
+	{ .val = 2, .div = 4 },
+	{ .val = 3, .div = 8 },
+	{ .val = 4, .div = 16 },
+	{ .val = 5, .div = 32 },
+	{ .val = 6, .div = 64 },
+	{ .val = 7, .div = 122 },
+	{ },
+};
+
+/* Abuse the fact that one parent is 32768 Hz, and the other is 4 MHz */
+static unsigned long ac100_clkout_recalc_rate(struct clk_hw *hw,
+					      unsigned long prate)
+{
+	struct ac100_clkout *clk = to_ac100_clkout(hw);
+	unsigned int reg, div;
+
+	regmap_read(clk->regmap, clk->offset, &reg);
+
+	/* Handle pre-divider first */
+	if (prate != AC100_RTC_32K_RATE) {
+		div = (reg >> AC100_CLKOUT_PRE_DIV_SHIFT) &
+			((1 << AC100_CLKOUT_PRE_DIV_WIDTH) - 1);
+		prate = divider_recalc_rate(hw, prate, div,
+					    ac100_clkout_prediv, 0);
+	}
+
+	div = (reg >> AC100_CLKOUT_DIV_SHIFT) &
+		(BIT(AC100_CLKOUT_DIV_WIDTH) - 1);
+	return divider_recalc_rate(hw, prate, div, NULL,
+				   CLK_DIVIDER_POWER_OF_TWO);
+}
+
+static long ac100_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long prate)
+{
+	unsigned long best_rate = 0, tmp_rate, tmp_prate;
+	int i;
+
+	if (prate == AC100_RTC_32K_RATE)
+		return divider_round_rate(hw, rate, &prate, NULL,
+					  AC100_CLKOUT_DIV_WIDTH,
+					  CLK_DIVIDER_POWER_OF_TWO);
+
+	for (i = 0; ac100_clkout_prediv[i].div; i++) {
+		tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[i].val);
+		tmp_rate = divider_round_rate(hw, rate, &tmp_prate, NULL,
+					      AC100_CLKOUT_DIV_WIDTH,
+					      CLK_DIVIDER_POWER_OF_TWO);
+
+		if (tmp_rate > rate)
+			continue;
+		if (rate - tmp_rate < best_rate - tmp_rate)
+			best_rate = tmp_rate;
+	}
+
+	return best_rate;
+}
+
+static int ac100_clkout_determine_rate(struct clk_hw *hw,
+				       struct clk_rate_request *req)
+{
+	struct clk_hw *best_parent;
+	unsigned long best = 0;
+	int i, num_parents = clk_hw_get_num_parents(hw);
+
+	for (i = 0; i < num_parents; i++) {
+		struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
+		unsigned long tmp, prate = clk_hw_get_rate(parent);
+
+		tmp = ac100_clkout_round_rate(hw, req->rate, prate);
+
+		if (tmp > req->rate)
+			continue;
+		if (req->rate - tmp < req->rate - best) {
+			best = tmp;
+			best_parent = parent;
+		}
+	}
+
+	if (!best)
+		return -EINVAL;
+
+	req->best_parent_hw = best_parent;
+	req->best_parent_rate = best;
+	req->rate = best;
+
+	return 0;
+}
+
+static int ac100_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long prate)
+{
+	struct ac100_clkout *clk = to_ac100_clkout(hw);
+	int div = 0, pre_div = 0;
+
+	do {
+		div = divider_get_val(rate * ac100_clkout_prediv[pre_div].div,
+				      prate, NULL, AC100_CLKOUT_DIV_WIDTH,
+				      CLK_DIVIDER_POWER_OF_TWO);
+		if (div >= 0)
+			break;
+	} while (prate != AC100_RTC_32K_RATE &&
+		 ac100_clkout_prediv[++pre_div].div);
+
+	if (div < 0)
+		return div;
+
+	pre_div = ac100_clkout_prediv[pre_div].val;
+
+	regmap_update_bits(clk->regmap, clk->offset,
+			   ((1 << AC100_CLKOUT_DIV_WIDTH) - 1) << AC100_CLKOUT_DIV_SHIFT |
+			   ((1 << AC100_CLKOUT_PRE_DIV_WIDTH) - 1) << AC100_CLKOUT_PRE_DIV_SHIFT,
+			   (div - 1) << AC100_CLKOUT_DIV_SHIFT |
+			   (pre_div - 1) << AC100_CLKOUT_PRE_DIV_SHIFT);
+
+	return 0;
+}
+
+static int ac100_clkout_prepare(struct clk_hw *hw)
+{
+	struct ac100_clkout *clk = to_ac100_clkout(hw);
+
+	return regmap_update_bits(clk->regmap, clk->offset, AC100_CLKOUT_EN,
+				  AC100_CLKOUT_EN);
+}
+
+static void ac100_clkout_unprepare(struct clk_hw *hw)
+{
+	struct ac100_clkout *clk = to_ac100_clkout(hw);
+
+	regmap_update_bits(clk->regmap, clk->offset, AC100_CLKOUT_EN, 0);
+}
+
+static int ac100_clkout_is_prepared(struct clk_hw *hw)
+{
+	struct ac100_clkout *clk = to_ac100_clkout(hw);
+	unsigned int reg;
+
+	regmap_read(clk->regmap, clk->offset, &reg);
+
+	return reg & AC100_CLKOUT_EN;
+}
+
+static u8 ac100_clkout_get_parent(struct clk_hw *hw)
+{
+	struct ac100_clkout *clk = to_ac100_clkout(hw);
+	unsigned int reg;
+
+	regmap_read(clk->regmap, clk->offset, &reg);
+
+	return (reg >> AC100_CLKOUT_MUX_SHIFT) & 0x1;
+}
+
+static int ac100_clkout_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct ac100_clkout *clk = to_ac100_clkout(hw);
+
+	return regmap_update_bits(clk->regmap, clk->offset,
+				  BIT(AC100_CLKOUT_MUX_SHIFT),
+				  index ? BIT(AC100_CLKOUT_MUX_SHIFT) : 0);
+}
+
+static const struct clk_ops ac100_clkout_ops = {
+	.prepare	= ac100_clkout_prepare,
+	.unprepare	= ac100_clkout_unprepare,
+	.is_prepared	= ac100_clkout_is_prepared,
+	.recalc_rate	= ac100_clkout_recalc_rate,
+	.determine_rate	= ac100_clkout_determine_rate,
+	.get_parent	= ac100_clkout_get_parent,
+	.set_parent	= ac100_clkout_set_parent,
+	.set_rate	= ac100_clkout_set_rate,
+};
+
+static int ac100_rtc_register_clks(struct ac100_rtc_dev *chip)
+{
+	struct device_node *np = chip->dev->of_node;
+	const char *parents[2] = {AC100_RTC_32K_NAME};
+	int i, ret;
+
+	chip->clk_data = devm_kzalloc(chip->dev, sizeof(*chip->clk_data) +
+						 sizeof(*chip->clk_data->hws) *
+						 AC100_CLKOUT_NUM,
+						 GFP_KERNEL);
+	if (!chip->clk_data)
+		return -ENOMEM;
+
+	chip->rtc_32k_clk = clk_hw_register_fixed_rate(chip->dev,
+						       AC100_RTC_32K_NAME,
+						       NULL, 0,
+						       AC100_RTC_32K_RATE);
+	if (IS_ERR(chip->rtc_32k_clk)) {
+		ret = PTR_ERR(chip->rtc_32k_clk);
+		dev_err(chip->dev, "Failed to register RTC-32k clock: %d\n",
+			ret);
+		return ret;
+	}
+
+	parents[1] = of_clk_get_parent_name(np, 0);
+	if (!parents[1]) {
+		dev_err(chip->dev, "Failed to get ADDA 4M clock\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < AC100_CLKOUT_NUM; i++) {
+		struct ac100_clkout *clk = &chip->clks[i];
+		struct clk_init_data init = {
+			.name = ac100_clkout_names[i],
+			.ops = &ac100_clkout_ops,
+			.parent_names = parents,
+			.num_parents = ARRAY_SIZE(parents),
+			.flags = 0,
+		};
+
+		clk->regmap = chip->regmap;
+		clk->offset = AC100_CLKOUT_CTRL1 + i;
+		clk->hw.init = &init;
+
+		ret = devm_clk_hw_register(chip->dev, &clk->hw);
+		if (ret) {
+			dev_err(chip->dev, "Failed to register clk '%s': %d\n",
+				init.name, ret);
+			goto err_unregister_rtc_32k;
+		}
+
+		chip->clk_data->hws[i] = &clk->hw;
+	}
+
+	chip->clk_data->num = i;
+	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, chip->clk_data);
+	if (ret)
+		goto err_unregister_rtc_32k;
+
+	return 0;
+
+err_unregister_rtc_32k:
+	clk_unregister_fixed_rate(chip->rtc_32k_clk->clk);
+
+	return ret;
+}
+
+static void ac100_rtc_unregister_clks(struct ac100_rtc_dev *chip)
+{
+	of_clk_del_provider(chip->dev->of_node);
+	clk_unregister_fixed_rate(chip->rtc_32k_clk->clk);
+}
+
+/**
+ * RTC related bits
+ */
 static int ac100_rtc_get_time(struct device *dev, struct rtc_time *rtc_tm)
 {
 	struct ac100_rtc_dev *chip = dev_get_drvdata(dev);
@@ -300,11 +588,24 @@  static int ac100_rtc_probe(struct platform_device *pdev)
 		return PTR_ERR(chip->rtc);
 	}
 
+	ret = ac100_rtc_register_clks(chip);
+	if (ret)
+		return ret;
+
 	dev_info(&pdev->dev, "RTC enabled\n");
 
 	return 0;
 }
 
+static int ac100_rtc_remove(struct platform_device *pdev)
+{
+	struct ac100_rtc_dev *chip = platform_get_drvdata(pdev);
+
+	ac100_rtc_unregister_clks(chip);
+
+	return 0;
+}
+
 static const struct of_device_id ac100_rtc_match[] = {
 	{ .compatible = "x-powers,ac100-rtc" },
 	{ },
@@ -313,6 +614,7 @@  MODULE_DEVICE_TABLE(of, ac100_rtc_match);
 
 static struct platform_driver ac100_rtc_driver = {
 	.probe		= ac100_rtc_probe,
+	.remove		= ac100_rtc_remove,
 	.driver		= {
 		.name		= "ac100-rtc",
 		.of_match_table	= of_match_ptr(ac100_rtc_match),