Qualcomm QCA639x is a family of WiFi + Bluetooth chips, with BT part
being controlled through the UART and WiFi being present on PCIe
bus. Both blocks use common power management unit wich should be turned
on before either of devices can be probed. Add common 'qcom-qca6390'
driver providing regulator to be used by both BT and PCIe parts.
Changes since v2:
- Rebase on top of linux-next to get wcn6750 changes.
- Switch from misc device providing power domains into regulator code.
- Use vddpe-3v3-supply as expected by pcie-qcom driver.
- Use enable_gpio functionality in hci_qca driver.
Changes since v1:
- Stopped using wildcard in the dts binding, stick to qcom,qca6390.
- Stopped using pcie0_phy for qca639x power domain.
- Describe root PCIe bridge in the dts and bind power domain to the
bridge.
- Add pci quirk to power up power domains connected to this bridge.
----------------------------------------------------------------
Dmitry Baryshkov (7):
dt-bindings: regulator: qcom,qca6390: add binding for QCA6390 device
regulator: qca6390: add support for QCA639x powerup sequence
Bluetooth: hci_qca: provide default device data
Bluetooth: hci_qca: merge qca_power into qca_serdev
Bluetooth: hci_qca: merge wcn & non-wcn code paths
Bluetooth: hci_qca: add power sequencer support to qca6390
arm64: dts: qcom: qrb5165-rb5: add QCA6391 WiFi+BT SoC
.../bindings/regulator/qcom,qca6390.yaml | 70 +++++++
arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 56 +++++
drivers/bluetooth/hci_qca.c | 225 ++++++++++-----------
drivers/regulator/Kconfig | 13 ++
drivers/regulator/Makefile | 1 +
drivers/regulator/qcom-qca639x.c | 157 ++++++++++++++
6 files changed, 401 insertions(+), 121 deletions(-)
create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390.yaml
create mode 100644 drivers/regulator/qcom-qca639x.c
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.
Signed-off-by: Dmitry Baryshkov <redacted>
---
drivers/regulator/Kconfig | 13 +++
drivers/regulator/Makefile | 1 +
drivers/regulator/qcom-qca639x.c | 157 +++++++++++++++++++++++++++++++
3 files changed, 171 insertions(+)
create mode 100644 drivers/regulator/qcom-qca639x.c
@@ -0,0 +1,157 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(c)2021,LinaroLimited+*/+#include<linux/delay.h>+#include<linux/of.h>+#include<linux/platform_device.h>+#include<linux/regulator/consumer.h>+#include<linux/regulator/driver.h>+#include<linux/regulator/of_regulator.h>+#include<linux/slab.h>++#define MAX_NUM_REGULATORS 8++staticstructvreg{+constchar*name;+unsignedintload_uA;+}vregs[MAX_NUM_REGULATORS]={+/* 2.0 V */+{"vddpcie2",15000},+{"vddrfa3",400000},++/* 0.95 V */+{"vddaon",100000},+{"vddpmu",1250000},+{"vddrfa1",200000},++/* 1.35 V */+{"vddrfa2",400000},+{"vddpcie1",35000},++/* 1.8 V */+{"vddio",20000},+};++structqca6390_data{+structdevice*dev;+structregulator_bulk_dataregulators[MAX_NUM_REGULATORS];+size_tnum_vregs;++structregulator_descdesc;+structregulator_dev*regulator_dev;+unsignedintenable_counter;+};++#define domain_to_data(domain) container_of(domain, struct qca6390_data, pd)++staticintqca6390_enable(structregulator_dev*rdev)+{+structqca6390_data*data=rdev_get_drvdata(rdev);+intret;++ret=regulator_bulk_enable(data->num_vregs,data->regulators);+if(ret){+dev_err(data->dev,"Failed to enable regulators");+returnret;+}++/* Wait for 1ms before toggling enable pins. */+usleep_range(1000,2000);++data->enable_counter++;++return0;+}++staticintqca6390_disable(structregulator_dev*rdev)+{+structqca6390_data*data=rdev_get_drvdata(rdev);++regulator_bulk_disable(data->num_vregs,data->regulators);++data->enable_counter--;++return0;+}++staticintqca6390_is_enabled(structregulator_dev*rdev)+{+structqca6390_data*data=rdev_get_drvdata(rdev);++returndata->enable_counter>0;+}++staticconststructregulator_opsqca6390_ops={+.enable=qca6390_enable,+.disable=qca6390_disable,+.is_enabled=qca6390_is_enabled,+};++staticintqca6390_probe(structplatform_device*pdev)+{+structqca6390_data*data;+structdevice*dev=&pdev->dev;+structregulator_configcfg={};+inti,ret;++data=devm_kzalloc(dev,sizeof(*data),GFP_KERNEL);+if(!data)+return-ENOMEM;++data->dev=dev;+data->num_vregs=ARRAY_SIZE(vregs);++for(i=0;i<data->num_vregs;i++)+data->regulators[i].supply=vregs[i].name;++ret=devm_regulator_bulk_get(dev,data->num_vregs,data->regulators);+if(ret<0)+returnret;++for(i=0;i<data->num_vregs;i++){+ret=regulator_set_load(data->regulators[i].consumer,vregs[i].load_uA);+if(ret)+returnret;+}++data->desc.name=devm_kstrdup(dev,dev_name(dev),GFP_KERNEL);+if(!data->desc.name)+return-ENOMEM;++data->desc.type=REGULATOR_VOLTAGE;+data->desc.owner=THIS_MODULE;+data->desc.ops=&qca6390_ops;++cfg.dev=dev;+cfg.of_node=dev->of_node;+cfg.driver_data=data;+cfg.init_data=of_get_regulator_init_data(dev,dev->of_node,&data->desc);++data->regulator_dev=devm_regulator_register(dev,&data->desc,&cfg);+if(IS_ERR(data->regulator_dev)){+ret=PTR_ERR(data->regulator_dev);+returnret;+}++platform_set_drvdata(pdev,data);++return0;+}++staticconststructof_device_idqca6390_of_match[]={+{.compatible="qcom,qca6390"},+};++staticstructplatform_driverqca6390_driver={+.probe=qca6390_probe,+.driver={+.name="qca6390",+.of_match_table=qca6390_of_match,+},+};++module_platform_driver(qca6390_driver);+MODULE_AUTHOR("Dmitry Baryshkov <dmitry.baryshkov@linaro.org>");+MODULE_DESCRIPTION("Power control for Qualcomm QCA6390/1 BT/WiFi chip");+MODULE_LICENSE("GPL v2");
From: Mark Brown <broonie@kernel.org> Date: 2021-06-22 11:29:10
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.
On Tue, 22 Jun 2021 at 14:29, Mark Brown [off-list ref] wrote:
On Tue, Jun 22, 2021 at 01:31:36AM +0300, Dmitry Baryshkov wrote:
quoted
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.
First attempt was designed to be an MFD. And Lee clearly stated that
this is wrong:
"This is not an MFD, since it utilised neither the MFD API nor
of_platform_populate() to register child devices." [1]
I've attempted implementing that as a genpd (in previous iterations),
but it results in worse design. PCIe controllers are not expected to
handle power domains for EP devices, especially in cases when the PD
must come up before the controller does link training and bus probe.
I've tried following Rob's suggestions on implementing things clearly,
but doing so results in too big restructure just for a single device.
Please make the entire comment a C++ one so things look more
intentional.
Ack.
quoted
+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.
It does, even with lockdep enabled. Moreover BT regularly does disable
and enable this regulator, so both enable and disable paths were well
tested.
Should I change this into some internal call to remove API recursiveness?
quoted
+ /* Wait for 1ms before toggling enable pins. */
+ usleep_range(1000, 2000);
There's core support for delays after power on, better to use it.
Ack.
quoted
+ data->enable_counter++;
You shouldn't assume that enable and disable calls are matched.
From: Mark Brown <broonie@kernel.org> Date: 2021-06-22 14:38:39
On Tue, Jun 22, 2021 at 05:17:28PM +0300, Dmitry Baryshkov wrote:
On Tue, 22 Jun 2021 at 14:29, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jun 22, 2021 at 01:31:36AM +0300, Dmitry Baryshkov wrote:
quoted
quoted
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.
quoted
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.
First attempt was designed to be an MFD. And Lee clearly stated that
this is wrong:
"This is not an MFD, since it utilised neither the MFD API nor
of_platform_populate() to register child devices." [1]
Well, perhaps it should do one of those things then? Like I say this is
very clearly not a regulator, it looks like a consumer of some kind.
The regulator API isn't there just to absorb things that need reference
counting, it's there to represent things providing supplies. This seems
to be very clearly not a supply given that it's grouping together a
bunch of other supplies and switching them on and off together without
providing a clear output supply.
I've tried following Rob's suggestions on implementing things clearly,
but doing so results in too big restructure just for a single device.
I don't know what that suggestion was? If there's only one device that
uses this why is it not implemented as part of that device?
quoted
quoted
+static int qca6390_enable(struct regulator_dev *rdev)
+{
+ struct qca6390_data *data = rdev_get_drvdata(rdev);
+ int ret;
quoted
quoted
+ ret = regulator_bulk_enable(data->num_vregs, data->regulators);
+ if (ret) {
+ dev_err(data->dev, "Failed to enable regulators");
+ return ret;
+ }
quoted
The regulator API is *not* recursive, I am astonished this works.
It does, even with lockdep enabled. Moreover BT regularly does disable
and enable this regulator, so both enable and disable paths were well
tested.
Should I change this into some internal call to remove API recursiveness?
You should not be implementing this as a regulator at all.
On Tue, 22 Jun 2021 at 17:38, Mark Brown [off-list ref] wrote:
On Tue, Jun 22, 2021 at 05:17:28PM +0300, Dmitry Baryshkov wrote:
quoted
On Tue, 22 Jun 2021 at 14:29, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jun 22, 2021 at 01:31:36AM +0300, Dmitry Baryshkov wrote:
quoted
quoted
quoted
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.
quoted
quoted
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.
quoted
First attempt was designed to be an MFD. And Lee clearly stated that
this is wrong:
"This is not an MFD, since it utilised neither the MFD API nor
of_platform_populate() to register child devices." [1]
Well, perhaps it should do one of those things then?
I don't think so. BT part is just a serdev sitting on top of UART,
WiFi is PCIe device (for qca6390). So using MFD API (which primarily
targets platform devices) does not seem logical and feasible.
Like I say this is
very clearly not a regulator, it looks like a consumer of some kind.
The regulator API isn't there just to absorb things that need reference
counting, it's there to represent things providing supplies. This seems
to be very clearly not a supply given that it's grouping together a
bunch of other supplies and switching them on and off together without
providing a clear output supply.
Ack.
quoted
I've tried following Rob's suggestions on implementing things clearly,
but doing so results in too big restructure just for a single device.
I don't know what that suggestion was? If there's only one device that
uses this why is it not implemented as part of that device?
One device = qca6390 (or 91). Because it is still a device sitting on
a PCI bus which is typically discoverable, while adding power
sequencer to the device driver would demand making the bus fully
descibiable (like PCI bus is on Spark).
quoted
quoted
quoted
+static int qca6390_enable(struct regulator_dev *rdev)
+{
+ struct qca6390_data *data = rdev_get_drvdata(rdev);
+ int ret;
quoted
quoted
quoted
+ ret = regulator_bulk_enable(data->num_vregs, data->regulators);
+ if (ret) {
+ dev_err(data->dev, "Failed to enable regulators");
+ return ret;
+ }
quoted
quoted
The regulator API is *not* recursive, I am astonished this works.
quoted
It does, even with lockdep enabled. Moreover BT regularly does disable
and enable this regulator, so both enable and disable paths were well
tested.
Should I change this into some internal call to remove API recursiveness?
You should not be implementing this as a regulator at all.
From: Mark Brown <broonie@kernel.org> Date: 2021-06-22 17:08:49
On Tue, Jun 22, 2021 at 07:46:08PM +0300, Dmitry Baryshkov wrote:
On Tue, 22 Jun 2021 at 17:38, Mark Brown [off-list ref] wrote:
quoted
Well, perhaps it should do one of those things then?
I don't think so. BT part is just a serdev sitting on top of UART,
WiFi is PCIe device (for qca6390). So using MFD API (which primarily
targets platform devices) does not seem logical and feasible.
That really does sound like a MFD - AIUI it's a single chip with
multiple interfaces that needs some glue logic to hold it together. It
doesn't fit well with the current framework that MFD offers but it's
definitely the same sort of one chip in multiple Linux frameworks sort
of thing. The only other thing I can think might fit is handling it
like a plug in module for a development board (eg, RPi hats) but we've
not been doing so great at getting them supported upstream.
+ Peter
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
[off-list ref] 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.
Signed-off-by: Dmitry Baryshkov <redacted>
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
Note that, this isn't specific to the PCIe bus, but rather to all
buses that may have discoverable devices attached and which need some
kind of pre-power-sequence before they can be discovered/probed. USB,
PCIe, SDIO, etc.
Long time ago, we fixed the problem for SDIO (that also can have WiFi,
UART, bluetooth chips attached), but unfortunately through an MMC
subsystem specific implementation, that can't be re-used in a generic
way.
In any case, I have looped in Peter Chen, maybe he can provide us with
a better update on how things have moved forward, if at all.
Kind regards
Uffe
[1]
https://www.spinics.net/lists/linux-usb/msg158451.html
@@ -0,0 +1,157 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*Copyright(c)2021,LinaroLimited+*/+#include<linux/delay.h>+#include<linux/of.h>+#include<linux/platform_device.h>+#include<linux/regulator/consumer.h>+#include<linux/regulator/driver.h>+#include<linux/regulator/of_regulator.h>+#include<linux/slab.h>++#define MAX_NUM_REGULATORS 8++staticstructvreg{+constchar*name;+unsignedintload_uA;+}vregs[MAX_NUM_REGULATORS]={+/* 2.0 V */+{"vddpcie2",15000},+{"vddrfa3",400000},++/* 0.95 V */+{"vddaon",100000},+{"vddpmu",1250000},+{"vddrfa1",200000},++/* 1.35 V */+{"vddrfa2",400000},+{"vddpcie1",35000},++/* 1.8 V */+{"vddio",20000},+};++structqca6390_data{+structdevice*dev;+structregulator_bulk_dataregulators[MAX_NUM_REGULATORS];+size_tnum_vregs;++structregulator_descdesc;+structregulator_dev*regulator_dev;+unsignedintenable_counter;+};++#define domain_to_data(domain) container_of(domain, struct qca6390_data, pd)++staticintqca6390_enable(structregulator_dev*rdev)+{+structqca6390_data*data=rdev_get_drvdata(rdev);+intret;++ret=regulator_bulk_enable(data->num_vregs,data->regulators);+if(ret){+dev_err(data->dev,"Failed to enable regulators");+returnret;+}++/* Wait for 1ms before toggling enable pins. */+usleep_range(1000,2000);++data->enable_counter++;++return0;+}++staticintqca6390_disable(structregulator_dev*rdev)+{+structqca6390_data*data=rdev_get_drvdata(rdev);++regulator_bulk_disable(data->num_vregs,data->regulators);++data->enable_counter--;++return0;+}++staticintqca6390_is_enabled(structregulator_dev*rdev)+{+structqca6390_data*data=rdev_get_drvdata(rdev);++returndata->enable_counter>0;+}++staticconststructregulator_opsqca6390_ops={+.enable=qca6390_enable,+.disable=qca6390_disable,+.is_enabled=qca6390_is_enabled,+};++staticintqca6390_probe(structplatform_device*pdev)+{+structqca6390_data*data;+structdevice*dev=&pdev->dev;+structregulator_configcfg={};+inti,ret;++data=devm_kzalloc(dev,sizeof(*data),GFP_KERNEL);+if(!data)+return-ENOMEM;++data->dev=dev;+data->num_vregs=ARRAY_SIZE(vregs);++for(i=0;i<data->num_vregs;i++)+data->regulators[i].supply=vregs[i].name;++ret=devm_regulator_bulk_get(dev,data->num_vregs,data->regulators);+if(ret<0)+returnret;++for(i=0;i<data->num_vregs;i++){+ret=regulator_set_load(data->regulators[i].consumer,vregs[i].load_uA);+if(ret)+returnret;+}++data->desc.name=devm_kstrdup(dev,dev_name(dev),GFP_KERNEL);+if(!data->desc.name)+return-ENOMEM;++data->desc.type=REGULATOR_VOLTAGE;+data->desc.owner=THIS_MODULE;+data->desc.ops=&qca6390_ops;++cfg.dev=dev;+cfg.of_node=dev->of_node;+cfg.driver_data=data;+cfg.init_data=of_get_regulator_init_data(dev,dev->of_node,&data->desc);++data->regulator_dev=devm_regulator_register(dev,&data->desc,&cfg);+if(IS_ERR(data->regulator_dev)){+ret=PTR_ERR(data->regulator_dev);+returnret;+}++platform_set_drvdata(pdev,data);++return0;+}++staticconststructof_device_idqca6390_of_match[]={+{.compatible="qcom,qca6390"},+};++staticstructplatform_driverqca6390_driver={+.probe=qca6390_probe,+.driver={+.name="qca6390",+.of_match_table=qca6390_of_match,+},+};++module_platform_driver(qca6390_driver);+MODULE_AUTHOR("Dmitry Baryshkov <dmitry.baryshkov@linaro.org>");+MODULE_DESCRIPTION("Power control for Qualcomm QCA6390/1 BT/WiFi chip");+MODULE_LICENSE("GPL v2");--
From: Mark Brown <broonie@kernel.org> Date: 2021-07-06 12:11:03
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
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.
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
- Peter (the email was bouncing)
On Tue, 6 Jul 2021 at 13:55, Mark Brown [off-list ref] wrote:
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
quoted
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
quoted
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.
quoted
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
Well, to me it looks very similar to those resouses we could manage
with the mmc pwrseq, for SDIO. It's also typically the same kind of
combo-chips that moved from supporting SDIO to PCIe, for improved
performance I guess. More importantly, the same constraint to
pre-power on the device is needed to allow it to be discovered/probed.
Therefore, I think it would be worth having a common solution for
this, rather than a solution per subsystem or even worse, per device.
Unfortunately, it looks like Peter's email is bouncing so we can't get
an update from him.
Kind regards
Uffe
Hi,
On Thu, 8 Jul 2021 at 13:10, Ulf Hansson [off-list ref] wrote:
- Peter (the email was bouncing)
+ Peter's kernel.org address
On Tue, 6 Jul 2021 at 13:55, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
quoted
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
quoted
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.
quoted
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
Well, to me it looks very similar to those resouses we could manage
with the mmc pwrseq, for SDIO. It's also typically the same kind of
combo-chips that moved from supporting SDIO to PCIe, for improved
performance I guess. More importantly, the same constraint to
pre-power on the device is needed to allow it to be discovered/probed.
In our case we'd definitely use pwrseq for PCIe bus and we can also
benefit from using pwrseq for serdev and for platform busses also (for
the same story of WiFi+BT chips).
I can take a look at rewriting pwrseq code to also handle the PCIe
bus. Rewriting it to be a generic lib seems like an easy task,
plugging it into PCIe code would be more fun.
Platform and serdev... Definitely even more fun.
Therefore, I think it would be worth having a common solution for
this, rather than a solution per subsystem or even worse, per device.
Unfortunately, it looks like Peter's email is bouncing so we can't get
an update from him.
Let's see if the kernel.org email will get to him.
--
With best wishes
Dmitry
From: Rob Herring <robh@kernel.org> Date: 2021-07-14 16:47:18
On Thu, Jul 08, 2021 at 02:37:44PM +0300, Dmitry Baryshkov wrote:
Hi,
On Thu, 8 Jul 2021 at 13:10, Ulf Hansson [off-list ref] wrote:
quoted
- Peter (the email was bouncing)
+ Peter's kernel.org address
quoted
On Tue, 6 Jul 2021 at 13:55, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
quoted
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
quoted
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.
quoted
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
Well, to me it looks very similar to those resouses we could manage
with the mmc pwrseq, for SDIO. It's also typically the same kind of
combo-chips that moved from supporting SDIO to PCIe, for improved
performance I guess. More importantly, the same constraint to
pre-power on the device is needed to allow it to be discovered/probed.
In our case we'd definitely use pwrseq for PCIe bus and we can also
benefit from using pwrseq for serdev and for platform busses also (for
the same story of WiFi+BT chips).
I can take a look at rewriting pwrseq code to also handle the PCIe
bus. Rewriting it to be a generic lib seems like an easy task,
plugging it into PCIe code would be more fun.
Platform and serdev... Definitely even more fun.
I don't want to see pwrseq (the binding) expanded to other buses. If
that was the answer, we wouldn't be having this discussion. It was a
mistake for MMC IMO.
If pwrseq works as a kernel library/api, then I have no issue with that.
quoted
Therefore, I think it would be worth having a common solution for
this, rather than a solution per subsystem or even worse, per device.
Power sequencing requirements are inheritently per device unless we're
talking about standard connectors.
This is a solved problem on MDIO. It's quite simple. If there's a DT
node for a device you haven't discovered, then probe it anyways.
Rob
On Thu, Jul 08, 2021 at 02:37:44PM +0300, Dmitry Baryshkov wrote:
quoted
Hi,
On Thu, 8 Jul 2021 at 13:10, Ulf Hansson [off-list ref] wrote:
quoted
- Peter (the email was bouncing)
+ Peter's kernel.org address
quoted
On Tue, 6 Jul 2021 at 13:55, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
quoted
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
quoted
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.
quoted
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
Well, to me it looks very similar to those resouses we could manage
with the mmc pwrseq, for SDIO. It's also typically the same kind of
combo-chips that moved from supporting SDIO to PCIe, for improved
performance I guess. More importantly, the same constraint to
pre-power on the device is needed to allow it to be discovered/probed.
In our case we'd definitely use pwrseq for PCIe bus and we can also
benefit from using pwrseq for serdev and for platform busses also (for
the same story of WiFi+BT chips).
I can take a look at rewriting pwrseq code to also handle the PCIe
bus. Rewriting it to be a generic lib seems like an easy task,
plugging it into PCIe code would be more fun.
Platform and serdev... Definitely even more fun.
I don't want to see pwrseq (the binding) expanded to other buses. If
that was the answer, we wouldn't be having this discussion. It was a
mistake for MMC IMO.
But what do you want to see?
We have a single piece of hardware that needs a specific power sequence,
which is interacted with using both UART and PCIe.
If pwrseq works as a kernel library/api, then I have no issue with that.
quoted
quoted
Therefore, I think it would be worth having a common solution for
this, rather than a solution per subsystem or even worse, per device.
Power sequencing requirements are inheritently per device unless we're
talking about standard connectors.
Do you mean "device" as in the IC or device as in struct device? Because
we do have one physical IC, that has a need for a specific power on
sequence, but we have two struct device, on two different busses in
Linux interacting with this thing.
This is a solved problem on MDIO. It's quite simple. If there's a DT
node for a device you haven't discovered, then probe it anyways.
Okay, so DT tells us that there's actually a WiFi thing on the PCIe bus,
even though we can't find it, so we probe something...then what?
Regards,
Bjorn
On Wed, 14 Jul 2021 at 18:47, Rob Herring [off-list ref] wrote:
On Thu, Jul 08, 2021 at 02:37:44PM +0300, Dmitry Baryshkov wrote:
quoted
Hi,
On Thu, 8 Jul 2021 at 13:10, Ulf Hansson [off-list ref] wrote:
quoted
- Peter (the email was bouncing)
+ Peter's kernel.org address
quoted
On Tue, 6 Jul 2021 at 13:55, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
quoted
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
quoted
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.
quoted
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
Well, to me it looks very similar to those resouses we could manage
with the mmc pwrseq, for SDIO. It's also typically the same kind of
combo-chips that moved from supporting SDIO to PCIe, for improved
performance I guess. More importantly, the same constraint to
pre-power on the device is needed to allow it to be discovered/probed.
In our case we'd definitely use pwrseq for PCIe bus and we can also
benefit from using pwrseq for serdev and for platform busses also (for
the same story of WiFi+BT chips).
I can take a look at rewriting pwrseq code to also handle the PCIe
bus. Rewriting it to be a generic lib seems like an easy task,
plugging it into PCIe code would be more fun.
Platform and serdev... Definitely even more fun.
I don't want to see pwrseq (the binding) expanded to other buses. If
that was the answer, we wouldn't be having this discussion. It was a
mistake for MMC IMO.
Let's make sure we get your point correctly. I think we have discussed
this in the past, but let's refresh our memories.
If I recall correctly, you are against the mmc pwrseq DT bindings
because we are using a separate pwrseq OF node, that we point to via a
"mmc-pwrseq" property that contains a phandle from the mmc controller
device node. Is that correct?
If we would have encoded the power sequence specific properties, from
within a child node for the mmc controller node, that would have been
okay for you, right?
If pwrseq works as a kernel library/api, then I have no issue with that.
That's what Peter Chen was trying to do. A generic interface, flexible
enough so it can be used for many similar configurations (but not
exactly the same).
Perhaps it was too generic though.
quoted
quoted
Therefore, I think it would be worth having a common solution for
this, rather than a solution per subsystem or even worse, per device.
Power sequencing requirements are inheritently per device unless we're
talking about standard connectors.
The requirements are certainly per device, but the way to manage them
doesn't have to be.
As you said above, a generic library that subsystems/drivers can call
to power on/off a discoverable device, before trying to probe it would
be a good start.
This is a solved problem on MDIO. It's quite simple. If there's a DT
node for a device you haven't discovered, then probe it anyways.
A child OF node?
Then what do you think about some common power sequence properties
that we can use in such node?
On Wed, 14 Jul 2021 at 18:47, Rob Herring [off-list ref] wrote:
quoted
On Thu, Jul 08, 2021 at 02:37:44PM +0300, Dmitry Baryshkov wrote:
quoted
Hi,
On Thu, 8 Jul 2021 at 13:10, Ulf Hansson [off-list ref] wrote:
quoted
- Peter (the email was bouncing)
+ Peter's kernel.org address
quoted
On Tue, 6 Jul 2021 at 13:55, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
quoted
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
quoted
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.
quoted
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
Well, to me it looks very similar to those resouses we could manage
with the mmc pwrseq, for SDIO. It's also typically the same kind of
combo-chips that moved from supporting SDIO to PCIe, for improved
performance I guess. More importantly, the same constraint to
pre-power on the device is needed to allow it to be discovered/probed.
In our case we'd definitely use pwrseq for PCIe bus and we can also
benefit from using pwrseq for serdev and for platform busses also (for
the same story of WiFi+BT chips).
I can take a look at rewriting pwrseq code to also handle the PCIe
bus. Rewriting it to be a generic lib seems like an easy task,
plugging it into PCIe code would be more fun.
Platform and serdev... Definitely even more fun.
I don't want to see pwrseq (the binding) expanded to other buses. If
that was the answer, we wouldn't be having this discussion. It was a
mistake for MMC IMO.
Let's make sure we get your point correctly. I think we have discussed
this in the past, but let's refresh our memories.
If I recall correctly, you are against the mmc pwrseq DT bindings
because we are using a separate pwrseq OF node, that we point to via a
"mmc-pwrseq" property that contains a phandle from the mmc controller
device node. Is that correct?
If we would have encoded the power sequence specific properties, from
within a child node for the mmc controller node, that would have been
okay for you, right?
In Dmitry's case, we have an external chip with that needs to be powered
on per a specific sequence, at which point the WiFi driver on PCIe and
BT driver on serdev will be able to communicate with the device.
The extended case of this is where we have an SDX55 modem soldered onto
the pcb next to the SoC, in which case the power sequencing is even more
complex and additionally there are incoming gpios used to detect things
such as the firmware of the modem has crashed and Linux needs to toggle
power and rescan the PCIe bus.
In both of these cases it seems quite reasonable to represent that
external chip (and it's power needs) as a separate DT node. But we need
a way to link the functional devices to that thing.
Regards,
Bjorn
quoted
If pwrseq works as a kernel library/api, then I have no issue with that.
That's what Peter Chen was trying to do. A generic interface, flexible
enough so it can be used for many similar configurations (but not
exactly the same).
Perhaps it was too generic though.
quoted
quoted
quoted
Therefore, I think it would be worth having a common solution for
this, rather than a solution per subsystem or even worse, per device.
Power sequencing requirements are inheritently per device unless we're
talking about standard connectors.
The requirements are certainly per device, but the way to manage them
doesn't have to be.
As you said above, a generic library that subsystems/drivers can call
to power on/off a discoverable device, before trying to probe it would
be a good start.
quoted
This is a solved problem on MDIO. It's quite simple. If there's a DT
node for a device you haven't discovered, then probe it anyways.
A child OF node?
Then what do you think about some common power sequence properties
that we can use in such node?
On Tue, 10 Aug 2021 at 18:03, Bjorn Andersson
[off-list ref] wrote:
On Tue 10 Aug 06:55 CDT 2021, Ulf Hansson wrote:
quoted
On Wed, 14 Jul 2021 at 18:47, Rob Herring [off-list ref] wrote:
quoted
On Thu, Jul 08, 2021 at 02:37:44PM +0300, Dmitry Baryshkov wrote:
quoted
Hi,
On Thu, 8 Jul 2021 at 13:10, Ulf Hansson [off-list ref] wrote:
quoted
- Peter (the email was bouncing)
+ Peter's kernel.org address
quoted
On Tue, 6 Jul 2021 at 13:55, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
quoted
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
quoted
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.
quoted
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
Well, to me it looks very similar to those resouses we could manage
with the mmc pwrseq, for SDIO. It's also typically the same kind of
combo-chips that moved from supporting SDIO to PCIe, for improved
performance I guess. More importantly, the same constraint to
pre-power on the device is needed to allow it to be discovered/probed.
In our case we'd definitely use pwrseq for PCIe bus and we can also
benefit from using pwrseq for serdev and for platform busses also (for
the same story of WiFi+BT chips).
I can take a look at rewriting pwrseq code to also handle the PCIe
bus. Rewriting it to be a generic lib seems like an easy task,
plugging it into PCIe code would be more fun.
Platform and serdev... Definitely even more fun.
I don't want to see pwrseq (the binding) expanded to other buses. If
that was the answer, we wouldn't be having this discussion. It was a
mistake for MMC IMO.
Let's make sure we get your point correctly. I think we have discussed
this in the past, but let's refresh our memories.
If I recall correctly, you are against the mmc pwrseq DT bindings
because we are using a separate pwrseq OF node, that we point to via a
"mmc-pwrseq" property that contains a phandle from the mmc controller
device node. Is that correct?
If we would have encoded the power sequence specific properties, from
within a child node for the mmc controller node, that would have been
okay for you, right?
In Dmitry's case, we have an external chip with that needs to be powered
on per a specific sequence, at which point the WiFi driver on PCIe and
BT driver on serdev will be able to communicate with the device.
Thanks for sharing more details.
So, not only do we have a discoverable device that needs to be powered
on in a device specific way before probing, but in fact we have two
consumers of that "combo chip", one (PCIe) for Wifi and one (serdev)
for Bluetooth.
The extended case of this is where we have an SDX55 modem soldered onto
the pcb next to the SoC, in which case the power sequencing is even more
complex and additionally there are incoming gpios used to detect things
such as the firmware of the modem has crashed and Linux needs to toggle
power and rescan the PCIe bus.
That sounds very similar to what we manage for the SDIO bus already.
We have a mmc pwrseq node to describe what resources that are needed
to power on/off the external chip. The driver for the functional
device (Wifi chip for example) may then call SDIO APIs provided by the
mmc core to power on/off the device, in case some kind of reset would
be needed.
Additionally, we have a child node below the mmc controller node,
allowing us to describe device specific things for the SDIO functional
device, like an out-of-band IRQ line for example.
Overall, this seems to work fine, even if the DT bindings may be questionable.
In both of these cases it seems quite reasonable to represent that
external chip (and it's power needs) as a separate DT node. But we need
a way to link the functional devices to that thing.
Don't get me wrong, I am not suggesting we should re-use the
mmc-pwrseq DT bindings - but just trying to share our experience
around them.
In the cases you describe, it certainly sounds like we need some kind
of minimal description in DT for these functional external devices.
For GPIO pins, for example.
How to describe this in DT is one thing, let's see if Rob can help to
point us in some direction of what could make sense.
When it comes to implementing a library/interface to manage these
functional devices, I guess we just have to continue to explore
various options. Perhaps just start simple with another subsystem,
like PCIe and see where this brings us.
On Tue, 10 Aug 2021 at 18:03, Bjorn Andersson
[off-list ref] wrote:
quoted
On Tue 10 Aug 06:55 CDT 2021, Ulf Hansson wrote:
quoted
On Wed, 14 Jul 2021 at 18:47, Rob Herring [off-list ref] wrote:
quoted
On Thu, Jul 08, 2021 at 02:37:44PM +0300, Dmitry Baryshkov wrote:
quoted
Hi,
On Thu, 8 Jul 2021 at 13:10, Ulf Hansson [off-list ref] wrote:
quoted
- Peter (the email was bouncing)
+ Peter's kernel.org address
quoted
On Tue, 6 Jul 2021 at 13:55, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
quoted
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
quoted
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.
quoted
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
Well, to me it looks very similar to those resouses we could manage
with the mmc pwrseq, for SDIO. It's also typically the same kind of
combo-chips that moved from supporting SDIO to PCIe, for improved
performance I guess. More importantly, the same constraint to
pre-power on the device is needed to allow it to be discovered/probed.
In our case we'd definitely use pwrseq for PCIe bus and we can also
benefit from using pwrseq for serdev and for platform busses also (for
the same story of WiFi+BT chips).
I can take a look at rewriting pwrseq code to also handle the PCIe
bus. Rewriting it to be a generic lib seems like an easy task,
plugging it into PCIe code would be more fun.
Platform and serdev... Definitely even more fun.
I don't want to see pwrseq (the binding) expanded to other buses. If
that was the answer, we wouldn't be having this discussion. It was a
mistake for MMC IMO.
Let's make sure we get your point correctly. I think we have discussed
this in the past, but let's refresh our memories.
If I recall correctly, you are against the mmc pwrseq DT bindings
because we are using a separate pwrseq OF node, that we point to via a
"mmc-pwrseq" property that contains a phandle from the mmc controller
device node. Is that correct?
If we would have encoded the power sequence specific properties, from
within a child node for the mmc controller node, that would have been
okay for you, right?
In Dmitry's case, we have an external chip with that needs to be powered
on per a specific sequence, at which point the WiFi driver on PCIe and
BT driver on serdev will be able to communicate with the device.
Thanks for sharing more details.
So, not only do we have a discoverable device that needs to be powered
on in a device specific way before probing, but in fact we have two
consumers of that "combo chip", one (PCIe) for Wifi and one (serdev)
for Bluetooth.
quoted
The extended case of this is where we have an SDX55 modem soldered onto
the pcb next to the SoC, in which case the power sequencing is even more
complex and additionally there are incoming gpios used to detect things
such as the firmware of the modem has crashed and Linux needs to toggle
power and rescan the PCIe bus.
That sounds very similar to what we manage for the SDIO bus already.
We have a mmc pwrseq node to describe what resources that are needed
to power on/off the external chip. The driver for the functional
device (Wifi chip for example) may then call SDIO APIs provided by the
mmc core to power on/off the device, in case some kind of reset would
be needed.
Additionally, we have a child node below the mmc controller node,
allowing us to describe device specific things for the SDIO functional
device, like an out-of-band IRQ line for example.
Overall, this seems to work fine, even if the DT bindings may be questionable.
quoted
In both of these cases it seems quite reasonable to represent that
external chip (and it's power needs) as a separate DT node. But we need
a way to link the functional devices to that thing.
Don't get me wrong, I am not suggesting we should re-use the
mmc-pwrseq DT bindings - but just trying to share our experience
around them.
In the cases you describe, it certainly sounds like we need some kind
of minimal description in DT for these functional external devices.
For GPIO pins, for example.
How to describe this in DT is one thing, let's see if Rob can help to
point us in some direction of what could make sense.
When it comes to implementing a library/interface to manage these
functional devices, I guess we just have to continue to explore
various options. Perhaps just start simple with another subsystem,
like PCIe and see where this brings us.
Thank you for your opinion and suggestions. In fact I'm probably going
to start working on non-discoverable busses first (by chaning support
for few other BT+WiFi Qualcomm chips), later shifting the attention to
the PCIe part. While this may seem like a longer path, I'd like to
narrow pwrseq subsystem first, before going into PCIe details.
--
With best wishes
Dmitry
- Peter (the email was bouncing)
On Tue, 6 Jul 2021 at 13:55, Mark Brown [off-list ref] wrote:
quoted
On Tue, Jul 06, 2021 at 09:54:03AM +0200, Ulf Hansson wrote:
quoted
On Tue, 22 Jun 2021 at 00:32, Dmitry Baryshkov
quoted
quoted
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.
quoted
Power sequencing of discoverable buses have been discussed several
times before at LKML. The last attempt [1] I am aware of, was in 2017
from Peter Chen. I don't think there is a common solution, yet.
This feels a bit different to the power sequencing problem - it's not
exposing the individual inputs to the device but rather is a block that
manages everything but needs a bit of a kick to get things going (I'd
guess that with ACPI it'd be triggered via AML). It's in the same space
but it's not quite the same issue I think, something that can handle
control of the individual resources might still struggle with this.
Well, to me it looks very similar to those resouses we could manage
with the mmc pwrseq, for SDIO. It's also typically the same kind of
combo-chips that moved from supporting SDIO to PCIe, for improved
performance I guess. More importantly, the same constraint to
pre-power on the device is needed to allow it to be discovered/probed.
Therefore, I think it would be worth having a common solution for
this, rather than a solution per subsystem or even worse, per device.
Representing the chip and its power needs, separate from the busses does
seem reasonable. It's pretty much what Dmitry suggested originally, but
his attempts to use either power-domain or regulator references to
ensure ordering has been objected.
Beyond this, there is a similar case (that you and I have talked about
earlier) in supporting the SDX55 PCIe modem found in some devices.
Where in addition to ensuring that the power rails are configured, a
couple of gpios needs to be controlled and there's an incoming gpio line
indicating that the firmware of the device has locked up and the power
needs to be toggled and the device re-enumerated.
Unfortunately, it looks like Peter's email is bouncing so we can't get
an update from him.
And for this second part, where we need some additional logic it seems
to go beyond what the power sequence discussions has touched upon so
far.
Regards,
Bjorn
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 binding to describe power
sequencing required to power up this device.
Signed-off-by: Dmitry Baryshkov <redacted>
---
.../bindings/regulator/qcom,qca6390.yaml | 70 +++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390.yaml
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=504629
---Test result---
Test Summary:
CheckPatch FAIL 1.51 seconds
GitLint PASS 0.87 seconds
BuildKernel PASS 618.69 seconds
TestRunner: Setup PASS 403.43 seconds
TestRunner: l2cap-tester PASS 2.93 seconds
TestRunner: bnep-tester PASS 2.06 seconds
TestRunner: mgmt-tester PASS 29.40 seconds
TestRunner: rfcomm-tester PASS 2.31 seconds
TestRunner: sco-tester PASS 2.22 seconds
TestRunner: smp-tester PASS 2.30 seconds
TestRunner: userchan-tester PASS 2.09 seconds
Details
##############################
Test: CheckPatch - FAIL - 1.51 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
dt-bindings: regulator: qcom,qca6390: add binding for QCA6390 device
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#15:
new file mode 100644
total: 0 errors, 1 warnings, 70 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] dt-bindings: regulator: qcom,qca6390: add binding for QCA6390" has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
regulator: qca6390: add support for QCA639x powerup sequence
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#50:
new file mode 100644
total: 0 errors, 1 warnings, 183 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] regulator: qca6390: add support for QCA639x powerup sequence" has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - PASS - 0.87 seconds
Run gitlint with rule in .gitlint
##############################
Test: BuildKernel - PASS - 618.69 seconds
Build Kernel with minimal configuration supports Bluetooth
##############################
Test: TestRunner: Setup - PASS - 403.43 seconds
Setup environment for running Test Runner
##############################
Test: TestRunner: l2cap-tester - PASS - 2.93 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: bnep-tester - PASS - 2.06 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: mgmt-tester - PASS - 29.40 seconds
Run test-runner with mgmt-tester
Total: 446, Passed: 433 (97.1%), Failed: 0, Not Run: 13
##############################
Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: sco-tester - PASS - 2.22 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: smp-tester - PASS - 2.30 seconds
Run test-runner with smp-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: userchan-tester - PASS - 2.09 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
In order to simplify probe function provide default device data. This
removes the rest of if (data) checks.
Signed-off-by: Dmitry Baryshkov <redacted>
---
drivers/bluetooth/hci_qca.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
There is no need to allocate separate structure for handling regulators
used by QCA chips, we gain nothing from it. Move all used data fields
directly to struct qca_serdev.
Signed-off-by: Dmitry Baryshkov <redacted>
---
drivers/bluetooth/hci_qca.c | 58 ++++++++++++++-----------------------
1 file changed, 22 insertions(+), 36 deletions(-)
On Mon 21 Jun 17:31 CDT 2021, Dmitry Baryshkov wrote:
There is no need to allocate separate structure for handling regulators
used by QCA chips, we gain nothing from it. Move all used data fields
directly to struct qca_serdev.
In preparation to add power sequencer support to qca6390, merge wcnxxxx
and non-wcn codepaths.
Signed-off-by: Dmitry Baryshkov <redacted>
---
drivers/bluetooth/hci_qca.c | 152 +++++++++++++++++-------------------
1 file changed, 71 insertions(+), 81 deletions(-)
@@ -254,6 +257,17 @@ static const char *qca_get_firmware_name(struct hci_uart *hu)}}+staticconstchar*qca_soc_name(structhci_uart*hu)+{+if(hu->serdev){+structqca_serdev*qsd=serdev_device_get_drvdata(hu->serdev);++returnqsd->name;+}else{+return"ROME";+}+}+staticvoid__serial_clock_on(structtty_struct*tty){/* TODO: Some chipset requires to enable UART clock on client
@@ -1623,14 +1637,16 @@ static int qca_regulator_init(struct hci_uart *hu)gpiod_set_value_cansleep(qcadev->bt_en,0);msleep(50);gpiod_set_value_cansleep(qcadev->bt_en,1);-msleep(50);+msleep(150);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);
@@ -1650,7 +1666,9 @@ static int qca_regulator_init(struct hci_uart *hu)returnret;}-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);return0;}
@@ -1658,8 +1676,6 @@ static int qca_regulator_init(struct hci_uart *hu)staticintqca_power_on(structhci_dev*hdev){structhci_uart*hu=hci_get_drvdata(hdev);-enumqca_btsoc_typesoc_type=qca_soc_type(hu);-structqca_serdev*qcadev;structqca_data*qca=hu->priv;intret=0;
@@ -1669,17 +1685,7 @@ static int qca_power_on(struct hci_dev *hdev)if(!hu->serdev)return0;-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);returnret;
@@ -1709,9 +1715,7 @@ static int qca_setup(struct hci_uart *hu)*/set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY,&hdev->quirks);-bt_dev_info(hdev,"setting up %s",-qca_is_wcn399x(soc_type)?"wcn399x":-(soc_type==QCA_WCN6750)?"wcn6750":"ROME/QCA6390");+bt_dev_info(hdev,"setting up %s",qca_soc_name(hu));qca->memdump_state=QCA_MEMDUMP_IDLE;
@@ -2034,71 +2044,51 @@ static int qca_serdev_probe(struct serdev_device *serdev)if(!qcadev->oper_speed)BT_DBG("UART will pick default operating speed");-if((qca_is_wcn399x(data->soc_type)||-qca_is_wcn6750(data->soc_type))){-qcadev->btsoc_type=data->soc_type;--err=qca_init_regulators(&serdev->dev,qcadev,data->vregs,-data->num_vregs);-if(err){-BT_ERR("Failed to init regulators:%d",err);-returnerr;-}+qcadev->name=data->name;+qcadev->btsoc_type=data->soc_type;-qcadev->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;-}+err=qca_init_regulators(&serdev->dev,qcadev,data->vregs,+data->num_vregs);+if(err){+BT_ERR("Failed to init regulators:%d",err);+returnerr;+}-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->vregs_on=false;-qcadev->susclk=devm_clk_get_optional(&serdev->dev,NULL);-if(IS_ERR(qcadev->susclk)){-dev_err(&serdev->dev,"failed to acquire clk\n");-returnPTR_ERR(qcadev->susclk);-}+qcadev->bt_en=devm_gpiod_get_optional(&serdev->dev,"enable",+GPIOD_OUT_LOW);+if(!qcadev->bt_en&&(data->capabilities&QCA_CAP_NEEDS_BT_ENABLE)){+dev_err(&serdev->dev,"failed to acquire BT_EN gpio\n");+power_ctrl_enabled=false;+}-err=hci_uart_register_device(&qcadev->serdev_hu,&qca_proto);-if(err){-BT_ERR("wcn3990 serdev registration failed");-returnerr;-}-}else{-qcadev->btsoc_type=data->soc_type;+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->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->susclk=devm_clk_get_optional(&serdev->dev,NULL);+if(IS_ERR(qcadev->susclk)){+dev_err(&serdev->dev,"failed to acquire clk\n");+returnPTR_ERR(qcadev->susclk);+}-qcadev->susclk=devm_clk_get_optional(&serdev->dev,NULL);-if(IS_ERR(qcadev->susclk)){-dev_warn(&serdev->dev,"failed to acquire clk\n");-returnPTR_ERR(qcadev->susclk);-}-err=clk_set_rate(qcadev->susclk,SUSCLK_RATE_32KHZ);-if(err)-returnerr;+err=clk_set_rate(qcadev->susclk,SUSCLK_RATE_32KHZ);+if(err)+returnerr;+if(!qca_is_wcn399x(qcadev->btsoc_type)&&+!qca_is_wcn6750(qcadev->btsoc_type)){err=clk_prepare_enable(qcadev->susclk);if(err)returnerr;+}-err=hci_uart_register_device(&qcadev->serdev_hu,&qca_proto);-if(err){-BT_ERR("Rome serdev registration failed");-clk_disable_unprepare(qcadev->susclk);-returnerr;-}+err=hci_uart_register_device(&qcadev->serdev_hu,&qca_proto);+if(err){+BT_ERR("%s serdev registration failed",qcadev->name);+returnerr;}hdev=qcadev->serdev_hu.hdev;
QCA6390 uses on-chip power sequencer (and power management unit) to
supply power to both WiFi and BT parts. Since this sequencer is
supported by a separate driver, use it as a single "vin" regulator.
Signed-off-by: Dmitry Baryshkov <redacted>
---
drivers/bluetooth/hci_qca.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
Describe the onboard WiFi+BT SoC QCA6391. It is connected to uart6 to
provide Bluetooth functionality (QCA) and to PCIe to provide WiFi
(ath11k). A separate power sequencer device node is used to describe
on-SoC power management unit common to both WiFi and BT parts.
Signed-off-by: Dmitry Baryshkov <redacted>
---
arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 56 ++++++++++++++++++++++++
1 file changed, 56 insertions(+)