Re: [PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence
On Mon, Dec 21, 2020 at 05:08:44PM +0300, Dmitry Baryshkov wrote: > Hello, > > On Mon, 21 Dec 2020 at 12:02, Lee Jones wrote: > > > > On Sun, 20 Dec 2020, Dmitry Baryshkov wrote: > > > > > Qualcomm QCA639x is a family of WiFi + Bluetooth SoCs, with BT part > > > being controlled through the UART and WiFi being present on PCIe > > > bus. Both blocks share common power sources. So add mfd device driver > > > handling power sequencing of QCA6390/1. > > > > > > Signed-off-by: Dmitry Baryshkov > > > --- > > > drivers/mfd/Kconfig| 12 +++ > > > drivers/mfd/Makefile | 1 + > > > drivers/mfd/qcom-qca639x.c | 168 + > > > 3 files changed, 181 insertions(+) > > > create mode 100644 drivers/mfd/qcom-qca639x.c > > > > This is not an MFD, since it utilised neither the MFD API nor > > of_platform_populate() to register child devices. > > It would use them if the WiFi part was not on a discoverable bus. PCI nodes have been supported in DT for forever. If you have non-discoverable additions to a PCI device, then the PCI device should be in DT. Rob
Re: [PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence
On Mon, 21 Dec 2020, Dmitry Baryshkov wrote: > Hello, > > On Mon, 21 Dec 2020 at 12:02, Lee Jones wrote: > > > > On Sun, 20 Dec 2020, Dmitry Baryshkov wrote: > > > > > Qualcomm QCA639x is a family of WiFi + Bluetooth SoCs, with BT part > > > being controlled through the UART and WiFi being present on PCIe > > > bus. Both blocks share common power sources. So add mfd device driver > > > handling power sequencing of QCA6390/1. > > > > > > Signed-off-by: Dmitry Baryshkov > > > --- > > > drivers/mfd/Kconfig| 12 +++ > > > drivers/mfd/Makefile | 1 + > > > drivers/mfd/qcom-qca639x.c | 168 + > > > 3 files changed, 181 insertions(+) > > > create mode 100644 drivers/mfd/qcom-qca639x.c > > > > This is not an MFD, since it utilised neither the MFD API nor > > of_platform_populate() to register child devices. > > It would use them if the WiFi part was not on a discoverable bus. This is a can of worms that I do not wish to open right now. However the TL;DR is: MFD is currently only *meant* for non-discoverable platform devices. > > Suggest you use drivers/power or similar to handle shared power > > sequencing requirements. > > What about drivers/soc/qcom? Or drivers/misc? These are 2 other subsystems, just like MFD, which are commonly used as dumping grounds for code which doesn't fit anywhere else. However, I implore you to please try to find a suitable subsystem for this to fit into first. If this driver only deals with power management, place it is a power related subsystem *before* considering drivers/soc or drivers/misc. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence
Hello, On Mon, 21 Dec 2020 at 12:02, Lee Jones wrote: > > On Sun, 20 Dec 2020, Dmitry Baryshkov wrote: > > > Qualcomm QCA639x is a family of WiFi + Bluetooth SoCs, with BT part > > being controlled through the UART and WiFi being present on PCIe > > bus. Both blocks share common power sources. So add mfd device driver > > handling power sequencing of QCA6390/1. > > > > Signed-off-by: Dmitry Baryshkov > > --- > > drivers/mfd/Kconfig| 12 +++ > > drivers/mfd/Makefile | 1 + > > drivers/mfd/qcom-qca639x.c | 168 + > > 3 files changed, 181 insertions(+) > > create mode 100644 drivers/mfd/qcom-qca639x.c > > This is not an MFD, since it utilised neither the MFD API nor > of_platform_populate() to register child devices. It would use them if the WiFi part was not on a discoverable bus. > Suggest you use drivers/power or similar to handle shared power > sequencing requirements. What about drivers/soc/qcom? Or drivers/misc? -- With best wishes Dmitry
Re: [PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence
On Sun, 20 Dec 2020, Dmitry Baryshkov wrote: > Qualcomm QCA639x is a family of WiFi + Bluetooth SoCs, with BT part > being controlled through the UART and WiFi being present on PCIe > bus. Both blocks share common power sources. So add mfd device driver > handling power sequencing of QCA6390/1. > > Signed-off-by: Dmitry Baryshkov > --- > drivers/mfd/Kconfig| 12 +++ > drivers/mfd/Makefile | 1 + > drivers/mfd/qcom-qca639x.c | 168 + > 3 files changed, 181 insertions(+) > create mode 100644 drivers/mfd/qcom-qca639x.c This is not an MFD, since it utilised neither the MFD API nor of_platform_populate() to register child devices. Suggest you use drivers/power or similar to handle shared power sequencing requirements. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
[PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence
Qualcomm QCA639x is a family of WiFi + Bluetooth SoCs, with BT part being controlled through the UART and WiFi being present on PCIe bus. Both blocks share common power sources. So add mfd device driver handling power sequencing of QCA6390/1. Signed-off-by: Dmitry Baryshkov --- drivers/mfd/Kconfig| 12 +++ drivers/mfd/Makefile | 1 + drivers/mfd/qcom-qca639x.c | 168 + 3 files changed, 181 insertions(+) create mode 100644 drivers/mfd/qcom-qca639x.c diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index bdfce7b15621..2fd6b9770ad0 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -1036,6 +1036,18 @@ config MFD_PM8XXX Say M here if you want to include support for PM8xxx chips as a module. This will build a module called "pm8xxx-core". +config MFD_QCOM_QCA639X + tristate "Qualcomm QCA639x WiFi/Bluetooth module support" + depends on REGULATOR && PM_GENERIC_DOMAINS + help + If you say yes to this option, support will be included for Qualcomm + QCA639x family of WiFi and Bluetooth SoCs. Note, this driver supports + only power control for this SoC, you still have to enable individual + Bluetooth and WiFi drivers. + + Say M here if you want to include support for QCA639x chips as a + module. This will build a module called "qcom-qca639x". + config MFD_QCOM_RPM tristate "Qualcomm Resource Power Manager (RPM)" depends on ARCH_QCOM && OF diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 14fdb188af02..da5747508faf 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -202,6 +202,7 @@ obj-$(CONFIG_MFD_SI476X_CORE) += si476x-core.o obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o obj-$(CONFIG_MFD_OMAP_USB_HOST)+= omap-usb-host.o omap-usb-tll.o obj-$(CONFIG_MFD_PM8XXX) += qcom-pm8xxx.o ssbi.o +obj-$(CONFIG_MFD_QCOM_QCA639X) += qcom-qca639x.o obj-$(CONFIG_MFD_QCOM_RPM) += qcom_rpm.o obj-$(CONFIG_MFD_SPMI_PMIC)+= qcom-spmi-pmic.o obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o diff --git a/drivers/mfd/qcom-qca639x.c b/drivers/mfd/qcom-qca639x.c new file mode 100644 index ..1ecc2e2e5bfd --- /dev/null +++ b/drivers/mfd/qcom-qca639x.c @@ -0,0 +1,168 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2020, Linaro Limited + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define MAX_NUM_REGULATORS 8 + +static struct vreg { + const char *name; + unsigned int load_uA; +} vregs[MAX_NUM_REGULATORS] = { + /* 2.0 V */ + { "vddpcie2", 15000 }, + { "vddrfa3", 40 }, + + /* 0.95 V */ + { "vddaon", 10 }, + { "vddpmu", 125 }, + { "vddrfa1", 20 }, + + /* 1.35 V */ + { "vddrfa2", 40 }, + { "vddpcie1", 35000 }, + + /* 1.8 V */ + { "vddio", 2 }, +}; + +struct qca639x_data { + struct regulator_bulk_data regulators[MAX_NUM_REGULATORS]; + size_t num_vregs; + struct device *dev; + struct pinctrl_state *active_state; + struct generic_pm_domain pd; +}; + +#define domain_to_data(domain) container_of(domain, struct qca639x_data, pd) + +static int qca639x_power_on(struct generic_pm_domain *domain) +{ + struct qca639x_data *data = domain_to_data(domain); + int ret; + + dev_warn(&domain->dev, "DUMMY POWER ON\n"); + + ret = regulator_bulk_enable(data->num_vregs, data->regulators); + if (ret) { + dev_err(data->dev, "Failed to enable regulators"); + return ret; + } + + /* Wait for 1ms before toggling enable pins. */ + usleep_range(1000, 2000); + + ret = pinctrl_select_state(data->dev->pins->p, data->active_state); + if (ret) { + dev_err(data->dev, "Failed to select active state"); + return ret; + } + + /* Wait for all power levels to stabilize */ + usleep_range(6000, 7000); + + return 0; +} + +static int qca639x_power_off(struct generic_pm_domain *domain) +{ + struct qca639x_data *data = domain_to_data(domain); + + dev_warn(&domain->dev, "DUMMY POWER OFF\n"); + + pinctrl_select_default_state(data->dev); + regulator_bulk_disable(data->num_vregs, data->regulators); + + return 0; +} + +static int qca639x_probe(struct platform_device *pdev) +{ + struct qca639x_data *data; + struct device *dev = &pdev->dev; + int i, ret; + + if (!dev->pins || IS_ERR_OR_NULL(dev->pins->default_state)) + return -EINVAL; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->dev = dev; + data->num_vregs = ARRAY_SIZE(vregs); + + data->active_state = pinctrl_lookup_state(dev->pins->p, "active"); + if (IS_ERR(