Hi Attila,

please include a commit message explains the changes.

> Signed-off-by: Attila Tőkés <[email protected]>
> ---
> drivers/bluetooth/hci_bcm.c | 61 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index ddbd8c6a..50c73eef 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -83,6 +83,16 @@
> * @hu: pointer to HCI UART controller struct,
> *     used to disable flow control during runtime suspend and system sleep
> * @is_suspended: whether flow control is currently disabled
> + *
> + *  SCO routing parameters:
> + *   used as the parameters for the bcm_set_pcm_int_params command
> + *   @sco_routing:
> + *    >= 255 (skip SCO routing configuration)
> + *    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)
> */
> struct bcm_device {
>       /* Must be the first member, hci_serdev.c expects this. */
> @@ -114,6 +124,13 @@ struct bcm_device {
>       struct hci_uart         *hu;
>       bool                    is_suspended;
> #endif
> +
> +     /* SCO routing parameters */
> +     u32                     sco_routing;
> +     u32                     pcm_interface_rate;
> +     u32                     pcm_frame_type;
> +     u32                     pcm_sync_mode;
> +     u32                     pcm_clock_mode;
> };

These should be all u8 since that is what they are in bcm_set_pcm_int_params.

> /* generic bcm uart resources */
> @@ -189,6 +206,35 @@ static int bcm_set_baudrate(struct hci_uart *hu, 
> unsigned int speed)
>       return 0;
> }
> 
> +static int bcm_configure_sco_routing(struct hci_uart *hu, struct bcm_device 
> *bcm_dev)
> +{
> +     struct hci_dev *hdev = hu->hdev;
> +     struct sk_buff *skb;
> +     struct bcm_set_pcm_int_params params;
> +
> +     bt_dev_dbg(hdev, "BCM: Configuring SCO routing (%d %d %d %d %d)",
> +                     bcm_dev->sco_routing, bcm_dev->pcm_interface_rate, 
> bcm_dev->pcm_frame_type,
> +                     bcm_dev->pcm_sync_mode, bcm_dev->pcm_clock_mode);
> +
> +     params.routing = bcm_dev->sco_routing;
> +     params.rate = bcm_dev->pcm_interface_rate;
> +     params.frame_sync = bcm_dev->pcm_frame_type;
> +     params.sync_mode = bcm_dev->pcm_sync_mode;
> +     params.clock_mode = bcm_dev->pcm_clock_mode;
> +
> +     // send the SCO routing configuration command

Please follow networking comment style here.

> +     skb = __hci_cmd_sync(hdev, 0xfc1c, sizeof(params), &params, 
> HCI_CMD_TIMEOUT);
> +     if (IS_ERR(skb)) {
> +             int err = PTR_ERR(skb);
> +             bt_dev_err(hdev, "BCM: failed to configure SCO routing (%d)", 
> err);
> +             return err;
> +     }
> +
> +     kfree_skb(skb);
> +
> +     return 0;
> +}
> +
> /* bcm_device_exists should be protected by bcm_device_lock */
> static bool bcm_device_exists(struct bcm_device *device)
> {
> @@ -534,6 +580,10 @@ static int bcm_setup(struct hci_uart *hu)
>                       host_set_baudrate(hu, speed);
>       }
> 
> +     if (bcm->dev->sco_routing < 0xff) {
> +             bcm_configure_sco_routing(hu, bcm->dev);
> +     }
> +

Move that check into bcm_configure_sco_routing to exit that function early.

I would also recommend to read the current settings first and deal with missing 
parameters in the DT. For example if you just change the routing part that 
should be fine as well.

> finalize:
>       release_firmware(fw);
> 
> @@ -1007,6 +1057,11 @@ static int bcm_acpi_probe(struct bcm_device *dev)
> static int bcm_of_probe(struct bcm_device *bdev)
> {
>       device_property_read_u32(bdev->dev, "max-speed", &bdev->oper_speed);
> +     device_property_read_u32(bdev->dev, "sco-routing", &bdev->sco_routing);
> +     device_property_read_u32(bdev->dev, "pcm-interface-rate", 
> &bdev->pcm_interface_rate);
> +     device_property_read_u32(bdev->dev, "pcm-frame-type", 
> &bdev->pcm_frame_type);
> +     device_property_read_u32(bdev->dev, "pcm-sync-mode", 
> &bdev->pcm_sync_mode);
> +     device_property_read_u32(bdev->dev, "pcm-clock-mode", 
> &bdev->pcm_clock_mode);
>       return 0;
> }

You need a patch documenting these DT settings.

> 
> @@ -1022,6 +1077,9 @@ static int bcm_probe(struct platform_device *pdev)
>       dev->dev = &pdev->dev;
>       dev->irq = platform_get_irq(pdev, 0);
> 
> +     /* SCO routing configuration is disabled by default */
> +     dev->sco_routing = 0xff;
> +
>       if (has_acpi_companion(&pdev->dev)) {
>               ret = bcm_acpi_probe(dev);
>               if (ret)
> @@ -1281,6 +1339,9 @@ static int bcm_serdev_probe(struct serdev_device 
> *serdev)
>       bcmdev->serdev_hu.serdev = serdev;
>       serdev_device_set_drvdata(serdev, bcmdev);
> 
> +     /* SCO routing configuration is disabled by default */
> +     bcmdev->sco_routing = 0xff;
> +
>       if (has_acpi_companion(&serdev->dev))
>               err = bcm_acpi_probe(bcmdev);
>       else

Regards

Marcel

Reply via email to