2019-04-04 06:38:06

by Rocky Liao

[permalink] [raw]
Subject: [PATCH] Bluetooth: hci_qca: Load customized NVM based on the device property

QCA BTSOC nvm is a customized file and different vendor/platoform may want
to have different BTSOC configuration via this file (e.g. Configure SCO over
PCM or I2S, Setting Tx power, etc.) This patch will allow vendors to download
different nvm file by reading a device property as the nvm file name postfix.

Signed-off-by: Rocky Liao <[email protected]>
---
drivers/bluetooth/btqca.c | 13 +++++++++----
drivers/bluetooth/btqca.h | 4 ++--
drivers/bluetooth/hci_qca.c | 18 +++++++++++++++++-
3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 6122685..81e42f3 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -332,7 +332,7 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);

int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
- enum qca_btsoc_type soc_type, u32 soc_ver)
+ enum qca_btsoc_type soc_type, u32 soc_ver, const char *nvm_postfix)
{
struct rome_config config;
int err;
@@ -368,9 +368,14 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
if (soc_type == QCA_WCN3990)
snprintf(config.fwname, sizeof(config.fwname),
"qca/crnv%02x.bin", rom_ver);
- else
- snprintf(config.fwname, sizeof(config.fwname),
- "qca/nvm_%08x.bin", soc_ver);
+ else {
+ if (NULL == nvm_postfix)
+ snprintf(config.fwname, sizeof(config.fwname),
+ "qca/nvm_%08x.bin", soc_ver);
+ else
+ snprintf(config.fwname, sizeof(config.fwname),
+ "qca/nvm_%08x_%s.bin", soc_ver, nvm_postfix);
+ }

err = qca_download_firmware(hdev, &config);
if (err < 0) {
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index 6fdc25d..60a868f 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -139,7 +139,7 @@ enum qca_btsoc_type {

int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
- enum qca_btsoc_type soc_type, u32 soc_ver);
+ enum qca_btsoc_type soc_type, u32 soc_ver, const char *nvm_postfix);
int qca_read_soc_version(struct hci_dev *hdev, u32 *soc_version);
int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
#else
@@ -150,7 +150,7 @@ static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdad
}

static inline int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
- enum qca_btsoc_type soc_type, u32 soc_ver)
+ enum qca_btsoc_type soc_type, u32 soc_ver, const char *nvm_postfix)
{
return -EOPNOTSUPP;
}
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 4ea995d..f495f3b 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -164,6 +164,7 @@ struct qca_serdev {
struct hci_uart serdev_hu;
struct gpio_desc *bt_en;
struct clk *susclk;
+ const char *nvm_postfix;
enum qca_btsoc_type btsoc_type;
struct qca_power *bt_power;
u32 init_speed;
@@ -189,6 +190,17 @@ static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
return soc_type;
}

+static const char *qca_get_nvm_postfix(struct hci_uart *hu)
+{
+ if (hu->serdev) {
+ struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
+
+ return qsd->nvm_postfix;
+ } else {
+ return NULL;
+ }
+}
+
static void __serial_clock_on(struct tty_struct *tty)
{
/* TODO: Some chipset requires to enable UART clock on client
@@ -1191,6 +1203,7 @@ static int qca_setup(struct hci_uart *hu)
struct qca_data *qca = hu->priv;
unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
enum qca_btsoc_type soc_type = qca_soc_type(hu);
+ const char *nvm_postfix = qca_get_nvm_postfix(hu);
int ret;
int soc_ver = 0;

@@ -1241,7 +1254,7 @@ static int qca_setup(struct hci_uart *hu)

bt_dev_info(hdev, "QCA controller version 0x%08x", soc_ver);
/* Setup patch / NVM configurations */
- ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver);
+ ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver, nvm_postfix);
if (!ret) {
set_bit(STATE_IN_BAND_SLEEP_ENABLED, &qca->flags);
qca_debugfs_init(hdev);
@@ -1462,6 +1475,9 @@ static int qca_serdev_probe(struct serdev_device *serdev)
return PTR_ERR(qcadev->bt_en);
}

+ device_property_read_string(&serdev->dev, "nvm-postfix",
+ &qcadev->nvm_postfix);
+
qcadev->susclk = devm_clk_get(&serdev->dev, NULL);
if (IS_ERR(qcadev->susclk)) {
dev_err(&serdev->dev, "failed to acquire clk\n");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project



2019-04-04 06:56:29

by Balakrishna Godavarthi

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_qca: Load customized NVM based on the device property

Hi Rocky,

On 2019-04-04 12:07, Rocky Liao wrote:
> QCA BTSOC nvm is a customized file and different vendor/platoform may
> want
> to have different BTSOC configuration via this file (e.g. Configure SCO
> over
> PCM or I2S, Setting Tx power, etc.) This patch will allow vendors to
> download
> different nvm file by reading a device property as the nvm file name
> postfix.
>

[Bala]: add the property which your reading to the documentation file.
"Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt"

> Signed-off-by: Rocky Liao <[email protected]>
> ---
> drivers/bluetooth/btqca.c | 13 +++++++++----
> drivers/bluetooth/btqca.h | 4 ++--
> drivers/bluetooth/hci_qca.c | 18 +++++++++++++++++-
> 3 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index 6122685..81e42f3 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -332,7 +332,7 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev,
> const bdaddr_t *bdaddr)
> EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);
>
> int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> - enum qca_btsoc_type soc_type, u32 soc_ver)
> + enum qca_btsoc_type soc_type, u32 soc_ver, const char
> *nvm_postfix)
> {
> struct rome_config config;
> int err;
> @@ -368,9 +368,14 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t
> baudrate,
> if (soc_type == QCA_WCN3990)
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/crnv%02x.bin", rom_ver);
> - else
> - snprintf(config.fwname, sizeof(config.fwname),
> - "qca/nvm_%08x.bin", soc_ver);
> + else {
> + if (NULL == nvm_postfix)
[Bala]:
something like this is more readable.
if (nvm_postfix)
append the nvm postfix string.
else
go with an default name.

> + snprintf(config.fwname, sizeof(config.fwname),
> + "qca/nvm_%08x.bin", soc_ver);
> + else
> + snprintf(config.fwname, sizeof(config.fwname),
> + "qca/nvm_%08x_%s.bin", soc_ver, nvm_postfix);
> + }
>
> err = qca_download_firmware(hdev, &config);
> if (err < 0) {
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index 6fdc25d..60a868f 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -139,7 +139,7 @@ enum qca_btsoc_type {
>
> int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> - enum qca_btsoc_type soc_type, u32 soc_ver);
> + enum qca_btsoc_type soc_type, u32 soc_ver, const char
> *nvm_postfix);
> int qca_read_soc_version(struct hci_dev *hdev, u32 *soc_version);
> int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> #else
> @@ -150,7 +150,7 @@ static inline int qca_set_bdaddr_rome(struct
> hci_dev *hdev, const bdaddr_t *bdad
> }
>
> static inline int qca_uart_setup(struct hci_dev *hdev, uint8_t
> baudrate,
> - enum qca_btsoc_type soc_type, u32 soc_ver)
> + enum qca_btsoc_type soc_type, u32 soc_ver, const char
> *nvm_postfix)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 4ea995d..f495f3b 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -164,6 +164,7 @@ struct qca_serdev {
> struct hci_uart serdev_hu;
> struct gpio_desc *bt_en;
> struct clk *susclk;
> + const char *nvm_postfix;

[Bala]: nit: add the entry at last is preferred.

> enum qca_btsoc_type btsoc_type;
> struct qca_power *bt_power;
> u32 init_speed;
> @@ -189,6 +190,17 @@ static enum qca_btsoc_type qca_soc_type(struct
> hci_uart *hu)
> return soc_type;
> }
>
> +static const char *qca_get_nvm_postfix(struct hci_uart *hu)
> +{
> + if (hu->serdev) {
> + struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
> +
> + return qsd->nvm_postfix;
> + } else {
> + return NULL;
> + }
> +}
> +
> static void __serial_clock_on(struct tty_struct *tty)
> {
> /* TODO: Some chipset requires to enable UART clock on client
> @@ -1191,6 +1203,7 @@ static int qca_setup(struct hci_uart *hu)
> struct qca_data *qca = hu->priv;
> unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
> enum qca_btsoc_type soc_type = qca_soc_type(hu);
> + const char *nvm_postfix = qca_get_nvm_postfix(hu);
> int ret;
> int soc_ver = 0;
>
> @@ -1241,7 +1254,7 @@ static int qca_setup(struct hci_uart *hu)
>
> bt_dev_info(hdev, "QCA controller version 0x%08x", soc_ver);
> /* Setup patch / NVM configurations */
> - ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver);
> + ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver,
> nvm_postfix);
> if (!ret) {
> set_bit(STATE_IN_BAND_SLEEP_ENABLED, &qca->flags);
> qca_debugfs_init(hdev);
> @@ -1462,6 +1475,9 @@ static int qca_serdev_probe(struct serdev_device
> *serdev)
> return PTR_ERR(qcadev->bt_en);
> }
>
> + device_property_read_string(&serdev->dev, "nvm-postfix",
> + &qcadev->nvm_postfix);
> +
> qcadev->susclk = devm_clk_get(&serdev->dev, NULL);
> if (IS_ERR(qcadev->susclk)) {
> dev_err(&serdev->dev, "failed to acquire clk\n");

--
Regards
Balakrishna.

2019-04-04 09:32:16

by Rocky Liao

[permalink] [raw]
Subject: [PATCH v2 2/2] dt-bindings: net: bluetooth: Add device property nvm-postfix for QCA6174

This patchs patch adds an optional device property nvm-postfix to allow the
driver to load customized nvm file based on this property

Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2:
* added the property to the document file
Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
* fixed coding style warnings
* moved the nvm-postfix to the last entry of the qca_serdev
---
Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
index 824c0e2..70cda4b 100644
--- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
+++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
@@ -16,6 +16,7 @@ Optional properties for compatible string qcom,qca6174-bt:

- enable-gpios: gpio specifier used to enable chip
- clocks: clock provided to the controller (SUSCLK_32KHZ)
+ - nvm-postfix: nvm file postfix to load customized nvm file

Required properties for compatible string qcom,wcn3990-bt:

@@ -39,6 +40,7 @@ serial@7570000 {

enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
clocks = <&divclk4>;
+ nvm-postfix = "i2s";
};
};

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


2019-04-04 09:32:43

by Rocky Liao

[permalink] [raw]
Subject: [PATCH v2 1/2] Bluetooth: hci_qca: Load customized NVM based on the device property

QCA BTSOC nvm is a customized file and different vendor/platoform may want
to have different BTSOC configuration via this file (e.g. Configure SCO
over PCM or I2S, Setting Tx power, etc.) This patch will allow vendors to
download different nvm file by reading a device property "nvm-postfix" as
the nvm file name postfix.

Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2:
* added the property to the document file
Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
* fixed coding style warnings
* moved the nvm-postfix to the last entry of the qca_serdev
---
drivers/bluetooth/btqca.c | 14 ++++++++++----
drivers/bluetooth/btqca.h | 6 ++++--
drivers/bluetooth/hci_qca.c | 19 ++++++++++++++++++-
3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 6122685..4e89286 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -332,7 +332,8 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);

int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
- enum qca_btsoc_type soc_type, u32 soc_ver)
+ enum qca_btsoc_type soc_type, u32 soc_ver,
+ const char *nvm_postfix)
{
struct rome_config config;
int err;
@@ -368,9 +369,14 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
if (soc_type == QCA_WCN3990)
snprintf(config.fwname, sizeof(config.fwname),
"qca/crnv%02x.bin", rom_ver);
- else
- snprintf(config.fwname, sizeof(config.fwname),
- "qca/nvm_%08x.bin", soc_ver);
+ else {
+ if (nvm_postfix)
+ snprintf(config.fwname, sizeof(config.fwname),
+ "qca/nvm_%08x_%s.bin", soc_ver, nvm_postfix);
+ else
+ snprintf(config.fwname, sizeof(config.fwname),
+ "qca/nvm_%08x.bin", soc_ver);
+ }

err = qca_download_firmware(hdev, &config);
if (err < 0) {
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index 6fdc25d..0dd6cd0 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -139,7 +139,8 @@ enum qca_btsoc_type {

int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
- enum qca_btsoc_type soc_type, u32 soc_ver);
+ enum qca_btsoc_type soc_type, u32 soc_ver,
+ const char *nvm_postfix);
int qca_read_soc_version(struct hci_dev *hdev, u32 *soc_version);
int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
#else
@@ -150,7 +151,8 @@ static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdad
}

static inline int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
- enum qca_btsoc_type soc_type, u32 soc_ver)
+ enum qca_btsoc_type soc_type, u32 soc_ver,
+ const char *nvm_postfix)
{
return -EOPNOTSUPP;
}
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 4ea995d..560e880 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -168,6 +168,7 @@ struct qca_serdev {
struct qca_power *bt_power;
u32 init_speed;
u32 oper_speed;
+ const char *nvm_postfix;
};

static int qca_power_setup(struct hci_uart *hu, bool on);
@@ -189,6 +190,17 @@ static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
return soc_type;
}

+static const char *qca_get_nvm_postfix(struct hci_uart *hu)
+{
+ if (hu->serdev) {
+ struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
+
+ return qsd->nvm_postfix;
+ } else {
+ return NULL;
+ }
+}
+
static void __serial_clock_on(struct tty_struct *tty)
{
/* TODO: Some chipset requires to enable UART clock on client
@@ -1191,6 +1203,7 @@ static int qca_setup(struct hci_uart *hu)
struct qca_data *qca = hu->priv;
unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
enum qca_btsoc_type soc_type = qca_soc_type(hu);
+ const char *nvm_postfix = qca_get_nvm_postfix(hu);
int ret;
int soc_ver = 0;

@@ -1241,7 +1254,8 @@ static int qca_setup(struct hci_uart *hu)

bt_dev_info(hdev, "QCA controller version 0x%08x", soc_ver);
/* Setup patch / NVM configurations */
- ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver);
+ ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver,
+ nvm_postfix);
if (!ret) {
set_bit(STATE_IN_BAND_SLEEP_ENABLED, &qca->flags);
qca_debugfs_init(hdev);
@@ -1462,6 +1476,9 @@ static int qca_serdev_probe(struct serdev_device *serdev)
return PTR_ERR(qcadev->bt_en);
}

+ device_property_read_string(&serdev->dev, "nvm-postfix",
+ &qcadev->nvm_postfix);
+
qcadev->susclk = devm_clk_get(&serdev->dev, NULL);
if (IS_ERR(qcadev->susclk)) {
dev_err(&serdev->dev, "failed to acquire clk\n");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


2019-04-04 10:00:09

by Balakrishna Godavarthi

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Bluetooth: hci_qca: Load customized NVM based on the device property

Hi Rocky,

On 2019-04-04 14:38, Rocky Liao wrote:
> QCA BTSOC nvm is a customized file and different vendor/platoform may
> want
> to have different BTSOC configuration via this file (e.g. Configure SCO
> over PCM or I2S, Setting Tx power, etc.) This patch will allow vendors
> to
> download different nvm file by reading a device property "nvm-postfix"
> as
> the nvm file name postfix.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
> Changes in v2:
> * added the property to the document file
> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> * fixed coding style warnings
> * moved the nvm-postfix to the last entry of the qca_serdev
> ---
> drivers/bluetooth/btqca.c | 14 ++++++++++----
> drivers/bluetooth/btqca.h | 6 ++++--
> drivers/bluetooth/hci_qca.c | 19 ++++++++++++++++++-
> 3 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index 6122685..4e89286 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -332,7 +332,8 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev,
> const bdaddr_t *bdaddr)
> EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);
>
> int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> - enum qca_btsoc_type soc_type, u32 soc_ver)
> + enum qca_btsoc_type soc_type, u32 soc_ver,
> + const char *nvm_postfix)
> {
> struct rome_config config;
> int err;
> @@ -368,9 +369,14 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t
> baudrate,
> if (soc_type == QCA_WCN3990)
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/crnv%02x.bin", rom_ver);
> - else
> - snprintf(config.fwname, sizeof(config.fwname),
> - "qca/nvm_%08x.bin", soc_ver);
> + else {
> + if (nvm_postfix)
> + snprintf(config.fwname, sizeof(config.fwname),
> + "qca/nvm_%08x_%s.bin", soc_ver, nvm_postfix);
> + else
> + snprintf(config.fwname, sizeof(config.fwname),
> + "qca/nvm_%08x.bin", soc_ver);
> + }
>
> err = qca_download_firmware(hdev, &config);
> if (err < 0) {
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index 6fdc25d..0dd6cd0 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -139,7 +139,8 @@ enum qca_btsoc_type {
>
> int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> - enum qca_btsoc_type soc_type, u32 soc_ver);
> + enum qca_btsoc_type soc_type, u32 soc_ver,
> + const char *nvm_postfix);
> int qca_read_soc_version(struct hci_dev *hdev, u32 *soc_version);
> int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> #else
> @@ -150,7 +151,8 @@ static inline int qca_set_bdaddr_rome(struct
> hci_dev *hdev, const bdaddr_t *bdad
> }
>
> static inline int qca_uart_setup(struct hci_dev *hdev, uint8_t
> baudrate,
> - enum qca_btsoc_type soc_type, u32 soc_ver)
> + enum qca_btsoc_type soc_type, u32 soc_ver,
> + const char *nvm_postfix)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 4ea995d..560e880 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -168,6 +168,7 @@ struct qca_serdev {
> struct qca_power *bt_power;
> u32 init_speed;
> u32 oper_speed;
> + const char *nvm_postfix;

[Bala]: I guess your trying to the read the type of communication for
SCO.
so i would recommend to change this to variable name to
sco_com_type or might be to the use understandable name.
just an suggestion. Don't respin right away wait for other
comments too :)

> };
>
> static int qca_power_setup(struct hci_uart *hu, bool on);
> @@ -189,6 +190,17 @@ static enum qca_btsoc_type qca_soc_type(struct
> hci_uart *hu)
> return soc_type;
> }
>
> +static const char *qca_get_nvm_postfix(struct hci_uart *hu)
> +{
> + if (hu->serdev) {
> + struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
> +
> + return qsd->nvm_postfix;
> + } else {
> + return NULL;
> + }
> +}
> +
> static void __serial_clock_on(struct tty_struct *tty)
> {
> /* TODO: Some chipset requires to enable UART clock on client
> @@ -1191,6 +1203,7 @@ static int qca_setup(struct hci_uart *hu)
> struct qca_data *qca = hu->priv;
> unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
> enum qca_btsoc_type soc_type = qca_soc_type(hu);
> + const char *nvm_postfix = qca_get_nvm_postfix(hu);
> int ret;
> int soc_ver = 0;
>
> @@ -1241,7 +1254,8 @@ static int qca_setup(struct hci_uart *hu)
>
> bt_dev_info(hdev, "QCA controller version 0x%08x", soc_ver);
> /* Setup patch / NVM configurations */
> - ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver);
> + ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver,
> + nvm_postfix);
> if (!ret) {
> set_bit(STATE_IN_BAND_SLEEP_ENABLED, &qca->flags);
> qca_debugfs_init(hdev);
> @@ -1462,6 +1476,9 @@ static int qca_serdev_probe(struct serdev_device
> *serdev)
> return PTR_ERR(qcadev->bt_en);
> }
>
> + device_property_read_string(&serdev->dev, "nvm-postfix",
> + &qcadev->nvm_postfix);
> +
> qcadev->susclk = devm_clk_get(&serdev->dev, NULL);
> if (IS_ERR(qcadev->susclk)) {
> dev_err(&serdev->dev, "failed to acquire clk\n");

--
Regards
Balakrishna.

2019-04-04 12:42:03

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: net: bluetooth: Add device property nvm-postfix for QCA6174

+robh

On 04/04/2019 11:08, Rocky Liao wrote:

> This patchs patch adds an optional device property nvm-postfix to allow the
> driver to load customized nvm file based on this property

While text /before/ is indeed called a "prefix", text /after/ is not a "postfix",
but a "suffix".


> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> index 824c0e2..70cda4b 100644
> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> @@ -16,6 +16,7 @@ Optional properties for compatible string qcom,qca6174-bt:
>
> - enable-gpios: gpio specifier used to enable chip
> - clocks: clock provided to the controller (SUSCLK_32KHZ)
> + - nvm-postfix: nvm file postfix to load customized nvm file

The device tree is supposed to describe hardware.

The name of which file to load can hardly be considered part of the HW.

Possible solutions:
1) derive the file name from the compatible string
2) pass the name as a module parameter
3) something else


> @@ -39,6 +40,7 @@ serial@7570000 {
>
> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
> clocks = <&divclk4>;
> + nvm-postfix = "i2s";
> };
> };

If one provides the entire suffix, including the underscore, then you can
simplify the code:

snprintf(config.fwname, sizeof(config.fwname), "qca/nvm_%08x%s.bin", soc_ver, suffix ? suffix : "");

Regards.

2019-04-09 10:15:35

by Rocky Liao

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: net: bluetooth: Add device property nvm-postfix for QCA6174

On 2019-04-04 20:32, Marc Gonzalez wrote:
> +robh
>
> On 04/04/2019 11:08, Rocky Liao wrote:
>
>> This patchs patch adds an optional device property nvm-postfix to
>> allow the
>> driver to load customized nvm file based on this property
>
> While text /before/ is indeed called a "prefix", text /after/ is not a
> "postfix",
> but a "suffix".
>
>
>> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> index 824c0e2..70cda4b 100644
>> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> @@ -16,6 +16,7 @@ Optional properties for compatible string
>> qcom,qca6174-bt:
>>
>> - enable-gpios: gpio specifier used to enable chip
>> - clocks: clock provided to the controller (SUSCLK_32KHZ)
>> + - nvm-postfix: nvm file postfix to load customized nvm file
>
> The device tree is supposed to describe hardware.
>
> The name of which file to load can hardly be considered part of the HW.
>
> Possible solutions:
> 1) derive the file name from the compatible string
> 2) pass the name as a module parameter
> 3) something else
>
>
>> @@ -39,6 +40,7 @@ serial@7570000 {
>>
>> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
>> clocks = <&divclk4>;
>> + nvm-postfix = "i2s";
>> };
>> };
>
> If one provides the entire suffix, including the underscore, then you
> can
> simplify the code:
>
> snprintf(config.fwname, sizeof(config.fwname), "qca/nvm_%08x%s.bin",
> soc_ver, suffix ? suffix : "");
>
> Regards
.
Hi Marc,

The major purpose for that property is about the BT audio bus type, can
it be considered as part of the HW? If yes maybe we can use a property
name "audio-bus" to reflect that.

If not then I will adopt the solution 1 to add a new compatible string
"{ .compatible = "qcom,qca6174-bt-i2s" }" and load specific nvm for this
compatible string, please feel free to let me know if any other
concerns.

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project

2019-04-09 13:54:27

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: net: bluetooth: Add device property nvm-postfix for QCA6174

On Thu, Apr 4, 2019 at 7:32 AM Marc Gonzalez <[email protected]> wrote:
>
> +robh
>
> On 04/04/2019 11:08, Rocky Liao wrote:
>
> > This patchs patch adds an optional device property nvm-postfix to allow the
> > driver to load customized nvm file based on this property
>
> While text /before/ is indeed called a "prefix", text /after/ is not a "postfix",
> but a "suffix".
>
>
> > Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> > index 824c0e2..70cda4b 100644
> > --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> > +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> > @@ -16,6 +16,7 @@ Optional properties for compatible string qcom,qca6174-bt:
> >
> > - enable-gpios: gpio specifier used to enable chip
> > - clocks: clock provided to the controller (SUSCLK_32KHZ)
> > + - nvm-postfix: nvm file postfix to load customized nvm file
>
> The device tree is supposed to describe hardware.
>
> The name of which file to load can hardly be considered part of the HW.
>
> Possible solutions:
> 1) derive the file name from the compatible string
> 2) pass the name as a module parameter
> 3) something else

Or use the 'firmware-name' property to define the full firmware filename.

Rob

2019-04-09 14:03:25

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: net: bluetooth: Add device property nvm-postfix for QCA6174

On Tue, Apr 9, 2019 at 5:15 AM Rocky Liao <[email protected]> wrote:
>
> On 2019-04-04 20:32, Marc Gonzalez wrote:
> > +robh
> >
> > On 04/04/2019 11:08, Rocky Liao wrote:
> >
> >> This patchs patch adds an optional device property nvm-postfix to
> >> allow the
> >> driver to load customized nvm file based on this property
> >
> > While text /before/ is indeed called a "prefix", text /after/ is not a
> > "postfix",
> > but a "suffix".
> >
> >
> >> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> >> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> >> index 824c0e2..70cda4b 100644
> >> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> >> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> >> @@ -16,6 +16,7 @@ Optional properties for compatible string
> >> qcom,qca6174-bt:
> >>
> >> - enable-gpios: gpio specifier used to enable chip
> >> - clocks: clock provided to the controller (SUSCLK_32KHZ)
> >> + - nvm-postfix: nvm file postfix to load customized nvm file
> >
> > The device tree is supposed to describe hardware.
> >
> > The name of which file to load can hardly be considered part of the HW.
> >
> > Possible solutions:
> > 1) derive the file name from the compatible string
> > 2) pass the name as a module parameter
> > 3) something else
> >
> >
> >> @@ -39,6 +40,7 @@ serial@7570000 {
> >>
> >> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
> >> clocks = <&divclk4>;
> >> + nvm-postfix = "i2s";
> >> };
> >> };
> >
> > If one provides the entire suffix, including the underscore, then you
> > can
> > simplify the code:
> >
> > snprintf(config.fwname, sizeof(config.fwname), "qca/nvm_%08x%s.bin",
> > soc_ver, suffix ? suffix : "");
> >
> > Regards
> .
> Hi Marc,
>
> The major purpose for that property is about the BT audio bus type, can
> it be considered as part of the HW? If yes maybe we can use a property
> name "audio-bus" to reflect that.
>
> If not then I will adopt the solution 1 to add a new compatible string
> "{ .compatible = "qcom,qca6174-bt-i2s" }" and load specific nvm for this
> compatible string, please feel free to let me know if any other
> concerns.

I don't think the suggestion was to add the nvm string to the
compatible, but rather compatible strings serve as a map key. Having
board specific firmware files for wifi/bt is pretty common, but
parameters for 'i2s' is a bit strange. So a better explanation of what
parameters this contains would help. How/when does it vary, for
example?

Also, if it is only a handful of parameters, making them DT properties
is preferred.

Rob

2019-04-09 14:49:05

by Rocky Liao

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: net: bluetooth: Add device property nvm-postfix for QCA6174

On 2019-04-09 22:03, Rob Herring wrote:
> On Tue, Apr 9, 2019 at 5:15 AM Rocky Liao <[email protected]>
> wrote:
>>
>> On 2019-04-04 20:32, Marc Gonzalez wrote:
>> > +robh
>> >
>> > On 04/04/2019 11:08, Rocky Liao wrote:
>> >
>> >> This patchs patch adds an optional device property nvm-postfix to
>> >> allow the
>> >> driver to load customized nvm file based on this property
>> >
>> > While text /before/ is indeed called a "prefix", text /after/ is not a
>> > "postfix",
>> > but a "suffix".
>> >
>> >
>> >> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
>> >> 1 file changed, 2 insertions(+)
>> >>
>> >> diff --git
>> >> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> >> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> >> index 824c0e2..70cda4b 100644
>> >> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> >> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> >> @@ -16,6 +16,7 @@ Optional properties for compatible string
>> >> qcom,qca6174-bt:
>> >>
>> >> - enable-gpios: gpio specifier used to enable chip
>> >> - clocks: clock provided to the controller (SUSCLK_32KHZ)
>> >> + - nvm-postfix: nvm file postfix to load customized nvm file
>> >
>> > The device tree is supposed to describe hardware.
>> >
>> > The name of which file to load can hardly be considered part of the HW.
>> >
>> > Possible solutions:
>> > 1) derive the file name from the compatible string
>> > 2) pass the name as a module parameter
>> > 3) something else
>> >
>> >
>> >> @@ -39,6 +40,7 @@ serial@7570000 {
>> >>
>> >> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
>> >> clocks = <&divclk4>;
>> >> + nvm-postfix = "i2s";
>> >> };
>> >> };
>> >
>> > If one provides the entire suffix, including the underscore, then you
>> > can
>> > simplify the code:
>> >
>> > snprintf(config.fwname, sizeof(config.fwname), "qca/nvm_%08x%s.bin",
>> > soc_ver, suffix ? suffix : "");
>> >
>> > Regards
>> .
>> Hi Marc,
>>
>> The major purpose for that property is about the BT audio bus type,
>> can
>> it be considered as part of the HW? If yes maybe we can use a property
>> name "audio-bus" to reflect that.
>>
>> If not then I will adopt the solution 1 to add a new compatible string
>> "{ .compatible = "qcom,qca6174-bt-i2s" }" and load specific nvm for
>> this
>> compatible string, please feel free to let me know if any other
>> concerns.
>
> I don't think the suggestion was to add the nvm string to the
> compatible, but rather compatible strings serve as a map key. Having
> board specific firmware files for wifi/bt is pretty common, but
> parameters for 'i2s' is a bit strange. So a better explanation of what
> parameters this contains would help. How/when does it vary, for
> example?
>
> Also, if it is only a handful of parameters, making them DT properties
> is preferred.
>
> Rob

Ok, I prefer to go with adding a device property "firmware-name" with
full firmware name as you suggested in previous comment.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project

2019-04-10 09:28:13

by Rocky Liao

[permalink] [raw]
Subject: [PATCH v3 1/2] Bluetooth: hci_qca: Load customized NVM based on the device property

QCA BTSOC NVM is a customized firmware file and different vendors may
want to have different BTSOC configuration (e.g. Configure SCO over PCM
or I2S, Setting Tx power, etc.) via this file. This patch will allow
vendors to download different NVM firmware file by reading a device
property "firmware-name".

Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v3:
* added firmware-name instead of nvm-postfix to specify full firmware name
---
drivers/bluetooth/btqca.c | 14 ++++++++++----
drivers/bluetooth/btqca.h | 6 ++++--
drivers/bluetooth/hci_qca.c | 19 ++++++++++++++++++-
3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 6122685..645a893 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -332,7 +332,8 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);

int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
- enum qca_btsoc_type soc_type, u32 soc_ver)
+ enum qca_btsoc_type soc_type, u32 soc_ver,
+ const char *firmware_name)
{
struct rome_config config;
int err;
@@ -368,9 +369,14 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
if (soc_type == QCA_WCN3990)
snprintf(config.fwname, sizeof(config.fwname),
"qca/crnv%02x.bin", rom_ver);
- else
- snprintf(config.fwname, sizeof(config.fwname),
- "qca/nvm_%08x.bin", soc_ver);
+ else {
+ if (firmware_name)
+ snprintf(config.fwname, sizeof(config.fwname),
+ "qca/%s", firmware_name);
+ else
+ snprintf(config.fwname, sizeof(config.fwname),
+ "qca/nvm_%08x.bin", soc_ver);
+ }

err = qca_download_firmware(hdev, &config);
if (err < 0) {
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index 6fdc25d..41dc806 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -139,7 +139,8 @@ enum qca_btsoc_type {

int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
- enum qca_btsoc_type soc_type, u32 soc_ver);
+ enum qca_btsoc_type soc_type, u32 soc_ver,
+ const char *firmware_name);
int qca_read_soc_version(struct hci_dev *hdev, u32 *soc_version);
int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
#else
@@ -150,7 +151,8 @@ static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdad
}

static inline int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
- enum qca_btsoc_type soc_type, u32 soc_ver)
+ enum qca_btsoc_type soc_type, u32 soc_ver,
+ const char *firmware_name)
{
return -EOPNOTSUPP;
}
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 4ea995d..9b8d4d7 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -168,6 +168,7 @@ struct qca_serdev {
struct qca_power *bt_power;
u32 init_speed;
u32 oper_speed;
+ const char *firmware_name;
};

static int qca_power_setup(struct hci_uart *hu, bool on);
@@ -189,6 +190,17 @@ static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
return soc_type;
}

+static const char *qca_get_firmware_name(struct hci_uart *hu)
+{
+ if (hu->serdev) {
+ struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
+
+ return qsd->firmware_name;
+ } else {
+ return NULL;
+ }
+}
+
static void __serial_clock_on(struct tty_struct *tty)
{
/* TODO: Some chipset requires to enable UART clock on client
@@ -1191,6 +1203,7 @@ static int qca_setup(struct hci_uart *hu)
struct qca_data *qca = hu->priv;
unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
enum qca_btsoc_type soc_type = qca_soc_type(hu);
+ const char *firmware_name = qca_get_firmware_name(hu);
int ret;
int soc_ver = 0;

@@ -1241,7 +1254,8 @@ static int qca_setup(struct hci_uart *hu)

bt_dev_info(hdev, "QCA controller version 0x%08x", soc_ver);
/* Setup patch / NVM configurations */
- ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver);
+ ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver,
+ firmware_name);
if (!ret) {
set_bit(STATE_IN_BAND_SLEEP_ENABLED, &qca->flags);
qca_debugfs_init(hdev);
@@ -1462,6 +1476,9 @@ static int qca_serdev_probe(struct serdev_device *serdev)
return PTR_ERR(qcadev->bt_en);
}

+ device_property_read_string(&serdev->dev, "firmware-name",
+ &qcadev->firmware_name);
+
qcadev->susclk = devm_clk_get(&serdev->dev, NULL);
if (IS_ERR(qcadev->susclk)) {
dev_err(&serdev->dev, "failed to acquire clk\n");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


2019-04-10 09:28:41

by Rocky Liao

[permalink] [raw]
Subject: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device property firmware-name for QCA6174

This patch adds an optional device property "firmware-name" to allow the
driver to load customized nvm firmware file based on this property.

Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v3:
* added firmware-name instead of nvm-postfix to specify full firmware name
---
Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
index 824c0e2..2bcea50 100644
--- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
+++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
@@ -16,6 +16,7 @@ Optional properties for compatible string qcom,qca6174-bt:

- enable-gpios: gpio specifier used to enable chip
- clocks: clock provided to the controller (SUSCLK_32KHZ)
+ - firmware-name: specify the name of nvm firmware to load

Required properties for compatible string qcom,wcn3990-bt:

@@ -39,6 +40,7 @@ serial@7570000 {

enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
clocks = <&divclk4>;
+ firmware-name = "nvm_00440302.bin";
};
};

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


2019-04-23 17:07:03

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device property firmware-name for QCA6174

Hi Rocky,

> This patch adds an optional device property "firmware-name" to allow the
> driver to load customized nvm firmware file based on this property.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
> Changes in v3:
> * added firmware-name instead of nvm-postfix to specify full firmware name
> ---
> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> index 824c0e2..2bcea50 100644
> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> @@ -16,6 +16,7 @@ Optional properties for compatible string qcom,qca6174-bt:
>
> - enable-gpios: gpio specifier used to enable chip
> - clocks: clock provided to the controller (SUSCLK_32KHZ)
> + - firmware-name: specify the name of nvm firmware to load
>
> Required properties for compatible string qcom,wcn3990-bt:
>
> @@ -39,6 +40,7 @@ serial@7570000 {
>
> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
> clocks = <&divclk4>;
> + firmware-name = "nvm_00440302.bin";
> };

and how is this a firmware-name property. Wouldn’t this be more like nvm-file or something along these lines. This really needs to be cleared with Rob to pick the right property name.

Regards

Marcel


2019-04-24 06:19:16

by Rocky Liao

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device property firmware-name for QCA6174

On 2019-04-24 01:06, Marcel Holtmann wrote:
> Hi Rocky,
>
>> This patch adds an optional device property "firmware-name" to allow
>> the
>> driver to load customized nvm firmware file based on this property.
>>
>> Signed-off-by: Rocky Liao <[email protected]>
>> ---
>> Changes in v3:
>> * added firmware-name instead of nvm-postfix to specify full firmware
>> name
>> ---
>> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> index 824c0e2..2bcea50 100644
>> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>> @@ -16,6 +16,7 @@ Optional properties for compatible string
>> qcom,qca6174-bt:
>>
>> - enable-gpios: gpio specifier used to enable chip
>> - clocks: clock provided to the controller (SUSCLK_32KHZ)
>> + - firmware-name: specify the name of nvm firmware to load
>>
>> Required properties for compatible string qcom,wcn3990-bt:
>>
>> @@ -39,6 +40,7 @@ serial@7570000 {
>>
>> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
>> clocks = <&divclk4>;
>> + firmware-name = "nvm_00440302.bin";
>> };
>
> and how is this a firmware-name property. Wouldn’t this be more like
> nvm-file or something along these lines. This really needs to be
> cleared with Rob to pick the right property name.
>
> Regards
>
> Marcel

Hi Rob,

Are you OK to use a property name "nvm-file" or "firmware-nvm-file"?
Actually we have two firmware files, one is the patch file which is
common to all of the products, the other is the nvm file which is
customized. Using a "nvm-file" or "firmware-nvm-file" property name
would be more clear.

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project

2019-04-26 17:46:48

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device property firmware-name for QCA6174

On Wed, Apr 24, 2019 at 1:19 AM Rocky Liao <[email protected]> wrote:
>
> On 2019-04-24 01:06, Marcel Holtmann wrote:
> > Hi Rocky,
> >
> >> This patch adds an optional device property "firmware-name" to allow
> >> the
> >> driver to load customized nvm firmware file based on this property.
> >>
> >> Signed-off-by: Rocky Liao <[email protected]>
> >> ---
> >> Changes in v3:
> >> * added firmware-name instead of nvm-postfix to specify full firmware
> >> name
> >> ---
> >> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> >> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> >> index 824c0e2..2bcea50 100644
> >> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> >> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
> >> @@ -16,6 +16,7 @@ Optional properties for compatible string
> >> qcom,qca6174-bt:
> >>
> >> - enable-gpios: gpio specifier used to enable chip
> >> - clocks: clock provided to the controller (SUSCLK_32KHZ)
> >> + - firmware-name: specify the name of nvm firmware to load
> >>
> >> Required properties for compatible string qcom,wcn3990-bt:
> >>
> >> @@ -39,6 +40,7 @@ serial@7570000 {
> >>
> >> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
> >> clocks = <&divclk4>;
> >> + firmware-name = "nvm_00440302.bin";
> >> };
> >
> > and how is this a firmware-name property. Wouldn’t this be more like
> > nvm-file or something along these lines. This really needs to be
> > cleared with Rob to pick the right property name.
> >
> > Regards
> >
> > Marcel
>
> Hi Rob,
>
> Are you OK to use a property name "nvm-file" or "firmware-nvm-file"?
> Actually we have two firmware files, one is the patch file which is
> common to all of the products, the other is the nvm file which is
> customized. Using a "nvm-file" or "firmware-nvm-file" property name
> would be more clear.

'firmware-name' is the standard name for specifying firmware file names.

Rob

2019-05-05 17:33:23

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] dt-bindings: net: bluetooth: Add device property firmware-name for QCA6174

Hi Rocky,

>>>>>> This patch adds an optional device property "firmware-name" to allow
>>>>>> the
>>>>>> driver to load customized nvm firmware file based on this property.
>>>>>> Signed-off-by: Rocky Liao <[email protected]>
>>>>>> ---
>>>>>> Changes in v3:
>>>>>> * added firmware-name instead of nvm-postfix to specify full firmware
>>>>>> name
>>>>>> ---
>>>>>> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
>>>>>> 1 file changed, 2 insertions(+)
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>>>>>> b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>>>>>> index 824c0e2..2bcea50 100644
>>>>>> --- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>>>>>> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
>>>>>> @@ -16,6 +16,7 @@ Optional properties for compatible string
>>>>>> qcom,qca6174-bt:
>>>>>> - enable-gpios: gpio specifier used to enable chip
>>>>>> - clocks: clock provided to the controller (SUSCLK_32KHZ)
>>>>>> + - firmware-name: specify the name of nvm firmware to load
>>>>>> Required properties for compatible string qcom,wcn3990-bt:
>>>>>> @@ -39,6 +40,7 @@ serial@7570000 {
>>>>>> enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
>>>>>> clocks = <&divclk4>;
>>>>>> + firmware-name = "nvm_00440302.bin";
>>>>>> };
>>>>> and how is this a firmware-name property. Wouldn’t this be more like
>>>>> nvm-file or something along these lines. This really needs to be
>>>>> cleared with Rob to pick the right property name.
>>>>> Regards
>>>>> Marcel
>>>> Hi Rob,
>>>> Are you OK to use a property name "nvm-file" or "firmware-nvm-file"?
>>>> Actually we have two firmware files, one is the patch file which is
>>>> common to all of the products, the other is the nvm file which is
>>>> customized. Using a "nvm-file" or "firmware-nvm-file" property name
>>>> would be more clear.
>>> 'firmware-name' is the standard name for specifying firmware file names.
>> but it is not a firmware file, it is a NVM file. What happens if in
>> the future they need a firmware file and a NVM file?
>> Regards
>> Marcel
>
> We won't need to specify a rampatch firmware file in future as it's a same file for all the boards with same chip, only the NVM firmware file may have board differences. NVM file is also one of the firmware files so I think it should be OK to use "firmware-name" property to specify it.

ok then, but I need patches that apply cleanly against bluetooth-next.

Regards

Marcel

2019-05-12 03:20:36

by Rocky Liao

[permalink] [raw]
Subject: [PATCH v4 2/2] dt-bindings: net: bluetooth: Add device property firmware-name for QCA6174

This patch adds an optional device property "firmware-name" to allow the
driver to load customized nvm firmware file based on this property.

Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v4:
* rebased the code base and merge with latest code
---
Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
index 7ef6118..7a3eda7 100644
--- a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
+++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt
@@ -17,6 +17,7 @@ Optional properties for compatible string qcom,qca6174-bt:

- enable-gpios: gpio specifier used to enable chip
- clocks: clock provided to the controller (SUSCLK_32KHZ)
+ - firmware-name: specify the name of nvm firmware to load

Required properties for compatible string qcom,wcn399x-bt:

@@ -40,6 +41,7 @@ serial@7570000 {

enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;
clocks = <&divclk4>;
+ firmware-name = "nvm_00440302.bin";
};
};

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project

2019-05-13 16:46:13

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] dt-bindings: net: bluetooth: Add device property firmware-name for QCA6174

On Sun, 12 May 2019 11:19:45 +0800, Rocky Liao wrote:
> This patch adds an optional device property "firmware-name" to allow the
> driver to load customized nvm firmware file based on this property.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
> Changes in v4:
> * rebased the code base and merge with latest code
> ---
> Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt | 2 ++
> 1 file changed, 2 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>