| Message ID | 20180314155514.3374-3-thierry.escande@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Hi Marcel, On 14/03/2018 20:51, Marcel Holtmann wrote: > Hi Bjorn, > >>>>>> + 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. >>> >> >> BT_DISABLE_N is the name of this pin in the datasheet of the QCA chip, >> not on the board, so this name is the same regardless of what you name >> the line or gpio your board connect it to. > > and QCA chip v1 and QCA chip v2 will use the same driver and same firmware loading mechanism. So why do we have to add a new GPIO naming if they decide to change the name in the data sheet. With Bluetooth it is pretty much all the same. Every UART chip has a shutdown/reset GPIO to enable/disable the chip behind the UART. > >>> 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. >>> >> >> I totally agree with the fact that the board should not affect the >> naming of the gpio in the driver. But I do enjoy when we refer to pins >> by their real name - instead of having to guess which pin in the _chip_ >> specification the driver actually refer to. >> >> >> That said, what name would you prefer for this? >> >> Afaict this is not "wakeup" and there are a few extra steps between the >> disabled state and "bluetooth is enabled", so "enable" feels slightly >> wrong. And it probably should be "bluetooth" and not just "device" as >> this refers to a pin on a WiFi/BT combo chip. > > The Broadcom side called it shutdown GPIO, it is essentially the shutdown/reset GPIO or power on/off GPIO. Personally I do not care what it is named, but it will be all the same for all Bluetooth chips. Take a poll from Broadcom, Intel, Realtek and Qualcomm and you can pick a reasonable common name. The Nokia driver has "bluetooth-wakeup" gpio. The Broadcom one has "device-wakeup" and "shutdown". The "shutdown" gpio is set to its active state to power on the chip which sounds reversed logic. Same for the "bt-disable-n" gpio in the Qualcomm driver, configured as ACTIVE_HIGH, and which is set to 1 to enable it... So for consistency, naming it as "shutdown" to stick to the bcm driver but it should be configured as ACTIVE_LOW in the dts so we actually do a gpiod_set_value(0) to un-shutdown it. Does that sound ok? Regards, Thierry > For the wakeup GPIOs, these are different and depend on if there is some low-power mode provided. You would need to check the data sheet to see if they provide more advanced low-power state handling. > > Regards > > Marcel > -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt b/Documentation/devicetree/bindings/net/qualcomm-bluetooth.txt new file mode 100644 index 000000000000..cdb14b96c229 --- /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" + - bt-disable-n-gpios: gpio specifier, used to enable chip during probe + - 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"; + + bt-disable-n-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; + + pinctrl-names = "default"; + pinctrl-0 = <&bt_en_pin_a>; + + clocks = <&divclk4>; + }; +};
Add binding document for serial bluetooth chips using Qualcomm protocol. Signed-off-by: Thierry Escande <thierry.escande@linaro.org> --- 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 -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html