* [PATCH v3 0/2] Enable Bluetooth functionality for wcn3998.
@ 2019-03-12 12:22 Harish Bandi
2019-03-12 12:22 ` [PATCH v3 1/2] Bluetooth: hci_qca: Added support " Harish Bandi
2019-03-12 12:22 ` [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 Harish Bandi
0 siblings, 2 replies; 12+ messages in thread
From: Harish Bandi @ 2019-03-12 12:22 UTC (permalink / raw)
To: marcel, johan.hedberg
Cc: mka, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm,
bgodavar, anubhavg, Harish Bandi
This patch series we add support for wcn3998 BT chip set. This new chipset
is based from the wcn3990 with minimal power numbers. So here in this patch
The major difference between wcn3990 and wcn3998 is only power numbers. where
as init process and fw download is same with wcn3990.So we add new compatible
for wcn3998.
Removed the Reading of regulator values from dts.
Added new compatible for wcn3998.
Changed in V3:
updated to latest code base.
Harish Bandi (2):
Bluetooth: hci_qca: Added support for wcn3998
dt-bindings: net: bluetooth: Add device tree bindings for QTI chip
wcn3998
.../devicetree/bindings/net/qualcomm-bluetooth.txt | 15 ++++++++
drivers/bluetooth/btqca.c | 4 +--
drivers/bluetooth/btqca.h | 3 +-
drivers/bluetooth/hci_qca.c | 40 ++++++++++++++--------
4 files changed, 45 insertions(+), 17 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v3 1/2] Bluetooth: hci_qca: Added support for wcn3998 2019-03-12 12:22 [PATCH v3 0/2] Enable Bluetooth functionality for wcn3998 Harish Bandi @ 2019-03-12 12:22 ` Harish Bandi 2019-03-12 16:29 ` Matthias Kaehlcke 2019-03-12 12:22 ` [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 Harish Bandi 1 sibling, 1 reply; 12+ messages in thread From: Harish Bandi @ 2019-03-12 12:22 UTC (permalink / raw) To: marcel, johan.hedberg Cc: mka, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg, Harish Bandi Added new compatible for wcn3998 and corresponding voltage and current values to wcn3998 compatible. Changed driver code to support wcn3998 Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> --- changes in v3: - updated to latest code base. --- drivers/bluetooth/btqca.c | 4 ++-- drivers/bluetooth/btqca.h | 3 ++- drivers/bluetooth/hci_qca.c | 40 ++++++++++++++++++++++++++-------------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c index 6122685..70cab13 100644 --- a/drivers/bluetooth/btqca.c +++ b/drivers/bluetooth/btqca.c @@ -344,7 +344,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, /* Download rampatch file */ config.type = TLV_TYPE_PATCH; - if (soc_type == QCA_WCN3990) { + if (soc_type >= QCA_WCN3990) { /* Firmware files to download are based on ROM version. * ROM version is derived from last two bytes of soc_ver. */ @@ -365,7 +365,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, /* Download NVM configuration */ config.type = TLV_TYPE_NVM; - if (soc_type == QCA_WCN3990) + if (soc_type >= QCA_WCN3990) snprintf(config.fwname, sizeof(config.fwname), "qca/crnv%02x.bin", rom_ver); else diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h index c72c56e..f03d96e 100644 --- a/drivers/bluetooth/btqca.h +++ b/drivers/bluetooth/btqca.h @@ -132,7 +132,8 @@ enum qca_btsoc_type { QCA_INVALID = -1, QCA_AR3002, QCA_ROME, - QCA_WCN3990 + QCA_WCN3990, + QCA_WCN3998 }; #if IS_ENABLED(CONFIG_BT_QCA) diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index 4ea995d..1221f5a 100644 --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -521,7 +521,7 @@ static int qca_open(struct hci_uart *hu) if (hu->serdev) { qcadev = serdev_device_get_drvdata(hu->serdev); - if (qcadev->btsoc_type != QCA_WCN3990) { + if (qcadev->btsoc_type == QCA_ROME) { gpiod_set_value_cansleep(qcadev->bt_en, 1); } else { hu->init_speed = qcadev->init_speed; @@ -627,7 +627,7 @@ static int qca_close(struct hci_uart *hu) if (hu->serdev) { qcadev = serdev_device_get_drvdata(hu->serdev); - if (qcadev->btsoc_type == QCA_WCN3990) + if (qcadev->btsoc_type >= QCA_WCN3990) qca_power_shutdown(hu); else gpiod_set_value_cansleep(qcadev->bt_en, 0); @@ -1008,7 +1008,7 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate) msecs_to_jiffies(CMD_TRANS_TIMEOUT_MS)); /* Give the controller time to process the request */ - if (qca_soc_type(hu) == QCA_WCN3990) + if (qca_soc_type(hu) >= QCA_WCN3990) msleep(10); else msleep(300); @@ -1084,7 +1084,7 @@ static unsigned int qca_get_speed(struct hci_uart *hu, static int qca_check_speeds(struct hci_uart *hu) { - if (qca_soc_type(hu) == QCA_WCN3990) { + if (qca_soc_type(hu) >= QCA_WCN3990) { if (!qca_get_speed(hu, QCA_INIT_SPEED) && !qca_get_speed(hu, QCA_OPER_SPEED)) return -EINVAL; @@ -1116,7 +1116,7 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type) /* Disable flow control for wcn3990 to deassert RTS while * changing the baudrate of chip and host. */ - if (soc_type == QCA_WCN3990) + if (soc_type >= QCA_WCN3990) hci_uart_set_flow_control(hu, true); qca_baudrate = qca_get_baudrate_value(speed); @@ -1128,7 +1128,7 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type) host_set_baudrate(hu, speed); error: - if (soc_type == QCA_WCN3990) + if (soc_type >= QCA_WCN3990) hci_uart_set_flow_control(hu, false); } @@ -1201,7 +1201,7 @@ static int qca_setup(struct hci_uart *hu) /* Patch downloading has to be done without IBS mode */ clear_bit(STATE_IN_BAND_SLEEP_ENABLED, &qca->flags); - if (soc_type == QCA_WCN3990) { + if (soc_type >= QCA_WCN3990) { bt_dev_info(hdev, "setting up wcn3990"); /* Enable NON_PERSISTENT_SETUP QUIRK to ensure to execute @@ -1232,7 +1232,7 @@ static int qca_setup(struct hci_uart *hu) qca_baudrate = qca_get_baudrate_value(speed); } - if (soc_type != QCA_WCN3990) { + if (soc_type == QCA_ROME) { /* Get QCA version information */ ret = qca_read_soc_version(hdev, &soc_ver); if (ret) @@ -1257,7 +1257,7 @@ static int qca_setup(struct hci_uart *hu) } /* Setup bdaddr */ - if (soc_type == QCA_WCN3990) + if (soc_type >= QCA_WCN3990) hu->hdev->set_bdaddr = qca_set_bdaddr; else hu->hdev->set_bdaddr = qca_set_bdaddr_rome; @@ -1280,7 +1280,7 @@ static int qca_setup(struct hci_uart *hu) .dequeue = qca_dequeue, }; -static const struct qca_vreg_data qca_soc_data = { +static const struct qca_vreg_data qca_soc_data_wcn3990 = { .soc_type = QCA_WCN3990, .vregs = (struct qca_vreg []) { { "vddio", 1800000, 1900000, 15000 }, @@ -1291,6 +1291,17 @@ static int qca_setup(struct hci_uart *hu) .num_vregs = 4, }; +static const struct qca_vreg_data qca_soc_data_wcn3998 = { + .soc_type = QCA_WCN3998, + .vregs = (struct qca_vreg []) { + { "vddio", 1800000, 1900000, 10000 }, + { "vddxo", 1800000, 1900000, 80000 }, + { "vddrf", 1300000, 1352000, 300000 }, + { "vddch0", 3300000, 3300000, 450000 }, + }, + .num_vregs = 4, +}; + static void qca_power_shutdown(struct hci_uart *hu) { struct qca_data *qca = hu->priv; @@ -1424,8 +1435,8 @@ static int qca_serdev_probe(struct serdev_device *serdev) qcadev->serdev_hu.serdev = serdev; data = of_device_get_match_data(&serdev->dev); serdev_device_set_drvdata(serdev, qcadev); - if (data && data->soc_type == QCA_WCN3990) { - qcadev->btsoc_type = QCA_WCN3990; + if (data && data->soc_type >= QCA_WCN3990) { + qcadev->btsoc_type = data->soc_type; qcadev->bt_power = devm_kzalloc(&serdev->dev, sizeof(struct qca_power), GFP_KERNEL); @@ -1489,7 +1500,7 @@ static void qca_serdev_remove(struct serdev_device *serdev) { struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev); - if (qcadev->btsoc_type == QCA_WCN3990) + if (qcadev->btsoc_type >= QCA_WCN3990) qca_power_shutdown(&qcadev->serdev_hu); else clk_disable_unprepare(qcadev->susclk); @@ -1499,7 +1510,8 @@ static void qca_serdev_remove(struct serdev_device *serdev) static const struct of_device_id qca_bluetooth_of_match[] = { { .compatible = "qcom,qca6174-bt" }, - { .compatible = "qcom,wcn3990-bt", .data = &qca_soc_data}, + { .compatible = "qcom,wcn3990-bt", .data = &qca_soc_data_wcn3990}, + { .compatible = "qcom,wcn3998-bt", .data = &qca_soc_data_wcn3998}, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] Bluetooth: hci_qca: Added support for wcn3998 2019-03-12 12:22 ` [PATCH v3 1/2] Bluetooth: hci_qca: Added support " Harish Bandi @ 2019-03-12 16:29 ` Matthias Kaehlcke 2019-03-13 6:22 ` c-hbandi 0 siblings, 1 reply; 12+ messages in thread From: Matthias Kaehlcke @ 2019-03-12 16:29 UTC (permalink / raw) To: Harish Bandi Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg Hi Harish, On Tue, Mar 12, 2019 at 05:52:58PM +0530, Harish Bandi wrote: > Added new compatible for wcn3998 and corresponding voltage > and current values to wcn3998 compatible. > Changed driver code to support wcn3998 > > Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> > --- > changes in v3: > - updated to latest code base. This is not useful, for future versions please describe what changed (e.g. 'specify regulator constraints in the driver instead of the DT') > --- > drivers/bluetooth/btqca.c | 4 ++-- > drivers/bluetooth/btqca.h | 3 ++- > drivers/bluetooth/hci_qca.c | 40 ++++++++++++++++++++++++++-------------- > 3 files changed, 30 insertions(+), 17 deletions(-) > > diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c > index 6122685..70cab13 100644 > --- a/drivers/bluetooth/btqca.c > +++ b/drivers/bluetooth/btqca.c > @@ -344,7 +344,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > > /* Download rampatch file */ > config.type = TLV_TYPE_PATCH; > - if (soc_type == QCA_WCN3990) { > + if (soc_type >= QCA_WCN3990) { That works, but isn't super-clear and might need to be adapted when future non-WCN399x controllers are added. Some possible alternatives: - is_wcn399x(soc_type) - have a family (Rome, Cherokee (IIRC this name was used for WCN3990)) and a chip id (QCA6174, WCN3990, WCN3998, ...) > /* Firmware files to download are based on ROM version. > * ROM version is derived from last two bytes of soc_ver. > */ > @@ -365,7 +365,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate, > > /* Download NVM configuration */ > config.type = TLV_TYPE_NVM; > - if (soc_type == QCA_WCN3990) > + if (soc_type >= QCA_WCN3990) > snprintf(config.fwname, sizeof(config.fwname), > "qca/crnv%02x.bin", rom_ver); > else > diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h > index c72c56e..f03d96e 100644 > --- a/drivers/bluetooth/btqca.h > +++ b/drivers/bluetooth/btqca.h > @@ -132,7 +132,8 @@ enum qca_btsoc_type { > QCA_INVALID = -1, > QCA_AR3002, > QCA_ROME, > - QCA_WCN3990 > + QCA_WCN3990, > + QCA_WCN3998 nit: if you add a comma after the last value the line doesn't need to be changed when a new type is added in the future. Is 'WCN3998' specific enough? You mentioned earlier that there are multiple WCN3998 variants with different requirements for regulator voltages/max currents. Which names does Qualcomm use to distinguish between them (e.g. WCN3998-A, WCN3998-B, ...)? Thanks Matthias ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] Bluetooth: hci_qca: Added support for wcn3998 2019-03-12 16:29 ` Matthias Kaehlcke @ 2019-03-13 6:22 ` c-hbandi 0 siblings, 0 replies; 12+ messages in thread From: c-hbandi @ 2019-03-13 6:22 UTC (permalink / raw) To: Matthias Kaehlcke Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg Hi Matthias, On 2019-03-12 21:59, Matthias Kaehlcke wrote: > Hi Harish, > > On Tue, Mar 12, 2019 at 05:52:58PM +0530, Harish Bandi wrote: >> Added new compatible for wcn3998 and corresponding voltage >> and current values to wcn3998 compatible. >> Changed driver code to support wcn3998 >> >> Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> >> --- >> changes in v3: >> - updated to latest code base. > > This is not useful, for future versions please describe what changed > (e.g. 'specify regulator constraints in the driver instead of the DT') > [Harish] -- added details in v2, and v3 uploaded just to rebase on tip of bluetooth-next for better understanding of code in review. From new patch onwards will add all patch version changes and add proper description. >> --- >> drivers/bluetooth/btqca.c | 4 ++-- >> drivers/bluetooth/btqca.h | 3 ++- >> drivers/bluetooth/hci_qca.c | 40 >> ++++++++++++++++++++++++++-------------- >> 3 files changed, 30 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c >> index 6122685..70cab13 100644 >> --- a/drivers/bluetooth/btqca.c >> +++ b/drivers/bluetooth/btqca.c >> @@ -344,7 +344,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t >> baudrate, >> >> /* Download rampatch file */ >> config.type = TLV_TYPE_PATCH; >> - if (soc_type == QCA_WCN3990) { >> + if (soc_type >= QCA_WCN3990) { > > That works, but isn't super-clear and might need to be adapted when > future non-WCN399x controllers are added. > > Some possible alternatives: > > - is_wcn399x(soc_type) > - have a family (Rome, Cherokee (IIRC this name was used for WCN3990)) > and a chip id (QCA6174, WCN3990, WCN3998, ...) > [Harish] -- Will change like is_wcn399x(soc_type) and come up with new patch >> /* Firmware files to download are based on ROM version. >> * ROM version is derived from last two bytes of soc_ver. >> */ >> @@ -365,7 +365,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t >> baudrate, >> >> /* Download NVM configuration */ >> config.type = TLV_TYPE_NVM; >> - if (soc_type == QCA_WCN3990) >> + if (soc_type >= QCA_WCN3990) >> snprintf(config.fwname, sizeof(config.fwname), >> "qca/crnv%02x.bin", rom_ver); >> else >> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h >> index c72c56e..f03d96e 100644 >> --- a/drivers/bluetooth/btqca.h >> +++ b/drivers/bluetooth/btqca.h >> @@ -132,7 +132,8 @@ enum qca_btsoc_type { >> QCA_INVALID = -1, >> QCA_AR3002, >> QCA_ROME, >> - QCA_WCN3990 >> + QCA_WCN3990, >> + QCA_WCN3998 > > nit: if you add a comma after the last value the line doesn't need to > be changed when a new type is added in the future. > [Harish] -- will take care in new patch > Is 'WCN3998' specific enough? You mentioned earlier that there are > multiple WCN3998 variants with different requirements for regulator > voltages/max currents. Which names does Qualcomm use to distinguish > between them (e.g. WCN3998-A, WCN3998-B, ...)? [Harish] -- for now we want to add WCN3998 support only, What i mean to say in my earlier explanation that. WCN3990 is base variant and on top of that we have variants like WCN3990, WCN3998 and WCN3998-0,WCN3998-1 like that.. > > Thanks > > Matthias Thanks, Harish ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 2019-03-12 12:22 [PATCH v3 0/2] Enable Bluetooth functionality for wcn3998 Harish Bandi 2019-03-12 12:22 ` [PATCH v3 1/2] Bluetooth: hci_qca: Added support " Harish Bandi @ 2019-03-12 12:22 ` Harish Bandi 2019-03-12 16:59 ` Matthias Kaehlcke 1 sibling, 1 reply; 12+ messages in thread From: Harish Bandi @ 2019-03-12 12:22 UTC (permalink / raw) To: marcel, johan.hedberg Cc: mka, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg, Harish Bandi This patch enables regulators for the Qualcomm Bluetooth wcn3998 controller. Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> --- changes in v3: - updated to latest code base. --- .../devicetree/bindings/net/qualcomm-bluetooth.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt index 824c0e2..1221535 100644 --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt @@ -53,3 +53,18 @@ serial@898000 { max-speed = <3200000>; }; }; + +&blsp1_uart3 { + pinctrl-names = "default"; + pinctrl-0 = <&blsp1_uart3_default>; + status = "okay"; + + bluetooth: wcn3998-bt { + compatible = "qcom,wcn3998-bt"; + vddio-supply = <&vreg_l6_1p8>; + vddxo-supply = <&vreg_l5_1p8>; + vddrf-supply = <&vreg_s5_1p35>; + vddch0-supply = <&vdd_ch0_3p3>; + max-speed = <3200000>; + }; +}; \ No newline at end of file -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 2019-03-12 12:22 ` [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 Harish Bandi @ 2019-03-12 16:59 ` Matthias Kaehlcke 2019-03-13 6:30 ` c-hbandi 0 siblings, 1 reply; 12+ messages in thread From: Matthias Kaehlcke @ 2019-03-12 16:59 UTC (permalink / raw) To: Harish Bandi Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg, Rob Herring, Mark Rutland, devicetree +DT folks Please add them in future versions (script/scripts/get_maintainer.pl <patches> should have listed them) On Tue, Mar 12, 2019 at 05:52:59PM +0530, Harish Bandi wrote: > This patch enables regulators for the Qualcomm Bluetooth wcn3998 > controller. No, it doesn't. The next version should probably say something like "Add compatible string for the Qualcomm WCN3998 Bluetooth controller. Is there any particular reason why QCA drivers folks use 'wcn' instead of 'WCN'? The QCA documentations calls it WCN399x, so I'd suggest to consistently use the uppercase name in comments and documentation (and log messages?). > Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> > --- > changes in v3: > - updated to latest code base. This comment is useless, please describe what changed wrt the previous version. > --- > .../devicetree/bindings/net/qualcomm-bluetooth.txt | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > index 824c0e2..1221535 100644 > --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > @@ -53,3 +53,18 @@ serial@898000 { > max-speed = <3200000>; > }; > }; > + > +&blsp1_uart3 { > + pinctrl-names = "default"; > + pinctrl-0 = <&blsp1_uart3_default>; > + status = "okay"; > + > + bluetooth: wcn3998-bt { > + compatible = "qcom,wcn3998-bt"; > + vddio-supply = <&vreg_l6_1p8>; > + vddxo-supply = <&vreg_l5_1p8>; > + vddrf-supply = <&vreg_s5_1p35>; > + vddch0-supply = <&vdd_ch0_3p3>; > + max-speed = <3200000>; > + }; > +}; > \ No newline at end of file I think the example isn't really needed since it's essentially the same as the one for 'qcom,wcn3990-bt'. But the important part is missing: add the new compatible string under ´Required properties´. You also want to update the documentation that mentiones 'qcom,wcn3990-bt' to 'qcom,wcn399x-bt' (assuming for now that other possible WCN399x chips would be similar). You mentioned in an earlier version of the series that there are multiple WCN3998 variants with different requirements for voltage/current. This seems to suggests that multiple compatible strings are needed to distinguish between them. Thanks Matthias ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 2019-03-12 16:59 ` Matthias Kaehlcke @ 2019-03-13 6:30 ` c-hbandi 2019-03-14 18:56 ` Matthias Kaehlcke 0 siblings, 1 reply; 12+ messages in thread From: c-hbandi @ 2019-03-13 6:30 UTC (permalink / raw) To: Matthias Kaehlcke Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg, Rob Herring, Mark Rutland, devicetree, linux-bluetooth-owner Hi Matthias, On 2019-03-12 22:29, Matthias Kaehlcke wrote: > +DT folks > > Please add them in future versions (script/scripts/get_maintainer.pl > <patches> should have listed them) [Harish] -- Will add them in new version of patches. > > On Tue, Mar 12, 2019 at 05:52:59PM +0530, Harish Bandi wrote: >> This patch enables regulators for the Qualcomm Bluetooth wcn3998 >> controller. > > No, it doesn't. > > The next version should probably say something like "Add compatible > string for the Qualcomm WCN3998 Bluetooth controller. > [Harish] -- From new patch onwards will add all patch version changes and add proper description. > Is there any particular reason why QCA drivers folks use 'wcn' instead > of 'WCN'? The QCA documentations calls it WCN399x, so I'd suggest to > consistently use the uppercase name in comments and documentation (and > log messages?). > [Harish] -- I think in DT we need to have small case like wcn, i think that is the reason it started using in code, comments and dt documentation. >> Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> >> --- >> changes in v3: >> - updated to latest code base. > > This comment is useless, please describe what changed wrt the previous > version. [Harish] -- added details in v2, and v3 uploaded just to rebase on tip of bluetooth-next for better understanding of code in review. From new patch onwards will add all patch version changes and add proper description. > >> --- >> .../devicetree/bindings/net/qualcomm-bluetooth.txt | 15 >> +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> index 824c0e2..1221535 100644 >> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> @@ -53,3 +53,18 @@ serial@898000 { >> max-speed = <3200000>; >> }; >> }; >> + >> +&blsp1_uart3 { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&blsp1_uart3_default>; >> + status = "okay"; >> + >> + bluetooth: wcn3998-bt { >> + compatible = "qcom,wcn3998-bt"; >> + vddio-supply = <&vreg_l6_1p8>; >> + vddxo-supply = <&vreg_l5_1p8>; >> + vddrf-supply = <&vreg_s5_1p35>; >> + vddch0-supply = <&vdd_ch0_3p3>; >> + max-speed = <3200000>; >> + }; >> +}; >> \ No newline at end of file > > I think the example isn't really needed since it's essentially the > same as the one for 'qcom,wcn3990-bt'. > > But the important part is missing: add the new compatible string under > ´Required properties´. You also want to update the documentation that > mentiones 'qcom,wcn3990-bt' to 'qcom,wcn399x-bt' (assuming for now > that other possible WCN399x chips would be similar). > [Harish] -- Will check the DT properties, documentation and update accordingly in new patch. > You mentioned in an earlier version of the series that there are > multiple WCN3998 variants with different requirements for > voltage/current. This seems to suggests that multiple compatible > strings are needed to distinguish between them. > [Harish] -- for now we want to add WCN3998 support only, What i mean to say in my earlier explanation that. WCN3990 is base variant and on top of that we have variants like WCN3990, WCN3998 and WCN3998-0,WCN3998-1 like that.. So I think wcn399x would make sense for this series. > Thanks > > Matthias Thanks, Harish ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 2019-03-13 6:30 ` c-hbandi @ 2019-03-14 18:56 ` Matthias Kaehlcke 2019-03-25 11:30 ` c-hbandi 0 siblings, 1 reply; 12+ messages in thread From: Matthias Kaehlcke @ 2019-03-14 18:56 UTC (permalink / raw) To: c-hbandi Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg, Rob Herring, Mark Rutland, devicetree, linux-bluetooth-owner Hi Harish, On Wed, Mar 13, 2019 at 12:00:06PM +0530, c-hbandi@codeaurora.org wrote: > Hi Matthias, > > > On 2019-03-12 22:29, Matthias Kaehlcke wrote: > > +DT folks > > > > Please add them in future versions (script/scripts/get_maintainer.pl > > <patches> should have listed them) > > [Harish] -- Will add them in new version of patches. > > > > On Tue, Mar 12, 2019 at 05:52:59PM +0530, Harish Bandi wrote: > > > This patch enables regulators for the Qualcomm Bluetooth wcn3998 > > > controller. > > > > No, it doesn't. > > > > The next version should probably say something like "Add compatible > > string for the Qualcomm WCN3998 Bluetooth controller. > > > [Harish] -- From new patch onwards will add all patch > version changes and add proper description. > > > Is there any particular reason why QCA drivers folks use 'wcn' instead > > of 'WCN'? The QCA documentations calls it WCN399x, so I'd suggest to > > consistently use the uppercase name in comments and documentation (and > > log messages?). > > > [Harish] -- I think in DT we need to have small case like wcn, agreed > i think that is the reason it started using in code, comments and dt > documentation. AFAIK there are no hard rules for everything, my suggestion would be: - use WCN399x - for general comments/documentation - commit messages - in DT context wcn3998-n seems ok - use wcn399x - for function and variable names - for compatible strings For logging: whatever, just be consistent. > > > Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> > > > --- > > > changes in v3: > > > - updated to latest code base. > > > > This comment is useless, please describe what changed wrt the previous > > version. > [Harish] -- added details in v2, and v3 uploaded just to rebase on tip of > bluetooth-next > for better understanding of code in review. From new patch onwards will add > all patch > version changes and add proper description. > > > > > --- > > > .../devicetree/bindings/net/qualcomm-bluetooth.txt | 15 > > > +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > > b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > > index 824c0e2..1221535 100644 > > > --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > > +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > > @@ -53,3 +53,18 @@ serial@898000 { > > > max-speed = <3200000>; > > > }; > > > }; > > > + > > > +&blsp1_uart3 { > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&blsp1_uart3_default>; > > > + status = "okay"; > > > + > > > + bluetooth: wcn3998-bt { > > > + compatible = "qcom,wcn3998-bt"; > > > + vddio-supply = <&vreg_l6_1p8>; > > > + vddxo-supply = <&vreg_l5_1p8>; > > > + vddrf-supply = <&vreg_s5_1p35>; > > > + vddch0-supply = <&vdd_ch0_3p3>; > > > + max-speed = <3200000>; > > > + }; > > > +}; > > > \ No newline at end of file > > > > I think the example isn't really needed since it's essentially the > > same as the one for 'qcom,wcn3990-bt'. > > > > But the important part is missing: add the new compatible string under > > ´Required properties´. You also want to update the documentation that > > mentiones 'qcom,wcn3990-bt' to 'qcom,wcn399x-bt' (assuming for now > > that other possible WCN399x chips would be similar). > > > [Harish] -- Will check the DT properties, documentation and update > accordingly in new patch. > > > You mentioned in an earlier version of the series that there are > > multiple WCN3998 variants with different requirements for > > voltage/current. This seems to suggests that multiple compatible > > strings are needed to distinguish between them. > > > [Harish] -- for now we want to add WCN3998 support only, What i mean to say > in my earlier > explanation that. WCN3990 is base variant and on top of that we have > variants like WCN3990, > WCN3998 and WCN3998-0,WCN3998-1 like that.. > So I think wcn399x would make sense for this series. If the variants have relevant differences between them (like different regulator requirements) I think you want unique names, rather than 'wcn399x' (I was referring to comments/documentation with this string). If there are variants wouldn't your first 'wcn3998' already be a 'wcn3998-n'? If 'wcn3998' without suffix is used I think it needs to be valid for all 'wcn3998-n' variants (it might be less power-efficient though than using the variant specific compatible string), otherwise things get confusing (a 'wcn3998-2' isn't a 'wcn3998'?) Thanks Matthias ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 2019-03-14 18:56 ` Matthias Kaehlcke @ 2019-03-25 11:30 ` c-hbandi 2019-03-26 23:54 ` Matthias Kaehlcke 0 siblings, 1 reply; 12+ messages in thread From: c-hbandi @ 2019-03-25 11:30 UTC (permalink / raw) To: Matthias Kaehlcke Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg, Rob Herring, Mark Rutland, devicetree, linux-bluetooth-owner Hi Matthias, On 2019-03-15 00:26, Matthias Kaehlcke wrote: > Hi Harish, > > On Wed, Mar 13, 2019 at 12:00:06PM +0530, c-hbandi@codeaurora.org > wrote: >> Hi Matthias, >> >> >> On 2019-03-12 22:29, Matthias Kaehlcke wrote: >> > +DT folks >> > >> > Please add them in future versions (script/scripts/get_maintainer.pl >> > <patches> should have listed them) >> >> [Harish] -- Will add them in new version of patches. >> > >> > On Tue, Mar 12, 2019 at 05:52:59PM +0530, Harish Bandi wrote: >> > > This patch enables regulators for the Qualcomm Bluetooth wcn3998 >> > > controller. >> > >> > No, it doesn't. >> > >> > The next version should probably say something like "Add compatible >> > string for the Qualcomm WCN3998 Bluetooth controller. >> > >> [Harish] -- From new patch onwards will add all patch >> version changes and add proper description. >> >> > Is there any particular reason why QCA drivers folks use 'wcn' instead >> > of 'WCN'? The QCA documentations calls it WCN399x, so I'd suggest to >> > consistently use the uppercase name in comments and documentation (and >> > log messages?). >> > >> [Harish] -- I think in DT we need to have small case like wcn, > > agreed > >> i think that is the reason it started using in code, comments and dt >> documentation. > > AFAIK there are no hard rules for everything, my suggestion would be: > > - use WCN399x > - for general comments/documentation > - commit messages > - in DT context wcn3998-n seems ok > - use wcn399x > - for function and variable names > - for compatible strings > > For logging: whatever, just be consistent. [Harish] --For Commit messages and all will try to follow change it new patch > >> > > Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> >> > > --- >> > > changes in v3: >> > > - updated to latest code base. >> > >> > This comment is useless, please describe what changed wrt the previous >> > version. >> [Harish] -- added details in v2, and v3 uploaded just to rebase on >> tip of >> bluetooth-next >> for better understanding of code in review. From new patch onwards >> will add >> all patch >> version changes and add proper description. >> > >> > > --- >> > > .../devicetree/bindings/net/qualcomm-bluetooth.txt | 15 >> > > +++++++++++++++ >> > > 1 file changed, 15 insertions(+) >> > > >> > > diff --git >> > > a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> > > b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> > > index 824c0e2..1221535 100644 >> > > --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> > > +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> > > @@ -53,3 +53,18 @@ serial@898000 { >> > > max-speed = <3200000>; >> > > }; >> > > }; >> > > + >> > > +&blsp1_uart3 { >> > > + pinctrl-names = "default"; >> > > + pinctrl-0 = <&blsp1_uart3_default>; >> > > + status = "okay"; >> > > + >> > > + bluetooth: wcn3998-bt { >> > > + compatible = "qcom,wcn3998-bt"; >> > > + vddio-supply = <&vreg_l6_1p8>; >> > > + vddxo-supply = <&vreg_l5_1p8>; >> > > + vddrf-supply = <&vreg_s5_1p35>; >> > > + vddch0-supply = <&vdd_ch0_3p3>; >> > > + max-speed = <3200000>; >> > > + }; >> > > +}; >> > > \ No newline at end of file >> > >> > I think the example isn't really needed since it's essentially the >> > same as the one for 'qcom,wcn3990-bt'. >> > >> > But the important part is missing: add the new compatible string under >> > ´Required properties´. You also want to update the documentation that >> > mentiones 'qcom,wcn3990-bt' to 'qcom,wcn399x-bt' (assuming for now >> > that other possible WCN399x chips would be similar). >> > >> [Harish] -- Will check the DT properties, documentation and update >> accordingly in new patch. >> >> > You mentioned in an earlier version of the series that there are >> > multiple WCN3998 variants with different requirements for >> > voltage/current. This seems to suggests that multiple compatible >> > strings are needed to distinguish between them. >> > >> [Harish] -- for now we want to add WCN3998 support only, What i mean >> to say >> in my earlier >> explanation that. WCN3990 is base variant and on top of that we have >> variants like WCN3990, >> WCN3998 and WCN3998-0,WCN3998-1 like that.. >> So I think wcn399x would make sense for this series. > > If the variants have relevant differences between them (like different > regulator requirements) I think you want unique names, rather than > 'wcn399x' (I was referring to comments/documentation with this > string). > > If there are variants wouldn't your first 'wcn3998' already be a > 'wcn3998-n'? If 'wcn3998' without suffix is used I think it needs to > be valid for all 'wcn3998-n' variants (it might be less > power-efficient though than using the variant specific compatible > string), otherwise things get confusing (a 'wcn3998-2' isn't a > 'wcn3998'?) > [Harish] -- for now WCN3990 and WCN3998 only, also wcn3998-2 and wcn3998 are same. So for now we are going to have only WCN3990 and WCN3998 for DT. > Thanks > > Matthias ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 2019-03-25 11:30 ` c-hbandi @ 2019-03-26 23:54 ` Matthias Kaehlcke 2019-03-27 10:23 ` Harish Bandi 0 siblings, 1 reply; 12+ messages in thread From: Matthias Kaehlcke @ 2019-03-26 23:54 UTC (permalink / raw) To: c-hbandi Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg, Rob Herring, Mark Rutland, devicetree, linux-bluetooth-owner On Mon, Mar 25, 2019 at 05:00:31PM +0530, c-hbandi@codeaurora.org wrote: > Hi Matthias, > > On 2019-03-15 00:26, Matthias Kaehlcke wrote: > > Hi Harish, > > > > On Wed, Mar 13, 2019 at 12:00:06PM +0530, c-hbandi@codeaurora.org wrote: > > > Hi Matthias, > > > > > > > > > On 2019-03-12 22:29, Matthias Kaehlcke wrote: > > > > +DT folks > > > > > > > > Please add them in future versions (script/scripts/get_maintainer.pl > > > > <patches> should have listed them) > > > > > > [Harish] -- Will add them in new version of patches. > > > > > > > > On Tue, Mar 12, 2019 at 05:52:59PM +0530, Harish Bandi wrote: > > > > > This patch enables regulators for the Qualcomm Bluetooth wcn3998 > > > > > controller. > > > > > > > > No, it doesn't. > > > > > > > > The next version should probably say something like "Add compatible > > > > string for the Qualcomm WCN3998 Bluetooth controller. > > > > > > > [Harish] -- From new patch onwards will add all patch > > > version changes and add proper description. > > > > > > > Is there any particular reason why QCA drivers folks use 'wcn' instead > > > > of 'WCN'? The QCA documentations calls it WCN399x, so I'd suggest to > > > > consistently use the uppercase name in comments and documentation (and > > > > log messages?). > > > > > > > [Harish] -- I think in DT we need to have small case like wcn, > > > > agreed > > > > > i think that is the reason it started using in code, comments and dt > > > documentation. > > > > AFAIK there are no hard rules for everything, my suggestion would be: > > > > - use WCN399x > > - for general comments/documentation > > - commit messages > > - in DT context wcn3998-n seems ok > > - use wcn399x > > - for function and variable names > > - for compatible strings > > > > For logging: whatever, just be consistent. > > [Harish] --For Commit messages and all will try to follow change it new > patch > > > > > > > > Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> > > > > > --- > > > > > changes in v3: > > > > > - updated to latest code base. > > > > > > > > This comment is useless, please describe what changed wrt the previous > > > > version. > > > [Harish] -- added details in v2, and v3 uploaded just to rebase on > > > tip of > > > bluetooth-next > > > for better understanding of code in review. From new patch onwards > > > will add > > > all patch > > > version changes and add proper description. > > > > > > > > > --- > > > > > .../devicetree/bindings/net/qualcomm-bluetooth.txt | 15 > > > > > +++++++++++++++ > > > > > 1 file changed, 15 insertions(+) > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > > > > b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > > > > index 824c0e2..1221535 100644 > > > > > --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > > > > +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > > > > @@ -53,3 +53,18 @@ serial@898000 { > > > > > max-speed = <3200000>; > > > > > }; > > > > > }; > > > > > + > > > > > +&blsp1_uart3 { > > > > > + pinctrl-names = "default"; > > > > > + pinctrl-0 = <&blsp1_uart3_default>; > > > > > + status = "okay"; > > > > > + > > > > > + bluetooth: wcn3998-bt { > > > > > + compatible = "qcom,wcn3998-bt"; > > > > > + vddio-supply = <&vreg_l6_1p8>; > > > > > + vddxo-supply = <&vreg_l5_1p8>; > > > > > + vddrf-supply = <&vreg_s5_1p35>; > > > > > + vddch0-supply = <&vdd_ch0_3p3>; > > > > > + max-speed = <3200000>; > > > > > + }; > > > > > +}; > > > > > \ No newline at end of file > > > > > > > > I think the example isn't really needed since it's essentially the > > > > same as the one for 'qcom,wcn3990-bt'. > > > > > > > > But the important part is missing: add the new compatible string under > > > > ´Required properties´. You also want to update the documentation that > > > > mentiones 'qcom,wcn3990-bt' to 'qcom,wcn399x-bt' (assuming for now > > > > that other possible WCN399x chips would be similar). > > > > > > > [Harish] -- Will check the DT properties, documentation and update > > > accordingly in new patch. > > > > > > > You mentioned in an earlier version of the series that there are > > > > multiple WCN3998 variants with different requirements for > > > > voltage/current. This seems to suggests that multiple compatible > > > > strings are needed to distinguish between them. > > > > > > > [Harish] -- for now we want to add WCN3998 support only, What i mean > > > to say > > > in my earlier > > > explanation that. WCN3990 is base variant and on top of that we have > > > variants like WCN3990, > > > WCN3998 and WCN3998-0,WCN3998-1 like that.. > > > So I think wcn399x would make sense for this series. > > > > If the variants have relevant differences between them (like different > > regulator requirements) I think you want unique names, rather than > > 'wcn399x' (I was referring to comments/documentation with this > > string). > > > > If there are variants wouldn't your first 'wcn3998' already be a > > 'wcn3998-n'? If 'wcn3998' without suffix is used I think it needs to > > be valid for all 'wcn3998-n' variants (it might be less > > power-efficient though than using the variant specific compatible > > string), otherwise things get confusing (a 'wcn3998-2' isn't a > > 'wcn3998'?) > > > > > [Harish] -- for now WCN3990 and WCN3998 only, also wcn3998-2 and wcn3998 > are same. > So for now we are going to have only WCN3990 and WCN3998 for DT. Since you brought up the different power requirements initially: are your reasonably sure that future wcn3998-3, wcn3998-4, ... can operate with the generic wcn3998 power settings? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 2019-03-26 23:54 ` Matthias Kaehlcke @ 2019-03-27 10:23 ` Harish Bandi 2019-03-27 17:22 ` Matthias Kaehlcke 0 siblings, 1 reply; 12+ messages in thread From: Harish Bandi @ 2019-03-27 10:23 UTC (permalink / raw) To: Matthias Kaehlcke Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg, Rob Herring, Mark Rutland, devicetree, linux-bluetooth-owner Hi Matthias, On 2019-03-27 05:24, Matthias Kaehlcke wrote: > On Mon, Mar 25, 2019 at 05:00:31PM +0530, c-hbandi@codeaurora.org > wrote: >> Hi Matthias, >> >> On 2019-03-15 00:26, Matthias Kaehlcke wrote: >> > Hi Harish, >> > >> > On Wed, Mar 13, 2019 at 12:00:06PM +0530, c-hbandi@codeaurora.org wrote: >> > > Hi Matthias, >> > > >> > > >> > > On 2019-03-12 22:29, Matthias Kaehlcke wrote: >> > > > +DT folks >> > > > >> > > > Please add them in future versions (script/scripts/get_maintainer.pl >> > > > <patches> should have listed them) >> > > >> > > [Harish] -- Will add them in new version of patches. >> > > > >> > > > On Tue, Mar 12, 2019 at 05:52:59PM +0530, Harish Bandi wrote: >> > > > > This patch enables regulators for the Qualcomm Bluetooth wcn3998 >> > > > > controller. >> > > > >> > > > No, it doesn't. >> > > > >> > > > The next version should probably say something like "Add compatible >> > > > string for the Qualcomm WCN3998 Bluetooth controller. >> > > > >> > > [Harish] -- From new patch onwards will add all patch >> > > version changes and add proper description. >> > > >> > > > Is there any particular reason why QCA drivers folks use 'wcn' instead >> > > > of 'WCN'? The QCA documentations calls it WCN399x, so I'd suggest to >> > > > consistently use the uppercase name in comments and documentation (and >> > > > log messages?). >> > > > >> > > [Harish] -- I think in DT we need to have small case like wcn, >> > >> > agreed >> > >> > > i think that is the reason it started using in code, comments and dt >> > > documentation. >> > >> > AFAIK there are no hard rules for everything, my suggestion would be: >> > >> > - use WCN399x >> > - for general comments/documentation >> > - commit messages >> > - in DT context wcn3998-n seems ok >> > - use wcn399x >> > - for function and variable names >> > - for compatible strings >> > >> > For logging: whatever, just be consistent. >> >> [Harish] --For Commit messages and all will try to follow change it >> new >> patch >> >> > >> > > > > Signed-off-by: Harish Bandi <c-hbandi@codeaurora.org> >> > > > > --- >> > > > > changes in v3: >> > > > > - updated to latest code base. >> > > > >> > > > This comment is useless, please describe what changed wrt the previous >> > > > version. >> > > [Harish] -- added details in v2, and v3 uploaded just to rebase on >> > > tip of >> > > bluetooth-next >> > > for better understanding of code in review. From new patch onwards >> > > will add >> > > all patch >> > > version changes and add proper description. >> > > > >> > > > > --- >> > > > > .../devicetree/bindings/net/qualcomm-bluetooth.txt | 15 >> > > > > +++++++++++++++ >> > > > > 1 file changed, 15 insertions(+) >> > > > > >> > > > > diff --git >> > > > > a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> > > > > b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> > > > > index 824c0e2..1221535 100644 >> > > > > --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> > > > > +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> > > > > @@ -53,3 +53,18 @@ serial@898000 { >> > > > > max-speed = <3200000>; >> > > > > }; >> > > > > }; >> > > > > + >> > > > > +&blsp1_uart3 { >> > > > > + pinctrl-names = "default"; >> > > > > + pinctrl-0 = <&blsp1_uart3_default>; >> > > > > + status = "okay"; >> > > > > + >> > > > > + bluetooth: wcn3998-bt { >> > > > > + compatible = "qcom,wcn3998-bt"; >> > > > > + vddio-supply = <&vreg_l6_1p8>; >> > > > > + vddxo-supply = <&vreg_l5_1p8>; >> > > > > + vddrf-supply = <&vreg_s5_1p35>; >> > > > > + vddch0-supply = <&vdd_ch0_3p3>; >> > > > > + max-speed = <3200000>; >> > > > > + }; >> > > > > +}; >> > > > > \ No newline at end of file >> > > > >> > > > I think the example isn't really needed since it's essentially the >> > > > same as the one for 'qcom,wcn3990-bt'. >> > > > >> > > > But the important part is missing: add the new compatible string under >> > > > ´Required properties´. You also want to update the documentation that >> > > > mentiones 'qcom,wcn3990-bt' to 'qcom,wcn399x-bt' (assuming for now >> > > > that other possible WCN399x chips would be similar). >> > > > >> > > [Harish] -- Will check the DT properties, documentation and update >> > > accordingly in new patch. >> > > >> > > > You mentioned in an earlier version of the series that there are >> > > > multiple WCN3998 variants with different requirements for >> > > > voltage/current. This seems to suggests that multiple compatible >> > > > strings are needed to distinguish between them. >> > > > >> > > [Harish] -- for now we want to add WCN3998 support only, What i mean >> > > to say >> > > in my earlier >> > > explanation that. WCN3990 is base variant and on top of that we have >> > > variants like WCN3990, >> > > WCN3998 and WCN3998-0,WCN3998-1 like that.. >> > > So I think wcn399x would make sense for this series. >> > >> > If the variants have relevant differences between them (like different >> > regulator requirements) I think you want unique names, rather than >> > 'wcn399x' (I was referring to comments/documentation with this >> > string). >> > >> > If there are variants wouldn't your first 'wcn3998' already be a >> > 'wcn3998-n'? If 'wcn3998' without suffix is used I think it needs to >> > be valid for all 'wcn3998-n' variants (it might be less >> > power-efficient though than using the variant specific compatible >> > string), otherwise things get confusing (a 'wcn3998-2' isn't a >> > 'wcn3998'?) >> > >> >> >> [Harish] -- for now WCN3990 and WCN3998 only, also wcn3998-2 and >> wcn3998 >> are same. >> So for now we are going to have only WCN3990 and WCN3998 for DT. > > Since you brought up the different power requirements initially: > > are your reasonably sure that future wcn3998-3, wcn3998-4, ... can > operate with the generic wcn3998 power settings? [Harish] - yes with power settings of wcn3998, wcn3998-3 and wcn3998-4 ... also will work. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 2019-03-27 10:23 ` Harish Bandi @ 2019-03-27 17:22 ` Matthias Kaehlcke 0 siblings, 0 replies; 12+ messages in thread From: Matthias Kaehlcke @ 2019-03-27 17:22 UTC (permalink / raw) To: Harish Bandi Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth, hemantg, linux-arm-msm, bgodavar, anubhavg, Rob Herring, Mark Rutland, devicetree, linux-bluetooth-owner On Wed, Mar 27, 2019 at 03:53:58PM +0530, Harish Bandi wrote: > Hi Matthias, > > > > > > > You mentioned in an earlier version of the series that there are > > > > > > multiple WCN3998 variants with different requirements for > > > > > > voltage/current. This seems to suggests that multiple compatible > > > > > > strings are needed to distinguish between them. > > > > > > > > > > > [Harish] -- for now we want to add WCN3998 support only, What i mean > > > > > to say > > > > > in my earlier > > > > > explanation that. WCN3990 is base variant and on top of that we have > > > > > variants like WCN3990, > > > > > WCN3998 and WCN3998-0,WCN3998-1 like that.. > > > > > So I think wcn399x would make sense for this series. > > > > > > > > If the variants have relevant differences between them (like different > > > > regulator requirements) I think you want unique names, rather than > > > > 'wcn399x' (I was referring to comments/documentation with this > > > > string). > > > > > > > > If there are variants wouldn't your first 'wcn3998' already be a > > > > 'wcn3998-n'? If 'wcn3998' without suffix is used I think it needs to > > > > be valid for all 'wcn3998-n' variants (it might be less > > > > power-efficient though than using the variant specific compatible > > > > string), otherwise things get confusing (a 'wcn3998-2' isn't a > > > > 'wcn3998'?) > > > > > > > > > > > > > [Harish] -- for now WCN3990 and WCN3998 only, also wcn3998-2 and > > > wcn3998 > > > are same. > > > So for now we are going to have only WCN3990 and WCN3998 for DT. > > > > Since you brought up the different power requirements initially: > > > > are your reasonably sure that future wcn3998-3, wcn3998-4, ... can > > operate with the generic wcn3998 power settings? > > [Harish] - yes with power settings of wcn3998, wcn3998-3 and wcn3998-4 ... > also will work. Thanks for the confirmation, in that case just using 'wcn3998' for now sounds good to me. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-03-27 17:22 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-12 12:22 [PATCH v3 0/2] Enable Bluetooth functionality for wcn3998 Harish Bandi 2019-03-12 12:22 ` [PATCH v3 1/2] Bluetooth: hci_qca: Added support " Harish Bandi 2019-03-12 16:29 ` Matthias Kaehlcke 2019-03-13 6:22 ` c-hbandi 2019-03-12 12:22 ` [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device tree bindings for QTI chip wcn3998 Harish Bandi 2019-03-12 16:59 ` Matthias Kaehlcke 2019-03-13 6:30 ` c-hbandi 2019-03-14 18:56 ` Matthias Kaehlcke 2019-03-25 11:30 ` c-hbandi 2019-03-26 23:54 ` Matthias Kaehlcke 2019-03-27 10:23 ` Harish Bandi 2019-03-27 17:22 ` Matthias Kaehlcke
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.