Thread (27 messages) 27 messages, 6 authors, 2021-08-12

Re: [PATCH v3 2/7] regulator: qca6390: add support for QCA639x powerup sequence

From: Mark Brown <broonie@kernel.org>
Date: 2021-06-22 11:29:10
Also in: linux-arm-msm, linux-devicetree, lkml

On Tue, Jun 22, 2021 at 01:31:36AM +0300, Dmitry Baryshkov wrote:
Qualcomm QCA6390/1 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. Add device driver handling
power sequencing of QCA6390/1.
Are you sure this is a regulator and not a MFD?  It appears to be a
consumer driver that turns on and off a bunch of regulators en masse
which for some reason exposes that on/off control as a single supply.
This looks like it'd be much more appropriate to implement as a MFD or
possibly power domain with the subdevices using runtime PM, it's clearly
not a regulator.
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021, Linaro Limited
+ */
Please make the entire comment a C++ one so things look more
intentional.
+static int qca6390_enable(struct regulator_dev *rdev)
+{
+	struct qca6390_data *data = rdev_get_drvdata(rdev);
+	int ret;
+
+	ret = regulator_bulk_enable(data->num_vregs, data->regulators);
+	if (ret) {
+		dev_err(data->dev, "Failed to enable regulators");
+		return ret;
+	}
The regulator API is *not* recursive, I am astonished this works.
+	/* Wait for 1ms before toggling enable pins. */
+	usleep_range(1000, 2000);
There's core support for delays after power on, better to use it.
+	data->enable_counter++;
You shouldn't assume that enable and disable calls are matched.

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help