[{"id":1761975,"web_url":"http://patchwork.ozlabs.org/comment/1761975/","msgid":"<5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>","list_archive_url":null,"date":"2017-09-01T20:47:23","subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","submitter":{"id":384,"url":"http://patchwork.ozlabs.org/api/people/384/","name":"Marcel Holtmann","email":"marcel@holtmann.org"},"content":"Hi Bjorn,\n\n> Bluetooth BD address can be retrieved in the same way as\n> for wcnss-wlan MAC address. This patch mainly stores the\n> local-mac-address property and sets the BD address during\n> hci device setup.\n> \n> Signed-off-by: Loic Poulain \n> Signed-off-by: Bjorn Andersson \n> ---\n> drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++\n> 1 file changed, 28 insertions(+)\n> \n> diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c\n> index d00c4fdae924..443bb2099329 100644\n> --- a/drivers/bluetooth/btqcomsmd.c\n> +++ b/drivers/bluetooth/btqcomsmd.c\n> @@ -26,6 +26,7 @@\n> struct btqcomsmd {\n> \tstruct hci_dev *hdev;\n> \n> +\tconst bdaddr_t *addr;\n> \tstruct rpmsg_endpoint *acl_channel;\n> \tstruct rpmsg_endpoint *cmd_channel;\n> };\n> @@ -100,6 +101,27 @@ static int btqcomsmd_close(struct hci_dev *hdev)\n> \treturn 0;\n> }\n> \n> +static int btqcomsmd_setup(struct hci_dev *hdev)\n> +{\n> +\tstruct btqcomsmd *btq = hci_get_drvdata(hdev);\n> +\tstruct sk_buff *skb;\n> +\n> +\tskb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);\n> +\tif (IS_ERR(skb))\n> +\t\treturn PTR_ERR(skb);\n> +\tkfree_skb(skb);\n> +\n> +\tif (btq->addr) {\n> +\t\tbdaddr_t bdaddr;\n> +\n> +\t\t/* btq->addr stored with most significant byte first */\n> +\t\tbaswap(&bdaddr, btq->addr);\n> +\t\treturn qca_set_bdaddr_rome(hdev, &bdaddr);\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> static int btqcomsmd_probe(struct platform_device *pdev)\n> {\n> \tstruct btqcomsmd *btq;\n> @@ -123,6 +145,11 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n> \tif (IS_ERR(btq->cmd_channel))\n> \t\treturn PTR_ERR(btq->cmd_channel);\n> \n> +\tbtq->addr = of_get_property(pdev->dev.of_node, \"local-mac-address\",\n> +\t\t\t\t &ret);\n> +\tif (ret != sizeof(bdaddr_t))\n> +\t\tbtq->addr = NULL;\n> +\n> \thdev = hci_alloc_dev();\n> \tif (!hdev)\n> \t\treturn -ENOMEM;\n> @@ -135,6 +162,7 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n> \thdev->open = btqcomsmd_open;\n> \thdev->close = btqcomsmd_close;\n> \thdev->send = btqcomsmd_send;\n> +\thdev->setup = btqcomsmd_setup;\n> \thdev->set_bdaddr = qca_set_bdaddr_rome;\n\nI do not like this patch. Why not just set HCI_QUIRK_INVALID_BDADDR and let a userspace tool deal with reading the BD_ADDR from some storage.\n\nFrankly I do not get this WiFI MAC address or BD_ADDR stored in DT. I assumed the DT is suppose to describe hardware and not some value that is normally retrieved for OTP or alike.\n\nRegards\n\nMarcel","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkWX61Zysz9sPt\n\tfor ;\n\tSat, 2 Sep 2017 06:47:34 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752504AbdIAUr2 convert rfc822-to-8bit (ORCPT\n\t);\n\tFri, 1 Sep 2017 16:47:28 -0400","from coyote.holtmann.net ([212.227.132.17]:45701 \"EHLO\n\tmail.holtmann.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752347AbdIAUr0 (ORCPT\n\t); Fri, 1 Sep 2017 16:47:26 -0400","from marcel-macpro.fritz.box (p5B3D2BCF.dip0.t-ipconnect.de\n\t[91.61.43.207])\n\tby mail.holtmann.org (Postfix) with ESMTPSA id 22235CEF8D;\n\tFri, 1 Sep 2017 22:52:46 +0200 (CEST)"],"Content-Type":"text/plain; charset=us-ascii","Mime-Version":"1.0 (Mac OS X Mail 10.3 \\(3273\\))","Subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","From":"Marcel Holtmann ","In-Reply-To":"<20170901204118.17123-3-bjorn.andersson@linaro.org>","Date":"Fri, 1 Sep 2017 22:47:23 +0200","Cc":"\"Gustavo F. Padovan\" ,\n\tJohan Hedberg ,\n\t\"David S. Miller\" ,\n\t\"open list:BLUETOOTH DRIVERS\" ,\n\tNetwork Development ,\n\tLKML , linux-arm-msm@vger.kernel.org,\n\tLoic Poulain , Rob Herring ","Content-Transfer-Encoding":"8BIT","Message-Id":"<5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>","References":"<20170901204118.17123-1-bjorn.andersson@linaro.org>\n\t<20170901204118.17123-3-bjorn.andersson@linaro.org>","To":"Bjorn Andersson ","X-Mailer":"Apple Mail (2.3273)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1761988,"web_url":"http://patchwork.ozlabs.org/comment/1761988/","msgid":"","list_archive_url":null,"date":"2017-09-01T21:26:53","subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","submitter":{"id":62529,"url":"http://patchwork.ozlabs.org/api/people/62529/","name":"Rob Herring (Arm)","email":"robh@kernel.org"},"content":"On Fri, Sep 1, 2017 at 3:47 PM, Marcel Holtmann wrote:\n> Hi Bjorn,\n>\n>> Bluetooth BD address can be retrieved in the same way as\n>> for wcnss-wlan MAC address. This patch mainly stores the\n>> local-mac-address property and sets the BD address during\n>> hci device setup.\n>>\n>> Signed-off-by: Loic Poulain \n>> Signed-off-by: Bjorn Andersson \n>> ---\n>> drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++\n>> 1 file changed, 28 insertions(+)\n>>\n>> diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c\n>> index d00c4fdae924..443bb2099329 100644\n>> --- a/drivers/bluetooth/btqcomsmd.c\n>> +++ b/drivers/bluetooth/btqcomsmd.c\n>> @@ -26,6 +26,7 @@\n>> struct btqcomsmd {\n>> struct hci_dev *hdev;\n>>\n>> + const bdaddr_t *addr;\n>> struct rpmsg_endpoint *acl_channel;\n>> struct rpmsg_endpoint *cmd_channel;\n>> };\n>> @@ -100,6 +101,27 @@ static int btqcomsmd_close(struct hci_dev *hdev)\n>> return 0;\n>> }\n>>\n>> +static int btqcomsmd_setup(struct hci_dev *hdev)\n>> +{\n>> + struct btqcomsmd *btq = hci_get_drvdata(hdev);\n>> + struct sk_buff *skb;\n>> +\n>> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);\n>> + if (IS_ERR(skb))\n>> + return PTR_ERR(skb);\n>> + kfree_skb(skb);\n>> +\n>> + if (btq->addr) {\n>> + bdaddr_t bdaddr;\n>> +\n>> + /* btq->addr stored with most significant byte first */\n>> + baswap(&bdaddr, btq->addr);\n>> + return qca_set_bdaddr_rome(hdev, &bdaddr);\n>> + }\n>> +\n>> + return 0;\n>> +}\n>> +\n>> static int btqcomsmd_probe(struct platform_device *pdev)\n>> {\n>> struct btqcomsmd *btq;\n>> @@ -123,6 +145,11 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n>> if (IS_ERR(btq->cmd_channel))\n>> return PTR_ERR(btq->cmd_channel);\n>>\n>> + btq->addr = of_get_property(pdev->dev.of_node, \"local-mac-address\",\n>> + &ret);\n>> + if (ret != sizeof(bdaddr_t))\n>> + btq->addr = NULL;\n>> +\n>> hdev = hci_alloc_dev();\n>> if (!hdev)\n>> return -ENOMEM;\n>> @@ -135,6 +162,7 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n>> hdev->open = btqcomsmd_open;\n>> hdev->close = btqcomsmd_close;\n>> hdev->send = btqcomsmd_send;\n>> + hdev->setup = btqcomsmd_setup;\n>> hdev->set_bdaddr = qca_set_bdaddr_rome;\n>\n> I do not like this patch. Why not just set HCI_QUIRK_INVALID_BDADDR and let a userspace tool deal with reading the BD_ADDR from some storage.\n>\n> Frankly I do not get this WiFI MAC address or BD_ADDR stored in DT. I assumed the DT is suppose to describe hardware and not some value that is normally retrieved for OTP or alike.\n\nUse of \"local-mac-address\" for ethernet at least has existed as long\nat OpenFirmware I think. For some platforms, DT is the only OTP. And\nsometimes, the bootloader (like u-boot) stores MAC addresses and then\npopulates them on boot.\n\nSeems like if we just let userspace deal with it, then we're back to a\nbtattach tool with every platform's specific way of reading the MAC\naddress.\n\nRob","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=robh@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkXQ93r0hz9sPt\n\tfor ;\n\tSat, 2 Sep 2017 07:27:29 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752541AbdIAV1R (ORCPT );\n\tFri, 1 Sep 2017 17:27:17 -0400","from mail.kernel.org ([198.145.29.99]:37924 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1752392AbdIAV1Q (ORCPT );\n\tFri, 1 Sep 2017 17:27:16 -0400","from mail-qt0-f172.google.com (mail-qt0-f172.google.com\n\t[209.85.216.172])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 2BEC321B7E;\n\tFri, 1 Sep 2017 21:27:15 +0000 (UTC)","by mail-qt0-f172.google.com with SMTP id w42so6181816qtg.5;\n\tFri, 01 Sep 2017 14:27:15 -0700 (PDT)","by 10.12.153.1 with HTTP; Fri, 1 Sep 2017 14:26:53 -0700 (PDT)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 2BEC321B7E","X-Gm-Message-State":"AHPjjUi+T5KgKBQsHus5a/PsftOr9QbWi7pwwRJz4yQXdw53xPfN/hIi\n\tpdgIinw7GaKkUiZEn95tRJzIUL+jMQ==","X-Google-Smtp-Source":"ADKCNb6oKFOe3lxEoi1HXg2aiLPt2f+Ap5YCl13TNzf4lD7BGeGYiTA792eMs2g5E8EecufRWs2oZFypWeRpLIQ3yh8=","X-Received":"by 10.200.13.142 with SMTP id s14mr4674105qti.162.1504301234352; \n\tFri, 01 Sep 2017 14:27:14 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>","References":"<20170901204118.17123-1-bjorn.andersson@linaro.org>\n\t<20170901204118.17123-3-bjorn.andersson@linaro.org>\n\t<5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>","From":"Rob Herring ","Date":"Fri, 1 Sep 2017 16:26:53 -0500","X-Gmail-Original-Message-ID":"","Message-ID":"","Subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","To":"Marcel Holtmann ","Cc":"Bjorn Andersson ,\n\t\"Gustavo F. Padovan\" ,\n\tJohan Hedberg ,\n\t\"David S. Miller\" ,\n\t\"open list:BLUETOOTH DRIVERS\" ,\n\tNetwork Development ,\n\tLKML ,\n\tlinux-arm-msm ,\n\tLoic Poulain ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1762011,"web_url":"http://patchwork.ozlabs.org/comment/1762011/","msgid":"<20170901221503.GA1165@minitux>","list_archive_url":null,"date":"2017-09-01T22:15:03","subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","submitter":{"id":68398,"url":"http://patchwork.ozlabs.org/api/people/68398/","name":"Bjorn Andersson","email":"bjorn.andersson@linaro.org"},"content":"On Fri 01 Sep 13:47 PDT 2017, Marcel Holtmann wrote:\n\n> Hi Bjorn,\n> \n> > Bluetooth BD address can be retrieved in the same way as\n> > for wcnss-wlan MAC address. This patch mainly stores the\n> > local-mac-address property and sets the BD address during\n> > hci device setup.\n> > \n> > Signed-off-by: Loic Poulain \n> > Signed-off-by: Bjorn Andersson \n> > ---\n> > drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++\n> > 1 file changed, 28 insertions(+)\n> > \n> > diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c\n> > index d00c4fdae924..443bb2099329 100644\n> > --- a/drivers/bluetooth/btqcomsmd.c\n> > +++ b/drivers/bluetooth/btqcomsmd.c\n> > @@ -26,6 +26,7 @@\n> > struct btqcomsmd {\n> > \tstruct hci_dev *hdev;\n> > \n> > +\tconst bdaddr_t *addr;\n> > \tstruct rpmsg_endpoint *acl_channel;\n> > \tstruct rpmsg_endpoint *cmd_channel;\n> > };\n> > @@ -100,6 +101,27 @@ static int btqcomsmd_close(struct hci_dev *hdev)\n> > \treturn 0;\n> > }\n> > \n> > +static int btqcomsmd_setup(struct hci_dev *hdev)\n> > +{\n> > +\tstruct btqcomsmd *btq = hci_get_drvdata(hdev);\n> > +\tstruct sk_buff *skb;\n> > +\n> > +\tskb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);\n> > +\tif (IS_ERR(skb))\n> > +\t\treturn PTR_ERR(skb);\n> > +\tkfree_skb(skb);\n> > +\n> > +\tif (btq->addr) {\n> > +\t\tbdaddr_t bdaddr;\n> > +\n> > +\t\t/* btq->addr stored with most significant byte first */\n> > +\t\tbaswap(&bdaddr, btq->addr);\n> > +\t\treturn qca_set_bdaddr_rome(hdev, &bdaddr);\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > static int btqcomsmd_probe(struct platform_device *pdev)\n> > {\n> > \tstruct btqcomsmd *btq;\n> > @@ -123,6 +145,11 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n> > \tif (IS_ERR(btq->cmd_channel))\n> > \t\treturn PTR_ERR(btq->cmd_channel);\n> > \n> > +\tbtq->addr = of_get_property(pdev->dev.of_node, \"local-mac-address\",\n> > +\t\t\t\t &ret);\n> > +\tif (ret != sizeof(bdaddr_t))\n> > +\t\tbtq->addr = NULL;\n> > +\n> > \thdev = hci_alloc_dev();\n> > \tif (!hdev)\n> > \t\treturn -ENOMEM;\n> > @@ -135,6 +162,7 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n> > \thdev->open = btqcomsmd_open;\n> > \thdev->close = btqcomsmd_close;\n> > \thdev->send = btqcomsmd_send;\n> > +\thdev->setup = btqcomsmd_setup;\n> > \thdev->set_bdaddr = qca_set_bdaddr_rome;\n> \n> I do not like this patch. Why not just set HCI_QUIRK_INVALID_BDADDR\n> and let a userspace tool deal with reading the BD_ADDR from some\n> storage.\n> \n\nThat's what we currently have, but we regularly get complaints from\ndevelopers using our board (DB410c).\n\nWe're maintaining a Debian-based and an OpenEmbedded-based build and at\nleast in the past btmgmt was not available in these - so we would have\nto maintain both a custom BlueZ package and then some scripts to inject\nthe appropriate mac address.\n\nBeyond these reference builds our users tend to build their own system\nimages and I was hoping that they would not be forced to have a custom\nhook running each time hci0 is registered.\n\n> Frankly I do not get this WiFI MAC address or BD_ADDR stored in DT. I\n> assumed the DT is suppose to describe hardware and not some value that\n> is normally retrieved for OTP or alike.\n> \n\nWhile I share your skepticism here I find it way superior over the\nvarious cases where this information is hard coded in some firmware file\nthat has to be patched for each device - in particular when considering\nthe out-of-tree workarounds that follow when said firmware file is not\nallowed to be modified on the device (e.g. in Android).\n\nAnd note that it's not _stored_ in DT, it's passed from the boot loader\nin DT - and it's still optional, so if an OEM has other means to\nprovision the BD_ADDR they can still handle this in user space.\n\nRegards,\nBjorn","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"RiQOjPVv\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkYTX6v5Xz9sPt\n\tfor ;\n\tSat, 2 Sep 2017 08:15:28 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752571AbdIAWPK (ORCPT );\n\tFri, 1 Sep 2017 18:15:10 -0400","from mail-pf0-f177.google.com ([209.85.192.177]:33011 \"EHLO\n\tmail-pf0-f177.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752515AbdIAWPI (ORCPT\n\t); Fri, 1 Sep 2017 18:15:08 -0400","by mail-pf0-f177.google.com with SMTP id n73so3968079pfj.0\n\tfor ; Fri, 01 Sep 2017 15:15:07 -0700 (PDT)","from minitux (ip68-111-217-79.sd.sd.cox.net. [68.111.217.79])\n\tby smtp.gmail.com with ESMTPSA id\n\tn19sm1522522pfj.54.2017.09.01.15.15.05\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 01 Sep 2017 15:15:06 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=Hho5hIU36kvrqwDtbkd/DSNKxr9dVmgeMKPX2LoRNkM=;\n\tb=RiQOjPVvKZWSIpXBEaQPV+G7rc5O1afLgszcxGbwqEKpBYzzaYXlu/tQ74J+ydy5Ps\n\t+tiBhNi/imM6j5PYjG4KzpuO3ExWCnIQoV9nIE3T8oLhjEuIAuUCH/+TWHTvNJdq0nRa\n\tI29MCKH5At4qMwQqBHvQZF+Hbz2HVAbe6XxP8=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=Hho5hIU36kvrqwDtbkd/DSNKxr9dVmgeMKPX2LoRNkM=;\n\tb=DysSGvgRAjM4p5CHLeaTjXsUpv0/2OwOjJn2PHowybwRbOjm8I5tjtg+1N7axs8sMS\n\tHXhkD6hzHL93fqBiBDcL9YPJ/RBFSTc7fLM/qQdeoQisLeFwOHerok/p4+BffsPFBlpr\n\tWO7LFjExeOce8pZ7tU8Vat9JBRxgK/NxNXS7ehu2QlLJI2p2yoAJ2rJx9x6Fqvd8Jj3D\n\tDq8JkF2uJKvLmu4wSxUoBukQ12iCBZYu94L3PsL0Z8/8+i0ZjQUnD91aOy9fPLG28s4Z\n\tp9v8PqNxp5nYJDu+HlIDTouhoeyXPFBuYJsiPULarAdAP4CE5LWdgPl1YLkeXluOq8rV\n\tM41w==","X-Gm-Message-State":"AHPjjUhorn+794qViIjwy1eh3aYNE/qbRdtcMoNXLJxvdXh0vdmJSNyG\n\tUBFEzquYKvXkmhdq","X-Google-Smtp-Source":"ADKCNb4+QErJia/TvCuOzvyzLxnoU+ZcADmYcERad7trGf6BWdqnVXTaErzyxT0NnZZf9WXI+Uodsg==","X-Received":"by 10.84.149.102 with SMTP id b35mr4268187plh.392.1504304107522; \n\tFri, 01 Sep 2017 15:15:07 -0700 (PDT)","Date":"Fri, 1 Sep 2017 15:15:03 -0700","From":"Bjorn Andersson ","To":"Marcel Holtmann ","Cc":"\"Gustavo F. Padovan\" ,\n\tJohan Hedberg ,\n\t\"David S. Miller\" ,\n\t\"open list:BLUETOOTH DRIVERS\" ,\n\tNetwork Development ,\n\tLKML , linux-arm-msm@vger.kernel.org,\n\tLoic Poulain , Rob Herring ","Subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","Message-ID":"<20170901221503.GA1165@minitux>","References":"<20170901204118.17123-1-bjorn.andersson@linaro.org>\n\t<20170901204118.17123-3-bjorn.andersson@linaro.org>\n\t<5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1762088,"web_url":"http://patchwork.ozlabs.org/comment/1762088/","msgid":"","list_archive_url":null,"date":"2017-09-02T06:12:17","subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","submitter":{"id":384,"url":"http://patchwork.ozlabs.org/api/people/384/","name":"Marcel Holtmann","email":"marcel@holtmann.org"},"content":"Hi Rob,\n\n>>> Bluetooth BD address can be retrieved in the same way as\n>>> for wcnss-wlan MAC address. This patch mainly stores the\n>>> local-mac-address property and sets the BD address during\n>>> hci device setup.\n>>> \n>>> Signed-off-by: Loic Poulain \n>>> Signed-off-by: Bjorn Andersson \n>>> ---\n>>> drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++\n>>> 1 file changed, 28 insertions(+)\n>>> \n>>> diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c\n>>> index d00c4fdae924..443bb2099329 100644\n>>> --- a/drivers/bluetooth/btqcomsmd.c\n>>> +++ b/drivers/bluetooth/btqcomsmd.c\n>>> @@ -26,6 +26,7 @@\n>>> struct btqcomsmd {\n>>> struct hci_dev *hdev;\n>>> \n>>> + const bdaddr_t *addr;\n>>> struct rpmsg_endpoint *acl_channel;\n>>> struct rpmsg_endpoint *cmd_channel;\n>>> };\n>>> @@ -100,6 +101,27 @@ static int btqcomsmd_close(struct hci_dev *hdev)\n>>> return 0;\n>>> }\n>>> \n>>> +static int btqcomsmd_setup(struct hci_dev *hdev)\n>>> +{\n>>> + struct btqcomsmd *btq = hci_get_drvdata(hdev);\n>>> + struct sk_buff *skb;\n>>> +\n>>> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);\n>>> + if (IS_ERR(skb))\n>>> + return PTR_ERR(skb);\n>>> + kfree_skb(skb);\n>>> +\n>>> + if (btq->addr) {\n>>> + bdaddr_t bdaddr;\n>>> +\n>>> + /* btq->addr stored with most significant byte first */\n>>> + baswap(&bdaddr, btq->addr);\n>>> + return qca_set_bdaddr_rome(hdev, &bdaddr);\n>>> + }\n>>> +\n>>> + return 0;\n>>> +}\n>>> +\n>>> static int btqcomsmd_probe(struct platform_device *pdev)\n>>> {\n>>> struct btqcomsmd *btq;\n>>> @@ -123,6 +145,11 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n>>> if (IS_ERR(btq->cmd_channel))\n>>> return PTR_ERR(btq->cmd_channel);\n>>> \n>>> + btq->addr = of_get_property(pdev->dev.of_node, \"local-mac-address\",\n>>> + &ret);\n>>> + if (ret != sizeof(bdaddr_t))\n>>> + btq->addr = NULL;\n>>> +\n>>> hdev = hci_alloc_dev();\n>>> if (!hdev)\n>>> return -ENOMEM;\n>>> @@ -135,6 +162,7 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n>>> hdev->open = btqcomsmd_open;\n>>> hdev->close = btqcomsmd_close;\n>>> hdev->send = btqcomsmd_send;\n>>> + hdev->setup = btqcomsmd_setup;\n>>> hdev->set_bdaddr = qca_set_bdaddr_rome;\n>> \n>> I do not like this patch. Why not just set HCI_QUIRK_INVALID_BDADDR and let a userspace tool deal with reading the BD_ADDR from some storage.\n>> \n>> Frankly I do not get this WiFI MAC address or BD_ADDR stored in DT. I assumed the DT is suppose to describe hardware and not some value that is normally retrieved for OTP or alike.\n> \n> Use of \"local-mac-address\" for ethernet at least has existed as long\n> at OpenFirmware I think. For some platforms, DT is the only OTP. And\n> sometimes, the bootloader (like u-boot) stores MAC addresses and then\n> populates them on boot.\n> \n> Seems like if we just let userspace deal with it, then we're back to a\n> btattach tool with every platform's specific way of reading the MAC\n> address.\n\nfor Bluetooth that is not true. We have Set Public Address command that is uniquely handling this and the HCI_QUIRK_INVALID_BDADDR address does the right magic to allow userspace to identify a missing address. It is done nicely and correctly and works fine.\n\nMind you this is even used when there actually is a BD_ADDR, but the device manufacturer wants to have one from its own OUI range compared to the chip manufacturer’s OUI range.\n\nIf DT is really the only place for the BD_ADDR and the bootloader kinda does add / merge it into the DT, then by all means that is fine. However if it is not, then this feature is dangerous since it can lead to multiple devices with the same address. I rather have these devices leave the kernel in unconfigured mode. And then force a userspace tool to use Set Public Address to bring it into configured mode.\n\nRegards\n\nMarcel","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkm3w11l6z9s7G\n\tfor ;\n\tSat, 2 Sep 2017 16:12:28 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752243AbdIBGMV convert rfc822-to-8bit (ORCPT\n\t);\n\tSat, 2 Sep 2017 02:12:21 -0400","from coyote.holtmann.net ([212.227.132.17]:33056 \"EHLO\n\tmail.holtmann.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751677AbdIBGMT (ORCPT\n\t); Sat, 2 Sep 2017 02:12:19 -0400","from marcel-macpro.fritz.box (p5B3D2BCF.dip0.t-ipconnect.de\n\t[91.61.43.207])\n\tby mail.holtmann.org (Postfix) with ESMTPSA id C463DCEF9E;\n\tSat, 2 Sep 2017 08:17:39 +0200 (CEST)"],"Content-Type":"text/plain; charset=utf-8","Mime-Version":"1.0 (Mac OS X Mail 10.3 \\(3273\\))","Subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","From":"Marcel Holtmann ","In-Reply-To":"","Date":"Sat, 2 Sep 2017 08:12:17 +0200","Cc":"Bjorn Andersson ,\n\t\"Gustavo F. Padovan\" ,\n\tJohan Hedberg ,\n\t\"David S. Miller\" ,\n\t\"open list:BLUETOOTH DRIVERS\" ,\n\tNetwork Development ,\n\tLKML ,\n\tlinux-arm-msm ,\n\tLoic Poulain ","Content-Transfer-Encoding":"8BIT","Message-Id":"","References":"<20170901204118.17123-1-bjorn.andersson@linaro.org>\n\t<20170901204118.17123-3-bjorn.andersson@linaro.org>\n\t<5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>\n\t","To":"Rob Herring ","X-Mailer":"Apple Mail (2.3273)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1762093,"web_url":"http://patchwork.ozlabs.org/comment/1762093/","msgid":"<60FC7975-BB1B-4CFF-9C63-A901F70D7A5B@holtmann.org>","list_archive_url":null,"date":"2017-09-02T06:38:26","subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","submitter":{"id":384,"url":"http://patchwork.ozlabs.org/api/people/384/","name":"Marcel Holtmann","email":"marcel@holtmann.org"},"content":"Hi Bjorn,\n\n>>> Bluetooth BD address can be retrieved in the same way as\n>>> for wcnss-wlan MAC address. This patch mainly stores the\n>>> local-mac-address property and sets the BD address during\n>>> hci device setup.\n>>> \n>>> Signed-off-by: Loic Poulain \n>>> Signed-off-by: Bjorn Andersson \n>>> ---\n>>> drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++\n>>> 1 file changed, 28 insertions(+)\n>>> \n>>> diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c\n>>> index d00c4fdae924..443bb2099329 100644\n>>> --- a/drivers/bluetooth/btqcomsmd.c\n>>> +++ b/drivers/bluetooth/btqcomsmd.c\n>>> @@ -26,6 +26,7 @@\n>>> struct btqcomsmd {\n>>> \tstruct hci_dev *hdev;\n>>> \n>>> +\tconst bdaddr_t *addr;\n>>> \tstruct rpmsg_endpoint *acl_channel;\n>>> \tstruct rpmsg_endpoint *cmd_channel;\n>>> };\n>>> @@ -100,6 +101,27 @@ static int btqcomsmd_close(struct hci_dev *hdev)\n>>> \treturn 0;\n>>> }\n>>> \n>>> +static int btqcomsmd_setup(struct hci_dev *hdev)\n>>> +{\n>>> +\tstruct btqcomsmd *btq = hci_get_drvdata(hdev);\n>>> +\tstruct sk_buff *skb;\n>>> +\n>>> +\tskb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);\n>>> +\tif (IS_ERR(skb))\n>>> +\t\treturn PTR_ERR(skb);\n>>> +\tkfree_skb(skb);\n>>> +\n>>> +\tif (btq->addr) {\n>>> +\t\tbdaddr_t bdaddr;\n>>> +\n>>> +\t\t/* btq->addr stored with most significant byte first */\n>>> +\t\tbaswap(&bdaddr, btq->addr);\n>>> +\t\treturn qca_set_bdaddr_rome(hdev, &bdaddr);\n>>> +\t}\n>>> +\n>>> +\treturn 0;\n>>> +}\n>>> +\n>>> static int btqcomsmd_probe(struct platform_device *pdev)\n>>> {\n>>> \tstruct btqcomsmd *btq;\n>>> @@ -123,6 +145,11 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n>>> \tif (IS_ERR(btq->cmd_channel))\n>>> \t\treturn PTR_ERR(btq->cmd_channel);\n>>> \n>>> +\tbtq->addr = of_get_property(pdev->dev.of_node, \"local-mac-address\",\n>>> +\t\t\t\t &ret);\n>>> +\tif (ret != sizeof(bdaddr_t))\n>>> +\t\tbtq->addr = NULL;\n>>> +\n>>> \thdev = hci_alloc_dev();\n>>> \tif (!hdev)\n>>> \t\treturn -ENOMEM;\n>>> @@ -135,6 +162,7 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n>>> \thdev->open = btqcomsmd_open;\n>>> \thdev->close = btqcomsmd_close;\n>>> \thdev->send = btqcomsmd_send;\n>>> +\thdev->setup = btqcomsmd_setup;\n>>> \thdev->set_bdaddr = qca_set_bdaddr_rome;\n>> \n>> I do not like this patch. Why not just set HCI_QUIRK_INVALID_BDADDR\n>> and let a userspace tool deal with reading the BD_ADDR from some\n>> storage.\n>> \n> \n> That's what we currently have, but we regularly get complaints from\n> developers using our board (DB410c).\n\nat least not in the upstream driver. It does not use HCI_QUIRK_INVALID_BDADDR to tell the system that its BD_ADDR is not valid. Which is something you still need to do if local-mac-address would not be found.\n\nWhat BD_ADDR is actually returned by default. Can someone send me a “btmon -w trace.log” for an init procedure of this chip?\n\n> We're maintaining a Debian-based and an OpenEmbedded-based build and at\n> least in the past btmgmt was not available in these - so we would have\n> to maintain both a custom BlueZ package and then some scripts to inject\n> the appropriate mac address.\n> \n> Beyond these reference builds our users tend to build their own system\n> images and I was hoping that they would not be forced to have a custom\n> hook running each time hci0 is registered.\n\nFrankly this has never been about btmgmt usage. That tool is really just for us to test the interface. What was needed is that we create a small daemon that can have backends for accessing the various OTPs. Or in dev mode just generate a random OUI from an unused OUI range. I would have put that into bluetoothd, but it seemed not a good idea since many companies were secret about their OTP access. So I assumed they build there own quick solution since mgmt API is fully documented and you only need to listen for Unconfigured Index event, send Set Public Address and leave. So something super simple.\n\nFor a LE only controller without a BD_ADDR, we recently added a pool of static addresses that it will generate and program. However that is specific since LE is capable of operating without a public address.\n\nWe could actually downgrade a dual-mode controller without a BD_ADDR into a single mode controller. That will automatically start using static addresses and be fully operational. That might be useful for people who get a dual-mode controller, but only care about LE. I have seen devices that only use the LE portion.\n\n>> Frankly I do not get this WiFI MAC address or BD_ADDR stored in DT. I\n>> assumed the DT is suppose to describe hardware and not some value that\n>> is normally retrieved for OTP or alike.\n>> \n> \n> While I share your skepticism here I find it way superior over the\n> various cases where this information is hard coded in some firmware file\n> that has to be patched for each device - in particular when considering\n> the out-of-tree workarounds that follow when said firmware file is not\n> allowed to be modified on the device (e.g. in Android).\n\nThat I full agree. Storing the BD_ADDR in the firmware file is utterly pointless. That is just insane way of operation and that is why we added the ability to make interfaces clearly as unconfigured when they miss the BD_ADDR. And have one way to userspace to configure it.\n\nThe DT file is nothing better if it is something that is generated statically once. It has the same problem. You are just papering over it and telling someone else to deal with it.\n\nWhen the bootloader dynamically generated the DT and inserts a local-mac-address field based on its own OTP, then I can see that this is possible, but for static DT that for example the kernel ships, this is a super bad idea. As bad as the BD_ADDR in the firmware file itself.\n\n> And note that it's not _stored_ in DT, it's passed from the boot loader\n> in DT - and it's still optional, so if an OEM has other means to\n> provision the BD_ADDR they can still handle this in user space.\n\nThat is really not clear to me and I do not even think the documentation has all the needed warnings. The Bluetooth devices is super critical for having its unique BD_ADDR for Bluetooth Classic operation. Things will really go bonkers. And they will since I had to already fly half around the world to debug issues with wrong mac addresses.\n\nIf we now all want to blame the bootloader, then so be it. Nevertheless you are still not using HCI_QUIRK_INVALID_BDADDR if the bootloader can’t be blamed. That is actually a bad idea since now also userspace thinks it is all fine.\n\nAnd I did write a lengthy email about this about 3 years ago:\n\nhttp://linux-bluetooth.vger.kernel.narkive.com/9JnHPGAf/some-notes-about-device-provisioning\n\nSo since I think this is actually dangerous to have the BD_ADDR come from DT, I think these needs a few extra bits of documentation on the usage of local-mac-address in the DT documentation, in addition we might want to print that the Bluetooth address comes from DT and which one in dmesg. So that at least there is some chance of debugging if you get this fully wrong. And you need to use HCI_QUIRK_INVALID_BDADDR. That is actually a real bug right now if the hardware never has a valid address.\n\nRegards\n\nMarcel","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkmf93Pg3z9s7c\n\tfor ;\n\tSat, 2 Sep 2017 16:38:41 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752150AbdIBGib convert rfc822-to-8bit (ORCPT\n\t);\n\tSat, 2 Sep 2017 02:38:31 -0400","from coyote.holtmann.net ([212.227.132.17]:54613 \"EHLO\n\tmail.holtmann.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750980AbdIBGi3 (ORCPT\n\t); Sat, 2 Sep 2017 02:38:29 -0400","from marcel-macpro.fritz.box (p5B3D2BCF.dip0.t-ipconnect.de\n\t[91.61.43.207])\n\tby mail.holtmann.org (Postfix) with ESMTPSA id E71E4CEF9E;\n\tSat, 2 Sep 2017 08:43:48 +0200 (CEST)"],"Content-Type":"text/plain; charset=utf-8","Mime-Version":"1.0 (Mac OS X Mail 10.3 \\(3273\\))","Subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","From":"Marcel Holtmann ","In-Reply-To":"<20170901221503.GA1165@minitux>","Date":"Sat, 2 Sep 2017 08:38:26 +0200","Cc":"\"Gustavo F. Padovan\" ,\n\tJohan Hedberg ,\n\t\"David S. Miller\" ,\n\t\"open list:BLUETOOTH DRIVERS\" ,\n\tNetwork Development ,\n\tLKML , linux-arm-msm@vger.kernel.org,\n\tLoic Poulain , Rob Herring ","Content-Transfer-Encoding":"8BIT","Message-Id":"<60FC7975-BB1B-4CFF-9C63-A901F70D7A5B@holtmann.org>","References":"<20170901204118.17123-1-bjorn.andersson@linaro.org>\n\t<20170901204118.17123-3-bjorn.andersson@linaro.org>\n\t<5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>\n\t<20170901221503.GA1165@minitux>","To":"Bjorn Andersson ","X-Mailer":"Apple Mail (2.3273)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1762332,"web_url":"http://patchwork.ozlabs.org/comment/1762332/","msgid":"<20170903211012.GJ2016@tuxbook>","list_archive_url":null,"date":"2017-09-03T21:10:12","subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","submitter":{"id":68398,"url":"http://patchwork.ozlabs.org/api/people/68398/","name":"Bjorn Andersson","email":"bjorn.andersson@linaro.org"},"content":"On Fri 01 Sep 23:12 PDT 2017, Marcel Holtmann wrote:\n\n> Hi Rob,\n> \n> >>> Bluetooth BD address can be retrieved in the same way as\n> >>> for wcnss-wlan MAC address. This patch mainly stores the\n> >>> local-mac-address property and sets the BD address during\n> >>> hci device setup.\n> >>> \n> >>> Signed-off-by: Loic Poulain \n> >>> Signed-off-by: Bjorn Andersson \n> >>> ---\n> >>> drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++\n> >>> 1 file changed, 28 insertions(+)\n> >>> \n> >>> diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c\n> >>> index d00c4fdae924..443bb2099329 100644\n> >>> --- a/drivers/bluetooth/btqcomsmd.c\n> >>> +++ b/drivers/bluetooth/btqcomsmd.c\n> >>> @@ -26,6 +26,7 @@\n> >>> struct btqcomsmd {\n> >>> struct hci_dev *hdev;\n> >>> \n> >>> + const bdaddr_t *addr;\n> >>> struct rpmsg_endpoint *acl_channel;\n> >>> struct rpmsg_endpoint *cmd_channel;\n> >>> };\n> >>> @@ -100,6 +101,27 @@ static int btqcomsmd_close(struct hci_dev *hdev)\n> >>> return 0;\n> >>> }\n> >>> \n> >>> +static int btqcomsmd_setup(struct hci_dev *hdev)\n> >>> +{\n> >>> + struct btqcomsmd *btq = hci_get_drvdata(hdev);\n> >>> + struct sk_buff *skb;\n> >>> +\n> >>> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);\n> >>> + if (IS_ERR(skb))\n> >>> + return PTR_ERR(skb);\n> >>> + kfree_skb(skb);\n> >>> +\n> >>> + if (btq->addr) {\n> >>> + bdaddr_t bdaddr;\n> >>> +\n> >>> + /* btq->addr stored with most significant byte first */\n> >>> + baswap(&bdaddr, btq->addr);\n> >>> + return qca_set_bdaddr_rome(hdev, &bdaddr);\n> >>> + }\n> >>> +\n> >>> + return 0;\n> >>> +}\n> >>> +\n> >>> static int btqcomsmd_probe(struct platform_device *pdev)\n> >>> {\n> >>> struct btqcomsmd *btq;\n> >>> @@ -123,6 +145,11 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n> >>> if (IS_ERR(btq->cmd_channel))\n> >>> return PTR_ERR(btq->cmd_channel);\n> >>> \n> >>> + btq->addr = of_get_property(pdev->dev.of_node, \"local-mac-address\",\n> >>> + &ret);\n> >>> + if (ret != sizeof(bdaddr_t))\n> >>> + btq->addr = NULL;\n> >>> +\n> >>> hdev = hci_alloc_dev();\n> >>> if (!hdev)\n> >>> return -ENOMEM;\n> >>> @@ -135,6 +162,7 @@ static int btqcomsmd_probe(struct platform_device *pdev)\n> >>> hdev->open = btqcomsmd_open;\n> >>> hdev->close = btqcomsmd_close;\n> >>> hdev->send = btqcomsmd_send;\n> >>> + hdev->setup = btqcomsmd_setup;\n> >>> hdev->set_bdaddr = qca_set_bdaddr_rome;\n> >> \n> >> I do not like this patch. Why not just set HCI_QUIRK_INVALID_BDADDR and let a userspace tool deal with reading the BD_ADDR from some storage.\n> >> \n> >> Frankly I do not get this WiFI MAC address or BD_ADDR stored in DT. I assumed the DT is suppose to describe hardware and not some value that is normally retrieved for OTP or alike.\n> > \n> > Use of \"local-mac-address\" for ethernet at least has existed as long\n> > at OpenFirmware I think. For some platforms, DT is the only OTP. And\n> > sometimes, the bootloader (like u-boot) stores MAC addresses and then\n> > populates them on boot.\n> > \n> > Seems like if we just let userspace deal with it, then we're back to a\n> > btattach tool with every platform's specific way of reading the MAC\n> > address.\n> \n> for Bluetooth that is not true. We have Set Public Address command\n> that is uniquely handling this and the HCI_QUIRK_INVALID_BDADDR\n> address does the right magic to allow userspace to identify a missing\n> address. It is done nicely and correctly and works fine.\n> \n\nYou're right in that we have a nice standardized way of informing user\nspace that the bd address is invalid and a nice standardized way for the\nuser to specify an address.\n\nWhat I believe Rob is saying (and what is my problem) is that the user\nspace tool reading the address from somewhere and calling this API is\nnot standard - so we end up maintaining some custom\n\"read-address-and-call-public-address\"-tool for both Debian and\nOpenEmbedded, plus we need to ensure that all our customers include and\nlaunch this tool in their own systems.\n\n> Mind you this is even used when there actually is a BD_ADDR, but the\n> device manufacturer wants to have one from its own OUI range compared\n> to the chip manufacturer’s OUI range.\n> \n> If DT is really the only place for the BD_ADDR and the bootloader\n> kinda does add / merge it into the DT, then by all means that is fine.\n> However if it is not, then this feature is dangerous since it can lead\n> to multiple devices with the same address. I rather have these devices\n> leave the kernel in unconfigured mode. And then force a userspace tool\n> to use Set Public Address to bring it into configured mode.\n> \n\nThe \"new\" property is optional and think that for devices that has not\nbeen provisioned with a bd address the boot loader should not add this\nproperty.\n\n\nBase on the fact that the firmware is built with the assumption that the\nhost will set the bd address I think the patch should be amended to\nspecify HCI_QUIRK_INVALID_BDADDR in the absence of this property.\n\nRegards,\nBjorn","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"hRStISmI\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xllxf6V9pz9sP3\n\tfor ;\n\tMon, 4 Sep 2017 07:10:30 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752945AbdICVKT (ORCPT );\n\tSun, 3 Sep 2017 17:10:19 -0400","from mail-pg0-f42.google.com ([74.125.83.42]:32908 \"EHLO\n\tmail-pg0-f42.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752942AbdICVKR (ORCPT\n\t); Sun, 3 Sep 2017 17:10:17 -0400","by mail-pg0-f42.google.com with SMTP id t3so13693847pgt.0\n\tfor ; Sun, 03 Sep 2017 14:10:17 -0700 (PDT)","from tuxbook (ip68-111-217-79.sd.sd.cox.net. [68.111.217.79])\n\tby smtp.gmail.com with ESMTPSA id\n\tq75sm1462547pfd.104.2017.09.03.14.10.14\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 03 Sep 2017 14:10:15 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=+yOvwgnE2wxlLpcJN+OeKU/xCaxSzEd2UqO20IaA2R4=;\n\tb=hRStISmIcyQELQlIwn5NkA5x/MDtjipiSnGRP92ZpFiNuPjwg7Pm755fWauEEKYTKy\n\tsZOn5fXQJoWfjMsyuHmW2nb06jNJUOXyJyKsq4KHz5Ix6NY3snSbxTDCSygTsJWr48en\n\twMd5SHtR/jLVsMwhcy4JKqeEFV6YgV2SHduFg=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=+yOvwgnE2wxlLpcJN+OeKU/xCaxSzEd2UqO20IaA2R4=;\n\tb=Jdmt/9yRm0wZGocEBWCl/QnWRAvibSlxv5Kq49uOnQZz4V11/ocVEKMio0yZAiHNEb\n\tz0FxkEq+AOB2yFtYRaw/S1QAevQkDKUunDbGrlHwgzN8WDijbkA9x5I4RbsNegQdgD9R\n\t9FqBnpo2182E8BoUp0MFwnqPgHenh7xel6wOTCd53I0qY0AZ85AsqxYiLZAqPPLuDXKV\n\toJsbI4fRKsQ1ZUxaIfMqX4g2fPx6FVHTDlZFyF0U2csEF5JYhJcNwORPkrmq0yOi+8z/\n\ti4H/aKkkd8Hwwr0K5sVDzkl4iszesygYGvXZEs+O/dBbWDjaQwkxVn7OzmGM3I0xpeHQ\n\tkgIw==","X-Gm-Message-State":"AHPjjUhluDVD2GyDxTRIwf0ZkwA/pZAtPhjmGaWmG1Todk4K54uhKywy\n\t/2e+jUznTN3bwe8y","X-Google-Smtp-Source":"ADKCNb686v9WuCNBldDNTDnie9S3o3PmswyyxORgNZsi8Aig0Nrw6IB+VoTI298/OvXaD4zf38+0jQ==","X-Received":"by 10.84.211.75 with SMTP id b69mr518572pli.277.1504473016569;\n\tSun, 03 Sep 2017 14:10:16 -0700 (PDT)","Date":"Sun, 3 Sep 2017 14:10:12 -0700","From":"Bjorn Andersson ","To":"Marcel Holtmann ","Cc":"Rob Herring ,\n\t\"Gustavo F. Padovan\" ,\n\tJohan Hedberg ,\n\t\"David S. Miller\" ,\n\t\"open list:BLUETOOTH DRIVERS\" ,\n\tNetwork Development ,\n\tLKML ,\n\tlinux-arm-msm ,\n\tLoic Poulain ","Subject":"Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup","Message-ID":"<20170903211012.GJ2016@tuxbook>","References":"<20170901204118.17123-1-bjorn.andersson@linaro.org>\n\t<20170901204118.17123-3-bjorn.andersson@linaro.org>\n\t<5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=windows-1252","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]