BCM chips may require configuration of PCM to operate correctly and
there is a vendor specific HCI command to do this. Add support in the
hci_bcm driver to parse this from devicetree and configure the chip.
Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/bluetooth/hci_bcm.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 6134bff58748..4ee0b45be7e2 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -88,6 +88,8 @@ struct bcm_device_data {
* used to disable flow control during runtime suspend and system sleep
* @is_suspended: whether flow control is currently disabled
* @disallow_set_baudrate: don't allow set_baudrate
+ * @has_pcm_params: whether PCM parameters need to be configured
+ * @pcm_params: PCM and routing parameters
*/
struct bcm_device {
/* Must be the first member, hci_serdev.c expects this. */
@@ -122,6 +124,9 @@ struct bcm_device {
bool is_suspended;
#endif
bool disallow_set_baudrate;
+
+ bool has_pcm_params;
+ struct bcm_set_pcm_int_params pcm_params;
};
/* generic bcm uart resources */
@@ -596,6 +601,16 @@ static int bcm_setup(struct hci_uart *hu)
host_set_baudrate(hu, speed);
}
+ /* PCM parameters if any*/
+ if (bcm->dev && bcm->dev->has_pcm_params) {
+ err = btbcm_set_pcm_int_params(hu->hdev, &bcm->dev->pcm_params);
+
+ if (err) {
+ bt_dev_info(hu->hdev, "BCM: Set pcm params failed (%d)",
+ err);
+ }
+ }
+
finalize:
release_firmware(fw);
@@ -1132,7 +1147,24 @@ static int bcm_acpi_probe(struct bcm_device *dev)
static int bcm_of_probe(struct bcm_device *bdev)
{
+ int err;
+
device_property_read_u32(bdev->dev, "max-speed", &bdev->oper_speed);
+
+ err = device_property_read_u8(bdev->dev, "brcm,bt-sco-routing",
+ &bdev->pcm_params.routing);
+ if (!err)
+ bdev->has_pcm_params = true;
+
+ device_property_read_u8(bdev->dev, "brcm,pcm-interface-rate",
+ &bdev->pcm_params.rate);
+ device_property_read_u8(bdev->dev, "brcm,pcm-frame-type",
+ &bdev->pcm_params.frame_sync);
+ device_property_read_u8(bdev->dev, "brcm,pcm-sync-mode",
+ &bdev->pcm_params.sync_mode);
+ device_property_read_u8(bdev->dev, "brcm,pcm-clock-mode",
+ &bdev->pcm_params.clock_mode);
+
return 0;
}
--
2.24.0.rc1.363.gb1bccd3e3d-goog
On Wed, Nov 13, 2019 at 9:29 PM Marcel Holtmann <[email protected]> wrote:
>
> Hi Abhishek,
>
> >>> BCM chips may require configuration of PCM to operate correctly and
> >>> there is a vendor specific HCI command to do this. Add support in the
> >>> hci_bcm driver to parse this from devicetree and configure the chip.
> >>>
> >>> Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
> >>> ---
> >>>
> >>> Changes in v4: None
> >>> Changes in v3: None
> >>> Changes in v2: None
> >>>
> >>> drivers/bluetooth/hci_bcm.c | 32 ++++++++++++++++++++++++++++++++
> >>> 1 file changed, 32 insertions(+)
> >>>
> >>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> >>> index 6134bff58748..4ee0b45be7e2 100644
> >>> --- a/drivers/bluetooth/hci_bcm.c
> >>> +++ b/drivers/bluetooth/hci_bcm.c
> >>> @@ -88,6 +88,8 @@ struct bcm_device_data {
> >>> * used to disable flow control during runtime suspend and system sleep
> >>> * @is_suspended: whether flow control is currently disabled
> >>> * @disallow_set_baudrate: don't allow set_baudrate
> >>> + * @has_pcm_params: whether PCM parameters need to be configured
> >>> + * @pcm_params: PCM and routing parameters
> >>> */
> >>> struct bcm_device {
> >>> /* Must be the first member, hci_serdev.c expects this. */
> >>> @@ -122,6 +124,9 @@ struct bcm_device {
> >>> bool is_suspended;
> >>> #endif
> >>> bool disallow_set_baudrate;
> >>> +
> >>> + bool has_pcm_params;
> >>> + struct bcm_set_pcm_int_params pcm_params;
> >>> };
> >>>
> >>> /* generic bcm uart resources */
> >>> @@ -596,6 +601,16 @@ static int bcm_setup(struct hci_uart *hu)
> >>> host_set_baudrate(hu, speed);
> >>> }
> >>>
> >>> + /* PCM parameters if any*/
> >>> + if (bcm->dev && bcm->dev->has_pcm_params) {
> >>> + err = btbcm_set_pcm_int_params(hu->hdev, &bcm->dev->pcm_params);
> >>> +
> >>> + if (err) {
> >>> + bt_dev_info(hu->hdev, "BCM: Set pcm params failed (%d)",
> >>> + err);
> >>> + }
> >>> + }
> >>> +
> >>> finalize:
> >>> release_firmware(fw);
> >>>
> >>> @@ -1132,7 +1147,24 @@ static int bcm_acpi_probe(struct bcm_device *dev)
> >>>
> >>> static int bcm_of_probe(struct bcm_device *bdev)
> >>> {
> >>> + int err;
> >>> +
> >>> device_property_read_u32(bdev->dev, "max-speed", &bdev->oper_speed);
> >>> +
> >>> + err = device_property_read_u8(bdev->dev, "brcm,bt-sco-routing",
> >>> + &bdev->pcm_params.routing);
> >>> + if (!err)
> >>> + bdev->has_pcm_params = true;
> >>
> >> I think in case of HCI as routing path, these should be using the default or zero values as defined by Broadcom.
> >
> > I'm not sure what these default values should be. Wouldn't it be
> > reasonable to expect the user/developer to set the various brcm
> > parameters in device tree?
> > If unset, it's just 0.
>
> if that works with the hardware I am fine with that. The other option is to actually first read the current values. And then only change the ones that are supplied by the DT.
I don't know of a read pcm params command (this would be nice to have).
I think it might be prudent to default the frame_mode and clock_mode
to master (0x1). I'll test how the hardware responds to 0x0 and update
the default to 0x1 if things fail badly.
>
> Regards
>
> Marcel
>