ath10k.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2 00/13] create power sequencing subsystem
@ 2021-08-29 13:12 Dmitry Baryshkov
  2021-08-29 13:12 ` [RFC v2 01/13] power: add power sequencer subsystem Dmitry Baryshkov
                   ` (13 more replies)
  0 siblings, 14 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:12 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

This is the second RFC on the proposed power sequencer subsystem. This
is a generification of the MMC pwrseq code. The subsystem tries to
abstract the idea of complex power-up/power-down/reset of the devices.

To ease migration to pwrseq and to provide compatibility with older
device trees, while keeping drivers simple, this iteration of RFC
introduces pwrseq fallback support: pwrseq driver can register fallback
providers. If another device driver requests pwrseq instance and none
was declared, the pwrseq fallback code would go through the list of
fallback providers and if the match is found, driver would return a
crafted pwrseq instance. For now this mechanism is limited to the OF
device matching, but it can be extended further to use any combination
of device IDs.

The primary set of devices that promted me to create this patchset is
the Qualcomm BT+WiFi family of chips. They reside on serial+platform or
serial + SDIO interfaces (older generations) or on serial+PCIe (newer
generations).  They require a set of external voltage regulators to be
powered on and (some of them) have separate WiFi and Bluetooth enable
GPIOs.

This patchset being an RFC tries to demonstrate the approach, design and
usage of the pwrseq subsystem. Following issues are present in the RFC
at this moment but will be fixed later if the overall approach would be
viewed as acceptable:

 - No documentation
   While the code tries to be self-documenting proper documentation
   would be required.

 - Minimal device tree bindings changes
   There are no proper updates for the DT bindings (thus neither Rob
   Herring nor devicetree are included in the To/Cc lists). The dt
   schema changes would be a part of v1.

 - Lack of proper PCIe integration
   At this moment support for PCIe is hacked up to be able to test the
   PCIe part of qca6390. Proper PCIe support would require automatically
   powering up the devices before the bus scan depending on the proper
   device structure in the device tree.

Changes since RFC v1:
 - Provider pwrseq fallback support
 - Implement fallback support in pwrseq_qca.
 - Mmove susclk handling to pwrseq_qca.
 - Significantly simplify hci_qca.c changes, by dropping all legacy
   code. Now hci_qca uses only pwrseq calls to power up/down bluetooth
   parts of the chip.



_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC v2 01/13] power: add power sequencer subsystem
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
@ 2021-08-29 13:12 ` Dmitry Baryshkov
  2021-09-10 10:02   ` Ulf Hansson
  2021-08-29 13:12 ` [RFC v2 02/13] pwrseq: port MMC's pwrseq drivers to new pwrseq subsystem Dmitry Baryshkov
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:12 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Basing on MMC's pwrseq support code, add separate power sequencer
subsystem. It will be used by other drivers to handle device power up
requirements.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/power/Kconfig           |   1 +
 drivers/power/Makefile          |   1 +
 drivers/power/pwrseq/Kconfig    |  11 +
 drivers/power/pwrseq/Makefile   |   6 +
 drivers/power/pwrseq/core.c     | 412 ++++++++++++++++++++++++++++++++
 include/linux/pwrseq/consumer.h |  88 +++++++
 include/linux/pwrseq/driver.h   |  75 ++++++
 7 files changed, 594 insertions(+)
 create mode 100644 drivers/power/pwrseq/Kconfig
 create mode 100644 drivers/power/pwrseq/Makefile
 create mode 100644 drivers/power/pwrseq/core.c
 create mode 100644 include/linux/pwrseq/consumer.h
 create mode 100644 include/linux/pwrseq/driver.h

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 696bf77a7042..c87cd2240a74 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
+source "drivers/power/pwrseq/Kconfig"
 source "drivers/power/reset/Kconfig"
 source "drivers/power/supply/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index effbf0377f32..1dbce454a8c4 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_POWER_RESET)	+= reset/
 obj-$(CONFIG_POWER_SUPPLY)	+= supply/
+obj-$(CONFIG_PWRSEQ)		+= pwrseq/
diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
new file mode 100644
index 000000000000..8904ec9ed541
--- /dev/null
+++ b/drivers/power/pwrseq/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menuconfig PWRSEQ
+	bool "Power Sequencer drivers"
+	help
+	  Provides support for special power sequencing drivers.
+
+	  Say Y here to enable support for such devices
+
+if PWRSEQ
+
+endif
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
new file mode 100644
index 000000000000..108429ff6445
--- /dev/null
+++ b/drivers/power/pwrseq/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for power sequencer drivers.
+#
+
+obj-$(CONFIG_PWRSEQ) += core.o
diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
new file mode 100644
index 000000000000..2e4e9d123e60
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,412 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2021 (c) Linaro Ltd.
+ * Author: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
+ *
+ * Based on phy-core.c:
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
+ */
+
+#include <linux/device.h>
+#include <linux/idr.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/pm_runtime.h>
+#include <linux/pwrseq/consumer.h>
+#include <linux/pwrseq/driver.h>
+#include <linux/slab.h>
+
+#define	to_pwrseq(a)	(container_of((a), struct pwrseq, dev))
+
+static DEFINE_IDA(pwrseq_ida);
+static DEFINE_MUTEX(pwrseq_provider_mutex);
+static LIST_HEAD(pwrseq_provider_list);
+
+struct pwrseq_provider {
+	struct device		*dev;
+	struct module		*owner;
+	struct list_head	list;
+	void			*data;
+	struct pwrseq * (*of_xlate)(void *data, struct of_phandle_args *args);
+};
+
+void pwrseq_put(struct device *dev, struct pwrseq *pwrseq)
+{
+	device_link_remove(dev, &pwrseq->dev);
+
+	module_put(pwrseq->owner);
+	put_device(&pwrseq->dev);
+}
+EXPORT_SYMBOL_GPL(pwrseq_put);
+
+static struct pwrseq_provider *of_pwrseq_provider_lookup(struct device_node *node)
+{
+	struct pwrseq_provider *pwrseq_provider;
+
+	list_for_each_entry(pwrseq_provider, &pwrseq_provider_list, list) {
+		if (pwrseq_provider->dev->of_node == node)
+			return pwrseq_provider;
+	}
+
+	return ERR_PTR(-EPROBE_DEFER);
+}
+
+static struct pwrseq *_of_pwrseq_get(struct device *dev, const char *id)
+{
+	struct pwrseq_provider *pwrseq_provider;
+	struct pwrseq *pwrseq;
+	struct of_phandle_args args;
+	char prop_name[64]; /* 64 is max size of property name */
+	int ret;
+
+	snprintf(prop_name, 64, "%s-pwrseq", id);
+	ret = of_parse_phandle_with_args(dev->of_node, prop_name, "#pwrseq-cells", 0, &args);
+	if (ret) {
+		/*
+		 * Parsing failed. Try locating old bindings for mmc-pwrseq,
+		 * which did not use #pwrseq-cells.
+		 */
+		if (strcmp(id, "mmc"))
+			return NULL;
+
+		ret = of_parse_phandle_with_args(dev->of_node, prop_name, NULL, 0, &args);
+		if (ret)
+			return NULL;
+
+		dev_warn(dev, "old mmc-pwrseq binding used, add #pwrseq-cells to the provider\n");
+	}
+
+	mutex_lock(&pwrseq_provider_mutex);
+	pwrseq_provider = of_pwrseq_provider_lookup(args.np);
+	if (IS_ERR(pwrseq_provider) || !try_module_get(pwrseq_provider->owner)) {
+		pwrseq = ERR_PTR(-EPROBE_DEFER);
+		goto out_unlock;
+	}
+
+	if (!of_device_is_available(args.np)) {
+		dev_warn(pwrseq_provider->dev, "Requested pwrseq is disabled\n");
+		pwrseq = ERR_PTR(-ENODEV);
+		goto out_put_module;
+	}
+
+	pwrseq = pwrseq_provider->of_xlate(pwrseq_provider->data, &args);
+
+out_put_module:
+	module_put(pwrseq_provider->owner);
+
+out_unlock:
+	mutex_unlock(&pwrseq_provider_mutex);
+	of_node_put(args.np);
+
+	return pwrseq;
+}
+
+struct pwrseq * __pwrseq_get(struct device *dev, const char *id, bool optional)
+{
+	struct pwrseq *pwrseq;
+	struct device_link *link;
+
+	pwrseq = _of_pwrseq_get(dev, id);
+	if (pwrseq == NULL)
+		return optional ? NULL : ERR_PTR(-ENODEV);
+	else if (IS_ERR(pwrseq))
+		return pwrseq;
+
+	if (!try_module_get(pwrseq->owner))
+		return ERR_PTR(-EPROBE_DEFER);
+
+	get_device(&pwrseq->dev);
+	link = device_link_add(dev, &pwrseq->dev, DL_FLAG_STATELESS);
+	if (!link)
+		dev_dbg(dev, "failed to create device link to %s\n",
+			dev_name(pwrseq->dev.parent));
+
+	return pwrseq;
+}
+
+struct pwrseq * pwrseq_get(struct device *dev, const char *id)
+{
+	return __pwrseq_get(dev, id, false);
+}
+EXPORT_SYMBOL_GPL(pwrseq_get);
+
+static void devm_pwrseq_release(struct device *dev, void *res)
+{
+	struct pwrseq *pwrseq = *(struct pwrseq **)res;
+
+	pwrseq_put(dev, pwrseq);
+}
+
+struct pwrseq * devm_pwrseq_get(struct device *dev, const char *id)
+{
+	struct pwrseq **ptr, *pwrseq;
+
+	ptr = devres_alloc(devm_pwrseq_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	pwrseq = pwrseq_get(dev, id);
+	if (!IS_ERR(pwrseq)) {
+		*ptr = pwrseq;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return pwrseq;
+}
+EXPORT_SYMBOL_GPL(devm_pwrseq_get);
+
+struct pwrseq * pwrseq_get_optional(struct device *dev, const char *id)
+{
+	return __pwrseq_get(dev, id, true);
+}
+EXPORT_SYMBOL_GPL(pwrseq_get_optional);
+
+struct pwrseq * devm_pwrseq_get_optional(struct device *dev, const char *id)
+{
+	struct pwrseq **ptr, *pwrseq;
+
+	ptr = devres_alloc(devm_pwrseq_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	pwrseq = pwrseq_get_optional(dev, id);
+	if (!IS_ERR_OR_NULL(pwrseq)) {
+		*ptr = pwrseq;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return pwrseq;
+}
+EXPORT_SYMBOL_GPL(devm_pwrseq_get_optional);
+
+int pwrseq_pre_power_on(struct pwrseq *pwrseq)
+{
+	if (pwrseq && pwrseq->ops->pre_power_on)
+		return pwrseq->ops->pre_power_on(pwrseq);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pwrseq_pre_power_on);
+
+int pwrseq_power_on(struct pwrseq *pwrseq)
+{
+	if (pwrseq && pwrseq->ops->power_on)
+		return pwrseq->ops->power_on(pwrseq);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pwrseq_power_on);
+
+void pwrseq_power_off(struct pwrseq *pwrseq)
+{
+	if (pwrseq && pwrseq->ops->power_off)
+		pwrseq->ops->power_off(pwrseq);
+}
+EXPORT_SYMBOL_GPL(pwrseq_power_off);
+
+void pwrseq_reset(struct pwrseq *pwrseq)
+{
+	if (pwrseq && pwrseq->ops->reset)
+		pwrseq->ops->reset(pwrseq);
+}
+EXPORT_SYMBOL_GPL(pwrseq_reset);
+
+static void pwrseq_dev_release(struct device *dev)
+{
+	struct pwrseq *pwrseq = to_pwrseq(dev);
+
+	ida_free(&pwrseq_ida, pwrseq->id);
+	of_node_put(dev->of_node);
+	kfree(pwrseq);
+}
+
+static struct class pwrseq_class = {
+	.name = "pwrseq",
+	.dev_release = pwrseq_dev_release,
+};
+
+struct pwrseq *__pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data)
+{
+	struct pwrseq *pwrseq;
+	int ret;
+
+	if (WARN_ON(!dev))
+		return ERR_PTR(-EINVAL);
+
+	pwrseq = kzalloc(sizeof(*pwrseq), GFP_KERNEL);
+	if (!pwrseq)
+		return ERR_PTR(-ENOMEM);
+
+	ret = ida_alloc(&pwrseq_ida, GFP_KERNEL);
+	if (ret < 0)
+		goto free_pwrseq;
+
+	pwrseq->id = ret;
+
+	device_initialize(&pwrseq->dev);
+
+	pwrseq->dev.class = &pwrseq_class;
+	pwrseq->dev.parent = dev;
+	pwrseq->dev.of_node = of_node_get(dev->of_node);
+	pwrseq->ops = ops;
+	pwrseq->owner = owner;
+
+	dev_set_drvdata(&pwrseq->dev, data);
+
+	ret = dev_set_name(&pwrseq->dev, "pwrseq-%s.%u", dev_name(dev), pwrseq->id);
+	if (ret)
+		goto put_dev;
+
+	ret = device_add(&pwrseq->dev);
+	if (ret)
+		goto put_dev;
+
+	if (pm_runtime_enabled(dev)) {
+		pm_runtime_enable(&pwrseq->dev);
+		pm_runtime_no_callbacks(&pwrseq->dev);
+	}
+
+	return pwrseq;
+
+put_dev:
+	/* will call pwrseq_dev_release() to free resources */
+	put_device(&pwrseq->dev);
+
+	return ERR_PTR(ret);
+
+free_pwrseq:
+	kfree(pwrseq);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(__pwrseq_create);
+
+void pwrseq_destroy(struct pwrseq *pwrseq)
+{
+	pm_runtime_disable(&pwrseq->dev);
+	device_unregister(&pwrseq->dev);
+}
+EXPORT_SYMBOL_GPL(pwrseq_destroy);
+
+static void devm_pwrseq_destroy(struct device *dev, void *res)
+{
+	struct pwrseq *pwrseq = *(struct pwrseq **)res;
+
+	pwrseq_destroy(pwrseq);
+}
+
+struct pwrseq *__devm_pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data)
+{
+	struct pwrseq **ptr, *pwrseq;
+
+	ptr = devres_alloc(devm_pwrseq_destroy, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	pwrseq = __pwrseq_create(dev, owner, ops, data);
+	if (!IS_ERR(pwrseq)) {
+		*ptr = pwrseq;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return pwrseq;
+}
+EXPORT_SYMBOL_GPL(__devm_pwrseq_create);
+
+struct pwrseq_provider *__of_pwrseq_provider_register(struct device *dev,
+	struct module *owner,
+	struct pwrseq * (*of_xlate)(void *data,
+				    struct of_phandle_args *args),
+	void *data)
+{
+	struct pwrseq_provider *pwrseq_provider;
+
+	pwrseq_provider = kzalloc(sizeof(*pwrseq_provider), GFP_KERNEL);
+	if (!pwrseq_provider)
+		return ERR_PTR(-ENOMEM);
+
+	pwrseq_provider->dev = dev;
+	pwrseq_provider->owner = owner;
+	pwrseq_provider->of_xlate = of_xlate;
+	pwrseq_provider->data = data;
+
+	mutex_lock(&pwrseq_provider_mutex);
+	list_add_tail(&pwrseq_provider->list, &pwrseq_provider_list);
+	mutex_unlock(&pwrseq_provider_mutex);
+
+	return pwrseq_provider;
+}
+EXPORT_SYMBOL_GPL(__of_pwrseq_provider_register);
+
+void of_pwrseq_provider_unregister(struct pwrseq_provider *pwrseq_provider)
+{
+	if (IS_ERR(pwrseq_provider))
+		return;
+
+	mutex_lock(&pwrseq_provider_mutex);
+	list_del(&pwrseq_provider->list);
+	kfree(pwrseq_provider);
+	mutex_unlock(&pwrseq_provider_mutex);
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_provider_unregister);
+
+static void devm_pwrseq_provider_unregister(struct device *dev, void *res)
+{
+	struct pwrseq_provider *pwrseq_provider = *(struct pwrseq_provider **)res;
+
+	of_pwrseq_provider_unregister(pwrseq_provider);
+}
+
+struct pwrseq_provider *__devm_of_pwrseq_provider_register(struct device *dev,
+	struct module *owner,
+	struct pwrseq * (*of_xlate)(void *data,
+				    struct of_phandle_args *args),
+	void *data)
+{
+	struct pwrseq_provider **ptr, *pwrseq_provider;
+
+	ptr = devres_alloc(devm_pwrseq_provider_unregister, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	pwrseq_provider = __of_pwrseq_provider_register(dev, owner, of_xlate, data);
+	if (!IS_ERR(pwrseq_provider)) {
+		*ptr = pwrseq_provider;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return pwrseq_provider;
+}
+EXPORT_SYMBOL_GPL(__devm_of_pwrseq_provider_register);
+
+struct pwrseq *of_pwrseq_xlate_onecell(void *data, struct of_phandle_args *args)
+{
+	struct pwrseq_onecell_data *pwrseq_data = data;
+	unsigned int idx;
+
+	if (args->args_count != 1)
+		return ERR_PTR(-EINVAL);
+
+	idx = args->args[0];
+	if (idx >= pwrseq_data->num) {
+		pr_err("%s: invalid index %u\n", __func__, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return pwrseq_data->pwrseqs[idx];
+}
+
+static int __init pwrseq_core_init(void)
+{
+	return class_register(&pwrseq_class);
+}
+device_initcall(pwrseq_core_init);
diff --git a/include/linux/pwrseq/consumer.h b/include/linux/pwrseq/consumer.h
new file mode 100644
index 000000000000..fbcdc1fc0751
--- /dev/null
+++ b/include/linux/pwrseq/consumer.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2021 Linaro Ltd.
+ */
+
+#ifndef __LINUX_PWRSEQ_CONSUMER_H__
+#define __LINUX_PWRSEQ_CONSUMER_H__
+
+struct pwrseq;
+struct device;
+
+#if defined(CONFIG_PWRSEQ)
+
+struct pwrseq *__must_check pwrseq_get(struct device *dev, const char *id);
+struct pwrseq *__must_check devm_pwrseq_get(struct device *dev, const char *id);
+
+struct pwrseq *__must_check pwrseq_get_optional(struct device *dev, const char *id);
+struct pwrseq *__must_check devm_pwrseq_get_optional(struct device *dev, const char *id);
+
+void pwrseq_put(struct device *dev, struct pwrseq *pwrseq);
+
+int pwrseq_pre_power_on(struct pwrseq *pwrseq);
+int pwrseq_power_on(struct pwrseq *pwrseq);
+void pwrseq_power_off(struct pwrseq *pwrseq);
+void pwrseq_reset(struct pwrseq *pwrseq);
+
+#else
+
+static inline struct pwrseq *__must_check
+pwrseq_get(struct device *dev, const char *id)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct pwrseq *__must_check
+devm_pwrseq_get(struct device *dev, const char *id)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct pwrseq *__must_check
+pwrseq_get_optional(struct device *dev, const char *id)
+{
+	return NULL;
+}
+
+static inline struct pwrseq *__must_check
+devm_pwrseq_get_optional(struct device *dev, const char *id)
+{
+	return NULL;
+}
+
+static inline void pwrseq_put(struct device *dev, struct pwrseq *pwrseq)
+{
+}
+
+static inline int pwrseq_pre_power_on(struct pwrseq *pwrseq)
+{
+	return -ENOSYS;
+}
+
+static inline int pwrseq_power_on(struct pwrseq *pwrseq)
+{
+	return -ENOSYS;
+}
+
+static inline void pwrseq_power_off(struct pwrseq *pwrseq)
+{
+}
+
+static inline void pwrseq_reset(struct pwrseq *pwrseq)
+{
+}
+
+#endif
+
+static inline int pwrseq_full_power_on(struct pwrseq *pwrseq)
+{
+	int ret;
+
+	ret = pwrseq_pre_power_on(pwrseq);
+	if (ret)
+		return ret;
+
+	return pwrseq_power_on(pwrseq);
+}
+
+#endif /* __LINUX_PWRSEQ_CONSUMER_H__ */
diff --git a/include/linux/pwrseq/driver.h b/include/linux/pwrseq/driver.h
new file mode 100644
index 000000000000..b2bc46624d7e
--- /dev/null
+++ b/include/linux/pwrseq/driver.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2021 Linaro Ltd.
+ */
+
+#ifndef __LINUX_PWRSEQ_DRIVER_H__
+#define __LINUX_PWRSEQ_DRIVER_H__
+
+#include <linux/device.h>
+
+struct pwrseq;
+
+struct pwrseq_ops {
+	int (*pre_power_on)(struct pwrseq *pwrseq);
+	int (*power_on)(struct pwrseq *pwrseq);
+	void (*power_off)(struct pwrseq *pwrseq);
+	void (*reset)(struct pwrseq *pwrseq);
+};
+
+struct module;
+
+struct pwrseq {
+	struct device dev;
+	const struct pwrseq_ops *ops;
+	unsigned int id;
+	struct module *owner;
+};
+
+struct pwrseq *__pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data);
+struct pwrseq *__devm_pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data);
+
+#define pwrseq_create(dev, ops, data) __pwrseq_create((dev), THIS_MODULE, (ops), (data))
+#define devm_pwrseq_create(dev, ops, data) __devm_pwrseq_create((dev), THIS_MODULE, (ops), (data))
+
+void pwrseq_destroy(struct pwrseq *pwrseq);
+
+static inline void *pwrseq_get_data(struct pwrseq *pwrseq)
+{
+	return dev_get_drvdata(&pwrseq->dev);
+}
+
+#define	of_pwrseq_provider_register(dev, xlate, data)	\
+	__of_pwrseq_provider_register((dev), THIS_MODULE, (xlate), (data))
+
+#define	devm_of_pwrseq_provider_register(dev, xlate, data)	\
+	__devm_of_pwrseq_provider_register((dev), THIS_MODULE, (xlate), (data))
+
+struct of_phandle_args;
+
+struct pwrseq_provider *__of_pwrseq_provider_register(struct device *dev,
+	struct module *owner,
+	struct pwrseq * (*of_xlate)(void *data,
+				    struct of_phandle_args *args),
+	void *data);
+struct pwrseq_provider *__devm_of_pwrseq_provider_register(struct device *dev,
+	struct module *owner,
+	struct pwrseq * (*of_xlate)(void *data,
+				    struct of_phandle_args *args),
+	void *data);
+void of_pwrseq_provider_unregister(struct pwrseq_provider *pwrseq_provider);
+
+static inline struct pwrseq *of_pwrseq_xlate_single(void *data,
+						    struct of_phandle_args *args)
+{
+	return data;
+}
+
+struct pwrseq_onecell_data {
+	unsigned int num;
+	struct pwrseq *pwrseqs[];
+};
+
+struct pwrseq *of_pwrseq_xlate_onecell(void *data, struct of_phandle_args *args);
+
+#endif /* __LINUX_PWRSEQ_DRIVER_H__ */
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 02/13] pwrseq: port MMC's pwrseq drivers to new pwrseq subsystem
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
  2021-08-29 13:12 ` [RFC v2 01/13] power: add power sequencer subsystem Dmitry Baryshkov
@ 2021-08-29 13:12 ` Dmitry Baryshkov
  2021-08-29 13:12 ` [RFC v2 03/13] mmc: core: switch " Dmitry Baryshkov
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:12 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Port MMC's all pwrseq drivers to new pwrseq subsystem.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../pwrseq}/mmc-pwrseq-emmc.yaml              |   0
 .../pwrseq}/mmc-pwrseq-sd8787.yaml            |   0
 .../pwrseq}/mmc-pwrseq-simple.yaml            |   0
 drivers/mmc/core/Kconfig                      |  32 ----
 drivers/mmc/core/Makefile                     |   3 -
 drivers/mmc/core/pwrseq_emmc.c                | 120 -------------
 drivers/mmc/core/pwrseq_sd8787.c              | 107 ------------
 drivers/mmc/core/pwrseq_simple.c              | 164 ------------------
 drivers/power/pwrseq/Kconfig                  |  32 ++++
 drivers/power/pwrseq/Makefile                 |   4 +
 drivers/power/pwrseq/pwrseq_emmc.c            | 119 +++++++++++++
 drivers/power/pwrseq/pwrseq_sd8787.c          |  98 +++++++++++
 drivers/power/pwrseq/pwrseq_simple.c          | 160 +++++++++++++++++
 13 files changed, 413 insertions(+), 426 deletions(-)
 rename Documentation/devicetree/bindings/{mmc => power/pwrseq}/mmc-pwrseq-emmc.yaml (100%)
 rename Documentation/devicetree/bindings/{mmc => power/pwrseq}/mmc-pwrseq-sd8787.yaml (100%)
 rename Documentation/devicetree/bindings/{mmc => power/pwrseq}/mmc-pwrseq-simple.yaml (100%)
 delete mode 100644 drivers/mmc/core/pwrseq_emmc.c
 delete mode 100644 drivers/mmc/core/pwrseq_sd8787.c
 delete mode 100644 drivers/mmc/core/pwrseq_simple.c
 create mode 100644 drivers/power/pwrseq/pwrseq_emmc.c
 create mode 100644 drivers/power/pwrseq/pwrseq_sd8787.c
 create mode 100644 drivers/power/pwrseq/pwrseq_simple.c

diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.yaml b/Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-emmc.yaml
similarity index 100%
rename from Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.yaml
rename to Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-emmc.yaml
diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.yaml b/Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-sd8787.yaml
similarity index 100%
rename from Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.yaml
rename to Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-sd8787.yaml
diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.yaml b/Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.yaml
similarity index 100%
rename from Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.yaml
rename to Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.yaml
diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
index ae8b69aee619..cf7df64ce009 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/mmc/core/Kconfig
@@ -2,38 +2,6 @@
 #
 # MMC core configuration
 #
-config PWRSEQ_EMMC
-	tristate "HW reset support for eMMC"
-	default y
-	depends on OF
-	help
-	  This selects Hardware reset support aka pwrseq-emmc for eMMC
-	  devices. By default this option is set to y.
-
-	  This driver can also be built as a module. If so, the module
-	  will be called pwrseq_emmc.
-
-config PWRSEQ_SD8787
-	tristate "HW reset support for SD8787 BT + Wifi module"
-	depends on OF && (MWIFIEX || BT_MRVL_SDIO || LIBERTAS_SDIO)
-	help
-	  This selects hardware reset support for the SD8787 BT + Wifi
-	  module. By default this option is set to n.
-
-	  This driver can also be built as a module. If so, the module
-	  will be called pwrseq_sd8787.
-
-config PWRSEQ_SIMPLE
-	tristate "Simple HW reset support for MMC"
-	default y
-	depends on OF
-	help
-	  This selects simple hardware reset support aka pwrseq-simple for MMC
-	  devices. By default this option is set to y.
-
-	  This driver can also be built as a module. If so, the module
-	  will be called pwrseq_simple.
-
 config MMC_BLOCK
 	tristate "MMC block device driver"
 	depends on BLOCK
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 6a907736cd7a..322eb69bd00e 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -10,9 +10,6 @@ mmc_core-y			:= core.o bus.o host.o \
 				   sdio_cis.o sdio_io.o sdio_irq.o \
 				   slot-gpio.o regulator.o
 mmc_core-$(CONFIG_OF)		+= pwrseq.o
-obj-$(CONFIG_PWRSEQ_SIMPLE)	+= pwrseq_simple.o
-obj-$(CONFIG_PWRSEQ_SD8787)	+= pwrseq_sd8787.o
-obj-$(CONFIG_PWRSEQ_EMMC)	+= pwrseq_emmc.o
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
 obj-$(CONFIG_MMC_BLOCK)		+= mmc_block.o
 mmc_block-objs			:= block.o queue.o
diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
deleted file mode 100644
index f6dde9edd7a3..000000000000
--- a/drivers/mmc/core/pwrseq_emmc.c
+++ /dev/null
@@ -1,120 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (C) 2015, Samsung Electronics Co., Ltd.
- *
- * Author: Marek Szyprowski <m.szyprowski@samsung.com>
- *
- * Simple eMMC hardware reset provider
- */
-#include <linux/delay.h>
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/platform_device.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/device.h>
-#include <linux/err.h>
-#include <linux/gpio/consumer.h>
-#include <linux/reboot.h>
-
-#include <linux/mmc/host.h>
-
-#include "pwrseq.h"
-
-struct mmc_pwrseq_emmc {
-	struct mmc_pwrseq pwrseq;
-	struct notifier_block reset_nb;
-	struct gpio_desc *reset_gpio;
-};
-
-#define to_pwrseq_emmc(p) container_of(p, struct mmc_pwrseq_emmc, pwrseq)
-
-static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
-{
-	struct mmc_pwrseq_emmc *pwrseq =  to_pwrseq_emmc(host->pwrseq);
-
-	gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
-	udelay(1);
-	gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
-	udelay(200);
-}
-
-static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
-				    unsigned long mode, void *cmd)
-{
-	struct mmc_pwrseq_emmc *pwrseq = container_of(this,
-					struct mmc_pwrseq_emmc, reset_nb);
-	gpiod_set_value(pwrseq->reset_gpio, 1);
-	udelay(1);
-	gpiod_set_value(pwrseq->reset_gpio, 0);
-	udelay(200);
-
-	return NOTIFY_DONE;
-}
-
-static const struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
-	.reset = mmc_pwrseq_emmc_reset,
-};
-
-static int mmc_pwrseq_emmc_probe(struct platform_device *pdev)
-{
-	struct mmc_pwrseq_emmc *pwrseq;
-	struct device *dev = &pdev->dev;
-
-	pwrseq = devm_kzalloc(dev, sizeof(*pwrseq), GFP_KERNEL);
-	if (!pwrseq)
-		return -ENOMEM;
-
-	pwrseq->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
-	if (IS_ERR(pwrseq->reset_gpio))
-		return PTR_ERR(pwrseq->reset_gpio);
-
-	if (!gpiod_cansleep(pwrseq->reset_gpio)) {
-		/*
-		 * register reset handler to ensure emmc reset also from
-		 * emergency_reboot(), priority 255 is the highest priority
-		 * so it will be executed before any system reboot handler.
-		 */
-		pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
-		pwrseq->reset_nb.priority = 255;
-		register_restart_handler(&pwrseq->reset_nb);
-	} else {
-		dev_notice(dev, "EMMC reset pin tied to a sleepy GPIO driver; reset on emergency-reboot disabled\n");
-	}
-
-	pwrseq->pwrseq.ops = &mmc_pwrseq_emmc_ops;
-	pwrseq->pwrseq.dev = dev;
-	pwrseq->pwrseq.owner = THIS_MODULE;
-	platform_set_drvdata(pdev, pwrseq);
-
-	return mmc_pwrseq_register(&pwrseq->pwrseq);
-}
-
-static int mmc_pwrseq_emmc_remove(struct platform_device *pdev)
-{
-	struct mmc_pwrseq_emmc *pwrseq = platform_get_drvdata(pdev);
-
-	unregister_restart_handler(&pwrseq->reset_nb);
-	mmc_pwrseq_unregister(&pwrseq->pwrseq);
-
-	return 0;
-}
-
-static const struct of_device_id mmc_pwrseq_emmc_of_match[] = {
-	{ .compatible = "mmc-pwrseq-emmc",},
-	{/* sentinel */},
-};
-
-MODULE_DEVICE_TABLE(of, mmc_pwrseq_emmc_of_match);
-
-static struct platform_driver mmc_pwrseq_emmc_driver = {
-	.probe = mmc_pwrseq_emmc_probe,
-	.remove = mmc_pwrseq_emmc_remove,
-	.driver = {
-		.name = "pwrseq_emmc",
-		.of_match_table = mmc_pwrseq_emmc_of_match,
-	},
-};
-
-module_platform_driver(mmc_pwrseq_emmc_driver);
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/core/pwrseq_sd8787.c b/drivers/mmc/core/pwrseq_sd8787.c
deleted file mode 100644
index 68a826f1c0a1..000000000000
--- a/drivers/mmc/core/pwrseq_sd8787.c
+++ /dev/null
@@ -1,107 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * pwrseq_sd8787.c - power sequence support for Marvell SD8787 BT + Wifi chip
- *
- * Copyright (C) 2016 Matt Ranostay <matt@ranostay.consulting>
- *
- * Based on the original work pwrseq_simple.c
- *  Copyright (C) 2014 Linaro Ltd
- *  Author: Ulf Hansson <ulf.hansson@linaro.org>
- */
-
-#include <linux/delay.h>
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/platform_device.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/device.h>
-#include <linux/err.h>
-#include <linux/gpio/consumer.h>
-
-#include <linux/mmc/host.h>
-
-#include "pwrseq.h"
-
-struct mmc_pwrseq_sd8787 {
-	struct mmc_pwrseq pwrseq;
-	struct gpio_desc *reset_gpio;
-	struct gpio_desc *pwrdn_gpio;
-};
-
-#define to_pwrseq_sd8787(p) container_of(p, struct mmc_pwrseq_sd8787, pwrseq)
-
-static void mmc_pwrseq_sd8787_pre_power_on(struct mmc_host *host)
-{
-	struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq);
-
-	gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
-
-	msleep(300);
-	gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 1);
-}
-
-static void mmc_pwrseq_sd8787_power_off(struct mmc_host *host)
-{
-	struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq);
-
-	gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 0);
-	gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
-}
-
-static const struct mmc_pwrseq_ops mmc_pwrseq_sd8787_ops = {
-	.pre_power_on = mmc_pwrseq_sd8787_pre_power_on,
-	.power_off = mmc_pwrseq_sd8787_power_off,
-};
-
-static const struct of_device_id mmc_pwrseq_sd8787_of_match[] = {
-	{ .compatible = "mmc-pwrseq-sd8787",},
-	{/* sentinel */},
-};
-MODULE_DEVICE_TABLE(of, mmc_pwrseq_sd8787_of_match);
-
-static int mmc_pwrseq_sd8787_probe(struct platform_device *pdev)
-{
-	struct mmc_pwrseq_sd8787 *pwrseq;
-	struct device *dev = &pdev->dev;
-
-	pwrseq = devm_kzalloc(dev, sizeof(*pwrseq), GFP_KERNEL);
-	if (!pwrseq)
-		return -ENOMEM;
-
-	pwrseq->pwrdn_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_LOW);
-	if (IS_ERR(pwrseq->pwrdn_gpio))
-		return PTR_ERR(pwrseq->pwrdn_gpio);
-
-	pwrseq->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
-	if (IS_ERR(pwrseq->reset_gpio))
-		return PTR_ERR(pwrseq->reset_gpio);
-
-	pwrseq->pwrseq.dev = dev;
-	pwrseq->pwrseq.ops = &mmc_pwrseq_sd8787_ops;
-	pwrseq->pwrseq.owner = THIS_MODULE;
-	platform_set_drvdata(pdev, pwrseq);
-
-	return mmc_pwrseq_register(&pwrseq->pwrseq);
-}
-
-static int mmc_pwrseq_sd8787_remove(struct platform_device *pdev)
-{
-	struct mmc_pwrseq_sd8787 *pwrseq = platform_get_drvdata(pdev);
-
-	mmc_pwrseq_unregister(&pwrseq->pwrseq);
-
-	return 0;
-}
-
-static struct platform_driver mmc_pwrseq_sd8787_driver = {
-	.probe = mmc_pwrseq_sd8787_probe,
-	.remove = mmc_pwrseq_sd8787_remove,
-	.driver = {
-		.name = "pwrseq_sd8787",
-		.of_match_table = mmc_pwrseq_sd8787_of_match,
-	},
-};
-
-module_platform_driver(mmc_pwrseq_sd8787_driver);
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
deleted file mode 100644
index ea4d3670560e..000000000000
--- a/drivers/mmc/core/pwrseq_simple.c
+++ /dev/null
@@ -1,164 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- *  Copyright (C) 2014 Linaro Ltd
- *
- * Author: Ulf Hansson <ulf.hansson@linaro.org>
- *
- *  Simple MMC power sequence management
- */
-#include <linux/clk.h>
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/platform_device.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/device.h>
-#include <linux/err.h>
-#include <linux/gpio/consumer.h>
-#include <linux/delay.h>
-#include <linux/property.h>
-
-#include <linux/mmc/host.h>
-
-#include "pwrseq.h"
-
-struct mmc_pwrseq_simple {
-	struct mmc_pwrseq pwrseq;
-	bool clk_enabled;
-	u32 post_power_on_delay_ms;
-	u32 power_off_delay_us;
-	struct clk *ext_clk;
-	struct gpio_descs *reset_gpios;
-};
-
-#define to_pwrseq_simple(p) container_of(p, struct mmc_pwrseq_simple, pwrseq)
-
-static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
-					      int value)
-{
-	struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
-
-	if (!IS_ERR(reset_gpios)) {
-		unsigned long *values;
-		int nvalues = reset_gpios->ndescs;
-
-		values = bitmap_alloc(nvalues, GFP_KERNEL);
-		if (!values)
-			return;
-
-		if (value)
-			bitmap_fill(values, nvalues);
-		else
-			bitmap_zero(values, nvalues);
-
-		gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
-					       reset_gpios->info, values);
-
-		kfree(values);
-	}
-}
-
-static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
-{
-	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
-
-	if (!IS_ERR(pwrseq->ext_clk) && !pwrseq->clk_enabled) {
-		clk_prepare_enable(pwrseq->ext_clk);
-		pwrseq->clk_enabled = true;
-	}
-
-	mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
-}
-
-static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
-{
-	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
-
-	mmc_pwrseq_simple_set_gpios_value(pwrseq, 0);
-
-	if (pwrseq->post_power_on_delay_ms)
-		msleep(pwrseq->post_power_on_delay_ms);
-}
-
-static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
-{
-	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
-
-	mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
-
-	if (pwrseq->power_off_delay_us)
-		usleep_range(pwrseq->power_off_delay_us,
-			2 * pwrseq->power_off_delay_us);
-
-	if (!IS_ERR(pwrseq->ext_clk) && pwrseq->clk_enabled) {
-		clk_disable_unprepare(pwrseq->ext_clk);
-		pwrseq->clk_enabled = false;
-	}
-}
-
-static const struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
-	.pre_power_on = mmc_pwrseq_simple_pre_power_on,
-	.post_power_on = mmc_pwrseq_simple_post_power_on,
-	.power_off = mmc_pwrseq_simple_power_off,
-};
-
-static const struct of_device_id mmc_pwrseq_simple_of_match[] = {
-	{ .compatible = "mmc-pwrseq-simple",},
-	{/* sentinel */},
-};
-MODULE_DEVICE_TABLE(of, mmc_pwrseq_simple_of_match);
-
-static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
-{
-	struct mmc_pwrseq_simple *pwrseq;
-	struct device *dev = &pdev->dev;
-
-	pwrseq = devm_kzalloc(dev, sizeof(*pwrseq), GFP_KERNEL);
-	if (!pwrseq)
-		return -ENOMEM;
-
-	pwrseq->ext_clk = devm_clk_get(dev, "ext_clock");
-	if (IS_ERR(pwrseq->ext_clk) && PTR_ERR(pwrseq->ext_clk) != -ENOENT)
-		return PTR_ERR(pwrseq->ext_clk);
-
-	pwrseq->reset_gpios = devm_gpiod_get_array(dev, "reset",
-							GPIOD_OUT_HIGH);
-	if (IS_ERR(pwrseq->reset_gpios) &&
-	    PTR_ERR(pwrseq->reset_gpios) != -ENOENT &&
-	    PTR_ERR(pwrseq->reset_gpios) != -ENOSYS) {
-		return PTR_ERR(pwrseq->reset_gpios);
-	}
-
-	device_property_read_u32(dev, "post-power-on-delay-ms",
-				 &pwrseq->post_power_on_delay_ms);
-	device_property_read_u32(dev, "power-off-delay-us",
-				 &pwrseq->power_off_delay_us);
-
-	pwrseq->pwrseq.dev = dev;
-	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
-	pwrseq->pwrseq.owner = THIS_MODULE;
-	platform_set_drvdata(pdev, pwrseq);
-
-	return mmc_pwrseq_register(&pwrseq->pwrseq);
-}
-
-static int mmc_pwrseq_simple_remove(struct platform_device *pdev)
-{
-	struct mmc_pwrseq_simple *pwrseq = platform_get_drvdata(pdev);
-
-	mmc_pwrseq_unregister(&pwrseq->pwrseq);
-
-	return 0;
-}
-
-static struct platform_driver mmc_pwrseq_simple_driver = {
-	.probe = mmc_pwrseq_simple_probe,
-	.remove = mmc_pwrseq_simple_remove,
-	.driver = {
-		.name = "pwrseq_simple",
-		.of_match_table = mmc_pwrseq_simple_of_match,
-	},
-};
-
-module_platform_driver(mmc_pwrseq_simple_driver);
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
index 8904ec9ed541..36339a456b03 100644
--- a/drivers/power/pwrseq/Kconfig
+++ b/drivers/power/pwrseq/Kconfig
@@ -8,4 +8,36 @@ menuconfig PWRSEQ
 
 if PWRSEQ
 
+config PWRSEQ_EMMC
+	tristate "HW reset support for eMMC"
+	default y
+	depends on OF
+	help
+	  This selects Hardware reset support aka pwrseq-emmc for eMMC
+	  devices. By default this option is set to y.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called pwrseq_emmc.
+
+config PWRSEQ_SD8787
+	tristate "HW reset support for SD8787 BT + Wifi module"
+	depends on OF
+	help
+	  This selects hardware reset support for the SD8787 BT + Wifi
+	  module. By default this option is set to n.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called pwrseq_sd8787.
+
+config PWRSEQ_SIMPLE
+	tristate "Simple HW reset support"
+	default y
+	depends on OF
+	help
+	  This selects simple hardware reset support aka pwrseq-simple.
+	  By default this option is set to y.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called pwrseq_simple.
+
 endif
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
index 108429ff6445..6f359d228843 100644
--- a/drivers/power/pwrseq/Makefile
+++ b/drivers/power/pwrseq/Makefile
@@ -4,3 +4,7 @@
 #
 
 obj-$(CONFIG_PWRSEQ) += core.o
+
+obj-$(CONFIG_PWRSEQ_EMMC)	+= pwrseq_emmc.o
+obj-$(CONFIG_PWRSEQ_SD8787)	+= pwrseq_sd8787.o
+obj-$(CONFIG_PWRSEQ_SIMPLE)	+= pwrseq_simple.o
diff --git a/drivers/power/pwrseq/pwrseq_emmc.c b/drivers/power/pwrseq/pwrseq_emmc.c
new file mode 100644
index 000000000000..4a2da837efd0
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_emmc.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2015, Samsung Electronics Co., Ltd.
+ *
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ *
+ * Simple eMMC hardware reset provider
+ */
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/reboot.h>
+#include <linux/pwrseq/driver.h>
+
+struct pwrseq_emmc {
+	struct notifier_block reset_nb;
+	struct gpio_desc *reset_gpio;
+};
+
+static void pwrseq_ereset(struct pwrseq *pwrseq)
+{
+	struct pwrseq_emmc *pwrseq_emmc = pwrseq_get_data(pwrseq);
+
+	gpiod_set_value_cansleep(pwrseq_emmc->reset_gpio, 1);
+	udelay(1);
+	gpiod_set_value_cansleep(pwrseq_emmc->reset_gpio, 0);
+	udelay(200);
+}
+
+static int pwrseq_ereset_nb(struct notifier_block *this,
+				    unsigned long mode, void *cmd)
+{
+	struct pwrseq_emmc *pwrseq_emmc = container_of(this,
+					struct pwrseq_emmc, reset_nb);
+	gpiod_set_value(pwrseq_emmc->reset_gpio, 1);
+	udelay(1);
+	gpiod_set_value(pwrseq_emmc->reset_gpio, 0);
+	udelay(200);
+
+	return NOTIFY_DONE;
+}
+
+static const struct pwrseq_ops pwrseq_eops = {
+	.reset = pwrseq_ereset,
+};
+
+static int pwrseq_eprobe(struct platform_device *pdev)
+{
+	struct pwrseq_emmc *pwrseq_emmc;
+	struct pwrseq *pwrseq;
+	struct pwrseq_provider *provider;
+	struct device *dev = &pdev->dev;
+
+	pwrseq_emmc = devm_kzalloc(dev, sizeof(*pwrseq_emmc), GFP_KERNEL);
+	if (!pwrseq_emmc)
+		return -ENOMEM;
+
+	pwrseq_emmc->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(pwrseq_emmc->reset_gpio))
+		return PTR_ERR(pwrseq_emmc->reset_gpio);
+
+	if (!gpiod_cansleep(pwrseq_emmc->reset_gpio)) {
+		/*
+		 * register reset handler to ensure emmc reset also from
+		 * emergency_reboot(), priority 255 is the highest priority
+		 * so it will be executed before any system reboot handler.
+		 */
+		pwrseq_emmc->reset_nb.notifier_call = pwrseq_ereset_nb;
+		pwrseq_emmc->reset_nb.priority = 255;
+		register_restart_handler(&pwrseq_emmc->reset_nb);
+	} else {
+		dev_notice(dev, "EMMC reset pin tied to a sleepy GPIO driver; reset on emergency-reboot disabled\n");
+	}
+
+	platform_set_drvdata(pdev, pwrseq_emmc);
+
+	pwrseq = devm_pwrseq_create(dev, &pwrseq_eops, pwrseq_emmc);
+	if (IS_ERR(pwrseq))
+		return PTR_ERR(pwrseq);
+
+	provider = devm_of_pwrseq_provider_register(dev, of_pwrseq_xlate_single, pwrseq);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static int pwrseq_eremove(struct platform_device *pdev)
+{
+	struct pwrseq_emmc *pwrseq_emmc = platform_get_drvdata(pdev);
+
+	unregister_restart_handler(&pwrseq_emmc->reset_nb);
+
+	return 0;
+}
+
+static const struct of_device_id pwrseq_eof_match[] = {
+	{ .compatible = "mmc-pwrseq-emmc",},
+	{/* sentinel */},
+};
+
+MODULE_DEVICE_TABLE(of, pwrseq_eof_match);
+
+static struct platform_driver pwrseq_edriver = {
+	.probe = pwrseq_eprobe,
+	.remove = pwrseq_eremove,
+	.driver = {
+		.name = "pwrseq_emmc",
+		.of_match_table = pwrseq_eof_match,
+	},
+};
+
+module_platform_driver(pwrseq_edriver);
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/power/pwrseq/pwrseq_sd8787.c b/drivers/power/pwrseq/pwrseq_sd8787.c
new file mode 100644
index 000000000000..c5d6c639380f
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_sd8787.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * pwrseq_sd8787.c - power sequence support for Marvell SD8787 BT + Wifi chip
+ *
+ * Copyright (C) 2016 Matt Ranostay <matt@ranostay.consulting>
+ *
+ * Based on the original work pwrseq_sd8787.c
+ *  Copyright (C) 2014 Linaro Ltd
+ *  Author: Ulf Hansson <ulf.hansson@linaro.org>
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+
+#include <linux/pwrseq/driver.h>
+
+struct pwrseq_sd8787 {
+	struct gpio_desc *reset_gpio;
+	struct gpio_desc *pwrdn_gpio;
+};
+
+static int pwrseq_sd8787_pre_power_on(struct pwrseq *pwrseq)
+{
+	struct pwrseq_sd8787 *pwrseq_sd8787 = pwrseq_get_data(pwrseq);
+
+	gpiod_set_value_cansleep(pwrseq_sd8787->reset_gpio, 1);
+
+	msleep(300);
+	gpiod_set_value_cansleep(pwrseq_sd8787->pwrdn_gpio, 1);
+
+	return 0;
+}
+
+static void pwrseq_sd8787_power_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_sd8787 *pwrseq_sd8787 = pwrseq_get_data(pwrseq);
+
+	gpiod_set_value_cansleep(pwrseq_sd8787->pwrdn_gpio, 0);
+	gpiod_set_value_cansleep(pwrseq_sd8787->reset_gpio, 0);
+}
+
+static const struct pwrseq_ops pwrseq_sd8787_ops = {
+	.pre_power_on = pwrseq_sd8787_pre_power_on,
+	.power_off = pwrseq_sd8787_power_off,
+};
+
+static const struct of_device_id pwrseq_sd8787_of_match[] = {
+	{ .compatible = "mmc-pwrseq-sd8787",},
+	{/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, pwrseq_sd8787_of_match);
+
+static int pwrseq_sd8787_probe(struct platform_device *pdev)
+{
+	struct pwrseq_sd8787 *pwrseq_sd8787;
+	struct pwrseq *pwrseq;
+	struct pwrseq_provider *provider;
+	struct device *dev = &pdev->dev;
+
+	pwrseq_sd8787 = devm_kzalloc(dev, sizeof(*pwrseq_sd8787), GFP_KERNEL);
+	if (!pwrseq_sd8787)
+		return -ENOMEM;
+
+	pwrseq_sd8787->pwrdn_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_LOW);
+	if (IS_ERR(pwrseq_sd8787->pwrdn_gpio))
+		return PTR_ERR(pwrseq_sd8787->pwrdn_gpio);
+
+	pwrseq_sd8787->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(pwrseq_sd8787->reset_gpio))
+		return PTR_ERR(pwrseq_sd8787->reset_gpio);
+
+	pwrseq = devm_pwrseq_create(dev, &pwrseq_sd8787_ops, pwrseq_sd8787);
+	if (IS_ERR(pwrseq))
+		return PTR_ERR(pwrseq);
+
+	provider = devm_of_pwrseq_provider_register(dev, of_pwrseq_xlate_single, pwrseq);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static struct platform_driver pwrseq_sd8787_driver = {
+	.probe = pwrseq_sd8787_probe,
+	.driver = {
+		.name = "pwrseq_sd8787",
+		.of_match_table = pwrseq_sd8787_of_match,
+	},
+};
+
+module_platform_driver(pwrseq_sd8787_driver);
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/power/pwrseq/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_simple.c
new file mode 100644
index 000000000000..f8d6e6e077df
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  Copyright (C) 2014 Linaro Ltd
+ *
+ * Author: Ulf Hansson <ulf.hansson@linaro.org>
+ *
+ *  Simple MMC power sequence management
+ */
+#include <linux/clk.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+#include <linux/property.h>
+#include <linux/pwrseq/driver.h>
+
+struct pwrseq_simple {
+	bool clk_enabled;
+	u32 post_power_on_delay_ms;
+	u32 power_off_delay_us;
+	struct clk *ext_clk;
+	struct gpio_descs *reset_gpios;
+};
+
+static int pwrseq_simple_set_gpios_value(struct pwrseq_simple *pwrseq_simple,
+					 int value)
+{
+	struct gpio_descs *reset_gpios = pwrseq_simple->reset_gpios;
+	unsigned long *values;
+	int nvalues;
+	int ret;
+
+	if (IS_ERR(reset_gpios))
+		return PTR_ERR(reset_gpios);
+
+	nvalues = reset_gpios->ndescs;
+
+	values = bitmap_alloc(nvalues, GFP_KERNEL);
+	if (!values)
+		return -ENOMEM;
+
+	if (value)
+		bitmap_fill(values, nvalues);
+	else
+		bitmap_zero(values, nvalues);
+
+	ret = gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
+				       reset_gpios->info, values);
+	kfree(values);
+
+	return ret;
+}
+
+static int pwrseq_simple_pre_power_on(struct pwrseq *pwrseq)
+{
+	struct pwrseq_simple *pwrseq_simple = pwrseq_get_data(pwrseq);
+
+	if (!IS_ERR(pwrseq_simple->ext_clk) && !pwrseq_simple->clk_enabled) {
+		clk_prepare_enable(pwrseq_simple->ext_clk);
+		pwrseq_simple->clk_enabled = true;
+	}
+
+	return pwrseq_simple_set_gpios_value(pwrseq_simple, 1);
+}
+
+static int pwrseq_simple_power_on(struct pwrseq *pwrseq)
+{
+	struct pwrseq_simple *pwrseq_simple = pwrseq_get_data(pwrseq);
+	int ret;
+
+	ret = pwrseq_simple_set_gpios_value(pwrseq_simple, 0);
+	if (ret)
+		return ret;
+
+	if (pwrseq_simple->post_power_on_delay_ms)
+		msleep(pwrseq_simple->post_power_on_delay_ms);
+
+	return 0;
+}
+
+static void pwrseq_simple_power_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_simple *pwrseq_simple = pwrseq_get_data(pwrseq);
+
+	pwrseq_simple_set_gpios_value(pwrseq_simple, 1);
+
+	if (pwrseq_simple->power_off_delay_us)
+		usleep_range(pwrseq_simple->power_off_delay_us,
+			2 * pwrseq_simple->power_off_delay_us);
+
+	if (!IS_ERR(pwrseq_simple->ext_clk) && pwrseq_simple->clk_enabled) {
+		clk_disable_unprepare(pwrseq_simple->ext_clk);
+		pwrseq_simple->clk_enabled = false;
+	}
+}
+
+static const struct pwrseq_ops pwrseq_simple_ops = {
+	.pre_power_on = pwrseq_simple_pre_power_on,
+	.power_on = pwrseq_simple_power_on,
+	.power_off = pwrseq_simple_power_off,
+};
+
+static const struct of_device_id pwrseq_simple_of_match[] = {
+	{ .compatible = "mmc-pwrseq-simple",}, /* MMC-specific compatible */
+	{/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, pwrseq_simple_of_match);
+
+static int pwrseq_simple_probe(struct platform_device *pdev)
+{
+	struct pwrseq_simple *pwrseq_simple;
+	struct pwrseq *pwrseq;
+	struct pwrseq_provider *provider;
+	struct device *dev = &pdev->dev;
+
+	pwrseq_simple = devm_kzalloc(dev, sizeof(*pwrseq_simple), GFP_KERNEL);
+	if (!pwrseq_simple)
+		return -ENOMEM;
+
+	pwrseq_simple->ext_clk = devm_clk_get(dev, "ext_clock");
+	if (IS_ERR(pwrseq_simple->ext_clk) && PTR_ERR(pwrseq_simple->ext_clk) != -ENOENT)
+		return PTR_ERR(pwrseq_simple->ext_clk);
+
+	pwrseq_simple->reset_gpios = devm_gpiod_get_array(dev, "reset",
+							GPIOD_OUT_HIGH);
+	if (IS_ERR(pwrseq_simple->reset_gpios) &&
+	    PTR_ERR(pwrseq_simple->reset_gpios) != -ENOENT &&
+	    PTR_ERR(pwrseq_simple->reset_gpios) != -ENOSYS) {
+		return PTR_ERR(pwrseq_simple->reset_gpios);
+	}
+
+	device_property_read_u32(dev, "post-power-on-delay-ms",
+				 &pwrseq_simple->post_power_on_delay_ms);
+	device_property_read_u32(dev, "power-off-delay-us",
+				 &pwrseq_simple->power_off_delay_us);
+
+	pwrseq = devm_pwrseq_create(dev, &pwrseq_simple_ops, pwrseq_simple);
+	if (IS_ERR(pwrseq))
+		return PTR_ERR(pwrseq);
+
+	provider = devm_of_pwrseq_provider_register(dev, of_pwrseq_xlate_single, pwrseq);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static struct platform_driver pwrseq_simple_driver = {
+	.probe = pwrseq_simple_probe,
+	.driver = {
+		.name = "pwrseq_simple",
+		.of_match_table = pwrseq_simple_of_match,
+	},
+};
+
+module_platform_driver(pwrseq_simple_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 03/13] mmc: core: switch to new pwrseq subsystem
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
  2021-08-29 13:12 ` [RFC v2 01/13] power: add power sequencer subsystem Dmitry Baryshkov
  2021-08-29 13:12 ` [RFC v2 02/13] pwrseq: port MMC's pwrseq drivers to new pwrseq subsystem Dmitry Baryshkov
@ 2021-08-29 13:12 ` Dmitry Baryshkov
  2021-08-29 13:12 ` [RFC v2 04/13] pwrseq: add support for QCA BT+WiFi power sequencer Dmitry Baryshkov
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:12 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Drop old MMC pwrseq code and use new pwrseq subsystem instead.
Individual drivers are already ported to new subsystem.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/mmc/core/Makefile |   1 -
 drivers/mmc/core/core.c   |   9 ++-
 drivers/mmc/core/host.c   |   8 ++-
 drivers/mmc/core/mmc.c    |   3 +-
 drivers/mmc/core/pwrseq.c | 117 --------------------------------------
 drivers/mmc/core/pwrseq.h |  58 -------------------
 include/linux/mmc/host.h  |   4 +-
 7 files changed, 12 insertions(+), 188 deletions(-)
 delete mode 100644 drivers/mmc/core/pwrseq.c
 delete mode 100644 drivers/mmc/core/pwrseq.h

diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 322eb69bd00e..a504d873cf8e 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -9,7 +9,6 @@ mmc_core-y			:= core.o bus.o host.o \
 				   sdio.o sdio_ops.o sdio_bus.o \
 				   sdio_cis.o sdio_io.o sdio_irq.o \
 				   slot-gpio.o regulator.o
-mmc_core-$(CONFIG_OF)		+= pwrseq.o
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
 obj-$(CONFIG_MMC_BLOCK)		+= mmc_block.o
 mmc_block-objs			:= block.o queue.o
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 95fedcf56e4a..c468af900a45 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -41,7 +41,6 @@
 #include "bus.h"
 #include "host.h"
 #include "sdio_bus.h"
-#include "pwrseq.h"
 
 #include "mmc_ops.h"
 #include "sd_ops.h"
@@ -1321,7 +1320,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	if (host->ios.power_mode == MMC_POWER_ON)
 		return;
 
-	mmc_pwrseq_pre_power_on(host);
+	pwrseq_pre_power_on(host->pwrseq);
 
 	host->ios.vdd = fls(ocr) - 1;
 	host->ios.power_mode = MMC_POWER_UP;
@@ -1336,7 +1335,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	 */
 	mmc_delay(host->ios.power_delay_ms);
 
-	mmc_pwrseq_post_power_on(host);
+	pwrseq_power_on(host->pwrseq);
 
 	host->ios.clock = host->f_init;
 
@@ -1355,7 +1354,7 @@ void mmc_power_off(struct mmc_host *host)
 	if (host->ios.power_mode == MMC_POWER_OFF)
 		return;
 
-	mmc_pwrseq_power_off(host);
+	pwrseq_power_off(host->pwrseq);
 
 	host->ios.clock = 0;
 	host->ios.vdd = 0;
@@ -1985,7 +1984,7 @@ EXPORT_SYMBOL(mmc_set_blocklen);
 
 static void mmc_hw_reset_for_init(struct mmc_host *host)
 {
-	mmc_pwrseq_reset(host);
+	pwrseq_reset(host->pwrseq);
 
 	if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
 		return;
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 0475d96047c4..214b9cfda723 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -28,7 +28,6 @@
 #include "crypto.h"
 #include "host.h"
 #include "slot-gpio.h"
-#include "pwrseq.h"
 #include "sdio_ops.h"
 
 #define cls_dev_to_mmc_host(d)	container_of(d, struct mmc_host, class_dev)
@@ -413,7 +412,11 @@ int mmc_of_parse(struct mmc_host *host)
 	device_property_read_u32(dev, "post-power-on-delay-ms",
 				 &host->ios.power_delay_ms);
 
-	return mmc_pwrseq_alloc(host);
+	host->pwrseq = devm_pwrseq_get_optional(dev, "mmc");
+	if (IS_ERR(host->pwrseq))
+		return PTR_ERR(host->pwrseq);
+
+	return 0;
 }
 
 EXPORT_SYMBOL(mmc_of_parse);
@@ -632,7 +635,6 @@ EXPORT_SYMBOL(mmc_remove_host);
  */
 void mmc_free_host(struct mmc_host *host)
 {
-	mmc_pwrseq_free(host);
 	put_device(&host->class_dev);
 }
 
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 838726b68ff3..59d0d26bb5c0 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -24,7 +24,6 @@
 #include "mmc_ops.h"
 #include "quirks.h"
 #include "sd_ops.h"
-#include "pwrseq.h"
 
 #define DEFAULT_CMD6_TIMEOUT_MS	500
 #define MIN_CACHE_EN_TIMEOUT_MS 1600
@@ -2220,7 +2219,7 @@ static int _mmc_hw_reset(struct mmc_host *host)
 	} else {
 		/* Do a brute force power cycle */
 		mmc_power_cycle(host, card->ocr);
-		mmc_pwrseq_reset(host);
+		pwrseq_reset(host->pwrseq);
 	}
 	return mmc_init_card(host, card->ocr, card);
 }
diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c
deleted file mode 100644
index ef675f364bf0..000000000000
--- a/drivers/mmc/core/pwrseq.c
+++ /dev/null
@@ -1,117 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- *  Copyright (C) 2014 Linaro Ltd
- *
- * Author: Ulf Hansson <ulf.hansson@linaro.org>
- *
- *  MMC power sequence management
- */
-#include <linux/kernel.h>
-#include <linux/err.h>
-#include <linux/module.h>
-#include <linux/of.h>
-
-#include <linux/mmc/host.h>
-
-#include "pwrseq.h"
-
-static DEFINE_MUTEX(pwrseq_list_mutex);
-static LIST_HEAD(pwrseq_list);
-
-int mmc_pwrseq_alloc(struct mmc_host *host)
-{
-	struct device_node *np;
-	struct mmc_pwrseq *p;
-
-	np = of_parse_phandle(host->parent->of_node, "mmc-pwrseq", 0);
-	if (!np)
-		return 0;
-
-	mutex_lock(&pwrseq_list_mutex);
-	list_for_each_entry(p, &pwrseq_list, pwrseq_node) {
-		if (p->dev->of_node == np) {
-			if (!try_module_get(p->owner))
-				dev_err(host->parent,
-					"increasing module refcount failed\n");
-			else
-				host->pwrseq = p;
-
-			break;
-		}
-	}
-
-	of_node_put(np);
-	mutex_unlock(&pwrseq_list_mutex);
-
-	if (!host->pwrseq)
-		return -EPROBE_DEFER;
-
-	dev_info(host->parent, "allocated mmc-pwrseq\n");
-
-	return 0;
-}
-
-void mmc_pwrseq_pre_power_on(struct mmc_host *host)
-{
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
-
-	if (pwrseq && pwrseq->ops->pre_power_on)
-		pwrseq->ops->pre_power_on(host);
-}
-
-void mmc_pwrseq_post_power_on(struct mmc_host *host)
-{
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
-
-	if (pwrseq && pwrseq->ops->post_power_on)
-		pwrseq->ops->post_power_on(host);
-}
-
-void mmc_pwrseq_power_off(struct mmc_host *host)
-{
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
-
-	if (pwrseq && pwrseq->ops->power_off)
-		pwrseq->ops->power_off(host);
-}
-
-void mmc_pwrseq_reset(struct mmc_host *host)
-{
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
-
-	if (pwrseq && pwrseq->ops->reset)
-		pwrseq->ops->reset(host);
-}
-
-void mmc_pwrseq_free(struct mmc_host *host)
-{
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
-
-	if (pwrseq) {
-		module_put(pwrseq->owner);
-		host->pwrseq = NULL;
-	}
-}
-
-int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
-{
-	if (!pwrseq || !pwrseq->ops || !pwrseq->dev)
-		return -EINVAL;
-
-	mutex_lock(&pwrseq_list_mutex);
-	list_add(&pwrseq->pwrseq_node, &pwrseq_list);
-	mutex_unlock(&pwrseq_list_mutex);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(mmc_pwrseq_register);
-
-void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq)
-{
-	if (pwrseq) {
-		mutex_lock(&pwrseq_list_mutex);
-		list_del(&pwrseq->pwrseq_node);
-		mutex_unlock(&pwrseq_list_mutex);
-	}
-}
-EXPORT_SYMBOL_GPL(mmc_pwrseq_unregister);
diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h
deleted file mode 100644
index f3bb103db9ad..000000000000
--- a/drivers/mmc/core/pwrseq.h
+++ /dev/null
@@ -1,58 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) 2014 Linaro Ltd
- *
- * Author: Ulf Hansson <ulf.hansson@linaro.org>
- */
-#ifndef _MMC_CORE_PWRSEQ_H
-#define _MMC_CORE_PWRSEQ_H
-
-#include <linux/types.h>
-
-struct mmc_host;
-struct device;
-struct module;
-
-struct mmc_pwrseq_ops {
-	void (*pre_power_on)(struct mmc_host *host);
-	void (*post_power_on)(struct mmc_host *host);
-	void (*power_off)(struct mmc_host *host);
-	void (*reset)(struct mmc_host *host);
-};
-
-struct mmc_pwrseq {
-	const struct mmc_pwrseq_ops *ops;
-	struct device *dev;
-	struct list_head pwrseq_node;
-	struct module *owner;
-};
-
-#ifdef CONFIG_OF
-
-int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq);
-void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq);
-
-int mmc_pwrseq_alloc(struct mmc_host *host);
-void mmc_pwrseq_pre_power_on(struct mmc_host *host);
-void mmc_pwrseq_post_power_on(struct mmc_host *host);
-void mmc_pwrseq_power_off(struct mmc_host *host);
-void mmc_pwrseq_reset(struct mmc_host *host);
-void mmc_pwrseq_free(struct mmc_host *host);
-
-#else
-
-static inline int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
-{
-	return -ENOSYS;
-}
-static inline void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq) {}
-static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
-static inline void mmc_pwrseq_pre_power_on(struct mmc_host *host) {}
-static inline void mmc_pwrseq_post_power_on(struct mmc_host *host) {}
-static inline void mmc_pwrseq_power_off(struct mmc_host *host) {}
-static inline void mmc_pwrseq_reset(struct mmc_host *host) {}
-static inline void mmc_pwrseq_free(struct mmc_host *host) {}
-
-#endif
-
-#endif
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 0abd47e9ef9b..1673e37f6028 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -16,6 +16,7 @@
 #include <linux/mmc/pm.h>
 #include <linux/dma-direction.h>
 #include <linux/keyslot-manager.h>
+#include <linux/pwrseq/consumer.h>
 
 struct mmc_ios {
 	unsigned int	clock;			/* clock rate */
@@ -278,7 +279,6 @@ struct mmc_context_info {
 };
 
 struct regulator;
-struct mmc_pwrseq;
 
 struct mmc_supply {
 	struct regulator *vmmc;		/* Card power supply */
@@ -294,7 +294,7 @@ struct mmc_host {
 	struct device		class_dev;
 	int			index;
 	const struct mmc_host_ops *ops;
-	struct mmc_pwrseq	*pwrseq;
+	struct pwrseq		*pwrseq;
 	unsigned int		f_min;
 	unsigned int		f_max;
 	unsigned int		f_init;
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 04/13] pwrseq: add support for QCA BT+WiFi power sequencer
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (2 preceding siblings ...)
  2021-08-29 13:12 ` [RFC v2 03/13] mmc: core: switch " Dmitry Baryshkov
@ 2021-08-29 13:12 ` Dmitry Baryshkov
  2021-08-29 13:12 ` [RFC v2 05/13] pwrseq: add fallback support Dmitry Baryshkov
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:12 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Add support for power sequencer used in the Qualcomm BT+WiFi SoCs. They
require several external volate regulators and some of them use separate
BT and WiFi enable GPIO pins. This code is mostly extracted from the
hci_qca.c bluetooth driver and ath10k WiFi driver. Instead of having
each of them manage pins, different requirements, regulator types, move
this knowledge to the common power sequencer driver. Currently original
drivers are not stripped from the regulator code, this will be done
later (to keep compatibility with the old device trees).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/power/pwrseq/Kconfig      |  14 ++
 drivers/power/pwrseq/Makefile     |   1 +
 drivers/power/pwrseq/pwrseq_qca.c | 371 ++++++++++++++++++++++++++++++
 3 files changed, 386 insertions(+)
 create mode 100644 drivers/power/pwrseq/pwrseq_qca.c

diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
index 36339a456b03..990c3164144e 100644
--- a/drivers/power/pwrseq/Kconfig
+++ b/drivers/power/pwrseq/Kconfig
@@ -19,6 +19,20 @@ config PWRSEQ_EMMC
 	  This driver can also be built as a module. If so, the module
 	  will be called pwrseq_emmc.
 
+config PWRSEQ_QCA
+	tristate "Power Sequencer for Qualcomm WiFi + BT SoCs"
+	depends on OF
+	help
+	  If you say yes to this option, support will be included for Qualcomm
+	  WCN399x,QCA639x,QCA67xx families of hybrid WiFi and Bluetooth SoCs.
+	  Note, this driver supports only power control for these SoC, you
+	  still have to enable individual Bluetooth and WiFi drivers. This
+	  driver is only necessary on ARM platforms with these chips. PCIe
+	  cards handle power sequencing on their own.
+
+	  Say M here if you want to include support for Qualcomm WiFi+BT SoCs
+	  as a module. This will build a module called "pwrseq_qca".
+
 config PWRSEQ_SD8787
 	tristate "HW reset support for SD8787 BT + Wifi module"
 	depends on OF
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
index 6f359d228843..556bf5582d47 100644
--- a/drivers/power/pwrseq/Makefile
+++ b/drivers/power/pwrseq/Makefile
@@ -6,5 +6,6 @@
 obj-$(CONFIG_PWRSEQ) += core.o
 
 obj-$(CONFIG_PWRSEQ_EMMC)	+= pwrseq_emmc.o
+obj-$(CONFIG_PWRSEQ_QCA)	+= pwrseq_qca.o
 obj-$(CONFIG_PWRSEQ_SD8787)	+= pwrseq_sd8787.o
 obj-$(CONFIG_PWRSEQ_SIMPLE)	+= pwrseq_simple.o
diff --git a/drivers/power/pwrseq/pwrseq_qca.c b/drivers/power/pwrseq/pwrseq_qca.c
new file mode 100644
index 000000000000..7aa5f2d94039
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_qca.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021, Linaro Ltd.
+ *
+ * Author: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
+ *
+ * Power Sequencer for Qualcomm WiFi + BT SoCs
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/pwrseq/driver.h>
+#include <linux/regulator/consumer.h>
+
+/* susclk rate */
+#define SUSCLK_RATE_32KHZ	32768
+
+/*
+ * Voltage regulator information required for configuring the
+ * QCA WiFi+Bluetooth chipset
+ */
+struct qca_vreg {
+	const char *name;
+	unsigned int load_uA;
+};
+
+struct qca_device_data {
+	bool has_enable_gpios;
+
+	/*
+	 * VDDIO has to be enabled before the rest of regulators, so we treat
+	 * it separately
+	 */
+	struct qca_vreg vddio;
+
+	size_t num_vregs;
+	struct qca_vreg vregs[];
+};
+
+struct pwrseq_qca_common {
+	struct gpio_desc *sw_ctrl;
+	struct clk *susclk;
+
+	/*
+	 * Again vddio is separate so that it can be enabled before enabling
+	 * other regulators.
+	 */
+	struct regulator *vddio;
+	int num_vregs;
+	struct regulator_bulk_data vregs[];
+};
+
+struct pwrseq_qca_one {
+	struct pwrseq_qca_common *common;
+	struct gpio_desc *enable;
+};
+
+#define PWRSEQ_QCA_WIFI 0
+#define PWRSEQ_QCA_BT 1
+
+#define PWRSEQ_QCA_MAX 2
+
+struct pwrseq_qca {
+	struct pwrseq_qca_one pwrseq_qcas[PWRSEQ_QCA_MAX];
+	struct pwrseq_qca_common common;
+};
+
+static int pwrseq_qca_pre_power_on(struct pwrseq *pwrseq)
+{
+	struct pwrseq_qca_one *qca_one = pwrseq_get_data(pwrseq);
+
+	if (qca_one->enable) {
+		gpiod_set_value_cansleep(qca_one->enable, 0);
+		msleep(50);
+	}
+
+	return 0;
+}
+
+static int pwrseq_qca_power_on(struct pwrseq *pwrseq)
+{
+	struct pwrseq_qca_one *qca_one = pwrseq_get_data(pwrseq);
+	int ret;
+
+	if (qca_one->common->vddio) {
+		ret = regulator_enable(qca_one->common->vddio);
+		if (ret)
+			return ret;
+	}
+
+	ret = regulator_bulk_enable(qca_one->common->num_vregs, qca_one->common->vregs);
+	if (ret)
+		goto err_bulk;
+
+	ret = clk_prepare_enable(qca_one->common->susclk);
+	if (ret)
+		goto err_clk;
+
+	if (qca_one->enable) {
+		gpiod_set_value_cansleep(qca_one->enable, 1);
+		msleep(150);
+	}
+
+	if (qca_one->common->sw_ctrl) {
+		bool sw_ctrl_state = gpiod_get_value_cansleep(qca_one->common->sw_ctrl);
+		dev_dbg(&pwrseq->dev, "SW_CTRL is %d", sw_ctrl_state);
+	}
+
+	return 0;
+
+err_clk:
+	regulator_bulk_disable(qca_one->common->num_vregs, qca_one->common->vregs);
+err_bulk:
+	regulator_disable(qca_one->common->vddio);
+
+	return ret;
+}
+
+static void pwrseq_qca_power_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_qca_one *qca_one = pwrseq_get_data(pwrseq);
+
+	if (qca_one->enable) {
+		gpiod_set_value_cansleep(qca_one->enable, 0);
+		msleep(50);
+	}
+
+	clk_disable_unprepare(qca_one->common->susclk);
+
+	regulator_bulk_disable(qca_one->common->num_vregs, qca_one->common->vregs);
+	regulator_disable(qca_one->common->vddio);
+
+	if (qca_one->common->sw_ctrl) {
+		bool sw_ctrl_state = gpiod_get_value_cansleep(qca_one->common->sw_ctrl);
+		dev_dbg(&pwrseq->dev, "SW_CTRL is %d", sw_ctrl_state);
+	}
+}
+
+static const struct pwrseq_ops pwrseq_qca_ops = {
+	.pre_power_on = pwrseq_qca_pre_power_on,
+	.power_on = pwrseq_qca_power_on,
+	.power_off = pwrseq_qca_power_off,
+};
+
+static int pwrseq_qca_common_init(struct device *dev, struct pwrseq_qca_common *qca_common,
+		const struct qca_device_data *data)
+{
+	int ret, i;
+
+	if (data->vddio.name) {
+		qca_common->vddio = devm_regulator_get(dev, data->vddio.name);
+		if (IS_ERR(qca_common->vddio))
+			return PTR_ERR(qca_common->vddio);
+
+		ret = regulator_set_load(qca_common->vddio, data->vddio.load_uA);
+		if (ret)
+			return ret;
+	}
+
+	qca_common->num_vregs = data->num_vregs;
+
+	for (i = 0; i < qca_common->num_vregs; i++)
+		qca_common->vregs[i].supply = data->vregs[i].name;
+
+	ret = devm_regulator_bulk_get(dev, qca_common->num_vregs, qca_common->vregs);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < qca_common->num_vregs; i++) {
+		if (!data->vregs[i].load_uA)
+			continue;
+
+		ret = regulator_set_load(qca_common->vregs[i].consumer, data->vregs[i].load_uA);
+		if (ret)
+			return ret;
+	}
+
+	qca_common->susclk = devm_clk_get_optional(dev, NULL);
+	if (IS_ERR(qca_common->susclk)) {
+		dev_err(dev, "failed to acquire clk\n");
+		return PTR_ERR(qca_common->susclk);
+	}
+
+	qca_common->sw_ctrl = devm_gpiod_get_optional(dev, "swctrl", GPIOD_IN);
+	if (IS_ERR(qca_common->sw_ctrl)) {
+		return dev_err_probe(dev, PTR_ERR(qca_common->sw_ctrl),
+				"failed to acquire SW_CTRL gpio\n");
+	} else if (!qca_common->sw_ctrl)
+		dev_info(dev, "No SW_CTRL gpio\n");
+
+	return 0;
+}
+
+static void pwrseq_qca_unprepare_susclk(void *data)
+{
+	struct pwrseq_qca_common *qca_common = data;
+
+	clk_disable_unprepare(qca_common->susclk);
+}
+
+static const struct qca_device_data qca_soc_data_default = {
+	.num_vregs = 0,
+	.has_enable_gpios = true,
+};
+
+static int pwrseq_qca_probe(struct platform_device *pdev)
+{
+	struct pwrseq_qca *pwrseq_qca;
+	struct pwrseq *pwrseq;
+	struct pwrseq_provider *provider;
+	struct device *dev = &pdev->dev;
+	struct pwrseq_onecell_data *onecell;
+	const struct qca_device_data *data;
+	int ret, i;
+
+	data = device_get_match_data(dev);
+	if (!data)
+		data = &qca_soc_data_default;
+
+	pwrseq_qca = devm_kzalloc(dev, struct_size(pwrseq_qca, common.vregs, data->num_vregs), GFP_KERNEL);
+	if (!pwrseq_qca)
+		return -ENOMEM;
+
+	onecell = devm_kzalloc(dev, struct_size(onecell, pwrseqs, PWRSEQ_QCA_MAX), GFP_KERNEL);
+	if (!onecell)
+		return -ENOMEM;
+
+	ret = pwrseq_qca_common_init(dev, &pwrseq_qca->common, data);
+	if (ret)
+		return ret;
+
+	if (data->has_enable_gpios) {
+		struct gpio_desc *gpiod;
+
+		gpiod = devm_gpiod_get_optional(dev, "wifi-enable", GPIOD_OUT_LOW);
+		if (IS_ERR(gpiod))
+			return dev_err_probe(dev, PTR_ERR(gpiod), "failed to acquire WIFI enable GPIO\n");
+		else if (!gpiod)
+			dev_warn(dev, "No WiFi enable GPIO declared\n");
+
+		pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_WIFI].enable = gpiod;
+
+		gpiod = devm_gpiod_get_optional(dev, "bt-enable", GPIOD_OUT_LOW);
+		if (IS_ERR(gpiod))
+			return dev_err_probe(dev, PTR_ERR(gpiod), "failed to acquire BT enable GPIO\n");
+		else if (!gpiod)
+			dev_warn(dev, "No BT enable GPIO declared\n");
+
+		pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_BT].enable = gpiod;
+	}
+
+	/* If we have no control over device's enablement, make sure that sleep clock is always running */
+	if (!pwrseq_qca->common.vddio ||
+	    !pwrseq_qca->common.num_vregs ||
+	    !(pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_BT].enable &&
+	      pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_WIFI].enable)) {
+		ret = clk_set_rate(pwrseq_qca->common.susclk, SUSCLK_RATE_32KHZ);
+		if (ret)
+			return ret;
+
+		ret = clk_prepare_enable(pwrseq_qca->common.susclk);
+		if (ret)
+			return ret;
+
+		ret = devm_add_action_or_reset(dev, pwrseq_qca_unprepare_susclk, &pwrseq_qca->common);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < PWRSEQ_QCA_MAX; i++) {
+		pwrseq_qca->pwrseq_qcas[i].common = &pwrseq_qca->common;
+
+		pwrseq = devm_pwrseq_create(dev, &pwrseq_qca_ops, &pwrseq_qca->pwrseq_qcas[i]);
+		if (IS_ERR(pwrseq))
+			return PTR_ERR(pwrseq);
+
+		onecell->pwrseqs[i] = pwrseq;
+	}
+
+	onecell->num = PWRSEQ_QCA_MAX;
+
+	provider = devm_of_pwrseq_provider_register(dev, of_pwrseq_xlate_onecell, onecell);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct qca_device_data qca_soc_data_qca6390 = {
+	.vddio = { "vddio", 20000 },
+	.vregs = {
+		/* 2.0 V */
+		{ "vddpcie2", 15000 },
+		{ "vddrfa3", 400000 },
+
+		/* 0.95 V */
+		{ "vddaon", 100000 },
+		{ "vddpmu", 1250000 },
+		{ "vddrfa1", 200000 },
+
+		/* 1.35 V */
+		{ "vddrfa2", 400000 },
+		{ "vddpcie1", 35000 },
+	},
+	.num_vregs = 7,
+	.has_enable_gpios = true,
+};
+
+/* Shared between wcn3990 and wcn3991 */
+static const struct qca_device_data qca_soc_data_wcn3990 = {
+	.vddio = { "vddio", 15000 },
+	.vregs = {
+		{ "vddxo", 80000  },
+		{ "vddrf", 300000 },
+		{ "vddch0", 450000 },
+		{ "vddch1", 450000 },
+	},
+	.num_vregs = 4,
+};
+
+static const struct qca_device_data qca_soc_data_wcn3998 = {
+	.vddio = { "vddio", 10000 },
+	.vregs = {
+		{ "vddxo", 80000  },
+		{ "vddrf", 300000 },
+		{ "vddch0", 450000 },
+		{ "vddch1", 450000 },
+	},
+	.num_vregs = 4,
+};
+
+static const struct qca_device_data qca_soc_data_wcn6750 = {
+	.vddio = { "vddio", 5000 },
+	.vregs = {
+		{ "vddaon", 26000 },
+		{ "vddbtcxmx", 126000 },
+		{ "vddrfacmn", 12500 },
+		{ "vddrfa0p8", 102000 },
+		{ "vddrfa1p7", 302000 },
+		{ "vddrfa1p2", 257000 },
+		{ "vddrfa2p2", 1700000 },
+		{ "vddasd", 200 },
+	},
+	.num_vregs = 8,
+	.has_enable_gpios = true,
+};
+
+static const struct of_device_id pwrseq_qca_of_match[] = {
+	{ .compatible = "qcom,qca6174-pwrseq", },
+	{ .compatible = "qcom,qca6390-pwrseq", .data = &qca_soc_data_qca6390 },
+	{ .compatible = "qcom,qca9377-pwrseq" },
+	{ .compatible = "qcom,wcn3990-pwrseq", .data = &qca_soc_data_wcn3990 },
+	{ .compatible = "qcom,wcn3991-pwrseq", .data = &qca_soc_data_wcn3990 },
+	{ .compatible = "qcom,wcn3998-pwrseq", .data = &qca_soc_data_wcn3998 },
+	{ .compatible = "qcom,wcn6750-pwrseq", .data = &qca_soc_data_wcn6750 },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, pwrseq_qca_of_match);
+
+static struct platform_driver pwrseq_qca_driver = {
+	.probe = pwrseq_qca_probe,
+	.driver = {
+		.name = "pwrseq_qca",
+		.of_match_table = pwrseq_qca_of_match,
+	},
+};
+
+module_platform_driver(pwrseq_qca_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 05/13] pwrseq: add fallback support
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (3 preceding siblings ...)
  2021-08-29 13:12 ` [RFC v2 04/13] pwrseq: add support for QCA BT+WiFi power sequencer Dmitry Baryshkov
@ 2021-08-29 13:12 ` Dmitry Baryshkov
  2021-08-29 13:12 ` [RFC v2 06/13] pwrseq: pwrseq_qca: implement " Dmitry Baryshkov
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:12 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Power sequencer support requires changing device tree. To ease migration
to pwrseq, add support for pwrseq 'fallback': let the power sequencer
driver register special handler that if matched will create pwrseq
instance basing on the consumer device tree data.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/power/pwrseq/Makefile   |  2 +-
 drivers/power/pwrseq/core.c     |  3 ++
 drivers/power/pwrseq/fallback.c | 75 +++++++++++++++++++++++++++++++++
 include/linux/pwrseq/fallback.h | 36 ++++++++++++++++
 4 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 drivers/power/pwrseq/fallback.c
 create mode 100644 include/linux/pwrseq/fallback.h

diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
index 556bf5582d47..949ec848cf00 100644
--- a/drivers/power/pwrseq/Makefile
+++ b/drivers/power/pwrseq/Makefile
@@ -3,7 +3,7 @@
 # Makefile for power sequencer drivers.
 #
 
-obj-$(CONFIG_PWRSEQ) += core.o
+obj-$(CONFIG_PWRSEQ) += core.o fallback.o
 
 obj-$(CONFIG_PWRSEQ_EMMC)	+= pwrseq_emmc.o
 obj-$(CONFIG_PWRSEQ_QCA)	+= pwrseq_qca.o
diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
index 2e4e9d123e60..a43977a0eed8 100644
--- a/drivers/power/pwrseq/core.c
+++ b/drivers/power/pwrseq/core.c
@@ -15,6 +15,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/pwrseq/consumer.h>
 #include <linux/pwrseq/driver.h>
+#include <linux/pwrseq/fallback.h>
 #include <linux/slab.h>
 
 #define	to_pwrseq(a)	(container_of((a), struct pwrseq, dev))
@@ -108,6 +109,8 @@ struct pwrseq * __pwrseq_get(struct device *dev, const char *id, bool optional)
 	struct device_link *link;
 
 	pwrseq = _of_pwrseq_get(dev, id);
+	if (pwrseq == NULL)
+		pwrseq = pwrseq_fallback_get(dev, id);
 	if (pwrseq == NULL)
 		return optional ? NULL : ERR_PTR(-ENODEV);
 	else if (IS_ERR(pwrseq))
diff --git a/drivers/power/pwrseq/fallback.c b/drivers/power/pwrseq/fallback.c
new file mode 100644
index 000000000000..6ecf24dd8f29
--- /dev/null
+++ b/drivers/power/pwrseq/fallback.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2021 (c) Linaro Ltd.
+ * Author: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_device.h>
+#include <linux/pwrseq/fallback.h>
+#include <linux/slab.h>
+
+static DEFINE_MUTEX(pwrseq_fallback_mutex);
+static LIST_HEAD(pwrseq_fallback_list);
+
+int __pwrseq_fallback_register(struct pwrseq_fallback *fallback, struct module *owner)
+{
+	if (!try_module_get(owner))
+		return -EPROBE_DEFER;
+
+	fallback->owner = owner;
+
+	mutex_lock(&pwrseq_fallback_mutex);
+	list_add_tail(&fallback->list, &pwrseq_fallback_list);
+	mutex_unlock(&pwrseq_fallback_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__pwrseq_fallback_register);
+
+void pwrseq_fallback_unregister(struct pwrseq_fallback *fallback)
+{
+	mutex_lock(&pwrseq_fallback_mutex);
+	list_del(&fallback->list);
+	mutex_unlock(&pwrseq_fallback_mutex);
+
+	module_put(fallback->owner);
+
+	kfree(fallback);
+}
+EXPORT_SYMBOL_GPL(pwrseq_fallback_unregister);
+
+static bool pwrseq_fallback_match(struct device *dev, struct pwrseq_fallback *fallback)
+{
+	if (of_match_device(fallback->of_match_table, dev) != NULL)
+		return true;
+
+	/* We might add support for other matching options later */
+
+	return false;
+}
+
+struct pwrseq *pwrseq_fallback_get(struct device *dev, const char *id)
+{
+	struct pwrseq_fallback *fallback;
+	struct pwrseq *pwrseq = ERR_PTR(-ENODEV);
+
+	mutex_lock(&pwrseq_fallback_mutex);
+
+	list_for_each_entry(fallback, &pwrseq_fallback_list, list) {
+		if (!pwrseq_fallback_match(dev, fallback))
+			continue;
+
+		pwrseq = fallback->get(dev, id);
+		break;
+	}
+
+	mutex_unlock(&pwrseq_fallback_mutex);
+
+	if (!IS_ERR_OR_NULL(pwrseq))
+		dev_warn(dev, "legacy pwrseq support used for the device\n");
+
+	return pwrseq;
+}
diff --git a/include/linux/pwrseq/fallback.h b/include/linux/pwrseq/fallback.h
new file mode 100644
index 000000000000..616049df179f
--- /dev/null
+++ b/include/linux/pwrseq/fallback.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2021 Linaro Ltd.
+ */
+
+#ifndef __LINUX_PWRSEQ_FALLBACK_H__
+#define __LINUX_PWRSEQ_FALLBACK_H__
+
+#include <linux/list.h>
+
+struct pwrseq;
+
+struct device;
+struct module;
+struct of_device_id;
+
+struct pwrseq_fallback {
+	struct list_head list;
+	struct module *owner;
+
+	const struct of_device_id *of_match_table;
+
+	struct pwrseq *(*get)(struct device *dev, const char *id);
+};
+
+/* provider interface */
+
+int __pwrseq_fallback_register(struct pwrseq_fallback *fallback, struct module *owner);
+#define pwrseq_fallback_register(fallback) __pwrseq_fallback_register(fallback, THIS_MODULE)
+
+void pwrseq_fallback_unregister(struct pwrseq_fallback *fallback);
+
+/* internal interface */
+struct pwrseq *pwrseq_fallback_get(struct device *dev, const char *id);
+
+#endif /* __LINUX_PWRSEQ_DRIVER_H__ */
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 06/13] pwrseq: pwrseq_qca: implement fallback support
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (4 preceding siblings ...)
  2021-08-29 13:12 ` [RFC v2 05/13] pwrseq: add fallback support Dmitry Baryshkov
@ 2021-08-29 13:12 ` Dmitry Baryshkov
  2021-08-29 13:12 ` [RFC v2 07/13] Bluetooth: hci_qca: switch to using pwrseq Dmitry Baryshkov
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:12 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

While we are waiting for all users of wcn399x-bt to be converted to the
pwrseq declaration in the device tree, provide support for the pwrseq
fallback: if the regulators are declared in the device itself, create
pwrseq instance. This way the hci_qca driver doesn't have to cope with
old and new dts bindings.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/power/pwrseq/pwrseq_qca.c | 148 +++++++++++++++++++++++++++++-
 1 file changed, 147 insertions(+), 1 deletion(-)

diff --git a/drivers/power/pwrseq/pwrseq_qca.c b/drivers/power/pwrseq/pwrseq_qca.c
index 7aa5f2d94039..d01f1ef4626b 100644
--- a/drivers/power/pwrseq/pwrseq_qca.c
+++ b/drivers/power/pwrseq/pwrseq_qca.c
@@ -11,9 +11,11 @@
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/mod_devicetable.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
 #include <linux/pwrseq/driver.h>
+#include <linux/pwrseq/fallback.h>
 #include <linux/regulator/consumer.h>
 
 /* susclk rate */
@@ -367,5 +369,149 @@ static struct platform_driver pwrseq_qca_driver = {
 	},
 };
 
-module_platform_driver(pwrseq_qca_driver);
+struct pwrseq_qca_fallback {
+	struct pwrseq_qca_one qca_one;
+	struct pwrseq_qca_common common;
+};
+
+static const struct of_device_id pwrseq_qca_bt_of_match[] = {
+	{ .compatible = "qcom,qca6174-bt" },
+	{ .compatible = "qcom,qca6390-bt", .data = &qca_soc_data_qca6390},
+	{ .compatible = "qcom,qca9377-bt" },
+	{ .compatible = "qcom,wcn3990-bt", .data = &qca_soc_data_wcn3990 },
+	{ .compatible = "qcom,wcn3991-bt", .data = &qca_soc_data_wcn3990 },
+	{ .compatible = "qcom,wcn3998-bt", .data = &qca_soc_data_wcn3998 },
+	{ .compatible = "qcom,wcn6750-bt", .data = &qca_soc_data_wcn6750 },
+	{ /* sentinel */ },
+};
+
+static const struct qca_device_data qca_soc_data_wifi = {
+	.vregs = {
+		{ "vdd-1.8-xo", 80000  },
+		{ "vdd-1.3-rfa", 300000 },
+		{ "vdd-3.3-ch0", 450000 },
+		{ "vdd-3.3-ch1", 450000 },
+	},
+	.num_vregs = 4,
+};
+
+static const struct of_device_id pwrseq_qca_wifi_of_match[] = {
+	{ .compatible = "qcom,wcn3990-wifi", .data = &qca_soc_data_wifi },
+	{ /* sentinel */ }
+};
+
+static struct pwrseq * pwrseq_qca_fallback_get(struct device *dev)
+{
+	struct pwrseq_qca_fallback *fallback;
+	const struct of_device_id *match;
+	const struct qca_device_data *data;
+	struct gpio_desc *gpiod;
+	int ret;
+
+	match = of_match_device(pwrseq_qca_bt_of_match, dev);
+	if (!match)
+		return ERR_PTR(-ENODEV);
+
+	data = match->data;
+	if (!data)
+		data = &qca_soc_data_default;
+
+	fallback = devm_kzalloc(dev, struct_size(fallback, common.vregs, data->num_vregs), GFP_KERNEL);
+	if (!data)
+		return ERR_PTR(-ENOMEM);
+
+	fallback->qca_one.common = &fallback->common;
+
+	ret = pwrseq_qca_common_init(dev, &fallback->common, data);
+	if (ret)
+		return ERR_PTR(ret);
+
+	if (data->has_enable_gpios) {
+		gpiod = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
+		if (IS_ERR(gpiod))
+			return ERR_PTR(dev_err_probe(dev, PTR_ERR(gpiod), "failed to acquire enable GPIO\n"));
+		fallback->qca_one.enable = gpiod;
+	}
+
+	/* If we have no control over device's enablement, make sure that sleep clock is always running */
+	if (!fallback->common.vddio ||
+	    !fallback->common.num_vregs ||
+	    !fallback->qca_one.enable) {
+		ret = clk_set_rate(fallback->common.susclk, SUSCLK_RATE_32KHZ);
+		if (ret)
+			return ERR_PTR(ret);
+
+		ret = clk_prepare_enable(fallback->common.susclk);
+		if (ret)
+			return ERR_PTR(ret);
+
+		ret = devm_add_action_or_reset(dev, pwrseq_qca_unprepare_susclk, &fallback->common);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
+	return devm_pwrseq_create(dev, &pwrseq_qca_ops, &fallback->qca_one);
+}
+
+static struct pwrseq * pwrseq_qca_fallback_get_bt(struct device *dev, const char *id)
+{
+	if (strcmp(id, "bt"))
+		return ERR_PTR(-ENODEV);
+
+	return pwrseq_qca_fallback_get(dev);
+}
+
+static struct pwrseq * pwrseq_qca_fallback_get_wifi(struct device *dev, const char *id)
+{
+	if (strcmp(id, "wifi"))
+		return ERR_PTR(-ENODEV);
+
+	return pwrseq_qca_fallback_get(dev);
+}
+
+static struct pwrseq_fallback pwrseq_qca_fallback_bt = {
+	.get = pwrseq_qca_fallback_get_bt,
+	.of_match_table = pwrseq_qca_bt_of_match,
+};
+
+static struct pwrseq_fallback pwrseq_qca_fallback_wifi = {
+	.get = pwrseq_qca_fallback_get_wifi,
+	.of_match_table = pwrseq_qca_wifi_of_match,
+};
+
+static int __init pwrseq_qca_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&pwrseq_qca_driver);
+	if (ret)
+		return ret;
+
+	ret = pwrseq_fallback_register(&pwrseq_qca_fallback_bt);
+	if (ret)
+		goto err_bt;
+
+	ret = pwrseq_fallback_register(&pwrseq_qca_fallback_wifi);
+	if (ret)
+		goto err_wifi;
+
+	return 0;
+
+err_wifi:
+	pwrseq_fallback_unregister(&pwrseq_qca_fallback_bt);
+err_bt:
+	platform_driver_unregister(&pwrseq_qca_driver);
+
+	return ret;
+}
+module_init(pwrseq_qca_init);
+
+static void __exit pwrseq_qca_exit(void)
+{
+	pwrseq_fallback_unregister(&pwrseq_qca_fallback_wifi);
+	pwrseq_fallback_unregister(&pwrseq_qca_fallback_bt);
+	platform_driver_unregister(&pwrseq_qca_driver);
+}
+module_exit(pwrseq_qca_exit);
+
 MODULE_LICENSE("GPL v2");
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 07/13] Bluetooth: hci_qca: switch to using pwrseq
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (5 preceding siblings ...)
  2021-08-29 13:12 ` [RFC v2 06/13] pwrseq: pwrseq_qca: implement " Dmitry Baryshkov
@ 2021-08-29 13:12 ` Dmitry Baryshkov
  2021-08-29 13:13 ` [RFC v2 08/13] ath10k: add support for pwrseq sequencing Dmitry Baryshkov
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:12 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/bluetooth/hci_qca.c | 260 +++++-------------------------------
 1 file changed, 37 insertions(+), 223 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 53deea2eb7b4..1e4416916533 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -16,19 +16,17 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/devcoredump.h>
 #include <linux/device.h>
-#include <linux/gpio/consumer.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/acpi.h>
 #include <linux/platform_device.h>
-#include <linux/regulator/consumer.h>
+#include <linux/pwrseq/consumer.h>
 #include <linux/serdev.h>
 #include <linux/mutex.h>
 #include <asm/unaligned.h>
@@ -54,9 +52,6 @@
 	(MEMDUMP_TIMEOUT_MS + FW_DOWNLOAD_TIMEOUT_MS)
 #define FW_DOWNLOAD_TIMEOUT_MS		3000
 
-/* susclk rate */
-#define SUSCLK_RATE_32KHZ	32768
-
 /* Controller debug log header */
 #define QCA_DEBUG_HANDLE	0x2EDC
 
@@ -200,28 +195,17 @@ struct qca_vreg {
 
 struct qca_device_data {
 	enum qca_btsoc_type soc_type;
-	struct qca_vreg *vregs;
-	size_t num_vregs;
 	uint32_t capabilities;
 };
 
 /*
  * Platform data for the QCA Bluetooth power driver.
  */
-struct qca_power {
-	struct device *dev;
-	struct regulator_bulk_data *vreg_bulk;
-	int num_vregs;
-	bool vregs_on;
-};
-
 struct qca_serdev {
 	struct hci_uart	 serdev_hu;
-	struct gpio_desc *bt_en;
-	struct gpio_desc *sw_ctrl;
-	struct clk	 *susclk;
 	enum qca_btsoc_type btsoc_type;
-	struct qca_power *bt_power;
+	struct pwrseq *pwrseq;
+	bool vregs_on;
 	u32 init_speed;
 	u32 oper_speed;
 	const char *firmware_name;
@@ -1596,13 +1580,12 @@ static int qca_regulator_init(struct hci_uart *hu)
 	enum qca_btsoc_type soc_type = qca_soc_type(hu);
 	struct qca_serdev *qcadev;
 	int ret;
-	bool sw_ctrl_state;
 
 	/* Check for vregs status, may be hci down has turned
 	 * off the voltage regulator.
 	 */
 	qcadev = serdev_device_get_drvdata(hu->serdev);
-	if (!qcadev->bt_power->vregs_on) {
+	if (!qcadev->vregs_on) {
 		serdev_device_close(hu->serdev);
 		ret = qca_regulator_enable(qcadev);
 		if (ret)
@@ -1623,19 +1606,9 @@ static int qca_regulator_init(struct hci_uart *hu)
 			return ret;
 	}
 
-	/* For wcn6750 need to enable gpio bt_en */
-	if (qcadev->bt_en) {
-		gpiod_set_value_cansleep(qcadev->bt_en, 0);
-		msleep(50);
-		gpiod_set_value_cansleep(qcadev->bt_en, 1);
-		msleep(50);
-		if (qcadev->sw_ctrl) {
-			sw_ctrl_state = gpiod_get_value_cansleep(qcadev->sw_ctrl);
-			bt_dev_dbg(hu->hdev, "SW_CTRL is %d", sw_ctrl_state);
-		}
-	}
-
-	qca_set_speed(hu, QCA_INIT_SPEED);
+	if (qca_is_wcn399x(soc_type) ||
+	    qca_is_wcn6750(soc_type))
+		qca_set_speed(hu, QCA_INIT_SPEED);
 
 	if (qca_is_wcn399x(soc_type)) {
 		ret = qca_send_power_pulse(hu, true);
@@ -1655,7 +1628,9 @@ static int qca_regulator_init(struct hci_uart *hu)
 		return ret;
 	}
 
-	hci_uart_set_flow_control(hu, false);
+	if (qca_is_wcn399x(soc_type) ||
+	    qca_is_wcn6750(soc_type))
+		hci_uart_set_flow_control(hu, false);
 
 	return 0;
 }
@@ -1663,8 +1638,6 @@ static int qca_regulator_init(struct hci_uart *hu)
 static int qca_power_on(struct hci_dev *hdev)
 {
 	struct hci_uart *hu = hci_get_drvdata(hdev);
-	enum qca_btsoc_type soc_type = qca_soc_type(hu);
-	struct qca_serdev *qcadev;
 	struct qca_data *qca = hu->priv;
 	int ret = 0;
 
@@ -1674,17 +1647,7 @@ static int qca_power_on(struct hci_dev *hdev)
 	if (!hu->serdev)
 		return 0;
 
-	if (qca_is_wcn399x(soc_type) ||
-	    qca_is_wcn6750(soc_type)) {
-		ret = qca_regulator_init(hu);
-	} else {
-		qcadev = serdev_device_get_drvdata(hu->serdev);
-		if (qcadev->bt_en) {
-			gpiod_set_value_cansleep(qcadev->bt_en, 1);
-			/* Controller needs time to bootup. */
-			msleep(150);
-		}
-	}
+	ret = qca_regulator_init(hu);
 
 	clear_bit(QCA_BT_OFF, &qca->flags);
 	return ret;
@@ -1820,57 +1783,23 @@ static const struct hci_uart_proto qca_proto = {
 
 static const struct qca_device_data qca_soc_data_wcn3990 = {
 	.soc_type = QCA_WCN3990,
-	.vregs = (struct qca_vreg []) {
-		{ "vddio", 15000  },
-		{ "vddxo", 80000  },
-		{ "vddrf", 300000 },
-		{ "vddch0", 450000 },
-	},
-	.num_vregs = 4,
 };
 
 static const struct qca_device_data qca_soc_data_wcn3991 = {
 	.soc_type = QCA_WCN3991,
-	.vregs = (struct qca_vreg []) {
-		{ "vddio", 15000  },
-		{ "vddxo", 80000  },
-		{ "vddrf", 300000 },
-		{ "vddch0", 450000 },
-	},
-	.num_vregs = 4,
 	.capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
 };
 
 static const struct qca_device_data qca_soc_data_wcn3998 = {
 	.soc_type = QCA_WCN3998,
-	.vregs = (struct qca_vreg []) {
-		{ "vddio", 10000  },
-		{ "vddxo", 80000  },
-		{ "vddrf", 300000 },
-		{ "vddch0", 450000 },
-	},
-	.num_vregs = 4,
 };
 
 static const struct qca_device_data qca_soc_data_qca6390 = {
 	.soc_type = QCA_QCA6390,
-	.num_vregs = 0,
 };
 
 static const struct qca_device_data qca_soc_data_wcn6750 = {
 	.soc_type = QCA_WCN6750,
-	.vregs = (struct qca_vreg []) {
-		{ "vddio", 5000 },
-		{ "vddaon", 26000 },
-		{ "vddbtcxmx", 126000 },
-		{ "vddrfacmn", 12500 },
-		{ "vddrfa0p8", 102000 },
-		{ "vddrfa1p7", 302000 },
-		{ "vddrfa1p2", 257000 },
-		{ "vddrfa2p2", 1700000 },
-		{ "vddasd", 200 },
-	},
-	.num_vregs = 9,
 	.capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
 };
 
@@ -1880,7 +1809,6 @@ static void qca_power_shutdown(struct hci_uart *hu)
 	struct qca_data *qca = hu->priv;
 	unsigned long flags;
 	enum qca_btsoc_type soc_type = qca_soc_type(hu);
-	bool sw_ctrl_state;
 
 	/* From this point we go into power off state. But serial port is
 	 * still open, stop queueing the IBS data and flush all the buffered
@@ -1902,19 +1830,10 @@ static void qca_power_shutdown(struct hci_uart *hu)
 	if (qca_is_wcn399x(soc_type)) {
 		host_set_baudrate(hu, 2400);
 		qca_send_power_pulse(hu, false);
-		qca_regulator_disable(qcadev);
-	} else if (soc_type == QCA_WCN6750) {
-		gpiod_set_value_cansleep(qcadev->bt_en, 0);
-		msleep(100);
-		qca_regulator_disable(qcadev);
-		if (qcadev->sw_ctrl) {
-			sw_ctrl_state = gpiod_get_value_cansleep(qcadev->sw_ctrl);
-			bt_dev_dbg(hu->hdev, "SW_CTRL is %d", sw_ctrl_state);
-		}
-	} else if (qcadev->bt_en) {
-		gpiod_set_value_cansleep(qcadev->bt_en, 0);
 	}
 
+	qca_regulator_disable(qcadev);
+
 	set_bit(QCA_BT_OFF, &qca->flags);
 }
 
@@ -1940,75 +1859,34 @@ static int qca_power_off(struct hci_dev *hdev)
 
 static int qca_regulator_enable(struct qca_serdev *qcadev)
 {
-	struct qca_power *power = qcadev->bt_power;
 	int ret;
 
 	/* Already enabled */
-	if (power->vregs_on)
+	if (qcadev->vregs_on)
 		return 0;
 
-	BT_DBG("enabling %d regulators)", power->num_vregs);
+	BT_DBG("enabling regulators)");
 
-	ret = regulator_bulk_enable(power->num_vregs, power->vreg_bulk);
+	ret = pwrseq_full_power_on(qcadev->pwrseq);
 	if (ret)
 		return ret;
 
-	power->vregs_on = true;
-
-	ret = clk_prepare_enable(qcadev->susclk);
-	if (ret)
-		qca_regulator_disable(qcadev);
+	qcadev->vregs_on = true;
 
 	return ret;
 }
 
 static void qca_regulator_disable(struct qca_serdev *qcadev)
 {
-	struct qca_power *power;
-
 	if (!qcadev)
 		return;
 
-	power = qcadev->bt_power;
-
 	/* Already disabled? */
-	if (!power->vregs_on)
+	if (!qcadev->vregs_on)
 		return;
 
-	regulator_bulk_disable(power->num_vregs, power->vreg_bulk);
-	power->vregs_on = false;
-
-	clk_disable_unprepare(qcadev->susclk);
-}
-
-static int qca_init_regulators(struct qca_power *qca,
-				const struct qca_vreg *vregs, size_t num_vregs)
-{
-	struct regulator_bulk_data *bulk;
-	int ret;
-	int i;
-
-	bulk = devm_kcalloc(qca->dev, num_vregs, sizeof(*bulk), GFP_KERNEL);
-	if (!bulk)
-		return -ENOMEM;
-
-	for (i = 0; i < num_vregs; i++)
-		bulk[i].supply = vregs[i].name;
-
-	ret = devm_regulator_bulk_get(qca->dev, num_vregs, bulk);
-	if (ret < 0)
-		return ret;
-
-	for (i = 0; i < num_vregs; i++) {
-		ret = regulator_set_load(bulk[i].consumer, vregs[i].load_uA);
-		if (ret)
-			return ret;
-	}
-
-	qca->vreg_bulk = bulk;
-	qca->num_vregs = num_vregs;
-
-	return 0;
+	pwrseq_power_off(qcadev->pwrseq);
+	qcadev->vregs_on = false;
 }
 
 static int qca_serdev_probe(struct serdev_device *serdev)
@@ -2017,7 +1895,6 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 	struct hci_dev *hdev;
 	const struct qca_device_data *data;
 	int err;
-	bool power_ctrl_enabled = true;
 
 	qcadev = devm_kzalloc(&serdev->dev, sizeof(*qcadev), GFP_KERNEL);
 	if (!qcadev)
@@ -2033,89 +1910,29 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 	if (!qcadev->oper_speed)
 		BT_DBG("UART will pick default operating speed");
 
-	if (data &&
-	    (qca_is_wcn399x(data->soc_type) ||
-	    qca_is_wcn6750(data->soc_type))) {
-		qcadev->btsoc_type = data->soc_type;
-		qcadev->bt_power = devm_kzalloc(&serdev->dev,
-						sizeof(struct qca_power),
-						GFP_KERNEL);
-		if (!qcadev->bt_power)
-			return -ENOMEM;
-
-		qcadev->bt_power->dev = &serdev->dev;
-		err = qca_init_regulators(qcadev->bt_power, data->vregs,
-					  data->num_vregs);
-		if (err) {
-			BT_ERR("Failed to init regulators:%d", err);
-			return err;
-		}
-
-		qcadev->bt_power->vregs_on = false;
-
-		qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
-					       GPIOD_OUT_LOW);
-		if (!qcadev->bt_en && data->soc_type == QCA_WCN6750) {
-			dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n");
-			power_ctrl_enabled = false;
-		}
 
-		qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl",
-					       GPIOD_IN);
-		if (!qcadev->sw_ctrl && data->soc_type == QCA_WCN6750)
-			dev_warn(&serdev->dev, "failed to acquire SW_CTRL gpio\n");
-
-		qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
-		if (IS_ERR(qcadev->susclk)) {
-			dev_err(&serdev->dev, "failed to acquire clk\n");
-			return PTR_ERR(qcadev->susclk);
-		}
-
-		err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
-		if (err) {
-			BT_ERR("wcn3990 serdev registration failed");
-			return err;
-		}
-	} else {
-		if (data)
-			qcadev->btsoc_type = data->soc_type;
-		else
-			qcadev->btsoc_type = QCA_ROME;
+	if (data)
+		qcadev->btsoc_type = data->soc_type;
+	else
+		qcadev->btsoc_type = QCA_ROME;
 
-		qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
-					       GPIOD_OUT_LOW);
-		if (!qcadev->bt_en) {
-			dev_warn(&serdev->dev, "failed to acquire enable gpio\n");
-			power_ctrl_enabled = false;
-		}
+	qcadev->pwrseq = devm_pwrseq_get(&serdev->dev, "bt");
+	if (IS_ERR(qcadev->pwrseq)) {
+		dev_err(&serdev->dev, "failed to acquire pwrseq\n");
+		return PTR_ERR(qcadev->pwrseq);
+	}
+	qcadev->vregs_on = false;
 
-		qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
-		if (IS_ERR(qcadev->susclk)) {
-			dev_warn(&serdev->dev, "failed to acquire clk\n");
-			return PTR_ERR(qcadev->susclk);
-		}
-		err = clk_set_rate(qcadev->susclk, SUSCLK_RATE_32KHZ);
-		if (err)
-			return err;
-
-		err = clk_prepare_enable(qcadev->susclk);
-		if (err)
-			return err;
-
-		err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
-		if (err) {
-			BT_ERR("Rome serdev registration failed");
-			clk_disable_unprepare(qcadev->susclk);
-			return err;
-		}
+	err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
+	if (err) {
+		BT_ERR("wcn3990 serdev registration failed");
+		return err;
 	}
 
 	hdev = qcadev->serdev_hu.hdev;
 
-	if (power_ctrl_enabled) {
-		set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
-		hdev->shutdown = qca_power_off;
-	}
+	set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
+	hdev->shutdown = qca_power_off;
 
 	if (data) {
 		/* Wideband speech support must be set per driver since it can't
@@ -2135,14 +1952,11 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 static void qca_serdev_remove(struct serdev_device *serdev)
 {
 	struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev);
-	struct qca_power *power = qcadev->bt_power;
 
 	if ((qca_is_wcn399x(qcadev->btsoc_type) ||
 	     qca_is_wcn6750(qcadev->btsoc_type)) &&
-	     power->vregs_on)
+	     qcadev->vregs_on)
 		qca_power_shutdown(&qcadev->serdev_hu);
-	else if (qcadev->susclk)
-		clk_disable_unprepare(qcadev->susclk);
 
 	hci_uart_unregister_device(&qcadev->serdev_hu);
 }
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 08/13] ath10k: add support for pwrseq sequencing
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (6 preceding siblings ...)
  2021-08-29 13:12 ` [RFC v2 07/13] Bluetooth: hci_qca: switch to using pwrseq Dmitry Baryshkov
@ 2021-08-29 13:13 ` Dmitry Baryshkov
  2021-08-29 13:13 ` [RFC v2 09/13] arm64: dts: qcom: sdm845-db845c: switch bt+wifi to qca power sequencer Dmitry Baryshkov
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Power sequencing for Qualcomm WiFi+BT chipsets are being reworked to use
pwrseq rather than individually handling all the regulators. Add support
for pwrseq to ath10k SNOC driver.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/net/wireless/ath/ath10k/snoc.c | 45 +++++++++++++-------------
 drivers/net/wireless/ath/ath10k/snoc.h |  4 +--
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index ea00fbb15601..8578c56982df 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -14,6 +14,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/of_address.h>
 #include <linux/iommu.h>
+#include <linux/pwrseq/consumer.h>
 
 #include "ce.h"
 #include "coredump.h"
@@ -41,14 +42,6 @@ static char *const ce_name[] = {
 	"WLAN_CE_11",
 };
 
-static const char * const ath10k_regulators[] = {
-	"vdd-0.8-cx-mx",
-	"vdd-1.8-xo",
-	"vdd-1.3-rfa",
-	"vdd-3.3-ch0",
-	"vdd-3.3-ch1",
-};
-
 static const char * const ath10k_clocks[] = {
 	"cxo_ref_clk_pin", "qdss",
 };
@@ -1010,10 +1003,14 @@ static int ath10k_hw_power_on(struct ath10k *ar)
 
 	ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power on\n");
 
-	ret = regulator_bulk_enable(ar_snoc->num_vregs, ar_snoc->vregs);
+	ret = pwrseq_full_power_on(ar_snoc->pwrseq);
 	if (ret)
 		return ret;
 
+	ret = regulator_enable(ar_snoc->vreg_cx_mx);
+	if (ret)
+		goto vreg_pwrseq_off;
+
 	ret = clk_bulk_prepare_enable(ar_snoc->num_clks, ar_snoc->clks);
 	if (ret)
 		goto vreg_off;
@@ -1021,11 +1018,14 @@ static int ath10k_hw_power_on(struct ath10k *ar)
 	return ret;
 
 vreg_off:
-	regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
+	regulator_disable(ar_snoc->vreg_cx_mx);
+vreg_pwrseq_off:
+	pwrseq_power_off(ar_snoc->pwrseq);
+
 	return ret;
 }
 
-static int ath10k_hw_power_off(struct ath10k *ar)
+static void ath10k_hw_power_off(struct ath10k *ar)
 {
 	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
 
@@ -1033,7 +1033,9 @@ static int ath10k_hw_power_off(struct ath10k *ar)
 
 	clk_bulk_disable_unprepare(ar_snoc->num_clks, ar_snoc->clks);
 
-	return regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
+	regulator_disable(ar_snoc->vreg_cx_mx);
+
+	pwrseq_power_off(ar_snoc->pwrseq);
 }
 
 static void ath10k_snoc_wlan_disable(struct ath10k *ar)
@@ -1691,20 +1693,19 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
 		goto err_release_resource;
 	}
 
-	ar_snoc->num_vregs = ARRAY_SIZE(ath10k_regulators);
-	ar_snoc->vregs = devm_kcalloc(&pdev->dev, ar_snoc->num_vregs,
-				      sizeof(*ar_snoc->vregs), GFP_KERNEL);
-	if (!ar_snoc->vregs) {
-		ret = -ENOMEM;
+	ar_snoc->pwrseq = devm_pwrseq_get(&pdev->dev, "wifi");
+	if (IS_ERR(ar_snoc->pwrseq)) {
+		ret = PTR_ERR(ar_snoc->pwrseq);
+		if (ret != -EPROBE_DEFER)
+			ath10k_warn(ar, "failed to acquire pwrseq: %d\n", ret);
 		goto err_free_irq;
 	}
-	for (i = 0; i < ar_snoc->num_vregs; i++)
-		ar_snoc->vregs[i].supply = ath10k_regulators[i];
 
-	ret = devm_regulator_bulk_get(&pdev->dev, ar_snoc->num_vregs,
-				      ar_snoc->vregs);
-	if (ret < 0)
+	ar_snoc->vreg_cx_mx = devm_regulator_get(&pdev->dev, "vdd-0.8-cx-mx");
+	if (IS_ERR(ar_snoc->vreg_cx_mx)) {
+		ret = PTR_ERR(ar_snoc->vreg_cx_mx);
 		goto err_free_irq;
+	}
 
 	ar_snoc->num_clks = ARRAY_SIZE(ath10k_clocks);
 	ar_snoc->clks = devm_kcalloc(&pdev->dev, ar_snoc->num_clks,
diff --git a/drivers/net/wireless/ath/ath10k/snoc.h b/drivers/net/wireless/ath/ath10k/snoc.h
index 5095d1893681..5188d6f6f850 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.h
+++ b/drivers/net/wireless/ath/ath10k/snoc.h
@@ -70,10 +70,10 @@ struct ath10k_snoc {
 	struct ath10k_snoc_ce_irq ce_irqs[CE_COUNT_MAX];
 	struct ath10k_ce ce;
 	struct timer_list rx_post_retry;
-	struct regulator_bulk_data *vregs;
-	size_t num_vregs;
+	struct regulator *vreg_cx_mx;
 	struct clk_bulk_data *clks;
 	size_t num_clks;
+	struct pwrseq *pwrseq;
 	struct ath10k_qmi *qmi;
 	unsigned long flags;
 	bool xo_cal_supported;
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 09/13] arm64: dts: qcom: sdm845-db845c: switch bt+wifi to qca power sequencer
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (7 preceding siblings ...)
  2021-08-29 13:13 ` [RFC v2 08/13] ath10k: add support for pwrseq sequencing Dmitry Baryshkov
@ 2021-08-29 13:13 ` Dmitry Baryshkov
  2021-08-29 13:13 ` [RFC v2 10/13] arm64: dts: qcom: qrb5165-rb5: add bluetooth support Dmitry Baryshkov
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Switch sdm845-db845c device tree to use new power sequencer driver
rather than separate regulators.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 21 ++++++++++++++-------
 arch/arm64/boot/dts/qcom/sdm845.dtsi       |  6 ++++++
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
index 2d5533dd4ec2..a6a34a959a91 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
@@ -629,6 +629,16 @@ &qupv3_id_1 {
 	status = "okay";
 };
 
+&qca_pwrseq {
+	status = "okay";
+
+	vddio-supply = <&vreg_s4a_1p8>;
+
+	vddxo-supply = <&vreg_l7a_1p8>;
+	vddrf-supply = <&vreg_l17a_1p3>;
+	vddch0-supply = <&vreg_l25a_3p3>;
+};
+
 &sdhc_2 {
 	status = "okay";
 
@@ -916,10 +926,8 @@ &uart6 {
 	bluetooth {
 		compatible = "qcom,wcn3990-bt";
 
-		vddio-supply = <&vreg_s4a_1p8>;
-		vddxo-supply = <&vreg_l7a_1p8>;
-		vddrf-supply = <&vreg_l17a_1p3>;
-		vddch0-supply = <&vreg_l25a_3p3>;
+		bt-pwrseq = <&qca_pwrseq 1>;
+
 		max-speed = <3200000>;
 	};
 };
@@ -1036,9 +1044,8 @@ &wifi {
 	status = "okay";
 
 	vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
-	vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
-	vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
-	vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
+
+	wifi-pwrseq = <&qca_pwrseq 0>;
 
 	qcom,snoc-host-cap-8bit-quirk;
 };
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0a86fe71a66d..78e889b2c8dd 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1051,6 +1051,12 @@ psci {
 		method = "smc";
 	};
 
+	qca_pwrseq: qca-pwrseq {
+		compatible = "qcom,wcn3990-pwrseq";
+		#pwrseq-cells = <1>;
+		status = "disabled";
+	};
+
 	soc: soc@0 {
 		#address-cells = <2>;
 		#size-cells = <2>;
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 10/13] arm64: dts: qcom: qrb5165-rb5: add bluetooth support
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (8 preceding siblings ...)
  2021-08-29 13:13 ` [RFC v2 09/13] arm64: dts: qcom: sdm845-db845c: switch bt+wifi to qca power sequencer Dmitry Baryshkov
@ 2021-08-29 13:13 ` Dmitry Baryshkov
  2021-08-29 13:13 ` [RFC v2 11/13] arm64: dts: qcom: sdm845-db845c: add second channel to qca power sequencer Dmitry Baryshkov
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Add support for the bluetooth part of the QCA6391 BT+WiFi chip present
on the RB5 board. WiFi is not supported yet, as it requires separate
handling of the PCIe device power.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 50 ++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index 8ac96f8e79d4..326330f528fc 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -19,6 +19,7 @@ / {
 
 	aliases {
 		serial0 = &uart12;
+		serial1 = &uart6;
 		sdhc2 = &sdhc_2;
 	};
 
@@ -98,6 +99,25 @@ lt9611_3v3: lt9611-3v3 {
 		regulator-always-on;
 	};
 
+	qca_pwrseq: qca-pwrseq {
+		compatible = "qcom,qca6390-pwrseq";
+
+		#pwrseq-cells = <1>;
+
+		vddaon-supply = <&vreg_s6a_0p95>;
+		vddpmu-supply = <&vreg_s2f_0p95>;
+		vddrfa1-supply = <&vreg_s2f_0p95>;
+		vddrfa2-supply = <&vreg_s8c_1p3>;
+		vddrfa3-supply = <&vreg_s5a_1p9>;
+		vddpcie1-supply = <&vreg_s8c_1p3>;
+		vddpcie2-supply = <&vreg_s5a_1p9>;
+		vddio-supply = <&vreg_s4a_1p8>;
+
+		bt-enable-gpios = <&tlmm 21 GPIO_ACTIVE_HIGH>;
+		wifi-enable-gpios = <&tlmm 20 GPIO_ACTIVE_HIGH>;
+		swctrl-gpios = <&tlmm 124 GPIO_ACTIVE_HIGH>;
+	};
+
 	thermal-zones {
 		conn-thermal {
 			polling-delay-passive = <0>;
@@ -804,6 +824,26 @@ lt9611_rst_pin: lt9611-rst-pin {
 	};
 };
 
+&qup_uart6_default {
+	ctsrx {
+		pins = "gpio16", "gpio19";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	rts {
+		pins = "gpio17";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	tx {
+		pins = "gpio18";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
@@ -1193,6 +1233,16 @@ sdc2_card_det_n: sd-card-det-n {
 	};
 };
 
+&uart6 {
+	status = "okay";
+	bluetooth {
+		compatible = "qcom,qca6390-bt";
+		clocks = <&sleep_clk>;
+
+		bt-pwrseq = <&qca_pwrseq 1>;
+	};
+};
+
 &uart12 {
 	status = "okay";
 };
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 11/13] arm64: dts: qcom: sdm845-db845c: add second channel to qca power sequencer
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (9 preceding siblings ...)
  2021-08-29 13:13 ` [RFC v2 10/13] arm64: dts: qcom: qrb5165-rb5: add bluetooth support Dmitry Baryshkov
@ 2021-08-29 13:13 ` Dmitry Baryshkov
  2021-08-29 13:13 ` [RFC v2 12/13] WIP: PCI: qcom: use pwrseq to power up bus devices Dmitry Baryshkov
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

On DB845c board WiFi/BT chip can use both RF channels/antennas, so add
vddch1-supply property.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
index a6a34a959a91..0f3214c60980 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
@@ -342,6 +342,12 @@ vreg_l21a_2p95: ldo21 {
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
+		vreg_l23a_3p3: ldo23 {
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3312000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
 		vreg_l24a_3p075: ldo24 {
 			regulator-min-microvolt = <3088000>;
 			regulator-max-microvolt = <3088000>;
@@ -637,6 +643,7 @@ &qca_pwrseq {
 	vddxo-supply = <&vreg_l7a_1p8>;
 	vddrf-supply = <&vreg_l17a_1p3>;
 	vddch0-supply = <&vreg_l25a_3p3>;
+	vddch1-supply = <&vreg_l23a_3p3>;
 };
 
 &sdhc_2 {
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 12/13] WIP: PCI: qcom: use pwrseq to power up bus devices
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (10 preceding siblings ...)
  2021-08-29 13:13 ` [RFC v2 11/13] arm64: dts: qcom: sdm845-db845c: add second channel to qca power sequencer Dmitry Baryshkov
@ 2021-08-29 13:13 ` Dmitry Baryshkov
  2021-08-29 13:13 ` [RFC v2 13/13] WIP: arm64: dts: qcom: qrb5165-rb5: add bus-pwrseq property to pcie0 Dmitry Baryshkov
  2021-09-13 23:39 ` [RFC v2 00/13] create power sequencing subsystem Steev Klimaszewski
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Use bus-pwrseq device tree node to power up the devices on the bus. This
is to be rewritten with the proper code parsing the device tree and
powering up individual devices.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 8a7a300163e5..a60d41fbcd6f 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -23,6 +23,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
+#include <linux/pwrseq/consumer.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
@@ -1467,6 +1468,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	struct pcie_port *pp;
 	struct dw_pcie *pci;
 	struct qcom_pcie *pcie;
+	struct pwrseq *pwrseq;
 	int ret;
 
 	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
@@ -1520,6 +1522,17 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 	pp->ops = &qcom_pcie_dw_ops;
 
+	pwrseq = devm_pwrseq_get_optional(dev, "bus");
+	if (IS_ERR(pwrseq)) {
+		ret = PTR_ERR(pwrseq);
+		goto err_pm_runtime_put;
+	}
+	if (pwrseq) {
+		ret = pwrseq_full_power_on(pwrseq);
+		if (ret)
+			goto err_pm_runtime_put;
+	}
+
 	ret = phy_init(pcie->phy);
 	if (ret) {
 		pm_runtime_disable(&pdev->dev);
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC v2 13/13] WIP: arm64: dts: qcom: qrb5165-rb5: add bus-pwrseq property to pcie0
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (11 preceding siblings ...)
  2021-08-29 13:13 ` [RFC v2 12/13] WIP: PCI: qcom: use pwrseq to power up bus devices Dmitry Baryshkov
@ 2021-08-29 13:13 ` Dmitry Baryshkov
  2021-09-13 23:39 ` [RFC v2 00/13] create power sequencing subsystem Steev Klimaszewski
  13 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-08-29 13:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index 326330f528fc..0c347cb6f8e0 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -689,6 +689,7 @@ wifi-therm@1 {
 
 &pcie0 {
 	status = "okay";
+	bus-pwrseq = <&qca_pwrseq 0>;
 };
 
 &pcie0_phy {
-- 
2.33.0


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC v2 01/13] power: add power sequencer subsystem
  2021-08-29 13:12 ` [RFC v2 01/13] power: add power sequencer subsystem Dmitry Baryshkov
@ 2021-09-10 10:02   ` Ulf Hansson
  2021-09-13 12:32     ` Dmitry Baryshkov
  0 siblings, 1 reply; 19+ messages in thread
From: Ulf Hansson @ 2021-09-10 10:02 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Andy Gross, Bjorn Andersson, Marcel Holtmann, Johan Hedberg,
	Luiz Augusto von Dentz, Kalle Valo, David S. Miller,
	Jakub Kicinski, Stanimir Varbanov, linux-arm-msm, linux-mmc,
	Linux Kernel Mailing List, linux-bluetooth, ath10k,
	linux-wireless, netdev

On Sun, 29 Aug 2021 at 15:13, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Basing on MMC's pwrseq support code, add separate power sequencer
> subsystem. It will be used by other drivers to handle device power up
> requirements.

This is far too vague. You are suggesting to add a new subsystem, I
think that deserves some more explanations as justifications.

Additionally, it wouldn't hurt to explain a bit how the actual
subsystem is supposed to work, at least from a toplevel point of view.

>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/power/Kconfig           |   1 +
>  drivers/power/Makefile          |   1 +
>  drivers/power/pwrseq/Kconfig    |  11 +
>  drivers/power/pwrseq/Makefile   |   6 +
>  drivers/power/pwrseq/core.c     | 412 ++++++++++++++++++++++++++++++++
>  include/linux/pwrseq/consumer.h |  88 +++++++
>  include/linux/pwrseq/driver.h   |  75 ++++++
>  7 files changed, 594 insertions(+)
>  create mode 100644 drivers/power/pwrseq/Kconfig
>  create mode 100644 drivers/power/pwrseq/Makefile
>  create mode 100644 drivers/power/pwrseq/core.c
>  create mode 100644 include/linux/pwrseq/consumer.h
>  create mode 100644 include/linux/pwrseq/driver.h

I noticed there is no update of the MAINTAINERS file. We need that to
be a part of the $subject patch as well, I think. But, let's discuss
that later.

>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 696bf77a7042..c87cd2240a74 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +source "drivers/power/pwrseq/Kconfig"
>  source "drivers/power/reset/Kconfig"
>  source "drivers/power/supply/Kconfig"
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index effbf0377f32..1dbce454a8c4 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-$(CONFIG_POWER_RESET)      += reset/
>  obj-$(CONFIG_POWER_SUPPLY)     += supply/
> +obj-$(CONFIG_PWRSEQ)           += pwrseq/
> diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
> new file mode 100644
> index 000000000000..8904ec9ed541
> --- /dev/null
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +menuconfig PWRSEQ
> +       bool "Power Sequencer drivers"
> +       help
> +         Provides support for special power sequencing drivers.

This needs more description. The name "power sequencer" isn't entirely
self-explanatory, for when this should be used. I am not saying you
should invent a new name, rather just extend the description so people
get a better idea of what this is supposed to be used for.

> +
> +         Say Y here to enable support for such devices
> +
> +if PWRSEQ
> +
> +endif
> diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
> new file mode 100644
> index 000000000000..108429ff6445
> --- /dev/null
> +++ b/drivers/power/pwrseq/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for power sequencer drivers.
> +#
> +
> +obj-$(CONFIG_PWRSEQ) += core.o
> diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
> new file mode 100644
> index 000000000000..2e4e9d123e60
> --- /dev/null
> +++ b/drivers/power/pwrseq/core.c
> @@ -0,0 +1,412 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2021 (c) Linaro Ltd.
> + * Author: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> + *
> + * Based on phy-core.c:
> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
> + */
> +
> +#include <linux/device.h>
> +#include <linux/idr.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pwrseq/consumer.h>
> +#include <linux/pwrseq/driver.h>
> +#include <linux/slab.h>
> +
> +#define        to_pwrseq(a)    (container_of((a), struct pwrseq, dev))
> +
> +static DEFINE_IDA(pwrseq_ida);
> +static DEFINE_MUTEX(pwrseq_provider_mutex);
> +static LIST_HEAD(pwrseq_provider_list);
> +
> +struct pwrseq_provider {
> +       struct device           *dev;
> +       struct module           *owner;
> +       struct list_head        list;
> +       void                    *data;
> +       struct pwrseq * (*of_xlate)(void *data, struct of_phandle_args *args);
> +};
> +
> +void pwrseq_put(struct device *dev, struct pwrseq *pwrseq)
> +{
> +       device_link_remove(dev, &pwrseq->dev);

device_links - why do we need these at this initial step?

Please drop them so we can start with a simple implementation - and
then possibly extend it.

> +
> +       module_put(pwrseq->owner);
> +       put_device(&pwrseq->dev);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_put);
> +
> +static struct pwrseq_provider *of_pwrseq_provider_lookup(struct device_node *node)
> +{
> +       struct pwrseq_provider *pwrseq_provider;
> +
> +       list_for_each_entry(pwrseq_provider, &pwrseq_provider_list, list) {
> +               if (pwrseq_provider->dev->of_node == node)
> +                       return pwrseq_provider;
> +       }
> +
> +       return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static struct pwrseq *_of_pwrseq_get(struct device *dev, const char *id)
> +{
> +       struct pwrseq_provider *pwrseq_provider;
> +       struct pwrseq *pwrseq;
> +       struct of_phandle_args args;
> +       char prop_name[64]; /* 64 is max size of property name */
> +       int ret;
> +
> +       snprintf(prop_name, 64, "%s-pwrseq", id);
> +       ret = of_parse_phandle_with_args(dev->of_node, prop_name, "#pwrseq-cells", 0, &args);

This means that you are parsing a new DT binding/property.

Please fold in a DT binding patch, preceding $subject patch, so that
new binding that it can be discussed as well.

> +       if (ret) {
> +               /*
> +                * Parsing failed. Try locating old bindings for mmc-pwrseq,
> +                * which did not use #pwrseq-cells.
> +                */
> +               if (strcmp(id, "mmc"))
> +                       return NULL;
> +
> +               ret = of_parse_phandle_with_args(dev->of_node, prop_name, NULL, 0, &args);
> +               if (ret)
> +                       return NULL;
> +
> +               dev_warn(dev, "old mmc-pwrseq binding used, add #pwrseq-cells to the provider\n");

To start simple and thus to also make review easier, I suggest to skip
the mmc-pwrseq binding for now. Let's see if we can deal with that as
a standalone change on top, later, instead.

> +       }
> +
> +       mutex_lock(&pwrseq_provider_mutex);
> +       pwrseq_provider = of_pwrseq_provider_lookup(args.np);
> +       if (IS_ERR(pwrseq_provider) || !try_module_get(pwrseq_provider->owner)) {
> +               pwrseq = ERR_PTR(-EPROBE_DEFER);
> +               goto out_unlock;
> +       }
> +
> +       if (!of_device_is_available(args.np)) {
> +               dev_warn(pwrseq_provider->dev, "Requested pwrseq is disabled\n");
> +               pwrseq = ERR_PTR(-ENODEV);
> +               goto out_put_module;
> +       }
> +
> +       pwrseq = pwrseq_provider->of_xlate(pwrseq_provider->data, &args);
> +
> +out_put_module:
> +       module_put(pwrseq_provider->owner);
> +
> +out_unlock:
> +       mutex_unlock(&pwrseq_provider_mutex);
> +       of_node_put(args.np);
> +
> +       return pwrseq;
> +}
> +
> +struct pwrseq * __pwrseq_get(struct device *dev, const char *id, bool optional)
> +{
> +       struct pwrseq *pwrseq;
> +       struct device_link *link;
> +
> +       pwrseq = _of_pwrseq_get(dev, id);
> +       if (pwrseq == NULL)
> +               return optional ? NULL : ERR_PTR(-ENODEV);

I think we can manage this without "optional". The optional should
typically be the default behaviour, I think.

The caller should expect to get a handle to a pwrseq - if there is
property in the DT file that says there should be one. If not, the
caller should be happy to just receive "NULL". And if there is an
error, we should return ERR_PTR, as you do.

> +       else if (IS_ERR(pwrseq))
> +               return pwrseq;
> +
> +       if (!try_module_get(pwrseq->owner))
> +               return ERR_PTR(-EPROBE_DEFER);
> +
> +       get_device(&pwrseq->dev);
> +       link = device_link_add(dev, &pwrseq->dev, DL_FLAG_STATELESS);
> +       if (!link)
> +               dev_dbg(dev, "failed to create device link to %s\n",
> +                       dev_name(pwrseq->dev.parent));
> +
> +       return pwrseq;
> +}
> +
> +struct pwrseq * pwrseq_get(struct device *dev, const char *id)
> +{
> +       return __pwrseq_get(dev, id, false);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_get);
> +
> +static void devm_pwrseq_release(struct device *dev, void *res)
> +{
> +       struct pwrseq *pwrseq = *(struct pwrseq **)res;
> +
> +       pwrseq_put(dev, pwrseq);
> +}
> +
> +struct pwrseq * devm_pwrseq_get(struct device *dev, const char *id)
> +{
> +       struct pwrseq **ptr, *pwrseq;
> +
> +       ptr = devres_alloc(devm_pwrseq_release, sizeof(*ptr), GFP_KERNEL);
> +       if (!ptr)
> +               return ERR_PTR(-ENOMEM);
> +
> +       pwrseq = pwrseq_get(dev, id);
> +       if (!IS_ERR(pwrseq)) {
> +               *ptr = pwrseq;
> +               devres_add(dev, ptr);
> +       } else {
> +               devres_free(ptr);
> +       }
> +
> +       return pwrseq;
> +}
> +EXPORT_SYMBOL_GPL(devm_pwrseq_get);
> +
> +struct pwrseq * pwrseq_get_optional(struct device *dev, const char *id)
> +{
> +       return __pwrseq_get(dev, id, true);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_get_optional);

This can be dropped, if we make this the default behaviour.

> +
> +struct pwrseq * devm_pwrseq_get_optional(struct device *dev, const char *id)
> +{
> +       struct pwrseq **ptr, *pwrseq;
> +
> +       ptr = devres_alloc(devm_pwrseq_release, sizeof(*ptr), GFP_KERNEL);
> +       if (!ptr)
> +               return ERR_PTR(-ENOMEM);
> +
> +       pwrseq = pwrseq_get_optional(dev, id);
> +       if (!IS_ERR_OR_NULL(pwrseq)) {
> +               *ptr = pwrseq;
> +               devres_add(dev, ptr);
> +       } else {
> +               devres_free(ptr);
> +       }
> +
> +       return pwrseq;
> +}
> +EXPORT_SYMBOL_GPL(devm_pwrseq_get_optional);

Ditto.

> +
> +int pwrseq_pre_power_on(struct pwrseq *pwrseq)
> +{
> +       if (pwrseq && pwrseq->ops->pre_power_on)
> +               return pwrseq->ops->pre_power_on(pwrseq);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_pre_power_on);
> +
> +int pwrseq_power_on(struct pwrseq *pwrseq)
> +{
> +       if (pwrseq && pwrseq->ops->power_on)
> +               return pwrseq->ops->power_on(pwrseq);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_power_on);
> +
> +void pwrseq_power_off(struct pwrseq *pwrseq)
> +{
> +       if (pwrseq && pwrseq->ops->power_off)
> +               pwrseq->ops->power_off(pwrseq);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_power_off);
> +
> +void pwrseq_reset(struct pwrseq *pwrseq)
> +{
> +       if (pwrseq && pwrseq->ops->reset)
> +               pwrseq->ops->reset(pwrseq);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_reset);
> +
> +static void pwrseq_dev_release(struct device *dev)
> +{
> +       struct pwrseq *pwrseq = to_pwrseq(dev);
> +
> +       ida_free(&pwrseq_ida, pwrseq->id);
> +       of_node_put(dev->of_node);
> +       kfree(pwrseq);
> +}
> +
> +static struct class pwrseq_class = {
> +       .name = "pwrseq",
> +       .dev_release = pwrseq_dev_release,
> +};
> +
> +struct pwrseq *__pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data)
> +{
> +       struct pwrseq *pwrseq;
> +       int ret;
> +
> +       if (WARN_ON(!dev))
> +               return ERR_PTR(-EINVAL);
> +
> +       pwrseq = kzalloc(sizeof(*pwrseq), GFP_KERNEL);
> +       if (!pwrseq)
> +               return ERR_PTR(-ENOMEM);
> +
> +       ret = ida_alloc(&pwrseq_ida, GFP_KERNEL);
> +       if (ret < 0)
> +               goto free_pwrseq;
> +
> +       pwrseq->id = ret;
> +
> +       device_initialize(&pwrseq->dev);
> +
> +       pwrseq->dev.class = &pwrseq_class;
> +       pwrseq->dev.parent = dev;
> +       pwrseq->dev.of_node = of_node_get(dev->of_node);
> +       pwrseq->ops = ops;
> +       pwrseq->owner = owner;
> +
> +       dev_set_drvdata(&pwrseq->dev, data);
> +
> +       ret = dev_set_name(&pwrseq->dev, "pwrseq-%s.%u", dev_name(dev), pwrseq->id);
> +       if (ret)
> +               goto put_dev;
> +
> +       ret = device_add(&pwrseq->dev);
> +       if (ret)
> +               goto put_dev;
> +
> +       if (pm_runtime_enabled(dev)) {
> +               pm_runtime_enable(&pwrseq->dev);
> +               pm_runtime_no_callbacks(&pwrseq->dev);
> +       }

I don't think we should bother with runtime PM, at least in this
initial step. Please drop it, to start simple.

> +
> +       return pwrseq;
> +
> +put_dev:
> +       /* will call pwrseq_dev_release() to free resources */
> +       put_device(&pwrseq->dev);
> +
> +       return ERR_PTR(ret);
> +
> +free_pwrseq:
> +       kfree(pwrseq);
> +
> +       return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(__pwrseq_create);
> +
> +void pwrseq_destroy(struct pwrseq *pwrseq)
> +{
> +       pm_runtime_disable(&pwrseq->dev);
> +       device_unregister(&pwrseq->dev);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_destroy);
> +
> +static void devm_pwrseq_destroy(struct device *dev, void *res)
> +{
> +       struct pwrseq *pwrseq = *(struct pwrseq **)res;
> +
> +       pwrseq_destroy(pwrseq);
> +}
> +
> +struct pwrseq *__devm_pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data)
> +{
> +       struct pwrseq **ptr, *pwrseq;
> +
> +       ptr = devres_alloc(devm_pwrseq_destroy, sizeof(*ptr), GFP_KERNEL);
> +       if (!ptr)
> +               return ERR_PTR(-ENOMEM);
> +
> +       pwrseq = __pwrseq_create(dev, owner, ops, data);
> +       if (!IS_ERR(pwrseq)) {
> +               *ptr = pwrseq;
> +               devres_add(dev, ptr);
> +       } else {
> +               devres_free(ptr);
> +       }
> +
> +       return pwrseq;
> +}
> +EXPORT_SYMBOL_GPL(__devm_pwrseq_create);
> +
> +struct pwrseq_provider *__of_pwrseq_provider_register(struct device *dev,
> +       struct module *owner,
> +       struct pwrseq * (*of_xlate)(void *data,
> +                                   struct of_phandle_args *args),
> +       void *data)
> +{
> +       struct pwrseq_provider *pwrseq_provider;
> +
> +       pwrseq_provider = kzalloc(sizeof(*pwrseq_provider), GFP_KERNEL);
> +       if (!pwrseq_provider)
> +               return ERR_PTR(-ENOMEM);
> +
> +       pwrseq_provider->dev = dev;
> +       pwrseq_provider->owner = owner;
> +       pwrseq_provider->of_xlate = of_xlate;
> +       pwrseq_provider->data = data;
> +
> +       mutex_lock(&pwrseq_provider_mutex);
> +       list_add_tail(&pwrseq_provider->list, &pwrseq_provider_list);
> +       mutex_unlock(&pwrseq_provider_mutex);
> +
> +       return pwrseq_provider;
> +}
> +EXPORT_SYMBOL_GPL(__of_pwrseq_provider_register);
> +
> +void of_pwrseq_provider_unregister(struct pwrseq_provider *pwrseq_provider)
> +{
> +       if (IS_ERR(pwrseq_provider))
> +               return;
> +
> +       mutex_lock(&pwrseq_provider_mutex);
> +       list_del(&pwrseq_provider->list);
> +       kfree(pwrseq_provider);
> +       mutex_unlock(&pwrseq_provider_mutex);
> +}
> +EXPORT_SYMBOL_GPL(of_pwrseq_provider_unregister);
> +
> +static void devm_pwrseq_provider_unregister(struct device *dev, void *res)
> +{
> +       struct pwrseq_provider *pwrseq_provider = *(struct pwrseq_provider **)res;
> +
> +       of_pwrseq_provider_unregister(pwrseq_provider);
> +}
> +
> +struct pwrseq_provider *__devm_of_pwrseq_provider_register(struct device *dev,
> +       struct module *owner,
> +       struct pwrseq * (*of_xlate)(void *data,
> +                                   struct of_phandle_args *args),
> +       void *data)
> +{
> +       struct pwrseq_provider **ptr, *pwrseq_provider;
> +
> +       ptr = devres_alloc(devm_pwrseq_provider_unregister, sizeof(*ptr), GFP_KERNEL);
> +       if (!ptr)
> +               return ERR_PTR(-ENOMEM);
> +
> +       pwrseq_provider = __of_pwrseq_provider_register(dev, owner, of_xlate, data);
> +       if (!IS_ERR(pwrseq_provider)) {
> +               *ptr = pwrseq_provider;
> +               devres_add(dev, ptr);
> +       } else {
> +               devres_free(ptr);
> +       }
> +
> +       return pwrseq_provider;
> +}
> +EXPORT_SYMBOL_GPL(__devm_of_pwrseq_provider_register);
> +
> +struct pwrseq *of_pwrseq_xlate_onecell(void *data, struct of_phandle_args *args)
> +{
> +       struct pwrseq_onecell_data *pwrseq_data = data;
> +       unsigned int idx;
> +
> +       if (args->args_count != 1)
> +               return ERR_PTR(-EINVAL);
> +
> +       idx = args->args[0];
> +       if (idx >= pwrseq_data->num) {
> +               pr_err("%s: invalid index %u\n", __func__, idx);
> +               return ERR_PTR(-EINVAL);
> +       }

In many cases it's reasonable to leave room for future extensions, so
that a provider could serve with more than one power-sequencer. I
guess that is what you intend to do here, right?

In my opinion, I don't think what would happen, especially since a
power-sequence is something that should be specific to one particular
device (a Qcom WiFi/Blutooth chip, for example).

That said, I suggest limiting this to a 1:1 mapping between the device
node and power-sequencer. I think that should simplify the code a bit.

> +
> +       return pwrseq_data->pwrseqs[idx];
> +}
> +
> +static int __init pwrseq_core_init(void)
> +{
> +       return class_register(&pwrseq_class);
> +}
> +device_initcall(pwrseq_core_init);
> diff --git a/include/linux/pwrseq/consumer.h b/include/linux/pwrseq/consumer.h
> new file mode 100644
> index 000000000000..fbcdc1fc0751
> --- /dev/null
> +++ b/include/linux/pwrseq/consumer.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2021 Linaro Ltd.
> + */
> +
> +#ifndef __LINUX_PWRSEQ_CONSUMER_H__
> +#define __LINUX_PWRSEQ_CONSUMER_H__
> +
> +struct pwrseq;
> +struct device;
> +
> +#if defined(CONFIG_PWRSEQ)
> +
> +struct pwrseq *__must_check pwrseq_get(struct device *dev, const char *id);
> +struct pwrseq *__must_check devm_pwrseq_get(struct device *dev, const char *id);
> +
> +struct pwrseq *__must_check pwrseq_get_optional(struct device *dev, const char *id);
> +struct pwrseq *__must_check devm_pwrseq_get_optional(struct device *dev, const char *id);
> +
> +void pwrseq_put(struct device *dev, struct pwrseq *pwrseq);
> +
> +int pwrseq_pre_power_on(struct pwrseq *pwrseq);
> +int pwrseq_power_on(struct pwrseq *pwrseq);
> +void pwrseq_power_off(struct pwrseq *pwrseq);
> +void pwrseq_reset(struct pwrseq *pwrseq);
> +
> +#else
> +
> +static inline struct pwrseq *__must_check
> +pwrseq_get(struct device *dev, const char *id)
> +{
> +       return ERR_PTR(-ENOSYS);
> +}
> +
> +static inline struct pwrseq *__must_check
> +devm_pwrseq_get(struct device *dev, const char *id)
> +{
> +       return ERR_PTR(-ENOSYS);
> +}
> +
> +static inline struct pwrseq *__must_check
> +pwrseq_get_optional(struct device *dev, const char *id)
> +{
> +       return NULL;
> +}
> +
> +static inline struct pwrseq *__must_check
> +devm_pwrseq_get_optional(struct device *dev, const char *id)
> +{
> +       return NULL;
> +}
> +
> +static inline void pwrseq_put(struct device *dev, struct pwrseq *pwrseq)
> +{
> +}
> +
> +static inline int pwrseq_pre_power_on(struct pwrseq *pwrseq)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline int pwrseq_power_on(struct pwrseq *pwrseq)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline void pwrseq_power_off(struct pwrseq *pwrseq)
> +{
> +}
> +
> +static inline void pwrseq_reset(struct pwrseq *pwrseq)
> +{
> +}
> +
> +#endif
> +
> +static inline int pwrseq_full_power_on(struct pwrseq *pwrseq)
> +{
> +       int ret;
> +
> +       ret = pwrseq_pre_power_on(pwrseq);
> +       if (ret)
> +               return ret;
> +
> +       return pwrseq_power_on(pwrseq);
> +}
> +
> +#endif /* __LINUX_PWRSEQ_CONSUMER_H__ */
> diff --git a/include/linux/pwrseq/driver.h b/include/linux/pwrseq/driver.h
> new file mode 100644
> index 000000000000..b2bc46624d7e
> --- /dev/null
> +++ b/include/linux/pwrseq/driver.h
> @@ -0,0 +1,75 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2021 Linaro Ltd.
> + */
> +
> +#ifndef __LINUX_PWRSEQ_DRIVER_H__
> +#define __LINUX_PWRSEQ_DRIVER_H__
> +
> +#include <linux/device.h>
> +
> +struct pwrseq;
> +
> +struct pwrseq_ops {
> +       int (*pre_power_on)(struct pwrseq *pwrseq);
> +       int (*power_on)(struct pwrseq *pwrseq);
> +       void (*power_off)(struct pwrseq *pwrseq);
> +       void (*reset)(struct pwrseq *pwrseq);
> +};
> +
> +struct module;
> +
> +struct pwrseq {
> +       struct device dev;
> +       const struct pwrseq_ops *ops;
> +       unsigned int id;
> +       struct module *owner;
> +};
> +
> +struct pwrseq *__pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data);
> +struct pwrseq *__devm_pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data);
> +
> +#define pwrseq_create(dev, ops, data) __pwrseq_create((dev), THIS_MODULE, (ops), (data))
> +#define devm_pwrseq_create(dev, ops, data) __devm_pwrseq_create((dev), THIS_MODULE, (ops), (data))
> +
> +void pwrseq_destroy(struct pwrseq *pwrseq);
> +
> +static inline void *pwrseq_get_data(struct pwrseq *pwrseq)
> +{
> +       return dev_get_drvdata(&pwrseq->dev);
> +}
> +
> +#define        of_pwrseq_provider_register(dev, xlate, data)   \
> +       __of_pwrseq_provider_register((dev), THIS_MODULE, (xlate), (data))
> +
> +#define        devm_of_pwrseq_provider_register(dev, xlate, data)      \
> +       __devm_of_pwrseq_provider_register((dev), THIS_MODULE, (xlate), (data))
> +
> +struct of_phandle_args;
> +
> +struct pwrseq_provider *__of_pwrseq_provider_register(struct device *dev,
> +       struct module *owner,
> +       struct pwrseq * (*of_xlate)(void *data,
> +                                   struct of_phandle_args *args),
> +       void *data);
> +struct pwrseq_provider *__devm_of_pwrseq_provider_register(struct device *dev,
> +       struct module *owner,
> +       struct pwrseq * (*of_xlate)(void *data,
> +                                   struct of_phandle_args *args),
> +       void *data);
> +void of_pwrseq_provider_unregister(struct pwrseq_provider *pwrseq_provider);
> +
> +static inline struct pwrseq *of_pwrseq_xlate_single(void *data,
> +                                                   struct of_phandle_args *args)
> +{
> +       return data;
> +}
> +
> +struct pwrseq_onecell_data {
> +       unsigned int num;
> +       struct pwrseq *pwrseqs[];
> +};

According to my earlier comment, I think a lot can be removed from
here - if you would limit the provider to only use
of_pwrseq_xlate_single.

Again, I think it's better to start simple, as it simplifies the review.

> +
> +struct pwrseq *of_pwrseq_xlate_onecell(void *data, struct of_phandle_args *args);
> +
> +#endif /* __LINUX_PWRSEQ_DRIVER_H__ */
> --
> 2.33.0
>

Other than my comments, overall I think this looks like a good start.

Kind regards
Uffe

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC v2 01/13] power: add power sequencer subsystem
  2021-09-10 10:02   ` Ulf Hansson
@ 2021-09-13 12:32     ` Dmitry Baryshkov
  2021-09-13 13:42       ` Ulf Hansson
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-09-13 12:32 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Andy Gross, Bjorn Andersson, Marcel Holtmann, Johan Hedberg,
	Luiz Augusto von Dentz, Kalle Valo, David S. Miller,
	Jakub Kicinski, Stanimir Varbanov, linux-arm-msm, linux-mmc,
	Linux Kernel Mailing List, linux-bluetooth, ath10k,
	linux-wireless, netdev

On 10/09/2021 13:02, Ulf Hansson wrote:
> On Sun, 29 Aug 2021 at 15:13, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> Basing on MMC's pwrseq support code, add separate power sequencer
>> subsystem. It will be used by other drivers to handle device power up
>> requirements.
> 
> This is far too vague. You are suggesting to add a new subsystem, I
> think that deserves some more explanations as justifications.
> 
> Additionally, it wouldn't hurt to explain a bit how the actual
> subsystem is supposed to work, at least from a toplevel point of view.

Ack, will add more explanations together with the some inline documentation.

> 
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/power/Kconfig           |   1 +
>>   drivers/power/Makefile          |   1 +
>>   drivers/power/pwrseq/Kconfig    |  11 +
>>   drivers/power/pwrseq/Makefile   |   6 +
>>   drivers/power/pwrseq/core.c     | 412 ++++++++++++++++++++++++++++++++
>>   include/linux/pwrseq/consumer.h |  88 +++++++
>>   include/linux/pwrseq/driver.h   |  75 ++++++
>>   7 files changed, 594 insertions(+)
>>   create mode 100644 drivers/power/pwrseq/Kconfig
>>   create mode 100644 drivers/power/pwrseq/Makefile
>>   create mode 100644 drivers/power/pwrseq/core.c
>>   create mode 100644 include/linux/pwrseq/consumer.h
>>   create mode 100644 include/linux/pwrseq/driver.h
> 
> I noticed there is no update of the MAINTAINERS file. We need that to
> be a part of the $subject patch as well, I think. But, let's discuss
> that later.
> 
>>
>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>> index 696bf77a7042..c87cd2240a74 100644
>> --- a/drivers/power/Kconfig
>> +++ b/drivers/power/Kconfig
>> @@ -1,3 +1,4 @@
>>   # SPDX-License-Identifier: GPL-2.0-only
>> +source "drivers/power/pwrseq/Kconfig"
>>   source "drivers/power/reset/Kconfig"
>>   source "drivers/power/supply/Kconfig"
>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>> index effbf0377f32..1dbce454a8c4 100644
>> --- a/drivers/power/Makefile
>> +++ b/drivers/power/Makefile
>> @@ -1,3 +1,4 @@
>>   # SPDX-License-Identifier: GPL-2.0-only
>>   obj-$(CONFIG_POWER_RESET)      += reset/
>>   obj-$(CONFIG_POWER_SUPPLY)     += supply/
>> +obj-$(CONFIG_PWRSEQ)           += pwrseq/
>> diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
>> new file mode 100644
>> index 000000000000..8904ec9ed541
>> --- /dev/null
>> +++ b/drivers/power/pwrseq/Kconfig
>> @@ -0,0 +1,11 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +menuconfig PWRSEQ
>> +       bool "Power Sequencer drivers"
>> +       help
>> +         Provides support for special power sequencing drivers.
> 
> This needs more description. The name "power sequencer" isn't entirely
> self-explanatory, for when this should be used. I am not saying you
> should invent a new name, rather just extend the description so people
> get a better idea of what this is supposed to be used for.
> 
>> +
>> +         Say Y here to enable support for such devices
>> +
>> +if PWRSEQ
>> +
>> +endif
>> diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
>> new file mode 100644
>> index 000000000000..108429ff6445
>> --- /dev/null
>> +++ b/drivers/power/pwrseq/Makefile
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# Makefile for power sequencer drivers.
>> +#
>> +
>> +obj-$(CONFIG_PWRSEQ) += core.o
>> diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
>> new file mode 100644
>> index 000000000000..2e4e9d123e60
>> --- /dev/null
>> +++ b/drivers/power/pwrseq/core.c
>> @@ -0,0 +1,412 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright 2021 (c) Linaro Ltd.
>> + * Author: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> + *
>> + * Based on phy-core.c:
>> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/idr.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/pwrseq/consumer.h>
>> +#include <linux/pwrseq/driver.h>
>> +#include <linux/slab.h>
>> +
>> +#define        to_pwrseq(a)    (container_of((a), struct pwrseq, dev))
>> +
>> +static DEFINE_IDA(pwrseq_ida);
>> +static DEFINE_MUTEX(pwrseq_provider_mutex);
>> +static LIST_HEAD(pwrseq_provider_list);
>> +
>> +struct pwrseq_provider {
>> +       struct device           *dev;
>> +       struct module           *owner;
>> +       struct list_head        list;
>> +       void                    *data;
>> +       struct pwrseq * (*of_xlate)(void *data, struct of_phandle_args *args);
>> +};
>> +
>> +void pwrseq_put(struct device *dev, struct pwrseq *pwrseq)
>> +{
>> +       device_link_remove(dev, &pwrseq->dev);
> 
> device_links - why do we need these at this initial step?
> 
> Please drop them so we can start with a simple implementation - and
> then possibly extend it.

Ack

> 
>> +
>> +       module_put(pwrseq->owner);
>> +       put_device(&pwrseq->dev);
>> +}
>> +EXPORT_SYMBOL_GPL(pwrseq_put);
>> +
>> +static struct pwrseq_provider *of_pwrseq_provider_lookup(struct device_node *node)
>> +{
>> +       struct pwrseq_provider *pwrseq_provider;
>> +
>> +       list_for_each_entry(pwrseq_provider, &pwrseq_provider_list, list) {
>> +               if (pwrseq_provider->dev->of_node == node)
>> +                       return pwrseq_provider;
>> +       }
>> +
>> +       return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +
>> +static struct pwrseq *_of_pwrseq_get(struct device *dev, const char *id)
>> +{
>> +       struct pwrseq_provider *pwrseq_provider;
>> +       struct pwrseq *pwrseq;
>> +       struct of_phandle_args args;
>> +       char prop_name[64]; /* 64 is max size of property name */
>> +       int ret;
>> +
>> +       snprintf(prop_name, 64, "%s-pwrseq", id);
>> +       ret = of_parse_phandle_with_args(dev->of_node, prop_name, "#pwrseq-cells", 0, &args);
> 
> This means that you are parsing a new DT binding/property.
> 
> Please fold in a DT binding patch, preceding $subject patch, so that
> new binding that it can be discussed as well.
> 
>> +       if (ret) {
>> +               /*
>> +                * Parsing failed. Try locating old bindings for mmc-pwrseq,
>> +                * which did not use #pwrseq-cells.
>> +                */
>> +               if (strcmp(id, "mmc"))
>> +                       return NULL;
>> +
>> +               ret = of_parse_phandle_with_args(dev->of_node, prop_name, NULL, 0, &args);
>> +               if (ret)
>> +                       return NULL;
>> +
>> +               dev_warn(dev, "old mmc-pwrseq binding used, add #pwrseq-cells to the provider\n");
> 
> To start simple and thus to also make review easier, I suggest to skip
> the mmc-pwrseq binding for now. Let's see if we can deal with that as
> a standalone change on top, later, instead.

Ack, will split to the separate patch

> 
>> +       }
>> +
>> +       mutex_lock(&pwrseq_provider_mutex);
>> +       pwrseq_provider = of_pwrseq_provider_lookup(args.np);
>> +       if (IS_ERR(pwrseq_provider) || !try_module_get(pwrseq_provider->owner)) {
>> +               pwrseq = ERR_PTR(-EPROBE_DEFER);
>> +               goto out_unlock;
>> +       }
>> +
>> +       if (!of_device_is_available(args.np)) {
>> +               dev_warn(pwrseq_provider->dev, "Requested pwrseq is disabled\n");
>> +               pwrseq = ERR_PTR(-ENODEV);
>> +               goto out_put_module;
>> +       }
>> +
>> +       pwrseq = pwrseq_provider->of_xlate(pwrseq_provider->data, &args);
>> +
>> +out_put_module:
>> +       module_put(pwrseq_provider->owner);
>> +
>> +out_unlock:
>> +       mutex_unlock(&pwrseq_provider_mutex);
>> +       of_node_put(args.np);
>> +
>> +       return pwrseq;
>> +}
>> +
>> +struct pwrseq * __pwrseq_get(struct device *dev, const char *id, bool optional)
>> +{
>> +       struct pwrseq *pwrseq;
>> +       struct device_link *link;
>> +
>> +       pwrseq = _of_pwrseq_get(dev, id);
>> +       if (pwrseq == NULL)
>> +               return optional ? NULL : ERR_PTR(-ENODEV);
> 
> I think we can manage this without "optional". The optional should
> typically be the default behaviour, I think.
> 
> The caller should expect to get a handle to a pwrseq - if there is
> property in the DT file that says there should be one. If not, the
> caller should be happy to just receive "NULL". And if there is an
> error, we should return ERR_PTR, as you do.

Ack.

> 
>> +       else if (IS_ERR(pwrseq))
>> +               return pwrseq;
>> +
>> +       if (!try_module_get(pwrseq->owner))
>> +               return ERR_PTR(-EPROBE_DEFER);
>> +
>> +       get_device(&pwrseq->dev);
>> +       link = device_link_add(dev, &pwrseq->dev, DL_FLAG_STATELESS);
>> +       if (!link)
>> +               dev_dbg(dev, "failed to create device link to %s\n",
>> +                       dev_name(pwrseq->dev.parent));
>> +
>> +       return pwrseq;
>> +}
>> +
>> +struct pwrseq * pwrseq_get(struct device *dev, const char *id)
>> +{
>> +       return __pwrseq_get(dev, id, false);
>> +}
>> +EXPORT_SYMBOL_GPL(pwrseq_get);
>> +
>> +static void devm_pwrseq_release(struct device *dev, void *res)
>> +{
>> +       struct pwrseq *pwrseq = *(struct pwrseq **)res;
>> +
>> +       pwrseq_put(dev, pwrseq);
>> +}
>> +
>> +struct pwrseq * devm_pwrseq_get(struct device *dev, const char *id)
>> +{
>> +       struct pwrseq **ptr, *pwrseq;
>> +
>> +       ptr = devres_alloc(devm_pwrseq_release, sizeof(*ptr), GFP_KERNEL);
>> +       if (!ptr)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       pwrseq = pwrseq_get(dev, id);
>> +       if (!IS_ERR(pwrseq)) {
>> +               *ptr = pwrseq;
>> +               devres_add(dev, ptr);
>> +       } else {
>> +               devres_free(ptr);
>> +       }
>> +
>> +       return pwrseq;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_pwrseq_get);
>> +
>> +struct pwrseq * pwrseq_get_optional(struct device *dev, const char *id)
>> +{
>> +       return __pwrseq_get(dev, id, true);
>> +}
>> +EXPORT_SYMBOL_GPL(pwrseq_get_optional);
> 
> This can be dropped, if we make this the default behaviour.
> 
>> +
>> +struct pwrseq * devm_pwrseq_get_optional(struct device *dev, const char *id)
>> +{
>> +       struct pwrseq **ptr, *pwrseq;
>> +
>> +       ptr = devres_alloc(devm_pwrseq_release, sizeof(*ptr), GFP_KERNEL);
>> +       if (!ptr)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       pwrseq = pwrseq_get_optional(dev, id);
>> +       if (!IS_ERR_OR_NULL(pwrseq)) {
>> +               *ptr = pwrseq;
>> +               devres_add(dev, ptr);
>> +       } else {
>> +               devres_free(ptr);
>> +       }
>> +
>> +       return pwrseq;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_pwrseq_get_optional);
> 
> Ditto.
> 
>> +
>> +int pwrseq_pre_power_on(struct pwrseq *pwrseq)
>> +{
>> +       if (pwrseq && pwrseq->ops->pre_power_on)
>> +               return pwrseq->ops->pre_power_on(pwrseq);
>> +
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(pwrseq_pre_power_on);
>> +
>> +int pwrseq_power_on(struct pwrseq *pwrseq)
>> +{
>> +       if (pwrseq && pwrseq->ops->power_on)
>> +               return pwrseq->ops->power_on(pwrseq);
>> +
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(pwrseq_power_on);
>> +
>> +void pwrseq_power_off(struct pwrseq *pwrseq)
>> +{
>> +       if (pwrseq && pwrseq->ops->power_off)
>> +               pwrseq->ops->power_off(pwrseq);
>> +}
>> +EXPORT_SYMBOL_GPL(pwrseq_power_off);
>> +
>> +void pwrseq_reset(struct pwrseq *pwrseq)
>> +{
>> +       if (pwrseq && pwrseq->ops->reset)
>> +               pwrseq->ops->reset(pwrseq);
>> +}
>> +EXPORT_SYMBOL_GPL(pwrseq_reset);
>> +
>> +static void pwrseq_dev_release(struct device *dev)
>> +{
>> +       struct pwrseq *pwrseq = to_pwrseq(dev);
>> +
>> +       ida_free(&pwrseq_ida, pwrseq->id);
>> +       of_node_put(dev->of_node);
>> +       kfree(pwrseq);
>> +}
>> +
>> +static struct class pwrseq_class = {
>> +       .name = "pwrseq",
>> +       .dev_release = pwrseq_dev_release,
>> +};
>> +
>> +struct pwrseq *__pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data)
>> +{
>> +       struct pwrseq *pwrseq;
>> +       int ret;
>> +
>> +       if (WARN_ON(!dev))
>> +               return ERR_PTR(-EINVAL);
>> +
>> +       pwrseq = kzalloc(sizeof(*pwrseq), GFP_KERNEL);
>> +       if (!pwrseq)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       ret = ida_alloc(&pwrseq_ida, GFP_KERNEL);
>> +       if (ret < 0)
>> +               goto free_pwrseq;
>> +
>> +       pwrseq->id = ret;
>> +
>> +       device_initialize(&pwrseq->dev);
>> +
>> +       pwrseq->dev.class = &pwrseq_class;
>> +       pwrseq->dev.parent = dev;
>> +       pwrseq->dev.of_node = of_node_get(dev->of_node);
>> +       pwrseq->ops = ops;
>> +       pwrseq->owner = owner;
>> +
>> +       dev_set_drvdata(&pwrseq->dev, data);
>> +
>> +       ret = dev_set_name(&pwrseq->dev, "pwrseq-%s.%u", dev_name(dev), pwrseq->id);
>> +       if (ret)
>> +               goto put_dev;
>> +
>> +       ret = device_add(&pwrseq->dev);
>> +       if (ret)
>> +               goto put_dev;
>> +
>> +       if (pm_runtime_enabled(dev)) {
>> +               pm_runtime_enable(&pwrseq->dev);
>> +               pm_runtime_no_callbacks(&pwrseq->dev);
>> +       }
> 
> I don't think we should bother with runtime PM, at least in this
> initial step. Please drop it, to start simple.

Ack

> 
>> +
>> +       return pwrseq;
>> +
>> +put_dev:
>> +       /* will call pwrseq_dev_release() to free resources */
>> +       put_device(&pwrseq->dev);
>> +
>> +       return ERR_PTR(ret);
>> +
>> +free_pwrseq:
>> +       kfree(pwrseq);
>> +
>> +       return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(__pwrseq_create);
>> +
>> +void pwrseq_destroy(struct pwrseq *pwrseq)
>> +{
>> +       pm_runtime_disable(&pwrseq->dev);
>> +       device_unregister(&pwrseq->dev);
>> +}
>> +EXPORT_SYMBOL_GPL(pwrseq_destroy);
>> +
>> +static void devm_pwrseq_destroy(struct device *dev, void *res)
>> +{
>> +       struct pwrseq *pwrseq = *(struct pwrseq **)res;
>> +
>> +       pwrseq_destroy(pwrseq);
>> +}
>> +
>> +struct pwrseq *__devm_pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data)
>> +{
>> +       struct pwrseq **ptr, *pwrseq;
>> +
>> +       ptr = devres_alloc(devm_pwrseq_destroy, sizeof(*ptr), GFP_KERNEL);
>> +       if (!ptr)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       pwrseq = __pwrseq_create(dev, owner, ops, data);
>> +       if (!IS_ERR(pwrseq)) {
>> +               *ptr = pwrseq;
>> +               devres_add(dev, ptr);
>> +       } else {
>> +               devres_free(ptr);
>> +       }
>> +
>> +       return pwrseq;
>> +}
>> +EXPORT_SYMBOL_GPL(__devm_pwrseq_create);
>> +
>> +struct pwrseq_provider *__of_pwrseq_provider_register(struct device *dev,
>> +       struct module *owner,
>> +       struct pwrseq * (*of_xlate)(void *data,
>> +                                   struct of_phandle_args *args),
>> +       void *data)
>> +{
>> +       struct pwrseq_provider *pwrseq_provider;
>> +
>> +       pwrseq_provider = kzalloc(sizeof(*pwrseq_provider), GFP_KERNEL);
>> +       if (!pwrseq_provider)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       pwrseq_provider->dev = dev;
>> +       pwrseq_provider->owner = owner;
>> +       pwrseq_provider->of_xlate = of_xlate;
>> +       pwrseq_provider->data = data;
>> +
>> +       mutex_lock(&pwrseq_provider_mutex);
>> +       list_add_tail(&pwrseq_provider->list, &pwrseq_provider_list);
>> +       mutex_unlock(&pwrseq_provider_mutex);
>> +
>> +       return pwrseq_provider;
>> +}
>> +EXPORT_SYMBOL_GPL(__of_pwrseq_provider_register);
>> +
>> +void of_pwrseq_provider_unregister(struct pwrseq_provider *pwrseq_provider)
>> +{
>> +       if (IS_ERR(pwrseq_provider))
>> +               return;
>> +
>> +       mutex_lock(&pwrseq_provider_mutex);
>> +       list_del(&pwrseq_provider->list);
>> +       kfree(pwrseq_provider);
>> +       mutex_unlock(&pwrseq_provider_mutex);
>> +}
>> +EXPORT_SYMBOL_GPL(of_pwrseq_provider_unregister);
>> +
>> +static void devm_pwrseq_provider_unregister(struct device *dev, void *res)
>> +{
>> +       struct pwrseq_provider *pwrseq_provider = *(struct pwrseq_provider **)res;
>> +
>> +       of_pwrseq_provider_unregister(pwrseq_provider);
>> +}
>> +
>> +struct pwrseq_provider *__devm_of_pwrseq_provider_register(struct device *dev,
>> +       struct module *owner,
>> +       struct pwrseq * (*of_xlate)(void *data,
>> +                                   struct of_phandle_args *args),
>> +       void *data)
>> +{
>> +       struct pwrseq_provider **ptr, *pwrseq_provider;
>> +
>> +       ptr = devres_alloc(devm_pwrseq_provider_unregister, sizeof(*ptr), GFP_KERNEL);
>> +       if (!ptr)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       pwrseq_provider = __of_pwrseq_provider_register(dev, owner, of_xlate, data);
>> +       if (!IS_ERR(pwrseq_provider)) {
>> +               *ptr = pwrseq_provider;
>> +               devres_add(dev, ptr);
>> +       } else {
>> +               devres_free(ptr);
>> +       }
>> +
>> +       return pwrseq_provider;
>> +}
>> +EXPORT_SYMBOL_GPL(__devm_of_pwrseq_provider_register);
>> +
>> +struct pwrseq *of_pwrseq_xlate_onecell(void *data, struct of_phandle_args *args)
>> +{
>> +       struct pwrseq_onecell_data *pwrseq_data = data;
>> +       unsigned int idx;
>> +
>> +       if (args->args_count != 1)
>> +               return ERR_PTR(-EINVAL);
>> +
>> +       idx = args->args[0];
>> +       if (idx >= pwrseq_data->num) {
>> +               pr_err("%s: invalid index %u\n", __func__, idx);
>> +               return ERR_PTR(-EINVAL);
>> +       }
> 
> In many cases it's reasonable to leave room for future extensions, so
> that a provider could serve with more than one power-sequencer. I
> guess that is what you intend to do here, right?
> 
> In my opinion, I don't think what would happen, especially since a
> power-sequence is something that should be specific to one particular
> device (a Qcom WiFi/Blutooth chip, for example).
> 
> That said, I suggest limiting this to a 1:1 mapping between the device
> node and power-sequencer. I think that should simplify the code a bit.

In fact the WiFi/BT example itself provides a non 1:1 mapping. In my 
current design the power sequencer provides two instances (one for WiFi, 
one for BT). This allows us to move the knowledge about "enable" pins to 
the pwrseq. Once the QCA BT driver acquires and powers up the pwrseq, 
the BT part is ready. No need to toggle any additional pins. Once the 
WiFi pwrseq is powered up, the WiFi part is present on the bus and 
ready, without any additional pin toggling.

I can move onecell support to the separate patch if you think this might 
simplify the code review.

> 
>> +
>> +       return pwrseq_data->pwrseqs[idx];
>> +}
>> +
>> +static int __init pwrseq_core_init(void)
>> +{
>> +       return class_register(&pwrseq_class);
>> +}
>> +device_initcall(pwrseq_core_init);
>> diff --git a/include/linux/pwrseq/consumer.h b/include/linux/pwrseq/consumer.h
>> new file mode 100644
>> index 000000000000..fbcdc1fc0751
>> --- /dev/null
>> +++ b/include/linux/pwrseq/consumer.h
>> @@ -0,0 +1,88 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (c) 2021 Linaro Ltd.
>> + */
>> +
>> +#ifndef __LINUX_PWRSEQ_CONSUMER_H__
>> +#define __LINUX_PWRSEQ_CONSUMER_H__
>> +
>> +struct pwrseq;
>> +struct device;
>> +
>> +#if defined(CONFIG_PWRSEQ)
>> +
>> +struct pwrseq *__must_check pwrseq_get(struct device *dev, const char *id);
>> +struct pwrseq *__must_check devm_pwrseq_get(struct device *dev, const char *id);
>> +
>> +struct pwrseq *__must_check pwrseq_get_optional(struct device *dev, const char *id);
>> +struct pwrseq *__must_check devm_pwrseq_get_optional(struct device *dev, const char *id);
>> +
>> +void pwrseq_put(struct device *dev, struct pwrseq *pwrseq);
>> +
>> +int pwrseq_pre_power_on(struct pwrseq *pwrseq);
>> +int pwrseq_power_on(struct pwrseq *pwrseq);
>> +void pwrseq_power_off(struct pwrseq *pwrseq);
>> +void pwrseq_reset(struct pwrseq *pwrseq);
>> +
>> +#else
>> +
>> +static inline struct pwrseq *__must_check
>> +pwrseq_get(struct device *dev, const char *id)
>> +{
>> +       return ERR_PTR(-ENOSYS);
>> +}
>> +
>> +static inline struct pwrseq *__must_check
>> +devm_pwrseq_get(struct device *dev, const char *id)
>> +{
>> +       return ERR_PTR(-ENOSYS);
>> +}
>> +
>> +static inline struct pwrseq *__must_check
>> +pwrseq_get_optional(struct device *dev, const char *id)
>> +{
>> +       return NULL;
>> +}
>> +
>> +static inline struct pwrseq *__must_check
>> +devm_pwrseq_get_optional(struct device *dev, const char *id)
>> +{
>> +       return NULL;
>> +}
>> +
>> +static inline void pwrseq_put(struct device *dev, struct pwrseq *pwrseq)
>> +{
>> +}
>> +
>> +static inline int pwrseq_pre_power_on(struct pwrseq *pwrseq)
>> +{
>> +       return -ENOSYS;
>> +}
>> +
>> +static inline int pwrseq_power_on(struct pwrseq *pwrseq)
>> +{
>> +       return -ENOSYS;
>> +}
>> +
>> +static inline void pwrseq_power_off(struct pwrseq *pwrseq)
>> +{
>> +}
>> +
>> +static inline void pwrseq_reset(struct pwrseq *pwrseq)
>> +{
>> +}
>> +
>> +#endif
>> +
>> +static inline int pwrseq_full_power_on(struct pwrseq *pwrseq)
>> +{
>> +       int ret;
>> +
>> +       ret = pwrseq_pre_power_on(pwrseq);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return pwrseq_power_on(pwrseq);
>> +}
>> +
>> +#endif /* __LINUX_PWRSEQ_CONSUMER_H__ */
>> diff --git a/include/linux/pwrseq/driver.h b/include/linux/pwrseq/driver.h
>> new file mode 100644
>> index 000000000000..b2bc46624d7e
>> --- /dev/null
>> +++ b/include/linux/pwrseq/driver.h
>> @@ -0,0 +1,75 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (c) 2021 Linaro Ltd.
>> + */
>> +
>> +#ifndef __LINUX_PWRSEQ_DRIVER_H__
>> +#define __LINUX_PWRSEQ_DRIVER_H__
>> +
>> +#include <linux/device.h>
>> +
>> +struct pwrseq;
>> +
>> +struct pwrseq_ops {
>> +       int (*pre_power_on)(struct pwrseq *pwrseq);
>> +       int (*power_on)(struct pwrseq *pwrseq);
>> +       void (*power_off)(struct pwrseq *pwrseq);
>> +       void (*reset)(struct pwrseq *pwrseq);
>> +};
>> +
>> +struct module;
>> +
>> +struct pwrseq {
>> +       struct device dev;
>> +       const struct pwrseq_ops *ops;
>> +       unsigned int id;
>> +       struct module *owner;
>> +};
>> +
>> +struct pwrseq *__pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data);
>> +struct pwrseq *__devm_pwrseq_create(struct device *dev, struct module *owner, const struct pwrseq_ops *ops, void *data);
>> +
>> +#define pwrseq_create(dev, ops, data) __pwrseq_create((dev), THIS_MODULE, (ops), (data))
>> +#define devm_pwrseq_create(dev, ops, data) __devm_pwrseq_create((dev), THIS_MODULE, (ops), (data))
>> +
>> +void pwrseq_destroy(struct pwrseq *pwrseq);
>> +
>> +static inline void *pwrseq_get_data(struct pwrseq *pwrseq)
>> +{
>> +       return dev_get_drvdata(&pwrseq->dev);
>> +}
>> +
>> +#define        of_pwrseq_provider_register(dev, xlate, data)   \
>> +       __of_pwrseq_provider_register((dev), THIS_MODULE, (xlate), (data))
>> +
>> +#define        devm_of_pwrseq_provider_register(dev, xlate, data)      \
>> +       __devm_of_pwrseq_provider_register((dev), THIS_MODULE, (xlate), (data))
>> +
>> +struct of_phandle_args;
>> +
>> +struct pwrseq_provider *__of_pwrseq_provider_register(struct device *dev,
>> +       struct module *owner,
>> +       struct pwrseq * (*of_xlate)(void *data,
>> +                                   struct of_phandle_args *args),
>> +       void *data);
>> +struct pwrseq_provider *__devm_of_pwrseq_provider_register(struct device *dev,
>> +       struct module *owner,
>> +       struct pwrseq * (*of_xlate)(void *data,
>> +                                   struct of_phandle_args *args),
>> +       void *data);
>> +void of_pwrseq_provider_unregister(struct pwrseq_provider *pwrseq_provider);
>> +
>> +static inline struct pwrseq *of_pwrseq_xlate_single(void *data,
>> +                                                   struct of_phandle_args *args)
>> +{
>> +       return data;
>> +}
>> +
>> +struct pwrseq_onecell_data {
>> +       unsigned int num;
>> +       struct pwrseq *pwrseqs[];
>> +};
> 
> According to my earlier comment, I think a lot can be removed from
> here - if you would limit the provider to only use
> of_pwrseq_xlate_single.
> 
> Again, I think it's better to start simple, as it simplifies the review.
> 
>> +
>> +struct pwrseq *of_pwrseq_xlate_onecell(void *data, struct of_phandle_args *args);
>> +
>> +#endif /* __LINUX_PWRSEQ_DRIVER_H__ */
>> --
>> 2.33.0
>>
> 
> Other than my comments, overall I think this looks like a good start.
> 
> Kind regards
> Uffe
> 


-- 
With best wishes
Dmitry

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC v2 01/13] power: add power sequencer subsystem
  2021-09-13 12:32     ` Dmitry Baryshkov
@ 2021-09-13 13:42       ` Ulf Hansson
  0 siblings, 0 replies; 19+ messages in thread
From: Ulf Hansson @ 2021-09-13 13:42 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Andy Gross, Bjorn Andersson, Marcel Holtmann, Johan Hedberg,
	Luiz Augusto von Dentz, Kalle Valo, David S. Miller,
	Jakub Kicinski, Stanimir Varbanov, linux-arm-msm, linux-mmc,
	Linux Kernel Mailing List, linux-bluetooth, ath10k,
	linux-wireless, netdev

[...]

> >> +
> >> +struct pwrseq *of_pwrseq_xlate_onecell(void *data, struct of_phandle_args *args)
> >> +{
> >> +       struct pwrseq_onecell_data *pwrseq_data = data;
> >> +       unsigned int idx;
> >> +
> >> +       if (args->args_count != 1)
> >> +               return ERR_PTR(-EINVAL);
> >> +
> >> +       idx = args->args[0];
> >> +       if (idx >= pwrseq_data->num) {
> >> +               pr_err("%s: invalid index %u\n", __func__, idx);
> >> +               return ERR_PTR(-EINVAL);
> >> +       }
> >
> > In many cases it's reasonable to leave room for future extensions, so
> > that a provider could serve with more than one power-sequencer. I
> > guess that is what you intend to do here, right?
> >
> > In my opinion, I don't think what would happen, especially since a
> > power-sequence is something that should be specific to one particular
> > device (a Qcom WiFi/Blutooth chip, for example).
> >
> > That said, I suggest limiting this to a 1:1 mapping between the device
> > node and power-sequencer. I think that should simplify the code a bit.
>
> In fact the WiFi/BT example itself provides a non 1:1 mapping. In my
> current design the power sequencer provides two instances (one for WiFi,
> one for BT). This allows us to move the knowledge about "enable" pins to
> the pwrseq. Once the QCA BT driver acquires and powers up the pwrseq,
> the BT part is ready. No need to toggle any additional pins. Once the
> WiFi pwrseq is powered up, the WiFi part is present on the bus and
> ready, without any additional pin toggling.

Aha, that seems reasonable.

>
> I can move onecell support to the separate patch if you think this might
> simplify the code review.

It doesn't matter, both options work for me.

[...]

Kind regards
Uffe

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC v2 00/13] create power sequencing subsystem
  2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
                   ` (12 preceding siblings ...)
  2021-08-29 13:13 ` [RFC v2 13/13] WIP: arm64: dts: qcom: qrb5165-rb5: add bus-pwrseq property to pcie0 Dmitry Baryshkov
@ 2021-09-13 23:39 ` Steev Klimaszewski
  2021-10-06  3:49   ` Dmitry Baryshkov
  13 siblings, 1 reply; 19+ messages in thread
From: Steev Klimaszewski @ 2021-09-13 23:39 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Ulf Hansson,
	Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	Kalle Valo, David S. Miller, Jakub Kicinski, Stanimir Varbanov
  Cc: linux-arm-msm, linux-mmc, linux-kernel, linux-bluetooth, ath10k,
	linux-wireless, netdev


On 8/29/21 8:12 AM, Dmitry Baryshkov wrote:
> This is the second RFC on the proposed power sequencer subsystem. This
> is a generification of the MMC pwrseq code. The subsystem tries to
> abstract the idea of complex power-up/power-down/reset of the devices.
>
> To ease migration to pwrseq and to provide compatibility with older
> device trees, while keeping drivers simple, this iteration of RFC
> introduces pwrseq fallback support: pwrseq driver can register fallback
> providers. If another device driver requests pwrseq instance and none
> was declared, the pwrseq fallback code would go through the list of
> fallback providers and if the match is found, driver would return a
> crafted pwrseq instance. For now this mechanism is limited to the OF
> device matching, but it can be extended further to use any combination
> of device IDs.
>
> The primary set of devices that promted me to create this patchset is
> the Qualcomm BT+WiFi family of chips. They reside on serial+platform or
> serial + SDIO interfaces (older generations) or on serial+PCIe (newer
> generations).  They require a set of external voltage regulators to be
> powered on and (some of them) have separate WiFi and Bluetooth enable
> GPIOs.
>
> This patchset being an RFC tries to demonstrate the approach, design and
> usage of the pwrseq subsystem. Following issues are present in the RFC
> at this moment but will be fixed later if the overall approach would be
> viewed as acceptable:
>
>  - No documentation
>    While the code tries to be self-documenting proper documentation
>    would be required.
>
>  - Minimal device tree bindings changes
>    There are no proper updates for the DT bindings (thus neither Rob
>    Herring nor devicetree are included in the To/Cc lists). The dt
>    schema changes would be a part of v1.
>
>  - Lack of proper PCIe integration
>    At this moment support for PCIe is hacked up to be able to test the
>    PCIe part of qca6390. Proper PCIe support would require automatically
>    powering up the devices before the bus scan depending on the proper
>    device structure in the device tree.
>
> Changes since RFC v1:
>  - Provider pwrseq fallback support
>  - Implement fallback support in pwrseq_qca.
>  - Mmove susclk handling to pwrseq_qca.
>  - Significantly simplify hci_qca.c changes, by dropping all legacy
>    code. Now hci_qca uses only pwrseq calls to power up/down bluetooth
>    parts of the chip.
>
I tested this here, on the Lenovo Yoga C630, after creating a patch to
do basically the same thing as the db845c does.  One thing I noticed, if
PWRSEQ=y and the rest are =m, there is a build error.  I suppose once
the full set is posted and not RFC, I can send the patch for that. 

One question I have, if you don't mind, in patch 11, you add a second
channel to qca power sequencer.  I've added that here, but in the c630's
dts, "vreg_l23a_3p3: ldo23" is empty, so I added the same numbers in for
the regulator, and I'm wondering how to test that it's actually working
correctly?

-- steev


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC v2 00/13] create power sequencing subsystem
  2021-09-13 23:39 ` [RFC v2 00/13] create power sequencing subsystem Steev Klimaszewski
@ 2021-10-06  3:49   ` Dmitry Baryshkov
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2021-10-06  3:49 UTC (permalink / raw)
  To: Steev Klimaszewski
  Cc: Andy Gross, Bjorn Andersson, Ulf Hansson, Marcel Holtmann,
	Johan Hedberg, Luiz Augusto von Dentz, Kalle Valo,
	David S. Miller, Jakub Kicinski, Stanimir Varbanov,
	open list:DRM DRIVER FOR MSM ADRENO GPU, linux-mmc, open list,
	open list:BLUETOOTH SUBSYSTEM, ath10k, linux-wireless, netdev

Hi Steev,

On Tue, 14 Sept 2021 at 02:39, Steev Klimaszewski <steev@kali.org> wrote:
>
>
> On 8/29/21 8:12 AM, Dmitry Baryshkov wrote:
> > This is the second RFC on the proposed power sequencer subsystem. This
> > is a generification of the MMC pwrseq code. The subsystem tries to
> > abstract the idea of complex power-up/power-down/reset of the devices.
> >
> > To ease migration to pwrseq and to provide compatibility with older
> > device trees, while keeping drivers simple, this iteration of RFC
> > introduces pwrseq fallback support: pwrseq driver can register fallback
> > providers. If another device driver requests pwrseq instance and none
> > was declared, the pwrseq fallback code would go through the list of
> > fallback providers and if the match is found, driver would return a
> > crafted pwrseq instance. For now this mechanism is limited to the OF
> > device matching, but it can be extended further to use any combination
> > of device IDs.
> >
> > The primary set of devices that promted me to create this patchset is
> > the Qualcomm BT+WiFi family of chips. They reside on serial+platform or
> > serial + SDIO interfaces (older generations) or on serial+PCIe (newer
> > generations).  They require a set of external voltage regulators to be
> > powered on and (some of them) have separate WiFi and Bluetooth enable
> > GPIOs.
> >
> > This patchset being an RFC tries to demonstrate the approach, design and
> > usage of the pwrseq subsystem. Following issues are present in the RFC
> > at this moment but will be fixed later if the overall approach would be
> > viewed as acceptable:
> >
> >  - No documentation
> >    While the code tries to be self-documenting proper documentation
> >    would be required.
> >
> >  - Minimal device tree bindings changes
> >    There are no proper updates for the DT bindings (thus neither Rob
> >    Herring nor devicetree are included in the To/Cc lists). The dt
> >    schema changes would be a part of v1.
> >
> >  - Lack of proper PCIe integration
> >    At this moment support for PCIe is hacked up to be able to test the
> >    PCIe part of qca6390. Proper PCIe support would require automatically
> >    powering up the devices before the bus scan depending on the proper
> >    device structure in the device tree.
> >
> > Changes since RFC v1:
> >  - Provider pwrseq fallback support
> >  - Implement fallback support in pwrseq_qca.
> >  - Mmove susclk handling to pwrseq_qca.
> >  - Significantly simplify hci_qca.c changes, by dropping all legacy
> >    code. Now hci_qca uses only pwrseq calls to power up/down bluetooth
> >    parts of the chip.
> >
> I tested this here, on the Lenovo Yoga C630, after creating a patch to
> do basically the same thing as the db845c does.  One thing I noticed, if
> PWRSEQ=y and the rest are =m, there is a build error.  I suppose once
> the full set is posted and not RFC, I can send the patch for that.

Please excuse me for the delay in the response. I was carried away by
other duties. Yes, could you please provide a fixup patch.
I'm going to send v1 now, containing mostly cosmetical and
documentation changes. I'll include your patch in v2.

> One question I have, if you don't mind, in patch 11, you add a second
> channel to qca power sequencer.  I've added that here, but in the c630's
> dts, "vreg_l23a_3p3: ldo23" is empty, so I added the same numbers in for
> the regulator, and I'm wondering how to test that it's actually working
> correctly?

That's a good question. I have not looked in the details in the ath10k
documentation. I'll try finding it.
Maybe Kalle Valo can answer your question. Could you please duplicate
your question on the ath10k mailing list?

-- 
With best wishes
Dmitry

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-10-06  3:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-29 13:12 [RFC v2 00/13] create power sequencing subsystem Dmitry Baryshkov
2021-08-29 13:12 ` [RFC v2 01/13] power: add power sequencer subsystem Dmitry Baryshkov
2021-09-10 10:02   ` Ulf Hansson
2021-09-13 12:32     ` Dmitry Baryshkov
2021-09-13 13:42       ` Ulf Hansson
2021-08-29 13:12 ` [RFC v2 02/13] pwrseq: port MMC's pwrseq drivers to new pwrseq subsystem Dmitry Baryshkov
2021-08-29 13:12 ` [RFC v2 03/13] mmc: core: switch " Dmitry Baryshkov
2021-08-29 13:12 ` [RFC v2 04/13] pwrseq: add support for QCA BT+WiFi power sequencer Dmitry Baryshkov
2021-08-29 13:12 ` [RFC v2 05/13] pwrseq: add fallback support Dmitry Baryshkov
2021-08-29 13:12 ` [RFC v2 06/13] pwrseq: pwrseq_qca: implement " Dmitry Baryshkov
2021-08-29 13:12 ` [RFC v2 07/13] Bluetooth: hci_qca: switch to using pwrseq Dmitry Baryshkov
2021-08-29 13:13 ` [RFC v2 08/13] ath10k: add support for pwrseq sequencing Dmitry Baryshkov
2021-08-29 13:13 ` [RFC v2 09/13] arm64: dts: qcom: sdm845-db845c: switch bt+wifi to qca power sequencer Dmitry Baryshkov
2021-08-29 13:13 ` [RFC v2 10/13] arm64: dts: qcom: qrb5165-rb5: add bluetooth support Dmitry Baryshkov
2021-08-29 13:13 ` [RFC v2 11/13] arm64: dts: qcom: sdm845-db845c: add second channel to qca power sequencer Dmitry Baryshkov
2021-08-29 13:13 ` [RFC v2 12/13] WIP: PCI: qcom: use pwrseq to power up bus devices Dmitry Baryshkov
2021-08-29 13:13 ` [RFC v2 13/13] WIP: arm64: dts: qcom: qrb5165-rb5: add bus-pwrseq property to pcie0 Dmitry Baryshkov
2021-09-13 23:39 ` [RFC v2 00/13] create power sequencing subsystem Steev Klimaszewski
2021-10-06  3:49   ` Dmitry Baryshkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).