2024-04-11 05:23:44

by Tony Lindgren

[permalink] [raw]
Subject: [RFC PATCH 0/4] Provide interconnect resets for ti-sysc users

Hi all,

Here are a few experimental WIP patches to make ti-sysc provide resets for
the devices connected to the interconnect. I've only tested this with
8250_omap.

I played with implementing all the resets automatically where available,
but we could of course map just the reset used via devicetree.

There are lots of resets, and not that many users. So likely using the
devicetree to map only the used resets makes most sense from memory
consumption point of view.

However, the reset control framework changes may be desired though.
For example, MFD child devices may not get the data via devicetree.

Note that for ti-sysc driver, this series depends on an earlier pending
clean-up series posted at [0].

Regards,

Tony

[0] https://lore.kernel.org/linux-omap/[email protected]/T/#md369ba556149a2662f2cd5413863d29f054b27b8

Tony Lindgren (4):
reset: Fall back to lookup if no reset node is found
reset: Allow removing a lookup
bus: ti-sysc: Implement reset control framework for soft reset
serial: 8250: omap: Use reset control for resets

drivers/bus/ti-sysc.c | 109 ++++++++++++++++++++++++++++
drivers/reset/core.c | 36 ++++++++-
drivers/tty/serial/8250/8250_omap.c | 66 ++++++-----------
include/linux/reset-controller.h | 7 ++
4 files changed, 174 insertions(+), 44 deletions(-)

--
2.44.0


2024-04-11 05:24:28

by Tony Lindgren

[permalink] [raw]
Subject: [RFC PATCH 3/4] bus: ti-sysc: Implement reset control framework for soft reset

We can implement reset control framework for ti-sysc for the connected
devices to use for the interconnect target reset.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/bus/ti-sysc.c | 109 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 109 insertions(+)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -25,6 +25,7 @@
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
#include <linux/reset.h>
+#include <linux/reset-controller.h>
#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/slab.h>
@@ -42,6 +43,7 @@

#define SOC_FLAG(match, flag) { .machine = match, .data = (void *)(flag), }

+#define TI_SYSC_SOFTRESET_ID 0
#define MAX_MODULE_SOFTRESET_WAIT 10000

enum sysc_soc {
@@ -79,6 +81,11 @@ struct sysc_soc_info {
struct notifier_block nb;
};

+struct sysc_reset_lookup {
+ struct list_head node;
+ struct reset_control_lookup lookup;
+};
+
enum sysc_clocks {
SYSC_FCK,
SYSC_ICK,
@@ -147,6 +154,9 @@ struct sysc {
const char **clock_roles;
int nr_clocks;
struct reset_control *rsts;
+ struct reset_controller_dev rcdev;
+ struct list_head child_resets;
+ struct mutex child_lock; /* child device data list lock */
const char *legacy_mode;
const struct sysc_capabilities *cap;
struct sysc_config cfg;
@@ -2194,6 +2204,46 @@ static int sysc_reset(struct sysc *ddata)
return error;
}

+/*
+ * Only handles the softreset for the interconnect target, does not consider
+ * the device specific external resets. We must ensure the interconnect target
+ * is runtime PM active for the reset. And we must restore the sysconfig
+ * register after reset. Locking is currently not needed as we only touch the
+ * sysconfig register on PM runtime state changes, and no other sysconfig
+ * register access happens when the interconnect target is runtime PM active.
+ * Interconnect targets with multiple children must coordinate the reset
+ * usage with reset_control_get_shared().
+ */
+static int ti_sysc_reset_control_reset(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct sysc *ddata;
+ int error;
+
+ if (id != TI_SYSC_SOFTRESET_ID)
+ return -EINVAL;
+
+ ddata = container_of(rcdev, struct sysc, rcdev);
+
+ error = pm_runtime_resume_and_get(ddata->dev);
+ if (error < 0)
+ return error;
+
+ error = sysc_reset(ddata);
+ if (error)
+ dev_warn(ddata->dev, "reset failed: %i\n", error);
+
+ sysc_write_sysconfig(ddata, ddata->sysconfig);
+
+ pm_runtime_put_sync(ddata->dev);
+
+ return error;
+}
+
+static const struct reset_control_ops ti_sysc_reset_ops = {
+ .reset = ti_sysc_reset_control_reset,
+};
+
/*
* At this point the module is configured enough to read the revision but
* module may not be completely configured yet to use PM runtime. Enable
@@ -2408,6 +2458,45 @@ static int sysc_child_add_clocks(struct sysc *ddata,
return 0;
}

+static int sysc_child_add_reset(struct sysc *ddata,
+ struct device *child)
+{
+ struct sysc_reset_lookup *srl;
+
+ srl = kzalloc(sizeof(*srl), GFP_KERNEL);
+ if (!srl)
+ return -ENOMEM;
+
+ srl->lookup.provider = dev_name(ddata->dev);
+ srl->lookup.index = TI_SYSC_SOFTRESET_ID;
+ srl->lookup.dev_id = dev_name(child);
+ srl->lookup.con_id = "softreset";
+ reset_controller_add_lookup(&srl->lookup, 1);
+ mutex_lock(&ddata->child_lock);
+ list_add(&srl->node, &ddata->child_resets);
+ mutex_unlock(&ddata->child_lock);
+
+ return 0;
+}
+
+static void sysc_child_remove_reset(struct sysc *ddata,
+ struct device *child)
+{
+ struct sysc_reset_lookup *srl;
+
+ mutex_lock(&ddata->child_lock);
+ list_for_each_entry(srl, &ddata->child_resets, node) {
+ if (srl->lookup.index == TI_SYSC_SOFTRESET_ID &&
+ !strcmp(dev_name(child), srl->lookup.dev_id)) {
+ reset_controller_remove_lookup(&srl->lookup, 1);
+ list_del(&srl->node);
+ kfree(srl);
+ break;
+ }
+ }
+ mutex_unlock(&ddata->child_lock);
+}
+
static const struct device_type sysc_device_type = {
};

@@ -2541,6 +2630,14 @@ static int sysc_notifier_call(struct notifier_block *nb,
error = sysc_child_add_clocks(ddata, dev);
if (error)
return error;
+
+ error = sysc_child_add_reset(ddata, dev);
+ if (error)
+ return error;
+
+ break;
+ case BUS_NOTIFY_REMOVED_DEVICE:
+ sysc_child_remove_reset(ddata, dev);
break;
default:
break;
@@ -3186,6 +3283,8 @@ static int sysc_probe(struct platform_device *pdev)
ddata->offsets[SYSC_SYSCONFIG] = -ENODEV;
ddata->offsets[SYSC_SYSSTATUS] = -ENODEV;
ddata->dev = &pdev->dev;
+ mutex_init(&ddata->child_lock);
+ INIT_LIST_HEAD(&ddata->child_resets);
platform_set_drvdata(pdev, ddata);

error = sysc_init_static_data(ddata);
@@ -3266,6 +3365,16 @@ static int sysc_probe(struct platform_device *pdev)

ddata->dev->type = &sysc_device_type;

+ ddata->rcdev.owner = THIS_MODULE;
+ ddata->rcdev.nr_resets = 1;
+ ddata->rcdev.ops = &ti_sysc_reset_ops;
+ ddata->rcdev.dev = &pdev->dev;
+ ddata->rcdev.of_node = ddata->dev->of_node;
+
+ error = devm_reset_controller_register(ddata->dev, &ddata->rcdev);
+ if (error)
+ goto err;
+
if (!ddata->reserved) {
error = of_platform_populate(ddata->dev->of_node,
sysc_match_table,
--
2.44.0

2024-04-11 06:32:55

by Tony Lindgren

[permalink] [raw]
Subject: [RFC PATCH 4/4] serial: 8250: omap: Use reset control for resets

For at least am335x and omap4, we set the UART_ERRATA_CLOCK_DISABLE quirk
that ends up calling reset for the interconnect target. We can do this with
reset control framework and simplify the 8250_omap driver.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/tty/serial/8250/8250_omap.c | 66 +++++++++++------------------
1 file changed, 24 insertions(+), 42 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -29,6 +29,7 @@
#include <linux/dma-mapping.h>
#include <linux/sys_soc.h>
#include <linux/pm_domain.h>
+#include <linux/reset.h>

#include "8250.h"

@@ -147,6 +148,7 @@ struct omap8250_priv {
struct pm_qos_request pm_qos_request;
struct work_struct qos_work;
struct uart_8250_dma omap8250_dma;
+ struct reset_control *reset;
spinlock_t rx_dma_lock;
bool rx_dma_broken;
bool throttled;
@@ -1490,6 +1492,14 @@ static int omap8250_probe(struct platform_device *pdev)
priv->line = -ENODEV;
priv->latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
priv->calc_latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
+
+ if (priv->habit & UART_ERRATA_CLOCK_DISABLE) {
+ priv->reset = devm_reset_control_get_exclusive(&pdev->dev,
+ "softreset");
+ if (IS_ERR(priv->reset))
+ return PTR_ERR(priv->reset);
+ }
+
cpu_latency_qos_add_request(&priv->pm_qos_request, priv->latency);
INIT_WORK(&priv->qos_work, omap8250_uart_qos_work);

@@ -1695,47 +1705,6 @@ static void uart_write(struct omap8250_priv *priv, u32 reg, u32 val)
writel(val, priv->membase + (reg << OMAP_UART_REGSHIFT));
}

-/* TODO: in future, this should happen via API in drivers/reset/ */
-static int omap8250_soft_reset(struct device *dev)
-{
- struct omap8250_priv *priv = dev_get_drvdata(dev);
- int timeout = 100;
- int sysc;
- int syss;
-
- /*
- * At least on omap4, unused uarts may not idle after reset without
- * a basic scr dma configuration even with no dma in use. The
- * module clkctrl status bits will be 1 instead of 3 blocking idle
- * for the whole clockdomain. The softreset below will clear scr,
- * and we restore it on resume so this is safe to do on all SoCs
- * needing omap8250_soft_reset() quirk. Do it in two writes as
- * recommended in the comment for omap8250_update_scr().
- */
- uart_write(priv, UART_OMAP_SCR, OMAP_UART_SCR_DMAMODE_1);
- uart_write(priv, UART_OMAP_SCR,
- OMAP_UART_SCR_DMAMODE_1 | OMAP_UART_SCR_DMAMODE_CTL);
-
- sysc = uart_read(priv, UART_OMAP_SYSC);
-
- /* softreset the UART */
- sysc |= OMAP_UART_SYSC_SOFTRESET;
- uart_write(priv, UART_OMAP_SYSC, sysc);
-
- /* By experiments, 1us enough for reset complete on AM335x */
- do {
- udelay(1);
- syss = uart_read(priv, UART_OMAP_SYSS);
- } while (--timeout && !(syss & OMAP_UART_SYSS_RESETDONE));
-
- if (!timeout) {
- dev_err(dev, "timed out waiting for reset done\n");
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
static int omap8250_runtime_suspend(struct device *dev)
{
struct omap8250_priv *priv = dev_get_drvdata(dev);
@@ -1747,7 +1716,20 @@ static int omap8250_runtime_suspend(struct device *dev)
if (priv->habit & UART_ERRATA_CLOCK_DISABLE) {
int ret;

- ret = omap8250_soft_reset(dev);
+ /*
+ * At least on omap4, unused uarts may not idle after reset without
+ * a basic scr dma configuration even with no dma in use. The
+ * module clkctrl status bits will be 1 instead of 3 blocking idle
+ * for the whole clockdomain. The softreset below will clear scr,
+ * and we restore it on resume so this is safe to do on all SoCs
+ * needing omap8250_soft_reset() quirk. Do it in two writes as
+ * recommended in the comment for omap8250_update_scr().
+ */
+ uart_write(priv, UART_OMAP_SCR, OMAP_UART_SCR_DMAMODE_1);
+ uart_write(priv, UART_OMAP_SCR,
+ OMAP_UART_SCR_DMAMODE_1 | OMAP_UART_SCR_DMAMODE_CTL);
+
+ ret = reset_control_reset(priv->reset);
if (ret)
return ret;

--
2.44.0

2024-04-11 06:39:33

by Tony Lindgren

[permalink] [raw]
Subject: [RFC PATCH 1/4] reset: Fall back to lookup if no reset node is found

Fall back to lookup if the reset node does not exist. When creating and
removing subdevices on an interconnect, the parent device may provide
resets for the children using struct reset_control_lookup instead using
devicetree.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/reset/core.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -1141,12 +1141,17 @@ struct reset_control *__reset_control_get(struct device *dev, const char *id,
int index, bool shared, bool optional,
bool acquired)
{
+ struct reset_control *rstc;
+
if (WARN_ON(shared && acquired))
return ERR_PTR(-EINVAL);

- if (dev->of_node)
- return __of_reset_control_get(dev->of_node, id, index, shared,
+ if (dev->of_node) {
+ rstc = __of_reset_control_get(dev->of_node, id, index, shared,
optional, acquired);
+ if (!(IS_ERR(rstc) && PTR_ERR(rstc) == -ENOENT))
+ return rstc;
+ }

return __reset_control_get_from_lookup(dev, id, shared, optional,
acquired);
--
2.44.0

2024-04-11 08:29:28

by Tony Lindgren

[permalink] [raw]
Subject: [RFC PATCH 2/4] reset: Allow removing a lookup

If a parent device provides resets for the child devices using a lookup
table, let's also allow removal of the lookup table when removing the
child devices.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/reset/core.c | 27 +++++++++++++++++++++++++++
include/linux/reset-controller.h | 7 +++++++
2 files changed, 34 insertions(+)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -217,6 +217,33 @@ void reset_controller_add_lookup(struct reset_control_lookup *lookup,
}
EXPORT_SYMBOL_GPL(reset_controller_add_lookup);

+/**
+ * reset_controller_remove_lookup - unregister a set of lookup entries
+ * @lookup: array of reset lookup entries
+ * @num_entries: number of entries in the lookup array
+ */
+void reset_controller_remove_lookup(struct reset_control_lookup *lookup,
+ unsigned int num_entries)
+{
+ struct reset_control_lookup *entry;
+ unsigned int i;
+
+ mutex_lock(&reset_lookup_mutex);
+ for (i = 0; i < num_entries; i++) {
+ entry = &lookup[i];
+
+ if (!entry->dev_id || !entry->provider) {
+ pr_warn("%s(): reset lookup entry badly specified, skipping\n",
+ __func__);
+ continue;
+ }
+
+ list_del(&entry->list);
+ }
+ mutex_unlock(&reset_lookup_mutex);
+}
+EXPORT_SYMBOL_GPL(reset_controller_remove_lookup);
+
static inline struct reset_control_array *
rstc_to_array(struct reset_control *rstc) {
return container_of(rstc, struct reset_control_array, base);
diff --git a/include/linux/reset-controller.h b/include/linux/reset-controller.h
--- a/include/linux/reset-controller.h
+++ b/include/linux/reset-controller.h
@@ -93,6 +93,8 @@ int devm_reset_controller_register(struct device *dev,

void reset_controller_add_lookup(struct reset_control_lookup *lookup,
unsigned int num_entries);
+void reset_controller_remove_lookup(struct reset_control_lookup *lookup,
+ unsigned int num_entries);
#else
static inline int reset_controller_register(struct reset_controller_dev *rcdev)
{
@@ -113,6 +115,11 @@ static inline void reset_controller_add_lookup(struct reset_control_lookup *look
unsigned int num_entries)
{
}
+
+static inline void reset_controller_remove_lookup(struct reset_control_lookup *lookup,
+ unsigned int num_entries)
+{
+}
#endif

#endif
--
2.44.0