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

From: Marcel Holtmann
Date: Fri Nov 15 2019 - 03:01:58 EST


Hi Abhishek,

> Add documentation for pcm parameters.
>
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx>
> ---
>
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> .../bindings/net/broadcom-bluetooth.txt | 20 +++++++++++-
> include/dt-bindings/bluetooth/brcm.h | 32 +++++++++++++++++++
> 2 files changed, 51 insertions(+), 1 deletion(-)
> create mode 100644 include/dt-bindings/bluetooth/brcm.h
>
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> index c749dc297624..a92da31daa79 100644
> --- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> @@ -29,10 +29,22 @@ 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: PCM, Transport, Codec, I2S
> + This value must be set in order for the latter
> + properties to take effect.
> + - brcm,bt-pcm-interface-rate: 128KBps, 256KBps, 512KBps, 1024KBps, 2048KBps
> + - brcm,bt-pcm-frame-type: short, long
> + - brcm,bt-pcm-sync-mode: slave, master
> + - brcm,bt-pcm-clock-mode: slave, master
> +
> +See include/dt-bindings/bluetooth/brcm.h for SCO/PCM parameters. The default
> +value for all these values are 0 (except for brcm,bt-sco-routing which requires
> +a value) if you choose to leave it out.
>
> Example:
>
> +#include <dt-bindings/bluetooth/brcm.h>
> +
> &uart2 {
> pinctrl-names = "default";
> pinctrl-0 = <&uart2_pins>;
> @@ -40,5 +52,11 @@ Example:
> bluetooth {
> compatible = "brcm,bcm43438-bt";
> max-speed = <921600>;
> +
> + brcm,bt-sco-routing = <BRCM_SCO_ROUTING_TRANSPORT>;
> + brcm,bt-pcm-interface-rate = <BRCM_PCM_IF_RATE_512KBPS>;
> + brcm,bt-pcm-frame-type = <BRCM_PCM_FRAME_TYPE_SHORT>;
> + brcm,bt-pcm-sync-mode = <BRCM_PCM_SYNC_MODE_MASTER>;
> + brcm,bt-pcm-clock-mode = <BRCM_PCM_CLOCK_MODE_MASTER>;
> };

I am not sure this makes this all that much more readable. I would have been fine with using actual integer values with a comment behind it. Especially since in the driver itself we will never compare against the constants, we set whatever the DT gives us.

Anyway, for the names, I like Rob to ack them before applying them.

Regards

Marcel