From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthias Kaehlcke Subject: Re: [PATCH v1] Bluetooth: hci_qca: Enable the ldisc for ROME for x86 platforms. Date: Thu, 7 Mar 2019 12:42:24 -0800 Message-ID: <20190307204224.GD138592@google.com> References: <20190307101722.25871-1-bgodavar@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Return-path: Content-Disposition: inline In-Reply-To: <20190307101722.25871-1-bgodavar@codeaurora.org> Sender: linux-kernel-owner@vger.kernel.org To: Balakrishna Godavarthi Cc: marcel@holtmann.org, johan.hedberg@gmail.com, linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org, hemantg@codeaurora.org, linux-arm-msm@vger.kernel.org, rjliao@codeaurora.org List-Id: linux-arm-msm@vger.kernel.org Hi Balakrishna, On Thu, Mar 07, 2019 at 03:47:22PM +0530, Balakrishna Godavarthi wrote: > When using btattach to setup Rome over ldisc we observed a crash > in qca_setup as it will try to access the serdev which is not > available in the ldisc proto. This patch will fix the crash by > support both the ldisc and serdev way in the qca hci_uart driver. > > Signed-off-by: Balakrishna Godavarthi Oh, I wasn't aware of the instantiation through ldisc and was actually considering to *remove* some of the seemingly unnecessary serdev checks. > --- > drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++++--------------- > 1 file changed, 28 insertions(+), 19 deletions(-) > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index 237aea34b69f..0a5c98d46864 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -963,7 +963,7 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate) > { > struct hci_uart *hu = hci_get_drvdata(hdev); > struct qca_data *qca = hu->priv; > - struct qca_serdev *qcadev; > + struct qca_serdev *qcadev = NULL; In many cases the only field that is accessed is qcadev->btsoc_type. I think something like 'qca_get_soc_type(struct hci_dev *hdev / struct hci_uart *hu)' would make things more readable. IMO the whole 'qcadev' vs 'qca(_data)' is confusing anyway, in this sense even better if we can make most of the 'qcadev' references disappear. Thanks Matthias