Re: [PATCH v7 2/8] Bluetooth: btqca: Rename string ROME to QCA in logs.

[Matthias Kaehlcke <mka@chromium.org>]

On Sat, Jun 16, 2018 at 11:57:12AM +0530, Balakrishna Godavarthi wrote:
> To have generic text in bt_dev_info, bt_dev_err and bt_dev_dbg
> updated from ROME to QCA where ever possible. This avoids
> confusion to user, when using the future Qualcomm Bluetooth SoC's.
> Updated BT_DBG, BT_ERR and BT_INFO with bt_dev_dbg, bt_dev_err and
> bt_dev_info where ever applicable.
> 
> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
> ---
> Changes in v7:
> 	* initial patch.
> 	* Updated the logs to generic.
> 
> ---
>  drivers/bluetooth/btqca.c | 56 +++++++++++++++++++--------------------
>  1 file changed, 27 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index 8219816c54a0..a36fae1f3c2c 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -35,36 +35,35 @@ static int rome_patch_ver_req(struct hci_dev *hdev, u32 *rome_version)
>  	char cmd;
>  	int err = 0;
>  
> -	BT_DBG("%s: ROME Patch Version Request", hdev->name);
> +	bt_dev_dbg(hdev, "QCA Version Request");
>  
>  	cmd = EDL_PATCH_VER_REQ_CMD;
>  	skb = __hci_cmd_sync_ev(hdev, EDL_PATCH_CMD_OPCODE, EDL_PATCH_CMD_LEN,
>  				&cmd, HCI_VENDOR_PKT, HCI_INIT_TIMEOUT);
>  	if (IS_ERR(skb)) {
>  		err = PTR_ERR(skb);
> -		BT_ERR("%s: Failed to read version of ROME (%d)", hdev->name,
> -		       err);
> +		bt_dev_err(hdev, "Reading QCA version information failed (%d)",
> +			   err);

[Not really the scope of this patch/series, but an idea for a future
improvement:

Some error messages put the error code inside parentheses, others put
it after a colon. Would be good to unify this (my preference would be
after the colon).]

Shouldn't this patch be squashed with "[3/8] Bluetooth: btqca: Rename
ROME related functions to Generic functions"? It's a bit odd to do the
renaming of the functions and log entries in two steps, even though
the end result is the same.

What would make sense is to have a separate patch for BT_XYZ() to
bt_dev_xyz(), though it's probably ok to do it in the patch that
changes the log strings.