Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Bjorn Andersson]

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 
> > [..]
> > > > + - 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@757 {
> > > > +   pinctrl-names = "default", "sleep";
> > > > +   pinctrl-0 = <_uart1_default>;
> > > > +   pinctrl-1 = <_uart1_sleep>;
> > > > +
> > > > +   bluetooth {
> > > > +   compatible = "qcom,qca6174-bt";
> > > > +
> > > > +   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
> > > > +
> > > > +   pinctrl-names = "default";
> > > > +   pinctrl-0 = <_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

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Bjorn Andersson]

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 
> > [..]
> > > > + - 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@757 {
> > > > +   pinctrl-names = "default", "sleep";
> > > > +   pinctrl-0 = <_uart1_default>;
> > > > +   pinctrl-1 = <_uart1_sleep>;
> > > > +
> > > > +   bluetooth {
> > > > +   compatible = "qcom,qca6174-bt";
> > > > +
> > > > +   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
> > > > +
> > > > +   pinctrl-names = "default";
> > > > +   pinctrl-0 = <_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

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Thierry Escande]

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 

[..]

+ - 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@757 {
+   pinctrl-names = "default", "sleep";
+   pinctrl-0 = <_uart1_default>;
+   pinctrl-1 = <_uart1_sleep>;
+
+   bluetooth {
+   compatible = "qcom,qca6174-bt";
+
+   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_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

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Thierry Escande]

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 

[..]

+ - 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@757 {
+   pinctrl-names = "default", "sleep";
+   pinctrl-0 = <_uart1_default>;
+   pinctrl-1 = <_uart1_sleep>;
+
+   bluetooth {
+   compatible = "qcom,qca6174-bt";
+
+   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_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

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Bjorn Andersson]

On Tue 20 Mar 23:58 HKT 2018, Marcel Holtmann wrote:
> > Signed-off-by: Thierry Escande 
[..]
> > + - 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@757 {
> > +   pinctrl-names = "default", "sleep";
> > +   pinctrl-0 = <_uart1_default>;
> > +   pinctrl-1 = <_uart1_sleep>;
> > +
> > +   bluetooth {
> > +   compatible = "qcom,qca6174-bt";
> > +
> > +   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
> > +
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_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

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Bjorn Andersson]

On Tue 20 Mar 23:58 HKT 2018, Marcel Holtmann wrote:
> > Signed-off-by: Thierry Escande 
[..]
> > + - 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@757 {
> > +   pinctrl-names = "default", "sleep";
> > +   pinctrl-0 = <_uart1_default>;
> > +   pinctrl-1 = <_uart1_sleep>;
> > +
> > +   bluetooth {
> > +   compatible = "qcom,qca6174-bt";
> > +
> > +   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
> > +
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_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

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Rob Herring]

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 
> ---
> 
> 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 

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Rob Herring]

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 
> ---
> 
> 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 

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Thierry Escande]

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 
---

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 ..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@757 {
+   pinctrl-names = "default", "sleep";
+   pinctrl-0 = <_uart1_default>;
+   pinctrl-1 = <_uart1_sleep>;
+
+   bluetooth {
+   compatible = "qcom,qca6174-bt";
+
+   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_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 = <>;


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

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Thierry Escande]

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 
---

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 ..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@757 {
+   pinctrl-names = "default", "sleep";
+   pinctrl-0 = <_uart1_default>;
+   pinctrl-1 = <_uart1_sleep>;
+
+   bluetooth {
+   compatible = "qcom,qca6174-bt";
+
+   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_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 = <>;


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

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Marcel Holtmann]

Hi Thierry,

> Add binding document for serial bluetooth chips using Qualcomm protocol.
> 
> Signed-off-by: Thierry Escande 
> ---
> 
> 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 ..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@757 {
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <_uart1_default>;
> + pinctrl-1 = <_uart1_sleep>;
> +
> + bluetooth {
> + compatible = "qcom,qca6174-bt";
> +
> + enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_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 = <>;

No clock-names?

Regards

Marcel

Re: [PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Marcel Holtmann]

Hi Thierry,

> Add binding document for serial bluetooth chips using Qualcomm protocol.
> 
> Signed-off-by: Thierry Escande 
> ---
> 
> 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 ..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@757 {
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <_uart1_default>;
> + pinctrl-1 = <_uart1_sleep>;
> +
> + bluetooth {
> + compatible = "qcom,qca6174-bt";
> +
> + enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_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 = <>;

No clock-names?

Regards

Marcel

[PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Thierry Escande]

Add binding document for serial bluetooth chips using Qualcomm protocol.

Signed-off-by: Thierry Escande 
---

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 ..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@757 {
+   pinctrl-names = "default", "sleep";
+   pinctrl-0 = <_uart1_default>;
+   pinctrl-1 = <_uart1_sleep>;
+
+   bluetooth {
+   compatible = "qcom,qca6174-bt";
+
+   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_en_pin_a>;
+
+   clocks = <>;
+   };
+};
-- 
2.14.1

[PATCH v5 2/3] dt-bindings: net: bluetooth: Add qualcomm-bluetooth

[Thierry Escande]

Add binding document for serial bluetooth chips using Qualcomm protocol.

Signed-off-by: Thierry Escande 
---

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 ..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@757 {
+   pinctrl-names = "default", "sleep";
+   pinctrl-0 = <_uart1_default>;
+   pinctrl-1 = <_uart1_sleep>;
+
+   bluetooth {
+   compatible = "qcom,qca6174-bt";
+
+   enable-gpios = <_gpios 19 GPIO_ACTIVE_HIGH>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_en_pin_a>;
+
+   clocks = <>;
+   };
+};
-- 
2.14.1