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-hba...@codeaurora.org>
> > > Reviewed-by: Matthias Kaehlcke <m...@chromium.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.
