diff mbox series

rtc: pcf85063: Add pcf85063 clkout control to common clock framework

Message ID 20190902043727.GA24684@michael-Latitude-5590
State Changes Requested
Headers show
Series rtc: pcf85063: Add pcf85063 clkout control to common clock framework | expand

Commit Message

Michael McCormick Sept. 2, 2019, 4:37 a.m. UTC
Signed-off-by: Michael McCormick <michael.mccormick@enatel.net>
---
 drivers/rtc/rtc-pcf85063.c | 153 +++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)

--
2.17.1



**CONFIDENTIALITY STATEMENT**
This message is intended for the sole use of the individual(s) and/or entity to whom it is addressed, and may contain information that is legally privileged, confidential, and exempt from disclosure under applicable law. If you are not the intended addressee, nor authorized to receive for the intended addressee, you are hereby notified that dissemination, distribution, copying or disclosure of this message is strictly prohibited. If you have received this message in error please immediately advise the sender by reply email, and delete the message.

Comments

Alexandre Belloni Jan. 22, 2020, 10:07 p.m. UTC | #1
Hi,

I'm sorry for the very late review, please Cc me on RTC patches.

It seems the patch has been mangled and tabs became spaces. Also, a few
alignments are off.

On 02/09/2019 16:37:27+1200, Michael McCormick wrote:
> Signed-off-by: Michael McCormick <michael.mccormick@enatel.net>
> ---
>  drivers/rtc/rtc-pcf85063.c | 153 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
> index 1afa6d9fa9fb..f47d3a6b997d 100644
> --- a/drivers/rtc/rtc-pcf85063.c
> +++ b/drivers/rtc/rtc-pcf85063.c
> @@ -9,6 +9,7 @@
>   * Copyright (C) 2019 Micro Crystal AG
>   * Author: Alexandre Belloni <alexandre.belloni@bootlin.com>
>   */
> +#include <linux/clk-provider.h>
>  #include <linux/i2c.h>
>  #include <linux/bcd.h>
>  #include <linux/rtc.h>
> @@ -44,6 +45,16 @@
>  #define PCF85063_OFFSET_STEP0          4340
>  #define PCF85063_OFFSET_STEP1          4069
> 
> +#define PCF85063_REG_CLKO_F_MASK       0x07 /* frequency mask */
> +#define PCF85063_REG_CLKO_F_32768HZ    0x00
> +#define PCF85063_REG_CLKO_F_16384HZ    0x01
> +#define PCF85063_REG_CLKO_F_8192HZ     0x02
> +#define PCF85063_REG_CLKO_F_4096HZ     0x03
> +#define PCF85063_REG_CLKO_F_2048HZ     0x04
> +#define PCF85063_REG_CLKO_F_1024HZ     0x05
> +#define PCF85063_REG_CLKO_F_1HZ                0x06

Most of those defines are not used, I don't think they are necessary.

> +static int pcf85063_clkout_control(struct clk_hw *hw, bool enable)
> +{
> +       struct pcf85063 *pcf85063 = clkout_hw_to_pcf85063(hw);
> +       unsigned char buf;
> +
> +       if (enable)
> +               buf = PCF85063_REG_CLKO_F_32768HZ;

This doesn't work properly if the clock was already enabled and had a
proper rate already set. I think you need to read out the currently set
rate and update PCF85063_REG_CTRL2 only if PCF85063_REG_CLKO_F_OFF is
set.

> +       else
> +               buf = PCF85063_REG_CLKO_F_OFF;
> +
> +       return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
> +                                 PCF85063_REG_CLKO_F_MASK, buf);
> +}
> +
> +static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063)
> +{
> +       struct clk *clk;
> +       struct clk_init_data init;
> +       int ret;
> +
> +       /* disable the clkout output */
> +       ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
> +                                PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF);

This has to be left to the common clock framework to figure that nothing
is using this clock and disable it. I guess this is the whole point of
this patch but you can't hardcode it here because this will introduce a
glitch for all the actual users of the clock.
diff mbox series

Patch

diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
index 1afa6d9fa9fb..f47d3a6b997d 100644
--- a/drivers/rtc/rtc-pcf85063.c
+++ b/drivers/rtc/rtc-pcf85063.c
@@ -9,6 +9,7 @@ 
  * Copyright (C) 2019 Micro Crystal AG
  * Author: Alexandre Belloni <alexandre.belloni@bootlin.com>
  */
+#include <linux/clk-provider.h>
 #include <linux/i2c.h>
 #include <linux/bcd.h>
 #include <linux/rtc.h>
@@ -44,6 +45,16 @@ 
 #define PCF85063_OFFSET_STEP0          4340
 #define PCF85063_OFFSET_STEP1          4069

+#define PCF85063_REG_CLKO_F_MASK       0x07 /* frequency mask */
+#define PCF85063_REG_CLKO_F_32768HZ    0x00
+#define PCF85063_REG_CLKO_F_16384HZ    0x01
+#define PCF85063_REG_CLKO_F_8192HZ     0x02
+#define PCF85063_REG_CLKO_F_4096HZ     0x03
+#define PCF85063_REG_CLKO_F_2048HZ     0x04
+#define PCF85063_REG_CLKO_F_1024HZ     0x05
+#define PCF85063_REG_CLKO_F_1HZ                0x06
+#define PCF85063_REG_CLKO_F_OFF                0x07
+
 #define PCF85063_REG_RAM               0x03

 #define PCF85063_REG_SC                        0x04 /* datetime */
@@ -61,6 +72,9 @@  struct pcf85063_config {
 struct pcf85063 {
        struct rtc_device       *rtc;
        struct regmap           *regmap;
+#ifdef CONFIG_COMMON_CLK
+       struct clk_hw           clkout_hw;
+#endif
 };

 static int pcf85063_rtc_read_time(struct device *dev, struct rtc_time *tm)
@@ -369,6 +383,140 @@  static int pcf85063_load_capacitance(struct pcf85063 *pcf85063,
                                  PCF85063_REG_CTRL1_CAP_SEL, reg);
 }

+#ifdef CONFIG_COMMON_CLK
+/*
+ * Handling of the clkout
+ */
+
+#define clkout_hw_to_pcf85063(_hw) container_of(_hw, struct pcf85063, clkout_hw)
+
+static int clkout_rates[] = {
+       32768,
+       16384,
+       8192,
+       4096,
+       2048,
+       1024,
+       1,
+};
+
+static unsigned long pcf85063_clkout_recalc_rate(struct clk_hw *hw,
+                                                unsigned long parent_rate)
+{
+       struct pcf85063 *pcf85063 = clkout_hw_to_pcf85063(hw);
+       unsigned int buf;
+       int ret = regmap_read(pcf85063->regmap, PCF85063_REG_CTRL2, &buf);
+
+       if (ret < 0)
+               return 0;
+
+       buf &= PCF85063_REG_CLKO_F_MASK;
+       return clkout_rates[buf];
+}
+
+static long pcf85063_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
+                                      unsigned long *prate)
+{
+       int i;
+
+       for (i = 0; i < ARRAY_SIZE(clkout_rates); i++)
+               if (clkout_rates[i] <= rate)
+                       return clkout_rates[i];
+
+       return 0;
+}
+
+static int pcf85063_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
+                                   unsigned long parent_rate)
+{
+       struct pcf85063 *pcf85063 = clkout_hw_to_pcf85063(hw);
+       int i;
+
+       for (i = 0; i < ARRAY_SIZE(clkout_rates); i++)
+               if (clkout_rates[i] == rate)
+                       return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
+                                                 PCF85063_REG_CLKO_F_MASK, i);
+
+       return -EINVAL;
+}
+
+static int pcf85063_clkout_control(struct clk_hw *hw, bool enable)
+{
+       struct pcf85063 *pcf85063 = clkout_hw_to_pcf85063(hw);
+       unsigned char buf;
+
+       if (enable)
+               buf = PCF85063_REG_CLKO_F_32768HZ;
+       else
+               buf = PCF85063_REG_CLKO_F_OFF;
+
+       return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
+                                 PCF85063_REG_CLKO_F_MASK, buf);
+}
+
+static int pcf85063_clkout_prepare(struct clk_hw *hw)
+{
+       return pcf85063_clkout_control(hw, 1);
+}
+
+static void pcf85063_clkout_unprepare(struct clk_hw *hw)
+{
+       pcf85063_clkout_control(hw, 0);
+}
+
+static int pcf85063_clkout_is_prepared(struct clk_hw *hw)
+{
+       struct pcf85063 *pcf85063 = clkout_hw_to_pcf85063(hw);
+       unsigned int buf;
+       int ret = regmap_read(pcf85063->regmap, PCF85063_REG_CTRL2, &buf);
+
+       if (ret < 0)
+               return 0;
+
+       return (buf & PCF85063_REG_CLKO_F_MASK) != PCF85063_REG_CLKO_F_OFF;
+}
+
+static const struct clk_ops pcf85063_clkout_ops = {
+       .prepare = pcf85063_clkout_prepare,
+       .unprepare = pcf85063_clkout_unprepare,
+       .is_prepared = pcf85063_clkout_is_prepared,
+       .recalc_rate = pcf85063_clkout_recalc_rate,
+       .round_rate = pcf85063_clkout_round_rate,
+       .set_rate = pcf85063_clkout_set_rate,
+};
+
+static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063)
+{
+       struct clk *clk;
+       struct clk_init_data init;
+       int ret;
+
+       /* disable the clkout output */
+       ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
+                                PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF);
+       if (ret < 0)
+               return ERR_PTR(ret);
+
+       init.name = "pcf85063-clkout";
+       init.ops = &pcf85063_clkout_ops;
+       init.flags = 0;
+       init.parent_names = NULL;
+       init.num_parents = 0;
+       pcf85063->clkout_hw.init = &init;
+
+       /* optional override of the clockname */
+       of_property_read_string(pcf85063->rtc->dev.of_node, "clock-output-names", &init.name);
+
+       /* register the clock */
+       clk = devm_clk_register(&pcf85063->rtc->dev, &pcf85063->clkout_hw);
+
+       if (!IS_ERR(clk))
+               of_clk_add_provider(pcf85063->rtc->dev.of_node, of_clk_src_simple_get, clk);
+
+       return clk;
+}
+#endif
+
 static const struct pcf85063_config pcf85063a_config = {
        .regmap = {
                .reg_bits = 8,
@@ -469,6 +617,11 @@  static int pcf85063_probe(struct i2c_client *client)
        nvmem_cfg.priv = pcf85063->regmap;
        rtc_nvmem_register(pcf85063->rtc, &nvmem_cfg);

+#ifdef CONFIG_COMMON_CLK
+       /* register clk in common clk framework */
+       pcf85063_clkout_register_clk(pcf85063);
+#endif
+
        return rtc_register_device(pcf85063->rtc);
 }