Return-Path: <linux-bluetooth-owner@vger.kernel.org>
Content-Type: text/plain; charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [PATCH 2/2] Bluetooth: hci_bcm: Add serdev support
From: Marcel Holtmann <marcel@holtmann.org>
In-Reply-To: <CALeDE9MWoNsiJqVhy_+FnbSKUGmikriqty5nrFd8V8ZtRYmTrA@mail.gmail.com>
Date: Mon, 7 Aug 2017 11:19:43 +0200
Cc: Loic Poulain <loic.poulain@gmail.com>,
        Rob Herring <robh+dt@kernel.org>, f.fainelli@gmail.com,
        rjui@broadcom.com, devicetree <devicetree@vger.kernel.org>,
        linux-rpi-kernel@lists.infradead.org,
        linux-bluetooth@vger.kernel.org
Message-Id: <F7B2F142-A202-43FB-81D0-35822909A7E2@holtmann.org>
References: <1501495414-17103-1-git-send-email-loic.poulain@gmail.com>
 <1501495414-17103-2-git-send-email-loic.poulain@gmail.com>
 <CALeDE9MWoNsiJqVhy_+FnbSKUGmikriqty5nrFd8V8ZtRYmTrA@mail.gmail.com>
To: Peter Robinson <pbrobinson@gmail.com>
Sender: linux-bluetooth-owner@vger.kernel.org
List-ID: <linux-bluetooth.vger.kernel.org>

Hi Peter,

>> Add basic support for Broadcom serial slave devices.
>> Probe the serial device, retrieve its maximum speed and
>> register a new hci uart device.
>> 
>> Tested/compatible with bcm43438 (RPi3).
>> 
>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>> ---
>> drivers/bluetooth/hci_bcm.c | 82 ++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 78 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>> index 6b42372..f824738 100644
>> --- a/drivers/bluetooth/hci_bcm.c
>> +++ b/drivers/bluetooth/hci_bcm.c
>> @@ -27,6 +27,7 @@
>> #include <linux/firmware.h>
>> #include <linux/module.h>
>> #include <linux/acpi.h>
>> +#include <linux/of.h>
>> #include <linux/platform_device.h>
>> #include <linux/clk.h>
>> #include <linux/gpio/consumer.h>
>> @@ -34,6 +35,7 @@
>> #include <linux/interrupt.h>
>> #include <linux/dmi.h>
>> #include <linux/pm_runtime.h>
>> +#include <linux/serdev.h>
>> 
>> #include <net/bluetooth/bluetooth.h>
>> #include <net/bluetooth/hci_core.h>
>> @@ -46,6 +48,7 @@
>> 
>> #define BCM_AUTOSUSPEND_DELAY  5000 /* default autosleep delay */
>> 
>> +/* platform device driver resources */
>> struct bcm_device {
>>        struct list_head        list;
>> 
>> @@ -68,6 +71,12 @@ struct bcm_device {
>> #endif
>> };
>> 
>> +/* serdev driver resources */
>> +struct bcm_bt_device {
>> +       struct hci_uart hu;
>> +};
>> +
>> +/* generic bcm uart resources */
>> struct bcm_data {
>>        struct sk_buff          *rx_skb;
>>        struct sk_buff_head     txq;
>> @@ -289,6 +298,11 @@ static int bcm_open(struct hci_uart *hu)
>> 
>>        hu->priv = bcm;
>> 
>> +       if (hu->serdev) {
>> +               serdev_device_open(hu->serdev);
>> +               goto out;
>> +       }
>> +
>>        if (!hu->tty->dev)
>>                goto out;
>> 
>> @@ -323,6 +337,9 @@ static int bcm_close(struct hci_uart *hu)
>> 
>>        bt_dev_dbg(hu->hdev, "hu %p", hu);
>> 
>> +       if (hu->serdev)
>> +               serdev_device_close(hu->serdev);
>> +
>>        /* Protect bcm->dev against removal of the device or driver */
>>        mutex_lock(&bcm_device_lock);
>>        if (bcm_device_exists(bdev)) {
>> @@ -397,8 +414,12 @@ static int bcm_setup(struct hci_uart *hu)
>>        else
>>                speed = 0;
>> 
>> -       if (speed)
>> -               hci_uart_set_baudrate(hu, speed);
>> +       if (speed) {
>> +               if (hu->serdev)
>> +                       serdev_device_set_baudrate(hu->serdev, speed);
>> +               else
>> +                       hci_uart_set_baudrate(hu, speed);
>> +       }
>> 
>>        /* Operational speed if any */
>>        if (hu->oper_speed)
>> @@ -410,8 +431,12 @@ static int bcm_setup(struct hci_uart *hu)
>> 
>>        if (speed) {
>>                err = bcm_set_baudrate(hu, speed);
>> -               if (!err)
>> -                       hci_uart_set_baudrate(hu, speed);
>> +               if (!err) {
>> +                       if (hu->serdev)
>> +                               serdev_device_set_baudrate(hu->serdev, speed);
>> +                       else
>> +                               hci_uart_set_baudrate(hu, speed);
>> +               }
>>        }
>> 
>> finalize:
>> @@ -903,9 +928,57 @@ static int bcm_remove(struct platform_device *pdev)
>>        },
>> };
>> 
>> +static int bcm_serdev_probe(struct serdev_device *serdev)
>> +{
>> +       struct bcm_bt_device *bcmdev;
>> +       u32 speed;
>> +       int err;
>> +
>> +       bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
>> +       if (!bcmdev)
>> +               return -ENOMEM;
>> +
>> +       bcmdev->hu.serdev = serdev;
>> +       serdev_device_set_drvdata(serdev, bcmdev);
>> +
>> +       err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
>> +       if (!err)
>> +               bcmdev->hu.oper_speed = speed;
>> +
>> +       return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
>> +}
>> +
>> +static void bcm_serdev_remove(struct serdev_device *serdev)
>> +{
>> +       struct bcm_bt_device *bcmdev = serdev_device_get_drvdata(serdev);
>> +
>> +       hci_uart_unregister_device(&bcmdev->hu);
>> +}
> 
> I'm seeing the following warning when building this with Fedora-26/gcc7:
> 
> drivers/bluetooth/hci_bcm.c: In function 'bcm_serdev_remove':
> drivers/bluetooth/hci_bcm.c:953:2: error: implicit declaration of
> function 'hci_uart_unregister_device'; did you mean
> 'hci_uart_register_device'? [-Werror=implicit-function-declaration]
>  hci_uart_unregister_device(&bcmdev->hu);
>  ^~~~~~~~~~~~~~~~~~~~~~~~~~
>  hci_uart_register_device
> cc1: some warnings being treated as errors

you might be missing some patches that are already in bluetooth-next and net-next.

Regards

Marcel