Minor Nit picks,
On 20/03/15 06:21, Rajendra Nayak wrote:
From: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
GDSCs (Global Distributed Switch Controllers) are responsible for
safely collapsing and restoring power to peripherals in the SoC.
These are best modelled as power domains using genpd and given
the registers are scattered throughout the clock controller register
space, its best to have the support added through the clock driver.
Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
Signed-off-by: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx>
---
drivers/clk/qcom/Kconfig | 5 ++
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/gdsc.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/qcom/gdsc.h | 46 +++++++++++++
4 files changed, 218 insertions(+)
create mode 100644 drivers/clk/qcom/gdsc.c
create mode 100644 drivers/clk/qcom/gdsc.h
...
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/export.h>
?? Do you need this?
+#include <linux/jiffies.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include "gdsc.h"
+
+#define PWR_ON_MASK BIT(31)
+#define EN_REST_WAIT_MASK GENMASK(23, 20)
+#define EN_FEW_WAIT_MASK GENMASK(19, 16)
+#define CLK_DIS_WAIT_MASK GENMASK(15, 12)
+#define SW_OVERRIDE_MASK BIT(2)
+#define HW_CONTROL_MASK BIT(1)
+#define SW_COLLAPSE_MASK BIT(0)
+
+/* Wait 2^n CXO cycles between all states. Here, n=2 (4 cycles). */
+#define EN_REST_WAIT_VAL (0x2 << 20)
+#define EN_FEW_WAIT_VAL (0x8 << 16)
+#define CLK_DIS_WAIT_VAL (0x2 << 12)
+
+#define TIMEOUT_US 100
+
+static int gdsc_is_enabled(struct gdsc *sc)
+{
+ u32 val;
+ int ret;
+
+ ret = regmap_read(sc->regmap, sc->gdscr, &val);
+ if (ret)
+ return ret;
Line after this would be good.
+ return !!(val & PWR_ON_MASK);
+}
+
+static int gdsc_toggle_logic(struct gdsc *sc, bool en)
+{
+ int ret;
+ u32 val = en ? 0 : SW_COLLAPSE_MASK;
+ u32 check = en ? PWR_ON_MASK : 0;
+ unsigned long timeout;
+
+ ret = regmap_update_bits(sc->regmap, sc->gdscr, SW_COLLAPSE_MASK, val);
+ if (ret)
+ return ret;
+
+ timeout = jiffies + usecs_to_jiffies(TIMEOUT_US);
+ do {
+ ret = regmap_read(sc->regmap, sc->gdscr, &val);
+ if (ret)
+ return ret;
+ if ((val & PWR_ON_MASK) == check)
+ return 0;
+ } while (time_before(jiffies, timeout));
+
+ ret = regmap_read(sc->regmap, sc->gdscr, &val);
+ if (ret)
+ return ret;
New line here.
+ if ((val & PWR_ON_MASK) == check)
+ return 0;
+
+ return -ETIMEDOUT;
+}
+
...
+
+int gdsc_register(struct device *dev, struct gdsc **scs, size_t num,
+ struct regmap *regmap)
+{
+ int i, ret;
+ struct genpd_onecell_data *data;
+
+ if (!num || !scs || !dev || !dev->of_node)
+ return 0;
Should it not return -EINVAL here?
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->domains = devm_kzalloc(dev, sizeof(*data->domains) * num,
+ GFP_KERNEL);
+ if (!data->domains)
+ return -ENOMEM;
+
+ data->num_domains = num;
+ for (i = 0; i < num; i++) {
+ if (!scs[i])
+ continue;
+ scs[i]->regmap = regmap;
+ ret = gdsc_init(scs[i]);
+ if (ret)
+ return ret;
+ data->domains[i] = &scs[i]->pd;
+ }
New line?
+ return of_genpd_add_provider_onecell(dev->of_node, data);
+}
+
+void gdsc_unregister(struct device *dev)
+{
+ of_genpd_del_provider(dev->of_node);
+}
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
new file mode 100644
index 0000000..02e2990
--- /dev/null
+++ b/drivers/clk/qcom/gdsc.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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.
+ */
+
+#ifndef __QCOM_GDSC_H__
+#define __QCOM_GDSC_H__
+
+#include <linux/pm_domain.h>
+
+struct regmap;
+
+/**
+ * struct gdsc - Globally Distributed Switch Controller
+ * @pd: generic power domain
+ * @regmap: regmap for MMIO accesses
+ * @gdscr: gsdc control register
+ */
+struct gdsc {
+ struct generic_pm_domain pd;
+ struct regmap *regmap;
+ unsigned int gdscr;
+};
+
+#define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)
+
+#ifdef CONFIG_QCOM_GDSC
+int gdsc_register(struct device *, struct gdsc **, size_t n, struct regmap *);
+void gdsc_unregister(struct device *);
+#else
+int gdsc_register(struct device *d, struct gdsc **g, size_t n, struct regmap *r)
static inline here??? else it would result in muliple definitions?
+{
+ return 0;
return -ENOSYS; I
+}
+
+void gdsc_unregister(struct device *d) {};
same static inline...
Braces should be in next line too.
+#endif /* CONFIG_QCOM_GDSC */
+#endif /* __QCOM_GDSC_H__ */
--srini
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
