Re: [PATCH] Bluetooth: hci_bcm: Configure SCO routing automatically

Hi Marcel, Rob,

I was thinking about doing these changes in two parts:

1. Making in hci_bcm.c the SCO routed over HCI by default

As in the Broadcom chips the SCO seems to be routed to PCM by default, this
may break the backward compatibility with some devices. I guess this would
be a problem, so we will need a way to keep this backward compatible.

2. Adding support to configure SCO routed over the other transports (PCM /
I2C / codec), based on the DT

I don't think we have clear solution yet here. The implementation could be
vendor specific or a more generic one. And there is the modes / timing
negotiation part that is unclear (at least for me).

What do you think?

Attila

On Mon, Jun 11, 2018 at 10:05 PM, Rob Herring [off-list ref] wrote:

On Mon, Jun 11, 2018 at 12:19 PM, Marcel Holtmann [off-list ref]
wrote:

quoted

Hi Rob,

quoted

quoted

quoted

quoted

quoted

quoted

Added support to automatically configure the SCO packet routing at

the

quoted

quoted

quoted

quoted

quoted

quoted

quoted

device setup. The SCO packets are used with the HSP / HFP

profiles, but in

quoted

quoted

quoted

quoted

quoted

quoted

quoted

some devices (ex. CYW43438) they are routed to a PCM output by

default. This

quoted

quoted

quoted

quoted

quoted

quoted

quoted

change allows sending the vendor specific HCI command to configure

the SCO

quoted

quoted

quoted

quoted

quoted

quoted

quoted

routing. The parameters of the command are loaded from the device

tree.

quoted

quoted

quoted

quoted

quoted

quoted

Please wrap your commit msg.

Sure.

quoted

quoted

Signed-off-by: Attila Tőkés <redacted>
---
.../bindings/net/broadcom-bluetooth.txt       |  7 ++

Please split bindings to separate patch.

Ok, I will split this in two.

quoted

quoted

drivers/bluetooth/hci_bcm.c                   | 72

+++++++++++++++++++

quoted

quoted

quoted

quoted

quoted

quoted

quoted

2 files changed, 79 insertions(+)

diff --git
a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
index 4194ff7e..aea3a094 100644

--- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt

@@ -21,6 +21,12 @@ Optional properties:
- clocks: clock specifier if external clock provided to the

controller

quoted

quoted

quoted

quoted

quoted

quoted

quoted

- clock-names: should be "extclk"

+ SCO routing parameters:
+ - sco-routing: 0-3 (PCM, Transport, Codec, I2S)
+ - pcm-interface-rate: 0-4 (128 Kbps - 2048 Kbps)
+ - pcm-frame-type: 0 (short), 1 (long)
+ - pcm-sync-mode: 0 (slave), 1 (master)
+ - pcm-clock-mode: 0 (slave), 1 (master)

Are these Broadcom specific? Properties need either vendor prefix or
to be documented in a common location. I think these look like the
latter.

These will be used as parameters of a vendor specific

(Broadcom/Cypress)

quoted

quoted

quoted

quoted

quoted

command configuring the SCO packet routing. See the

Write_SCO_PCM_Int_Param

quoted

quoted

quoted

quoted

quoted

command from: http://www.cypress.com/file/298311/download.

The DT should just describe how the h/w is hooked-up. What the s/w has
to do based on that is the driver's problem which is certainly
vendor/chip specific, but that is all irrelevant to the binding.

quoted

What would be the property names with a Broadcom / Cypress vendor

prefix?

quoted

quoted

quoted

quoted

quoted

  brcm,sco-routing
  brcm,pcm-interface-rate
  brcm,pcm-frame-type
  brcm,pcm-sync-mode
  brcm,pcm-clock-mode

?

Yes.

we can do this. However all pcm-* are optional if you switch the HCI

transport. And sco-routing should default to HCI if that is not present.
Meaning a driver should actively trying to change this. Nevertheless, it
would be good if a driver reads the current settings.

quoted

quoted

quoted

In theory we could make sco-routing generic, but so many vendors have

different modes, that we better keep this vendor specific.

quoted

quoted

Even if vendor specific, the properties for not HCI transport case are
still incomplete IMO.

By modes, you mean PCM vs. I2S and all the flavors of timings you can
have within those or something else? For the former, that's often
going to be a process of solving what each end support and if that
doesn't work, then IIRC we already have properties for setting
modes/timing. All the same issues exist with audio codecs and this is
really not any different.

this is what Broadcom uses to configure their PCM transport. So I think

for now, we make them brcm, specific and see how that goes. We can always
generalize them later if enough chip manufactures provide support for it.

We already have properties doing the same thing defined in
Documentation/devicetree/bindings/sound/simple-card.txt. Use and
extend that. We don't need new properties especially for something
that is not complete. For example If I have 2 host ports (every SoC
has at least 2), how do I indicate which one is connected to BT.

Rob