mbox series

[v2,0/3] Bluetooth: hci_qca: Add serdev support

Message ID 20180313164444.19881-1-thierry.escande@linaro.org
Headers show
Series Bluetooth: hci_qca: Add serdev support | expand

Message

Thierry Escande March 13, 2018, 4:44 p.m. UTC 
Hi,

This patchset enables the Qualcomm BT controller QCA6174 node in the
device tree of the db820c board. This allows the bluetooth chipset to
be probed and registered against the hci layer by using the serdev
framework.

This patchset also contains the documentation for the compatible
string "qcom,qca6174-bt" related to this chipset.

v2:
- Fixed author email

Thierry Escande (3):
  arm64: dts: apq8096-db820c: enable bluetooth node
  dt-bindings: net: bluetooth: Add qualcomm-bluetooth
  Bluetooth: hci_qca: Add serdev support

 .../devicetree/bindings/net/qualcomm-bluetooth.txt |  34 +++++++
 arch/arm64/boot/dts/qcom/apq8096-db820c-pins.dtsi  |  14 +++
 .../boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi    |  17 ++++
 arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi       |  32 +++++++
 arch/arm64/boot/dts/qcom/msm8996.dtsi              |  10 ++
 drivers/bluetooth/Kconfig                          |   2 +-
 drivers/bluetooth/hci_qca.c                        | 102 ++++++++++++++++++++-
 7 files changed, 208 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt

-- 
2.14.1

Comments

Andy Shevchenko March 13, 2018, 4:50 p.m. UTC | #1 
On Tue, Mar 13, 2018 at 6:44 PM, Thierry Escande
<thierry.escande@linaro.org> wrote:
> Add support for Qualcomm serial slave devices. Probe the serial device,

> retrieve its maximum speed and register a new hci uart device.


> +#include <linux/of.h>


What exactly this is used for?

> +       qcadev->bt_en = devm_gpiod_get(&serdev->dev, "bt-disable-n",

> +                                      GPIOD_OUT_LOW);

> +       if (IS_ERR(qcadev->bt_en)) {

> +               dev_err(&serdev->dev, "failed to acquire bt-disable-n gpio\n");

> +               return PTR_ERR(qcadev->bt_en);

> +       }


> +       gpiod_set_value(qcadev->bt_en, 0);


Redundant.

> +       clk_set_rate(qcadev->divclk4, DIVCLK4_RATE_32KHZ);


> +       clk_prepare_enable(qcadev->divclk4);


This may fail.

> +       return hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);

> +}


> +       clk_disable(qcadev->divclk4);

> +       clk_unprepare(qcadev->divclk4);


One call.

> +}


> +static const struct of_device_id qca_bluetooth_of_match[] = {

> +       { .compatible = "qcom,qca6174-bt" },


> +       { },


No comma.

> +};


-- 
With Best Regards,
Andy Shevchenko
Bjorn Andersson March 13, 2018, 5:45 p.m. UTC | #2 
On Tue 13 Mar 09:44 PDT 2018, Thierry Escande wrote:
> @@ -50,6 +54,9 @@

>  #define IBS_TX_IDLE_TIMEOUT_MS		2000

>  #define BAUDRATE_SETTLE_TIMEOUT_MS	300

>  

> +/* divclk4 rate */


The clock is called "susclk" in the BT chip, "divclk4" is the board
specific name for the clock source.

> +#define DIVCLK4_RATE_32KHZ	32768

> +

>  /* HCI_IBS transmit side sleep protocol states */

>  enum tx_ibs_states {

>  	HCI_IBS_TX_ASLEEP,

> @@ -111,6 +118,12 @@ struct qca_data {

>  	u64 votes_off;

>  };

>  

> +struct qca_serdev {

> +	struct hci_uart	 serdev_hu;

> +	struct gpio_desc *bt_en;

> +	struct clk	 *divclk4;


Rename this to "susclk", or simply "clk".

> +};


Apart from this and Andy's comments this looks good.

Regards,
Bjorn