linux-kernel - Re: [PATCH v7 1/2] Bluetooth: hci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190425164644.GG112750@google.com>
Date:   Thu, 25 Apr 2019 09:46:44 -0700
From:   Matthias Kaehlcke <mka@...omium.org>
To:     Harish Bandi <c-hbandi@...eaurora.org>
Cc:     Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        linux-kernel@...r.kernel.org, linux-bluetooth@...r.kernel.org,
        hemantg@...eaurora.org, linux-arm-msm@...r.kernel.org,
        bgodavar@...eaurora.org, anubhavg@...eaurora.org,
        devicetree@...r.kernel.org, mark.rutland@....com,
        robh+dt@...nel.org
Subject: Re: [PATCH v7 1/2] Bluetooth: hci_qca: Added support for WCN3998

On Thu, Apr 25, 2019 at 07:10:51PM +0530, Harish Bandi wrote:
> Hi Marcel,
> 
> On 2019-04-25 18:17, Marcel Holtmann wrote:
> > Hi Harish,
> > 
> > > Added new compatible for WCN3998 and corresponding voltage
> > > and current values to WCN3998 compatible.
> > > Changed driver code to support WCN3998
> > > 
> > > Signed-off-by: Harish Bandi <c-hbandi@...eaurora.org>
> > > Reviewed-by: Matthias Kaehlcke <mka@...omium.org>
> > > ---
> > > Changes in V7:
> > > - Initialized rom_ver to 0 to fix compiler warning
> > > ---
> > > drivers/bluetooth/btqca.c   | 12 +++++++++---
> > > drivers/bluetooth/btqca.h   |  8 +++++++-
> > > drivers/bluetooth/hci_qca.c | 40
> > > ++++++++++++++++++++++++++--------------
> > > 3 files changed, 42 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> > > index 6122685..495a52f 100644
> > > --- a/drivers/bluetooth/btqca.c
> > > +++ b/drivers/bluetooth/btqca.c
> > > @@ -336,7 +336,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t
> > > baudrate,
> > > {
> > > 	struct rome_config config;
> > > 	int err;
> > > -	u8 rom_ver;
> > > +	u8 rom_ver = 0;
> > 
> > what is this change for?
> [Harish] - kbuild test robot gave compiler warning. So initialized.
> drivers/bluetooth/btqca.c:369:3: warning: 'rom_ver' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> 
> > 
> > > 	bt_dev_dbg(hdev, "QCA setup on UART");
> > > 
> > > @@ -344,7 +344,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t
> > > baudrate,
> > > 
> > > 	/* Download rampatch file */
> > > 	config.type = TLV_TYPE_PATCH;
> > > -	if (soc_type == QCA_WCN3990) {
> > > +	if (qca_is_wcn399x(soc_type)) {
> > > 		/* Firmware files to download are based on ROM version.
> > > 		 * ROM version is derived from last two bytes of soc_ver.
> > > 		 */
> > > @@ -365,7 +365,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t
> > > baudrate,
> > > 
> > > 	/* Download NVM configuration */
> > > 	config.type = TLV_TYPE_NVM;
> > > -	if (soc_type == QCA_WCN3990)
> > > +	if (qca_is_wcn399x(soc_type))
> > > 		snprintf(config.fwname, sizeof(config.fwname),
> > > 			 "qca/crnv%02x.bin", rom_ver);
> > > 	else
> > > @@ -410,6 +410,12 @@ int qca_set_bdaddr(struct hci_dev *hdev, const
> > > bdaddr_t *bdaddr)
> > > }
> > > EXPORT_SYMBOL_GPL(qca_set_bdaddr);
> > > 
> > > +bool qca_is_wcn399x(enum qca_btsoc_type soc_type)
> > > +{
> > > +	return ((soc_type == QCA_WCN3990) || (soc_type == QCA_WCN3998));
> > 
> > no () needed around soc_type = check.
> 
> [Harish]  - OK, will change it.
> > 
> > > +}
> > > +EXPORT_SYMBOL_GPL(qca_is_wcn399x);
> > > +
> > 
> > Why is this exported. Make this an inline function in btqca.h.
> [Harish] - This was used in btqca.c also to check the soc_type

true, but this is still possible with an inline function in btqca.h.