* [PATCH v5 0/3] Bluetooth: hci_qca: Add serdev support @ 2018-03-20 3:23 Thierry Escande 2018-03-20 3:23 ` [PATCH v5 1/3] arm64: dts: apq8096-db820c: enable bluetooth node Thierry Escande ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Thierry Escande @ 2018-03-20 3:23 UTC (permalink / raw) To: Rob Herring, Andy Gross, Marcel Holtmann, Johan Hedberg, David Brown, Mark Rutland Cc: Andy Shevchenko, Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel 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. v5: - Rename 'bt-disable-n' gpio as 'enable' 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 | 33 +++++++ arch/arm64/boot/dts/qcom/msm8996.dtsi | 10 ++ drivers/bluetooth/Kconfig | 1 + drivers/bluetooth/hci_qca.c | 109 ++++++++++++++++++++- 7 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt -- 2.14.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 1/3] arm64: dts: apq8096-db820c: enable bluetooth node 2018-03-20 3:23 [PATCH v5 0/3] Bluetooth: hci_qca: Add serdev support Thierry Escande @ 2018-03-20 3:23 ` Thierry Escande 2018-03-20 3:23 ` [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth Thierry Escande 2018-03-20 3:23 ` [PATCH v5 3/3] Bluetooth: hci_qca: Add serdev support Thierry Escande 2 siblings, 0 replies; 13+ messages in thread From: Thierry Escande @ 2018-03-20 3:23 UTC (permalink / raw) To: Rob Herring, Andy Gross, Marcel Holtmann, Johan Hedberg, David Brown, Mark Rutland Cc: Andy Shevchenko, Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel Add a new serial node for the Qualcomm BT controller QCA6174. This allows automatic probing and hci registration through the serdev framework instead of relying on the userspace helpers. Signed-off-by: Thierry Escande <thierry.escande@linaro.org> --- v5: - Rename 'bt-disable-n' gpio as 'enable' v4: no change v3: no change v2: - Fix author email 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 | 33 ++++++++++++++++++++++ arch/arm64/boot/dts/qcom/msm8996.dtsi | 10 +++++++ 4 files changed, 74 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c-pins.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c-pins.dtsi index 24552f19b3fa..172165d84669 100644 --- a/arch/arm64/boot/dts/qcom/apq8096-db820c-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c-pins.dtsi @@ -36,4 +36,18 @@ drive-strength = <2>; /* 2 MA */ }; }; + + blsp1_uart1_default: blsp1_uart1_default { + function = "blsp_uart2"; + pins = "gpio41", "gpio42", "gpio43", "gpio44"; + drive-strength = <16>; + bias-disable; + }; + + blsp1_uart1_sleep: blsp1_uart1_sleep { + function = "gpio"; + pins = "gpio41", "gpio42", "gpio43", "gpio44"; + drive-strength = <2>; + bias-disable; + }; }; diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi index 59b29ddfb6e9..f8d2a3b10b1f 100644 --- a/arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi @@ -26,6 +26,23 @@ }; }; + divclk4_pin_a: divclk4 { + pins = "gpio18"; + function = "func2"; + + bias-disable; + power-source = <PM8994_GPIO_S4>; + }; + + bt_en_pin_a: bt-en-active { + pins = "gpio19"; + function = "normal"; + + output-low; + power-source = <PM8994_GPIO_S4>; + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>; + }; + usb3_vbus_det_gpio: pm8996_gpio22 { pinconf { pins = "gpio22"; diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi index 1c8f1b86472d..8b3bcdf0d718 100644 --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi @@ -23,6 +23,7 @@ aliases { serial0 = &blsp2_uart1; serial1 = &blsp2_uart2; + serial2 = &blsp1_uart1; i2c0 = &blsp1_i2c2; i2c1 = &blsp2_i2c1; i2c2 = &blsp2_i2c0; @@ -34,7 +35,39 @@ stdout-path = "serial0:115200n8"; }; + clocks { + divclk4: divclk4 { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <32768>; + clock-output-names = "divclk4"; + + pinctrl-names = "default"; + pinctrl-0 = <&divclk4_pin_a>; + }; + }; + soc { + serial@7570000 { + label = "BT-UART"; + status = "okay"; + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&blsp1_uart1_default>; + pinctrl-1 = <&blsp1_uart1_sleep>; + + bluetooth { + compatible = "qcom,qca6174-bt"; + + /* bt_disable_n gpio */ + enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; + + pinctrl-names = "default"; + pinctrl-0 = <&bt_en_pin_a>; + + clocks = <&divclk4>; + }; + }; + serial@75b0000 { label = "LS-UART1"; status = "okay"; diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi index 0a6f7952bbb1..2d54a86a027f 100644 --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi @@ -408,6 +408,16 @@ #clock-cells = <1>; }; + blsp1_uart1: serial@7570000 { + compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm"; + reg = <0x07570000 0x1000>; + interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, + <&gcc GCC_BLSP1_AHB_CLK>; + clock-names = "core", "iface"; + status = "disabled"; + }; + blsp1_spi0: spi@7575000 { compatible = "qcom,spi-qup-v2.2.1"; reg = <0x07575000 0x600>; -- 2.14.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth 2018-03-20 3:23 [PATCH v5 0/3] Bluetooth: hci_qca: Add serdev support Thierry Escande 2018-03-20 3:23 ` [PATCH v5 1/3] arm64: dts: apq8096-db820c: enable bluetooth node Thierry Escande @ 2018-03-20 3:23 ` Thierry Escande 2018-03-20 15:58 ` Marcel Holtmann 2018-03-26 22:23 ` Rob Herring 2018-03-20 3:23 ` [PATCH v5 3/3] Bluetooth: hci_qca: Add serdev support Thierry Escande 2 siblings, 2 replies; 13+ messages in thread From: Thierry Escande @ 2018-03-20 3:23 UTC (permalink / raw) To: Rob Herring, Andy Gross, Marcel Holtmann, Johan Hedberg, David Brown, Mark Rutland Cc: Andy Shevchenko, Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel Add binding document for serial bluetooth chips using Qualcomm protocol. Signed-off-by: Thierry Escande <thierry.escande@linaro.org> --- v5: - Rename 'bt-disable-n' gpio as 'enable' v4: - Move bt-disable-n-gpios to required properties section - Add clocks and pinctrl-0 as required properties v3: no change v2: no change .../devicetree/bindings/net/qualcomm-bluetooth.txt | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt new file mode 100644 index 000000000000..bbc2973634b2 --- /dev/null +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt @@ -0,0 +1,34 @@ +Qualcomm Bluetooth Chips +--------------------- + +This documents the binding structure and common properties for serial +attached Qualcomm devices. + +Serial attached Qualcomm devices shall be a child node of the host UART +device the slave device is attached to. + +Required properties: + - compatible: should contain one of the following: + * "qcom,qca6174-bt" + - enable-gpios: gpio specifier used to enable chip + - pinctrl-0: pin phandle for bt_en gpio + - clocks: clock phandle for SUSCLK_32KHZ + +Example: + +serial@7570000 { + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&blsp1_uart1_default>; + pinctrl-1 = <&blsp1_uart1_sleep>; + + bluetooth { + compatible = "qcom,qca6174-bt"; + + enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; + + pinctrl-names = "default"; + pinctrl-0 = <&bt_en_pin_a>; + + clocks = <&divclk4>; + }; +}; -- 2.14.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth 2018-03-20 3:23 ` [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth Thierry Escande @ 2018-03-20 15:58 ` Marcel Holtmann 2018-03-26 16:45 ` Thierry Escande 2018-03-26 22:51 ` Bjorn Andersson 2018-03-26 22:23 ` Rob Herring 1 sibling, 2 replies; 13+ messages in thread From: Marcel Holtmann @ 2018-03-20 15:58 UTC (permalink / raw) To: Thierry Escande Cc: Rob Herring, Andy Gross, Johan Hedberg, David Brown, Mark Rutland, Andy Shevchenko, Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel Hi Thierry, > Add binding document for serial bluetooth chips using Qualcomm protocol. > > Signed-off-by: Thierry Escande <thierry.escande@linaro.org> > --- > > v5: > - Rename 'bt-disable-n' gpio as 'enable' > > v4: > - Move bt-disable-n-gpios to required properties section > - Add clocks and pinctrl-0 as required properties > > v3: no change > v2: no change > > .../devicetree/bindings/net/qualcomm-bluetooth.txt | 34 ++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > > diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > new file mode 100644 > index 000000000000..bbc2973634b2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt > @@ -0,0 +1,34 @@ > +Qualcomm Bluetooth Chips > +--------------------- > + > +This documents the binding structure and common properties for serial > +attached Qualcomm devices. > + > +Serial attached Qualcomm devices shall be a child node of the host UART > +device the slave device is attached to. > + > +Required properties: > + - compatible: should contain one of the following: > + * "qcom,qca6174-bt" > + - enable-gpios: gpio specifier used to enable chip > + - pinctrl-0: pin phandle for bt_en gpio > + - clocks: clock phandle for SUSCLK_32KHZ if I compare this with broadcom-bluetooth.txt or ti-bluetooth.txt then besides compatible, everything else is optional. The nokia-bluetooth.txt has everything required, but that is also a really specific platform. Can we be less restrictive for a QCA general purpose chip? > + > +Example: > + > +serial@7570000 { > + pinctrl-names = "default", "sleep"; > + pinctrl-0 = <&blsp1_uart1_default>; > + pinctrl-1 = <&blsp1_uart1_sleep>; > + > + bluetooth { > + compatible = "qcom,qca6174-bt"; > + > + enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&bt_en_pin_a>; This one I do not understand and you might want to shed some light into why this is done that way. > + > + clocks = <&divclk4>; No clock-names? Regards Marcel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth 2018-03-20 15:58 ` Marcel Holtmann @ 2018-03-26 16:45 ` Thierry Escande 2018-03-26 22:51 ` Bjorn Andersson 1 sibling, 0 replies; 13+ messages in thread From: Thierry Escande @ 2018-03-26 16:45 UTC (permalink / raw) To: Marcel Holtmann Cc: Rob Herring, Andy Gross, Johan Hedberg, David Brown, Mark Rutland, Andy Shevchenko, Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel Hi Marcel, On 20/03/2018 16:58, Marcel Holtmann wrote: > Hi Thierry, > >> Add binding document for serial bluetooth chips using Qualcomm protocol. >> >> Signed-off-by: Thierry Escande <thierry.escande@linaro.org> >> --- >> >> v5: >> - Rename 'bt-disable-n' gpio as 'enable' >> >> v4: >> - Move bt-disable-n-gpios to required properties section >> - Add clocks and pinctrl-0 as required properties >> >> v3: no change >> v2: no change >> >> .../devicetree/bindings/net/qualcomm-bluetooth.txt | 34 ++++++++++++++++++++++ >> 1 file changed, 34 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> >> diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> new file mode 100644 >> index 000000000000..bbc2973634b2 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt >> @@ -0,0 +1,34 @@ >> +Qualcomm Bluetooth Chips >> +--------------------- >> + >> +This documents the binding structure and common properties for serial >> +attached Qualcomm devices. >> + >> +Serial attached Qualcomm devices shall be a child node of the host UART >> +device the slave device is attached to. >> + >> +Required properties: >> + - compatible: should contain one of the following: >> + * "qcom,qca6174-bt" >> + - enable-gpios: gpio specifier used to enable chip >> + - pinctrl-0: pin phandle for bt_en gpio >> + - clocks: clock phandle for SUSCLK_32KHZ > > if I compare this with broadcom-bluetooth.txt or ti-bluetooth.txt then besides compatible, everything else is optional. The nokia-bluetooth.txt has everything required, but that is also a really specific platform. > > Can we be less restrictive for a QCA general purpose chip? Ok. To me, at least the enable gpio seems required. > >> + >> +Example: >> + >> +serial@7570000 { >> + pinctrl-names = "default", "sleep"; >> + pinctrl-0 = <&blsp1_uart1_default>; >> + pinctrl-1 = <&blsp1_uart1_sleep>; >> + >> + bluetooth { >> + compatible = "qcom,qca6174-bt"; >> + >> + enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; >> + >> + pinctrl-names = "default"; >> + pinctrl-0 = <&bt_en_pin_a>; > > This one I do not understand and you might want to shed some light into why this is done that way. Sure. It reclaims this gpio pin for this device. Will add it to the documentation. > >> + >> + clocks = <&divclk4>; > > No clock-names? No need for a name as there is only one clk obtained by passing a NULL id to devm_gpiod_get(). Regards, Thierry > > Regards > > Marcel > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth 2018-03-20 15:58 ` Marcel Holtmann 2018-03-26 16:45 ` Thierry Escande @ 2018-03-26 22:51 ` Bjorn Andersson 2018-03-27 15:56 ` Thierry Escande 1 sibling, 1 reply; 13+ messages in thread From: Bjorn Andersson @ 2018-03-26 22:51 UTC (permalink / raw) To: Marcel Holtmann Cc: Thierry Escande, Rob Herring, Andy Gross, Johan Hedberg, David Brown, Mark Rutland, Andy Shevchenko, Loic Poulain, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel On Tue 20 Mar 23:58 HKT 2018, Marcel Holtmann wrote: > > Signed-off-by: Thierry Escande <thierry.escande@linaro.org> [..] > > + - clocks: clock phandle for SUSCLK_32KHZ > > if I compare this with broadcom-bluetooth.txt or ti-bluetooth.txt then > besides compatible, everything else is optional. The > nokia-bluetooth.txt has everything required, but that is also a really > specific platform. > > Can we be less restrictive for a QCA general purpose chip? > The way we deal with this in other bindings is that we tie such requirements to the compatible; i.e. we would say that qcom,qca6174-bt requires a clock and we would have something like qcom,qca-bt that makes it optional. The beauty of this is that the driver will tell you if you forgot to specify the clock when it actually is required, which saves you considerable amount of debugging time. NB. The way the bcm driver handles this is insufficient, as it treats any error from clk_get as "there's no clock specified". The driver should accept a clock not being specified, but should fail properly when a clock is specified but can't be acquired (e.g. due to clk_get() returning EPROBE_DEFER). > > + > > +Example: > > + > > +serial@7570000 { > > + pinctrl-names = "default", "sleep"; > > + pinctrl-0 = <&blsp1_uart1_default>; > > + pinctrl-1 = <&blsp1_uart1_sleep>; > > + > > + bluetooth { > > + compatible = "qcom,qca6174-bt"; > > + > > + enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; > > + > > + pinctrl-names = "default"; > > + pinctrl-0 = <&bt_en_pin_a>; > > This one I do not understand and you might want to shed some light > into why this is done that way. > This is completely generic and only relates to getting the electrical properties of the gpio pin set up correctly. So I would recommend that we omit this from the binding and example (including the pinctrl properties for the serial above). Regards, Bjorn ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth 2018-03-26 22:51 ` Bjorn Andersson @ 2018-03-27 15:56 ` Thierry Escande 2018-03-27 18:47 ` Bjorn Andersson 0 siblings, 1 reply; 13+ messages in thread From: Thierry Escande @ 2018-03-27 15:56 UTC (permalink / raw) To: Bjorn Andersson, Marcel Holtmann Cc: Rob Herring, Andy Gross, Johan Hedberg, David Brown, Mark Rutland, Andy Shevchenko, Loic Poulain, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel Hi Bjorn, On 27/03/2018 00:51, Bjorn Andersson wrote: > On Tue 20 Mar 23:58 HKT 2018, Marcel Holtmann wrote: >>> Signed-off-by: Thierry Escande <thierry.escande@linaro.org> > [..] >>> + - clocks: clock phandle for SUSCLK_32KHZ >> >> if I compare this with broadcom-bluetooth.txt or ti-bluetooth.txt then >> besides compatible, everything else is optional. The >> nokia-bluetooth.txt has everything required, but that is also a really >> specific platform. >> >> Can we be less restrictive for a QCA general purpose chip? >> > > The way we deal with this in other bindings is that we tie such > requirements to the compatible; i.e. we would say that qcom,qca6174-bt > requires a clock and we would have something like qcom,qca-bt that makes > it optional. > > The beauty of this is that the driver will tell you if you forgot to > specify the clock when it actually is required, which saves you > considerable amount of debugging time. > > > NB. The way the bcm driver handles this is insufficient, as it treats > any error from clk_get as "there's no clock specified". The driver > should accept a clock not being specified, but should fail properly when > a clock is specified but can't be acquired (e.g. due to clk_get() > returning EPROBE_DEFER). > >>> + >>> +Example: >>> + >>> +serial@7570000 { >>> + pinctrl-names = "default", "sleep"; >>> + pinctrl-0 = <&blsp1_uart1_default>; >>> + pinctrl-1 = <&blsp1_uart1_sleep>; >>> + >>> + bluetooth { >>> + compatible = "qcom,qca6174-bt"; >>> + >>> + enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; >>> + >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&bt_en_pin_a>; >> >> This one I do not understand and you might want to shed some light >> into why this is done that way. >> > > This is completely generic and only relates to getting the electrical > properties of the gpio pin set up correctly. So I would recommend that > we omit this from the binding and example (including the pinctrl > properties for the serial above). If I remove the pinctrl entry in the bluetooth node, the gpio19 is then marked as unclaimed. The drive strength also defaults to low but that doesn't seem to be an issue and the the chip can still be enabled through gpio19. Is it ok to have it unclaimed? If so I can remove it from the binding and the doc then. Regarding the blsp1_uart1_default of the serial node, I can still enable the chip if I remove it but the hci commands all end in timeout. It seems that the function for these pins has to be explicitly set to blsp_uart2. So at least, the default pinctrl seems mandatory. Regards, Thierry > > Regards, > Bjorn > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth 2018-03-27 15:56 ` Thierry Escande @ 2018-03-27 18:47 ` Bjorn Andersson 0 siblings, 0 replies; 13+ messages in thread From: Bjorn Andersson @ 2018-03-27 18:47 UTC (permalink / raw) To: Thierry Escande Cc: Marcel Holtmann, Rob Herring, Andy Gross, Johan Hedberg, David Brown, Mark Rutland, Andy Shevchenko, Loic Poulain, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel On Tue 27 Mar 08:56 PDT 2018, Thierry Escande wrote: > Hi Bjorn, > > On 27/03/2018 00:51, Bjorn Andersson wrote: > > On Tue 20 Mar 23:58 HKT 2018, Marcel Holtmann wrote: > > > > Signed-off-by: Thierry Escande <thierry.escande@linaro.org> > > [..] > > > > + - clocks: clock phandle for SUSCLK_32KHZ > > > > > > if I compare this with broadcom-bluetooth.txt or ti-bluetooth.txt then > > > besides compatible, everything else is optional. The > > > nokia-bluetooth.txt has everything required, but that is also a really > > > specific platform. > > > > > > Can we be less restrictive for a QCA general purpose chip? > > > > > > > The way we deal with this in other bindings is that we tie such > > requirements to the compatible; i.e. we would say that qcom,qca6174-bt > > requires a clock and we would have something like qcom,qca-bt that makes > > it optional. > > > > The beauty of this is that the driver will tell you if you forgot to > > specify the clock when it actually is required, which saves you > > considerable amount of debugging time. > > > > > > NB. The way the bcm driver handles this is insufficient, as it treats > > any error from clk_get as "there's no clock specified". The driver > > should accept a clock not being specified, but should fail properly when > > a clock is specified but can't be acquired (e.g. due to clk_get() > > returning EPROBE_DEFER). > > > > > > + > > > > +Example: > > > > + > > > > +serial@7570000 { > > > > + pinctrl-names = "default", "sleep"; > > > > + pinctrl-0 = <&blsp1_uart1_default>; > > > > + pinctrl-1 = <&blsp1_uart1_sleep>; > > > > + > > > > + bluetooth { > > > > + compatible = "qcom,qca6174-bt"; > > > > + > > > > + enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; > > > > + > > > > + pinctrl-names = "default"; > > > > + pinctrl-0 = <&bt_en_pin_a>; > > > > > > This one I do not understand and you might want to shed some light > > > into why this is done that way. > > > > > > > This is completely generic and only relates to getting the electrical > > properties of the gpio pin set up correctly. So I would recommend that > > we omit this from the binding and example (including the pinctrl > > properties for the serial above). > > If I remove the pinctrl entry in the bluetooth node, the gpio19 is then > marked as unclaimed. The drive strength also defaults to low but that > doesn't seem to be an issue and the the chip can still be enabled through > gpio19. Is it ok to have it unclaimed? If so I can remove it from the > binding and the doc then. > > Regarding the blsp1_uart1_default of the serial node, I can still enable the > chip if I remove it but the hci commands all end in timeout. It seems that > the function for these pins has to be explicitly set to blsp_uart2. So at > least, the default pinctrl seems mandatory. > Our board needs these properties to get the uart and gpio in the right state, but this is unrelated to BT - that's why I suggested that you omit these properties from the BT binding. Regards, Bjorn ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth 2018-03-20 3:23 ` [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth Thierry Escande 2018-03-20 15:58 ` Marcel Holtmann @ 2018-03-26 22:23 ` Rob Herring 1 sibling, 0 replies; 13+ messages in thread From: Rob Herring @ 2018-03-26 22:23 UTC (permalink / raw) To: Thierry Escande Cc: Andy Gross, Marcel Holtmann, Johan Hedberg, David Brown, Mark Rutland, Andy Shevchenko, Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel On Tue, Mar 20, 2018 at 04:23:30AM +0100, Thierry Escande wrote: > Add binding document for serial bluetooth chips using Qualcomm protocol. > > Signed-off-by: Thierry Escande <thierry.escande@linaro.org> > --- > > v5: > - Rename 'bt-disable-n' gpio as 'enable' > > v4: > - Move bt-disable-n-gpios to required properties section > - Add clocks and pinctrl-0 as required properties > > v3: no change > v2: no change > > .../devicetree/bindings/net/qualcomm-bluetooth.txt | 34 ++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 3/3] Bluetooth: hci_qca: Add serdev support 2018-03-20 3:23 [PATCH v5 0/3] Bluetooth: hci_qca: Add serdev support Thierry Escande 2018-03-20 3:23 ` [PATCH v5 1/3] arm64: dts: apq8096-db820c: enable bluetooth node Thierry Escande 2018-03-20 3:23 ` [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth Thierry Escande @ 2018-03-20 3:23 ` Thierry Escande 2018-03-20 10:54 ` Andy Shevchenko 2018-03-20 15:49 ` Marcel Holtmann 2 siblings, 2 replies; 13+ messages in thread From: Thierry Escande @ 2018-03-20 3:23 UTC (permalink / raw) To: Rob Herring, Andy Gross, Marcel Holtmann, Johan Hedberg, David Brown, Mark Rutland Cc: Andy Shevchenko, Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel Add support for Qualcomm serial slave devices. Probe the serial device, retrieve its maximum speed and register a new hci uart device. Signed-off-by: Thierry Escande <thierry.escande@linaro.org> --- v5: - Use gpio new name 'enable' v4: - Rename divclk4 as susclk (its name in the bt chip) - Use gpiod_set_value_cansleep() - Replace #include <linux/of.h> with <linux/mod_devicetable.h> - Restore dependency on BT_HCIUART v3: - Remove redundant call to gpiod_set_value() after devm_gpiod_get() - Check returned values for clk_set_rate() and clk_prepare_enable() - Use clk_disable_unprepare() v2: - Fix author email drivers/bluetooth/Kconfig | 1 + drivers/bluetooth/hci_qca.c | 109 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 108 insertions(+), 2 deletions(-) diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig index 07e55cd8f8c8..e0f1a6609b68 100644 --- a/drivers/bluetooth/Kconfig +++ b/drivers/bluetooth/Kconfig @@ -196,6 +196,7 @@ config BT_HCIUART_BCM config BT_HCIUART_QCA bool "Qualcomm Atheros protocol support" depends on BT_HCIUART + depends on BT_HCIUART_SERDEV select BT_HCIUART_H4 select BT_QCA help diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index 05ec530b8a3a..6e6042f77784 100644 --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -29,7 +29,11 @@ */ #include <linux/kernel.h> +#include <linux/clk.h> #include <linux/debugfs.h> +#include <linux/gpio/consumer.h> +#include <linux/mod_devicetable.h> +#include <linux/serdev.h> #include <net/bluetooth/bluetooth.h> #include <net/bluetooth/hci_core.h> @@ -50,6 +54,9 @@ #define IBS_TX_IDLE_TIMEOUT_MS 2000 #define BAUDRATE_SETTLE_TIMEOUT_MS 300 +/* susclk rate */ +#define SUSCLK_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 *susclk; +}; + static void __serial_clock_on(struct tty_struct *tty) { /* TODO: Some chipset requires to enable UART clock on client @@ -386,6 +399,7 @@ static void hci_ibs_wake_retrans_timeout(struct timer_list *t) /* Initialize protocol */ static int qca_open(struct hci_uart *hu) { + struct qca_serdev *qcadev; struct qca_data *qca; BT_DBG("hu %p qca_open", hu); @@ -444,6 +458,13 @@ static int qca_open(struct hci_uart *hu) timer_setup(&qca->tx_idle_timer, hci_ibs_tx_idle_timeout, 0); qca->tx_idle_delay = IBS_TX_IDLE_TIMEOUT_MS; + if (hu->serdev) { + serdev_device_open(hu->serdev); + + qcadev = serdev_device_get_drvdata(hu->serdev); + gpiod_set_value_cansleep(qcadev->bt_en, 1); + } + BT_DBG("HCI_UART_QCA open, tx_idle_delay=%u, wake_retrans=%u", qca->tx_idle_delay, qca->wake_retrans); @@ -512,6 +533,7 @@ static int qca_flush(struct hci_uart *hu) /* Close protocol */ static int qca_close(struct hci_uart *hu) { + struct qca_serdev *qcadev; struct qca_data *qca = hu->priv; BT_DBG("hu %p qca close", hu); @@ -525,6 +547,13 @@ static int qca_close(struct hci_uart *hu) destroy_workqueue(qca->workqueue); qca->hu = NULL; + if (hu->serdev) { + serdev_device_close(hu->serdev); + + qcadev = serdev_device_get_drvdata(hu->serdev); + gpiod_set_value_cansleep(qcadev->bt_en, 0); + } + kfree_skb(qca->rx_skb); hu->priv = NULL; @@ -885,6 +914,14 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate) return 0; } +static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed) +{ + if (hu->serdev) + serdev_device_set_baudrate(hu->serdev, speed); + else + hci_uart_set_baudrate(hu, speed); +} + static int qca_setup(struct hci_uart *hu) { struct hci_dev *hdev = hu->hdev; @@ -905,7 +942,7 @@ static int qca_setup(struct hci_uart *hu) speed = hu->proto->init_speed; if (speed) - hci_uart_set_baudrate(hu, speed); + host_set_baudrate(hu, speed); /* Setup user speed if needed */ speed = 0; @@ -924,7 +961,7 @@ static int qca_setup(struct hci_uart *hu) ret); return ret; } - hci_uart_set_baudrate(hu, speed); + host_set_baudrate(hu, speed); } /* Setup patch / NVM configurations */ @@ -958,12 +995,80 @@ static struct hci_uart_proto qca_proto = { .dequeue = qca_dequeue, }; +static int qca_serdev_probe(struct serdev_device *serdev) +{ + struct qca_serdev *qcadev; + int err; + + qcadev = devm_kzalloc(&serdev->dev, sizeof(*qcadev), GFP_KERNEL); + if (!qcadev) + return -ENOMEM; + + qcadev->serdev_hu.serdev = serdev; + serdev_device_set_drvdata(serdev, qcadev); + + qcadev->bt_en = devm_gpiod_get(&serdev->dev, "enable", + 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); + } + + qcadev->susclk = devm_clk_get(&serdev->dev, NULL); + if (IS_ERR(qcadev->susclk)) { + dev_err(&serdev->dev, "failed to acquire clk\n"); + return PTR_ERR(qcadev->susclk); + } + + err = clk_set_rate(qcadev->susclk, SUSCLK_RATE_32KHZ); + if (err) + return err; + + err = clk_prepare_enable(qcadev->susclk); + if (err) + return err; + + err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto); + if (err) + clk_disable_unprepare(qcadev->susclk); + + return err; +} + +static void qca_serdev_remove(struct serdev_device *serdev) +{ + struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev); + + hci_uart_unregister_device(&qcadev->serdev_hu); + + clk_disable_unprepare(qcadev->susclk); +} + +static const struct of_device_id qca_bluetooth_of_match[] = { + { .compatible = "qcom,qca6174-bt" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match); + +static struct serdev_device_driver qca_serdev_driver = { + .probe = qca_serdev_probe, + .remove = qca_serdev_remove, + .driver = { + .name = "hci_uart_qca", + .of_match_table = qca_bluetooth_of_match, + }, +}; + int __init qca_init(void) { + serdev_device_driver_register(&qca_serdev_driver); + return hci_uart_register_proto(&qca_proto); } int __exit qca_deinit(void) { + serdev_device_driver_unregister(&qca_serdev_driver); + return hci_uart_unregister_proto(&qca_proto); } -- 2.14.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/3] Bluetooth: hci_qca: Add serdev support 2018-03-20 3:23 ` [PATCH v5 3/3] Bluetooth: hci_qca: Add serdev support Thierry Escande @ 2018-03-20 10:54 ` Andy Shevchenko 2018-03-20 15:49 ` Marcel Holtmann 1 sibling, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2018-03-20 10:54 UTC (permalink / raw) To: Thierry Escande, Rob Herring, Andy Gross, Marcel Holtmann, Johan Hedberg, David Brown, Mark Rutland Cc: Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel On Tue, 2018-03-20 at 04:23 +0100, Thierry Escande wrote: > Add support for Qualcomm serial slave devices. Probe the serial > device, > retrieve its maximum speed and register a new hci uart device. > Tell me, if I'm not mistaken, I gave you my tag for this. If no, FWIW, Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Thierry Escande <thierry.escande@linaro.org> > --- > > v5: > - Use gpio new name 'enable' > > v4: > - Rename divclk4 as susclk (its name in the bt chip) > - Use gpiod_set_value_cansleep() > - Replace #include <linux/of.h> with <linux/mod_devicetable.h> > - Restore dependency on BT_HCIUART > > v3: > - Remove redundant call to gpiod_set_value() after devm_gpiod_get() > - Check returned values for clk_set_rate() and clk_prepare_enable() > - Use clk_disable_unprepare() > > v2: > - Fix author email > > drivers/bluetooth/Kconfig | 1 + > drivers/bluetooth/hci_qca.c | 109 > +++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 108 insertions(+), 2 deletions(-) > > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig > index 07e55cd8f8c8..e0f1a6609b68 100644 > --- a/drivers/bluetooth/Kconfig > +++ b/drivers/bluetooth/Kconfig > @@ -196,6 +196,7 @@ config BT_HCIUART_BCM > config BT_HCIUART_QCA > bool "Qualcomm Atheros protocol support" > depends on BT_HCIUART > + depends on BT_HCIUART_SERDEV > select BT_HCIUART_H4 > select BT_QCA > help > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index 05ec530b8a3a..6e6042f77784 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -29,7 +29,11 @@ > */ > > #include <linux/kernel.h> > +#include <linux/clk.h> > #include <linux/debugfs.h> > +#include <linux/gpio/consumer.h> > +#include <linux/mod_devicetable.h> > +#include <linux/serdev.h> > > #include <net/bluetooth/bluetooth.h> > #include <net/bluetooth/hci_core.h> > @@ -50,6 +54,9 @@ > #define IBS_TX_IDLE_TIMEOUT_MS 2000 > #define BAUDRATE_SETTLE_TIMEOUT_MS 300 > > +/* susclk rate */ > +#define SUSCLK_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 *susclk; > +}; > + > static void __serial_clock_on(struct tty_struct *tty) > { > /* TODO: Some chipset requires to enable UART clock on client > @@ -386,6 +399,7 @@ static void hci_ibs_wake_retrans_timeout(struct > timer_list *t) > /* Initialize protocol */ > static int qca_open(struct hci_uart *hu) > { > + struct qca_serdev *qcadev; > struct qca_data *qca; > > BT_DBG("hu %p qca_open", hu); > @@ -444,6 +458,13 @@ static int qca_open(struct hci_uart *hu) > timer_setup(&qca->tx_idle_timer, hci_ibs_tx_idle_timeout, 0); > qca->tx_idle_delay = IBS_TX_IDLE_TIMEOUT_MS; > > + if (hu->serdev) { > + serdev_device_open(hu->serdev); > + > + qcadev = serdev_device_get_drvdata(hu->serdev); > + gpiod_set_value_cansleep(qcadev->bt_en, 1); > + } > + > BT_DBG("HCI_UART_QCA open, tx_idle_delay=%u, > wake_retrans=%u", > qca->tx_idle_delay, qca->wake_retrans); > > @@ -512,6 +533,7 @@ static int qca_flush(struct hci_uart *hu) > /* Close protocol */ > static int qca_close(struct hci_uart *hu) > { > + struct qca_serdev *qcadev; > struct qca_data *qca = hu->priv; > > BT_DBG("hu %p qca close", hu); > @@ -525,6 +547,13 @@ static int qca_close(struct hci_uart *hu) > destroy_workqueue(qca->workqueue); > qca->hu = NULL; > > + if (hu->serdev) { > + serdev_device_close(hu->serdev); > + > + qcadev = serdev_device_get_drvdata(hu->serdev); > + gpiod_set_value_cansleep(qcadev->bt_en, 0); > + } > + > kfree_skb(qca->rx_skb); > > hu->priv = NULL; > @@ -885,6 +914,14 @@ static int qca_set_baudrate(struct hci_dev *hdev, > uint8_t baudrate) > return 0; > } > > +static inline void host_set_baudrate(struct hci_uart *hu, unsigned > int speed) > +{ > + if (hu->serdev) > + serdev_device_set_baudrate(hu->serdev, speed); > + else > + hci_uart_set_baudrate(hu, speed); > +} > + > static int qca_setup(struct hci_uart *hu) > { > struct hci_dev *hdev = hu->hdev; > @@ -905,7 +942,7 @@ static int qca_setup(struct hci_uart *hu) > speed = hu->proto->init_speed; > > if (speed) > - hci_uart_set_baudrate(hu, speed); > + host_set_baudrate(hu, speed); > > /* Setup user speed if needed */ > speed = 0; > @@ -924,7 +961,7 @@ static int qca_setup(struct hci_uart *hu) > ret); > return ret; > } > - hci_uart_set_baudrate(hu, speed); > + host_set_baudrate(hu, speed); > } > > /* Setup patch / NVM configurations */ > @@ -958,12 +995,80 @@ static struct hci_uart_proto qca_proto = { > .dequeue = qca_dequeue, > }; > > +static int qca_serdev_probe(struct serdev_device *serdev) > +{ > + struct qca_serdev *qcadev; > + int err; > + > + qcadev = devm_kzalloc(&serdev->dev, sizeof(*qcadev), > GFP_KERNEL); > + if (!qcadev) > + return -ENOMEM; > + > + qcadev->serdev_hu.serdev = serdev; > + serdev_device_set_drvdata(serdev, qcadev); > + > + qcadev->bt_en = devm_gpiod_get(&serdev->dev, "enable", > + 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); > + } > + > + qcadev->susclk = devm_clk_get(&serdev->dev, NULL); > + if (IS_ERR(qcadev->susclk)) { > + dev_err(&serdev->dev, "failed to acquire clk\n"); > + return PTR_ERR(qcadev->susclk); > + } > + > + err = clk_set_rate(qcadev->susclk, SUSCLK_RATE_32KHZ); > + if (err) > + return err; > + > + err = clk_prepare_enable(qcadev->susclk); > + if (err) > + return err; > + > + err = hci_uart_register_device(&qcadev->serdev_hu, > &qca_proto); > + if (err) > + clk_disable_unprepare(qcadev->susclk); > + > + return err; > +} > + > +static void qca_serdev_remove(struct serdev_device *serdev) > +{ > + struct qca_serdev *qcadev = > serdev_device_get_drvdata(serdev); > + > + hci_uart_unregister_device(&qcadev->serdev_hu); > + > + clk_disable_unprepare(qcadev->susclk); > +} > + > +static const struct of_device_id qca_bluetooth_of_match[] = { > + { .compatible = "qcom,qca6174-bt" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match); > + > +static struct serdev_device_driver qca_serdev_driver = { > + .probe = qca_serdev_probe, > + .remove = qca_serdev_remove, > + .driver = { > + .name = "hci_uart_qca", > + .of_match_table = qca_bluetooth_of_match, > + }, > +}; > + > int __init qca_init(void) > { > + serdev_device_driver_register(&qca_serdev_driver); > + > return hci_uart_register_proto(&qca_proto); > } > > int __exit qca_deinit(void) > { > + serdev_device_driver_unregister(&qca_serdev_driver); > + > return hci_uart_unregister_proto(&qca_proto); > } -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/3] Bluetooth: hci_qca: Add serdev support 2018-03-20 3:23 ` [PATCH v5 3/3] Bluetooth: hci_qca: Add serdev support Thierry Escande 2018-03-20 10:54 ` Andy Shevchenko @ 2018-03-20 15:49 ` Marcel Holtmann 2018-03-26 16:44 ` Thierry Escande 1 sibling, 1 reply; 13+ messages in thread From: Marcel Holtmann @ 2018-03-20 15:49 UTC (permalink / raw) To: Thierry Escande Cc: Rob Herring, Andy Gross, Johan Hedberg, David Brown, Mark Rutland, Andy Shevchenko, Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel Hi Thierry, > Add support for Qualcomm serial slave devices. Probe the serial device, > retrieve its maximum speed and register a new hci uart device. > > Signed-off-by: Thierry Escande <thierry.escande@linaro.org> > --- > > v5: > - Use gpio new name 'enable' > > v4: > - Rename divclk4 as susclk (its name in the bt chip) > - Use gpiod_set_value_cansleep() > - Replace #include <linux/of.h> with <linux/mod_devicetable.h> > - Restore dependency on BT_HCIUART > > v3: > - Remove redundant call to gpiod_set_value() after devm_gpiod_get() > - Check returned values for clk_set_rate() and clk_prepare_enable() > - Use clk_disable_unprepare() > > v2: > - Fix author email > > drivers/bluetooth/Kconfig | 1 + > drivers/bluetooth/hci_qca.c | 109 +++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 108 insertions(+), 2 deletions(-) > > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig > index 07e55cd8f8c8..e0f1a6609b68 100644 > --- a/drivers/bluetooth/Kconfig > +++ b/drivers/bluetooth/Kconfig > @@ -196,6 +196,7 @@ config BT_HCIUART_BCM > config BT_HCIUART_QCA > bool "Qualcomm Atheros protocol support" > depends on BT_HCIUART > + depends on BT_HCIUART_SERDEV > select BT_HCIUART_H4 > select BT_QCA > help > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index 05ec530b8a3a..6e6042f77784 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -29,7 +29,11 @@ > */ > > #include <linux/kernel.h> > +#include <linux/clk.h> > #include <linux/debugfs.h> > +#include <linux/gpio/consumer.h> > +#include <linux/mod_devicetable.h> > +#include <linux/serdev.h> > > #include <net/bluetooth/bluetooth.h> > #include <net/bluetooth/hci_core.h> > @@ -50,6 +54,9 @@ > #define IBS_TX_IDLE_TIMEOUT_MS 2000 > #define BAUDRATE_SETTLE_TIMEOUT_MS 300 > > +/* susclk rate */ > +#define SUSCLK_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 *susclk; > +}; > + > static void __serial_clock_on(struct tty_struct *tty) > { > /* TODO: Some chipset requires to enable UART clock on client > @@ -386,6 +399,7 @@ static void hci_ibs_wake_retrans_timeout(struct timer_list *t) > /* Initialize protocol */ > static int qca_open(struct hci_uart *hu) > { > + struct qca_serdev *qcadev; > struct qca_data *qca; > > BT_DBG("hu %p qca_open", hu); > @@ -444,6 +458,13 @@ static int qca_open(struct hci_uart *hu) > timer_setup(&qca->tx_idle_timer, hci_ibs_tx_idle_timeout, 0); > qca->tx_idle_delay = IBS_TX_IDLE_TIMEOUT_MS; > > + if (hu->serdev) { > + serdev_device_open(hu->serdev); > + > + qcadev = serdev_device_get_drvdata(hu->serdev); > + gpiod_set_value_cansleep(qcadev->bt_en, 1); > + } > + > BT_DBG("HCI_UART_QCA open, tx_idle_delay=%u, wake_retrans=%u", > qca->tx_idle_delay, qca->wake_retrans); > > @@ -512,6 +533,7 @@ static int qca_flush(struct hci_uart *hu) > /* Close protocol */ > static int qca_close(struct hci_uart *hu) > { > + struct qca_serdev *qcadev; > struct qca_data *qca = hu->priv; > > BT_DBG("hu %p qca close", hu); > @@ -525,6 +547,13 @@ static int qca_close(struct hci_uart *hu) > destroy_workqueue(qca->workqueue); > qca->hu = NULL; > > + if (hu->serdev) { > + serdev_device_close(hu->serdev); > + > + qcadev = serdev_device_get_drvdata(hu->serdev); > + gpiod_set_value_cansleep(qcadev->bt_en, 0); > + } > + > kfree_skb(qca->rx_skb); > > hu->priv = NULL; > @@ -885,6 +914,14 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate) > return 0; > } > > +static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed) > +{ > + if (hu->serdev) > + serdev_device_set_baudrate(hu->serdev, speed); > + else > + hci_uart_set_baudrate(hu, speed); > +} > + > static int qca_setup(struct hci_uart *hu) > { > struct hci_dev *hdev = hu->hdev; > @@ -905,7 +942,7 @@ static int qca_setup(struct hci_uart *hu) > speed = hu->proto->init_speed; > > if (speed) > - hci_uart_set_baudrate(hu, speed); > + host_set_baudrate(hu, speed); > > /* Setup user speed if needed */ > speed = 0; > @@ -924,7 +961,7 @@ static int qca_setup(struct hci_uart *hu) > ret); > return ret; > } > - hci_uart_set_baudrate(hu, speed); > + host_set_baudrate(hu, speed); > } > > /* Setup patch / NVM configurations */ > @@ -958,12 +995,80 @@ static struct hci_uart_proto qca_proto = { > .dequeue = qca_dequeue, > }; > > +static int qca_serdev_probe(struct serdev_device *serdev) > +{ > + struct qca_serdev *qcadev; > + int err; > + > + qcadev = devm_kzalloc(&serdev->dev, sizeof(*qcadev), GFP_KERNEL); > + if (!qcadev) > + return -ENOMEM; > + > + qcadev->serdev_hu.serdev = serdev; > + serdev_device_set_drvdata(serdev, qcadev); > + > + qcadev->bt_en = devm_gpiod_get(&serdev->dev, "enable", > + 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); > + } > + > + qcadev->susclk = devm_clk_get(&serdev->dev, NULL); > + if (IS_ERR(qcadev->susclk)) { > + dev_err(&serdev->dev, "failed to acquire clk\n"); > + return PTR_ERR(qcadev->susclk); > + } > + > + err = clk_set_rate(qcadev->susclk, SUSCLK_RATE_32KHZ); > + if (err) > + return err; > + > + err = clk_prepare_enable(qcadev->susclk); > + if (err) > + return err; > + > + err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto); > + if (err) > + clk_disable_unprepare(qcadev->susclk); > + > + return err; > +} so this a more generic question. Does the clk setup has to be done in serdev probe or can we just do that within qca_open callback. I asked because I really want to move towards btuart.c and integrate the vendor specific pieces there nicely. So what I did was that I posted a v2 that has the vendor abstraction build in and it would be super simple to add qca support to it. However I have no vendor specific handling from within the probe callback. If that is not needed and we can do all the clk and GPIO setup in the vendor open callback, then it should be fairly simple to do (I am ignoring IBS support for now, but I realize it is there). That all said, the hci_qca.c code has __serial_clock_on() and __serial_clock_off() empty stubs. Is this about the susclk or is that something totally different? Regards Marcel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/3] Bluetooth: hci_qca: Add serdev support 2018-03-20 15:49 ` Marcel Holtmann @ 2018-03-26 16:44 ` Thierry Escande 0 siblings, 0 replies; 13+ messages in thread From: Thierry Escande @ 2018-03-26 16:44 UTC (permalink / raw) To: Marcel Holtmann Cc: Rob Herring, Andy Gross, Johan Hedberg, David Brown, Mark Rutland, Andy Shevchenko, Loic Poulain, Bjorn Andersson, Srinivas Kandagatla, linux-bluetooth, linux-arm-msm, devicetree, linux-kernel Hi Marcel, On 20/03/2018 16:49, Marcel Holtmann wrote: > Hi Thierry, > >> Add support for Qualcomm serial slave devices. Probe the serial device, >> retrieve its maximum speed and register a new hci uart device. >> >> Signed-off-by: Thierry Escande <thierry.escande@linaro.org> >> --- >> >> v5: >> - Use gpio new name 'enable' >> >> v4: >> - Rename divclk4 as susclk (its name in the bt chip) >> - Use gpiod_set_value_cansleep() >> - Replace #include <linux/of.h> with <linux/mod_devicetable.h> >> - Restore dependency on BT_HCIUART >> >> v3: >> - Remove redundant call to gpiod_set_value() after devm_gpiod_get() >> - Check returned values for clk_set_rate() and clk_prepare_enable() >> - Use clk_disable_unprepare() >> >> v2: >> - Fix author email >> >> drivers/bluetooth/Kconfig | 1 + >> drivers/bluetooth/hci_qca.c | 109 +++++++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 108 insertions(+), 2 deletions(-) >> <snip> > > so this a more generic question. Does the clk setup has to be done in serdev probe or can we just do that within qca_open callback. I asked because I really want to move towards btuart.c and integrate the vendor specific pieces there nicely. So what I did was that I posted a v2 that has the vendor abstraction build in and it would be super simple to add qca support to it. However I have no vendor specific handling from within the probe callback. If that is not needed and we can do all the clk and GPIO setup in the vendor open callback, then it should be fairly simple to do (I am ignoring IBS support for now, but I realize it is there). I did test that and doing clk and gpio setups in qca_open seems ok. > > That all said, the hci_qca.c code has __serial_clock_on() and __serial_clock_off() empty stubs. Is this about the susclk or is that something totally different? afaiu these stubs are used to control host UART clock. The susclk concerns the bt chip itself. Regards, Thierry > > Regards > > Marcel > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-03-27 18:47 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-20 3:23 [PATCH v5 0/3] Bluetooth: hci_qca: Add serdev support Thierry Escande 2018-03-20 3:23 ` [PATCH v5 1/3] arm64: dts: apq8096-db820c: enable bluetooth node Thierry Escande 2018-03-20 3:23 ` [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth Thierry Escande 2018-03-20 15:58 ` Marcel Holtmann 2018-03-26 16:45 ` Thierry Escande 2018-03-26 22:51 ` Bjorn Andersson 2018-03-27 15:56 ` Thierry Escande 2018-03-27 18:47 ` Bjorn Andersson 2018-03-26 22:23 ` Rob Herring 2018-03-20 3:23 ` [PATCH v5 3/3] Bluetooth: hci_qca: Add serdev support Thierry Escande 2018-03-20 10:54 ` Andy Shevchenko 2018-03-20 15:49 ` Marcel Holtmann 2018-03-26 16:44 ` Thierry Escande
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).