On 20.8.2019 2.01, Suman Anna wrote:
Hi Tero,
On 8/19/19 4:32 AM, Tero Kristo wrote:
On 08/08/2019 08:26, Keerthy wrote:
On 07/08/19 1:18 PM, Tero Kristo wrote:
Add initial PRM (Power and Reset Management) driver for TI OMAP class
SoCs. Initially this driver only supports reset control, but can be
extended to support rest of the functionality, like powerdomain
control, PRCM irq support etc.
Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
---
arch/arm/mach-omap2/Kconfig | 1 +
drivers/soc/ti/Makefile | 1 +
drivers/soc/ti/omap_prm.c | 216
++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 218 insertions(+)
create mode 100644 drivers/soc/ti/omap_prm.c
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index fdb6743..42ad063 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -109,6 +109,7 @@ config ARCH_OMAP2PLUS
select TI_SYSC
select OMAP_IRQCHIP
select CLKSRC_TI_32K
+ select RESET_CONTROLLER
Use ARCH_HAS_RESET_CONTROLLER instead.
Ok.
help
Systems based on OMAP2, OMAP3, OMAP4 or OMAP5
diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
index b3868d3..788b5cd 100644
--- a/drivers/soc/ti/Makefile
+++ b/drivers/soc/ti/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_QMSS) += knav_qmss.o
knav_qmss-y := knav_qmss_queue.o knav_qmss_acc.o
obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA) += knav_dma.o
obj-$(CONFIG_AMX3_PM) += pm33xx.o
+obj-$(CONFIG_ARCH_OMAP2PLUS) += omap_prm.o
obj-$(CONFIG_WKUP_M3_IPC) += wkup_m3_ipc.o
obj-$(CONFIG_TI_SCI_PM_DOMAINS) += ti_sci_pm_domains.o
obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN) += ti_sci_inta_msi.o
diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c
new file mode 100644
index 0000000..7c89eb8
--- /dev/null
+++ b/drivers/soc/ti/omap_prm.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * OMAP2+ PRM driver
+ *
+ * Copyright (C) 2019 Texas Instruments Incorporated -
http://www.ti.com/
+ * Tero Kristo <t-kristo@xxxxxx>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/delay.h>
+
+struct omap_rst_map {
+ s8 rst;
+ s8 st;
+};
+
+struct omap_prm_data {
+ u32 base;
+ const char *name;
+ u16 pwstctrl;
+ u16 pwstst;
+ u16 rstctl;
Minor nit, can you use rstctrl instead here so that it is in sync with
the other variables and with the register names used in TRM.
Ok.
+ u16 rstst;
+ struct omap_rst_map *rstmap;
+ u8 flags;
+};
+
+struct omap_prm {
+ const struct omap_prm_data *data;
+ void __iomem *base;
+};
+
+struct omap_reset_data {
+ struct reset_controller_dev rcdev;
+ struct omap_prm *prm;
+};
+
+#define to_omap_reset_data(p) container_of((p), struct
omap_reset_data, rcdev)
+
+#define OMAP_MAX_RESETS 8
+#define OMAP_RESET_MAX_WAIT 10000
+
+#define OMAP_PRM_NO_RSTST BIT(0)
+
+static const struct of_device_id omap_prm_id_table[] = {
+ { },
+};
This table is blank and we are doing of_match_device against it.
Yes, it gets populated with other patches in series, one entry per soc.
+
+static int omap_reset_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct omap_reset_data *reset = to_omap_reset_data(rcdev);
+ u32 v;
+
+ v = readl_relaxed(reset->prm->base + reset->prm->data->rstst);
+ v &= 1 << id;
+ v >>= id;
+
+ return v;
+}
+
+static int omap_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct omap_reset_data *reset = to_omap_reset_data(rcdev);
+ u32 v;
+
+ /* assert the reset control line */
+ v = readl_relaxed(reset->prm->base + reset->prm->data->rstctl);
+ v |= 1 << id;
+ writel_relaxed(v, reset->prm->base + reset->prm->data->rstctl);
+
+ return 0;
+}
+
+static int omap_reset_get_st_bit(struct omap_reset_data *reset,
+ unsigned long id)
+{
+ struct omap_rst_map *map = reset->prm->data->rstmap;
+
+ while (map && map->rst >= 0) {
+ if (map->rst == id)
+ return map->st;
+
+ map++;
+ }
+
+ return id;
+}
+
+/*
+ * Note that status will not change until clocks are on, and clocks
cannot be
+ * enabled until reset is deasserted. Consumer drivers must check
status
+ * separately after enabling clocks.
+ */
+static int omap_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct omap_reset_data *reset = to_omap_reset_data(rcdev);
+ u32 v;
+ int st_bit = id;
No need to initialize this, will always get overwritten below.
Hmm right, must be a leftover from some earlier code.
+ bool has_rstst;
+
+ /* check the current status to avoid de-asserting the line twice */
+ v = readl_relaxed(reset->prm->base + reset->prm->data->rstctl);
+ if (!(v & BIT(id)))
+ return -EEXIST;
+
+ has_rstst = !(reset->prm->data->flags & OMAP_PRM_NO_RSTST);
+
+ if (has_rstst) {
+ st_bit = omap_reset_get_st_bit(reset, id);
+
+ /* Clear the reset status by writing 1 to the status bit */
+ v = readl_relaxed(reset->prm->base + reset->prm->data->rstst);
+ v |= 1 << st_bit;
+ writel_relaxed(v, reset->prm->base + reset->prm->data->rstst);
+ }
+
+ /* de-assert the reset control line */
+ v = readl_relaxed(reset->prm->base + reset->prm->data->rstctl);
+ v &= ~(1 << id);
+ writel_relaxed(v, reset->prm->base + reset->prm->data->rstctl);
+
+ return 0;
+}
+
+static const struct reset_control_ops omap_reset_ops = {
+ .assert = omap_reset_assert,
+ .deassert = omap_reset_deassert,
+ .status = omap_reset_status,
+};
+
+static int omap_prm_reset_probe(struct platform_device *pdev,
+ struct omap_prm *prm)
Call this omap_prm_reset_init or something similar to avoid confusion.
Ok, can change this.
+{
+ struct omap_reset_data *reset;
+
+ /*
+ * Check if we have resets. If either rstctl or rstst is
+ * non-zero, we have reset registers in place. Additionally
+ * the flag OMAP_PRM_NO_RSTST implies that we have resets.
+ */
+ if (!prm->data->rstctl && !prm->data->rstst &&
+ !(prm->data->flags & OMAP_PRM_NO_RSTST))
+ return 0;
+
+ reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
+ if (!reset)
+ return -ENOMEM;
+
+ reset->rcdev.owner = THIS_MODULE;
+ reset->rcdev.ops = &omap_reset_ops;
+ reset->rcdev.of_node = pdev->dev.of_node;
+ reset->rcdev.nr_resets = OMAP_MAX_RESETS;
Suggest adding a number of resets to prm->data, and using it so that we
don't even entertain any resets beyond the actual number of resets.
Hmm why bother? Accessing a stale reset bit will just cause access to a
reserved bit in the reset register, doing basically nothing. Also, this
would not work for am3/am4 wkup, as there is a single reset bit at an
arbitrary position.
You actually seem to be using the bit-position directly in client data
instead of a reset number. I am not sure if this is accepted practice
with reset controllers, do you incur any memory wastage?
Reset numbering almost always seems to start from 0, I think the only
exception to this is wkup_m3 on am3/am4. Introducing an additional
arbitrary mapping for this doesn't seem to make any sense.
Also, resets are allocated on-need-basis, so it only allocates one
instance for the reset control whatever the index is.
+
+ reset->prm = prm;
+
+ return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
+}
+
+static int omap_prm_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ const struct omap_prm_data *data;
+ struct omap_prm *prm;
+ const struct of_device_id *match;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+
+ match = of_match_device(omap_prm_id_table, &pdev->dev);
+ if (!match)
+ return -ENOTSUPP;
Use of_device_get_match_data() instead to do both match and get the
data. That can perhaps be the first block.
+
+ prm = devm_kzalloc(&pdev->dev, sizeof(*prm), GFP_KERNEL);
+ if (!prm)
+ return -ENOMEM;
Perhaps move the allocate after the match check to streamline.
Ok, will check these two out.
+
+ data = match->data;
+
+ while (data->base != res->start) {
+ if (!data->base)
+ return -EINVAL;
+ data++;
+ }
+
+ prm->data = data;
+
+ prm->base = devm_ioremap_resource(&pdev->dev, res);
+ if (!prm->base)
+ return -ENOMEM;
+
+ return omap_prm_reset_probe(pdev, prm);
+}
+
+static struct platform_driver omap_prm_driver = {
+ .probe = omap_prm_probe,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = omap_prm_id_table,
+ },
+};
+builtin_platform_driver(omap_prm_driver);
+
+MODULE_ALIAS("platform:prm");
Drop this and use MODULE_DEVICE_TABLE instead on omap_prm_id_table if
retaining, but I don't think these need to be defined.
Ok, will ditch them.
-Tero
regards
Suman
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("omap2+ prm driver");
It is a builtin_platform_driver so do we need the MODULE*?
Well, thats debatable, however some existing drivers do introduce this
even if they are builtin.
-Tero
--
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
