mbox series

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

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

Message

Thierry Escande March 14, 2018, 3:55 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.

v4:
- Fix dt binding documentation
- Address some other issues in patch #3

v3:
- Address comments for patch #3 (details in patch)

v2:
- Fix 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                          |   1 +
 drivers/bluetooth/hci_qca.c                        | 109 ++++++++++++++++++++-
 7 files changed, 215 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt

-- 
2.14.1

Comments

Bjorn Andersson March 14, 2018, 6:30 p.m. UTC | #1 
On Wed 14 Mar 09:13 PDT 2018, Marcel Holtmann wrote:
> > +		bt-disable-n-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;

> 

> can we use a common name here. I think that Nokia and Broadcom drivers

> define one. And if this is the enable/shutdown GPIO, we should name it

> consistently across all manufacturers. It essentially does the same on

> Bluetooth UART chips no matter what chip is behind them.

> 


Broadcomm has a device-wakup-gpios and Nokia has bluetooth-wakup-gpios.
It might be that these behave in the same way, but from the description
they only trigger the wakeup.

The reason for the proposed naming here is that the pin is named
"BT_DISABLE_N" in the datasheet.

Regards,
Bjorn
Marcel Holtmann March 14, 2018, 6:42 p.m. UTC | #2 
Hi Bjoern,

>>> +		bt-disable-n-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>;

>> 

>> can we use a common name here. I think that Nokia and Broadcom drivers

>> define one. And if this is the enable/shutdown GPIO, we should name it

>> consistently across all manufacturers. It essentially does the same on

>> Bluetooth UART chips no matter what chip is behind them.

>> 

> 

> Broadcomm has a device-wakup-gpios and Nokia has bluetooth-wakup-gpios.

> It might be that these behave in the same way, but from the description

> they only trigger the wakeup.


that is something that we might need to start fixing. I really prefer if we name the GPIOs a bit more consistent.

> The reason for the proposed naming here is that the pin is named

> "BT_DISABLE_N" in the datasheet.


That is not a reason I buy. So the next board comes around that labels it in the data sheet BT_DISABLE_YEAH_SUPER_GREAT and you send me a patch to the driver to look for that name. If the GPIO does the same thing, I couldn’t care less what the data sheet says. That might be a comment in the DT file, but it should not pollute the driver code.

A new board should not require driver changes, you just ship a new DT for that board and an existing driver hopefully just does the job. No matter how someone named a GPIO in a piece of paper.

Regards

Marcel