Re: [PATCH v4 4/4] dt-bindings: net: broadcom-bluetooth: Add pcm config

On Wed, Nov 13, 2019 at 01:21:06AM +0100, Marcel Holtmann wrote:

Hi Abhishek,

quoted

Add documentation for pcm parameters.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

---

Changes in v4:
- Fix incorrect function name in hci_bcm

Changes in v3:
- Change disallow baudrate setting to return -EBUSY if called before
 ready. bcm_proto is no longer modified and is back to being const.
- Changed btbcm_set_pcm_params to btbcm_set_pcm_int_params
- Changed brcm,sco-routing to brcm,bt-sco-routing

Changes in v2:
- Use match data to disallow baudrate setting
- Parse pcm parameters by name instead of as a byte string
- Fix prefix for dt-bindings commit

.../devicetree/bindings/net/broadcom-bluetooth.txt    | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
index c749dc297624..42fb2fa8143d 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt

@@ -29,6 +29,11 @@ Optional properties:
   - "lpo": external low power 32.768 kHz clock
 - vbat-supply: phandle to regulator supply for VBAT
 - vddio-supply: phandle to regulator supply for VDDIO
+ - brcm,bt-sco-routing: 0-3 (PCM, Transport, Codec, I2S)
+ - brcm,pcm-interface-rate: 0-4 (128KBps, 256KBps, 512KBps, 1024KBps, 2048KBps)
+ - brcm,pcm-frame-type: 0-1 (short, long)
+ - brcm,pcm-sync-mode: 0-1 (slave, master)
+ - brcm,pcm-clock-mode: 0-1 (slave, master)

I think that all of them need to start with brcm,bt- prefix since it is rather Bluetooth specific.

quoted

Example:

@@ -40,5 +45,11 @@ Example:
       bluetooth {
               compatible = "brcm,bcm43438-bt";
               max-speed = <921600>;
+
+               brcm,bt-sco-routing = [01];
+               brcm,pcm-interface-rate = [02];
+               brcm,pcm-frame-type = [00];
+               brcm,pcm-sync-mode = [01];
+               brcm,pcm-clock-mode = [01];
       };

My personal taste would be to add a comment after each entry that gives the human readable setting.

I'd suggest to define constants in include/dt-bindings/bluetooth/brcm.h
and use them instead of literals, with this we wouldn't rely on (optional)
comments to make the configuration human readable.