On 07/21/2015 04:07 AM, Vaibhav Hiremath wrote:
diff --git a/drivers/clk/clk-88pm800.c b/drivers/clk/clk-88pm800.c
new file mode 100644
index 0000000..cf1c162
--- /dev/null
+++ b/drivers/clk/clk-88pm800.c
@@ -0,0 +1,345 @@
+/*
+ * clk-88pm800.c - Clock driver for 88PM800 family of devices
+ *
+ * This driver is created based on clk-s2mps11.c
+ *
+ * Copyright (C) 2015 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/regmap.h>
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/88pm80x.h>
+
+/* Number of clocks output from 88pm800 family of device */
+enum {
+       PM800_CLK32K_1 = 0,
+       PM800_CLK32K_2,
+       PM800_CLK32K_3,
+       PM800_CLKS_NUM,
+};
+
+struct pm800_clk {
+       struct pm80x_chip *chip;
+       struct device_node *clk_np;
+       struct clk_hw hw;
+       struct clk *clk;
+       struct clk_lookup *lookup;
+       u32 mask;
+       u32 shift;
+       unsigned int reg;
+};
+
+static struct pm800_clk *to_pm800_clk(struct clk_hw *hw)
+{
+       return container_of(hw, struct pm800_clk, hw);
+}
+
+static int pm800_clk_prepare(struct clk_hw *hw)
+{
+       struct pm800_clk *pm800 = to_pm800_clk(hw);
+       int ret;
+
+       /* We always want to use XO clock */
+       ret = regmap_update_bits(pm800->chip->regmap,
+                                pm800->reg,
+                                pm800->mask,
+                                PM800_32K_OUTX_SEL_XO_32KHZ << pm800->shift);
+
+       return ret;

Can be simplified to:

    return regmap_update_bits()

+}
+
+static void pm800_clk_unprepare(struct clk_hw *hw)
+{
+       struct pm800_clk *pm800 = to_pm800_clk(hw);
+       int ret;
+
+       ret = regmap_update_bits(pm800->chip->regmap, pm800->reg,
+                               pm800->mask, ~pm800->mask);

Why have ret at all in a void function?

+}
+
+static int pm800_clk_is_prepared(struct clk_hw *hw)
+{
+       int ret;
+       u32 val;
+       struct pm800_clk *pm800 = to_pm800_clk(hw);
+
+       ret = regmap_read(pm800->chip->regmap,
+                               pm800->reg, &val);
+       if (ret < 0)
+               return -EINVAL;
+
+       return ((val & pm800->mask) >> pm800->shift);

One too many parentheses here.

+}
+
+static unsigned long pm800_clk_recalc_rate(struct clk_hw *hw,
+                                            unsigned long parent_rate)
+{
+       return 32768;
+}
+
+static struct clk_ops pm800_clk_ops = {

const?

+       .prepare        = pm800_clk_prepare,
+       .unprepare      = pm800_clk_unprepare,
+       .is_prepared    = pm800_clk_is_prepared,
+       .recalc_rate    = pm800_clk_recalc_rate,
+};
+
+static struct clk_init_data pm800_clks_init[PM800_CLKS_NUM] = {
+       [PM800_CLK32K_1] = {
+               .name   = "pm800_clk32k_1",
+               .ops    = &pm800_clk_ops,
+               .flags  = CLK_IS_ROOT,
+       },
+       [PM800_CLK32K_2] = {
+               .name   = "pm800_clk32k_2",
+               .ops    = &pm800_clk_ops,
+               .flags  = CLK_IS_ROOT,
+       },
+       [PM800_CLK32K_3] = {
+               .name   = "pm800_clk32k_3",
+               .ops    = &pm800_clk_ops,
+               .flags  = CLK_IS_ROOT,
+       },
+};
+
+static struct clk_init_data pm860_clks_init[PM800_CLKS_NUM] = {
+       [PM800_CLK32K_1] = {
+               .name   = "pm800_clk32k_1",
+               .ops    = &pm800_clk_ops,
+               .flags  = CLK_IS_ROOT,
+       },
+       [PM800_CLK32K_2] = {
+               .name   = "pm800_clk32k_2",
+               .ops    = &pm800_clk_ops,
+               .flags  = CLK_IS_ROOT,
+       },
+};
+
+static int pm800_init_clk(struct pm800_clk *pm800_clks)
+{
+
+       int ret;
+
+       /* Enable XO_LJ : Low jitter clock of 32KHz from XO */
+       ret = regmap_update_bits(pm800_clks->chip->regmap, PM800_LOW_POWER2,
+                       PM800_LOW_POWER2_XO_LJ_EN, PM800_LOW_POWER2_XO_LJ_EN);
+       if (ret)
+               return ret;
+
+       /* Enable USE_XO : Use XO clock for all internal timing reference */
+       ret = regmap_update_bits(pm800_clks->chip->regmap, PM800_RTC_CONTROL,
+                       PM800_RTC1_USE_XO, PM800_RTC1_USE_XO);
+       if (ret)
+               return ret;
+
+       /* OSC_FREERUN: Enable Osc free running mode by clearing the bit */
+       ret = regmap_update_bits(pm800_clks->chip->regmap, PM800_OSC_CNTRL1,
+                       PM800_OSC_CNTRL1_OSC_FREERUN_EN, 0);
+       if (ret)
+               return ret;
+
+       return 0;
+}
+
+static struct device_node *pm800_clk_parse_dt(struct platform_device *pdev,
+               struct clk_init_data *clks_init)
+{
+       struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
+       struct device_node *clk_np;
+       int i;
+
+       if (!chip->dev->of_node)
+               return ERR_PTR(-EINVAL);
+
+       clk_np = of_get_child_by_name(chip->dev->of_node, "clocks");
+       if (!clk_np) {
+               dev_err(&pdev->dev, "could not find clock sub-node\n");
+               return ERR_PTR(-EINVAL);
+       }
+
+       for (i = 0; i < PM800_CLKS_NUM; i++) {
+               if (!clks_init[i].name)
+                       continue; /* Skip clocks not present in some devices */
+
+               of_property_read_string_index(clk_np, "clock-output-names", i,
+                               &clks_init[i].name);
+       }
+
+       return clk_np;
+}
+
+static int pm800_clk_probe(struct platform_device *pdev)
+{
+       struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
+       struct pm800_clk *pm800_clks;
+       struct clk_init_data *clks_init;
+       static struct clk **clk_table;
+       static struct clk_onecell_data *of_clk_data;
+       int i, ret = 0;

Drop assignment to ret please.

+
+       pm800_clks = devm_kzalloc(&pdev->dev,
+                               sizeof(*pm800_clks) * PM800_CLKS_NUM,
+                               GFP_KERNEL);

devm_kcalloc()

+       if (!pm800_clks)
+               return -ENOMEM;
+
+       clk_table = devm_kzalloc(&pdev->dev,
+                               sizeof(struct clk *) * PM800_CLKS_NUM,
+                               GFP_KERNEL);

devm_kcalloc()

+       if (!clk_table)
+               return -ENOMEM;
+
+       switch (platform_get_device_id(pdev)->driver_data) {
+       case CHIP_PM800:
+               clks_init = pm800_clks_init;
+               break;
+       case CHIP_PM860:
+               clks_init = pm860_clks_init;
+               break;
+       default:
+               dev_err(&pdev->dev, "Invalid device type\n");
+               return -EINVAL;
+       }
+
+       /* Store clocks of_node in first element of pm800_clks array */
+       pm800_clks->clk_np = pm800_clk_parse_dt(pdev, clks_init);
+       if (IS_ERR(pm800_clks->clk_np))
+               return PTR_ERR(pm800_clks->clk_np);
+
+       of_clk_data = devm_kzalloc(&pdev->dev,
+                               sizeof(*of_clk_data),
+                               GFP_KERNEL);
+       if (!of_clk_data)
+               return -ENOMEM;
+
+       for (i = 0; i < PM800_CLKS_NUM; i++) {
+               if (!clks_init[i].name)
+                       continue; /* Skip clocks not present in some devices */
+
+               pm800_clks[i].chip = chip;
+               pm800_clks[i].hw.init = &clks_init[i];
+               /*
+                * As of now, mask and shift formula below works for both
+                * 88PM800 and it's family of devices,
+                *
+                *  PM800_RTC_MISC2:
+                *      1:0     = CK_32K_OUT1_SEL
+                *      3:2     = CK_32K_OUT2_SEL
+                *      5:4     = CK_32K_OUT3_SEL
+                */
+               pm800_clks[i].shift = (i * 2);

Drop parentheses please.

+               pm800_clks[i].mask = PM800_32K_OUTX_SEL_MASK << 
pm800_clks[i].shift;
+               pm800_clks[i].reg = PM800_RTC_MISC2;
+
+               pm800_clks[i].clk = devm_clk_register(&pdev->dev,
+                                                       &pm800_clks[i].hw);
+               if (IS_ERR(pm800_clks[i].clk)) {
+                       dev_err(&pdev->dev, "Fail to register : %s\n",
+                                               clks_init[i].name);
+                       ret = PTR_ERR(pm800_clks[i].clk);
+                       goto err;
+               }
+
+               pm800_clks[i].lookup = clkdev_create(pm800_clks[i].clk,
+                                       clks_init[i].name, NULL);
+               if (!pm800_clks[i].lookup) {
+                       ret = -ENOMEM;
+                       goto err;
+               }
+
+               clk_table[i] = pm800_clks[i].clk;
+       }
+
+       of_clk_data->clks = clk_table;
+       of_clk_data->clk_num = PM800_CLKS_NUM;
+       ret = of_clk_add_provider(pm800_clks->clk_np, of_clk_src_onecell_get,
+                       of_clk_data);
+       if (ret) {
+               dev_err(&pdev->dev, "Fail to add OF clk provider : %d\n", ret);
+               goto err;
+       }
+
+       /* Common for all 32KHz clock output */
+       ret = pm800_init_clk(&pm800_clks[0]);

Shouldn't we do this before registering the clocks with the framework?

+       if (ret) {
+               dev_err(&pdev->dev, "Failed to initialize clk : %d\n", ret);
+               goto err;
+       }
+
+       platform_set_drvdata(pdev, pm800_clks);
+
+       return 0;
+
+err:
+       for (i = 0; i < PM800_CLKS_NUM; i++) {
+               if (pm800_clks[i].lookup)
+                       clkdev_drop(pm800_clks[i].lookup);
+       }
+
+       return ret;
+}
+
+static int pm800_clk_remove(struct platform_device *pdev)
+{
+       struct pm800_clk *pm800_clks = platform_get_drvdata(pdev);
+       int i;
+
+       of_clk_del_provider(pm800_clks[0].clk_np);
+       /* Drop the reference obtained in pm800_clk_parse_dt */
+       of_node_put(pm800_clks[0].clk_np);

This is odd. Why are we keeping the reference in the driver?

+
+       for (i = 0; i < PM800_CLKS_NUM; i++) {
+               if (pm800_clks[i].lookup)
+                       clkdev_drop(pm800_clks[i].lookup);
+       }
+
+       return 0;
+}
+
[..]
+
+static int __init pm800_clk_init(void)
+{
+       return platform_driver_register(&pm800_clk_driver);
+}
+subsys_initcall(pm800_clk_init);
+
+static void __init pm800_clk_cleanup(void)

s/__init/__exit/

+{
+       platform_driver_unregister(&pm800_clk_driver);
+}
+module_exit(pm800_clk_cleanup);
+
+MODULE_DESCRIPTION("88PM800 Clock Driver");
+MODULE_AUTHOR("Vaibhav Hiremath <vaibhav.hirem...@linaro.org>");
+MODULE_LICENSE("GPL");


--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to