Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Jul 2023 08:59:25 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Tim Jiang <quic_tjiang@quicinc.com>
Cc:     marcel@holtmann.org, linux-kernel@vger.kernel.org,
        linux-bluetooth@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        quic_bgodavar@quicinc.com, quic_hemantg@quicinc.com
Subject: Re: [PATCH v12 1/2] Bluetooth: hci_qca: Add support for Qualcomm
 Bluetooth SoC QCA2066
Message-ID: <ZMDETVMUmJxmbIvW@hovoldconsulting.com>
References: <20230726052245.609987-1-quic_tjiang@quicinc.com>
 <20230726052245.609987-2-quic_tjiang@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230726052245.609987-2-quic_tjiang@quicinc.com>
Precedence: bulk

Please fix up the subject of this patch, which should be different from
patch 2/2 and instead summarise what *this* patch does.

Note that keeping the same 2/2 subject on the cover letter is fine as
2/2 is the primary patch and sort of summarises the series.

On Wed, Jul 26, 2023 at 01:22:44PM +0800, Tim Jiang wrote:
> This patch make the print btsoc type expression more clearly.
> 
> Signed-off-by: Tim Jiang <quic_tjiang@quicinc.com>
> ---
>  drivers/bluetooth/btqca.h   |  1 +
>  drivers/bluetooth/hci_qca.c | 34 ++++++++++++++++++++++++++++++----
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index b884095bcd9d..529382f0abb1 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -148,6 +148,7 @@ enum qca_btsoc_type {
>  	QCA_QCA6390,
>  	QCA_WCN6750,
>  	QCA_WCN6855,
> +	QCA_QCA2066,

This belongs in the next patch.

These are currently not sorted by probably should be to make it easier
to look up and add new entries. This could be done in a third,
preparatory, patch.

>  };
>  
>  #if IS_ENABLED(CONFIG_BT_QCA)
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 9b785c947d96..453000df7aec 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1748,6 +1748,7 @@ static int qca_setup(struct hci_uart *hu)
>  	const char *firmware_name = qca_get_firmware_name(hu);
>  	int ret;
>  	struct qca_btsoc_version ver;
> +	const char *soc_name;
>  
>  	ret = qca_check_speeds(hu);
>  	if (ret)
> @@ -1762,10 +1763,35 @@ static int qca_setup(struct hci_uart *hu)
>  	 */
>  	set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
>  
> -	bt_dev_info(hdev, "setting up %s",
> -		qca_is_wcn399x(soc_type) ? "wcn399x" :
> -		(soc_type == QCA_WCN6750) ? "wcn6750" :
> -		(soc_type == QCA_WCN6855) ? "wcn6855" : "ROME/QCA6390");
> +	switch (soc_type) {
> +	case QCA_AR3002:
> +		soc_name = "ar300x";
> +		break;
> +	case QCA_ROME:
> +		soc_name = "ROME";
> +		break;
> +	case QCA_WCN3990:
> +	case QCA_WCN3991:
> +	case QCA_WCN3998:
> +		soc_name = "wcn399x";
> +		break;
> +	case QCA_QCA2066:
> +		soc_name = "QCA2066";
> +		break;

This also belongs in the next patch.

> +	case QCA_QCA6390:
> +		soc_name = "QCA6390";
> +		break;
> +	case QCA_WCN6750:
> +		soc_name = "wcn6750";
> +		break;
> +	case QCA_WCN6855:
> +		soc_name = "wcn6855";
> +		break;
> +	default:
> +		soc_name = "unknown soc";
> +		break;
> +	}

And you should probably sort the above as well.

> +	bt_dev_info(hdev, "setting up %s", soc_name);
>  
>  	qca->memdump_state = QCA_MEMDUMP_IDLE;

Johan