Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3830-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <687c8f6a-d5d6-4918-bfd5-93d4b04da086@molgen.mpg.de>
Date: Mon, 22 Apr 2024 09:32:47 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] Bluetooth: Populate hci_set_hw_info for Intel and Realtek
To: Archie Pusaka <apusaka@google.com>, Archie Pusaka <apusaka@chromium.org>
Cc: linux-bluetooth@vger.kernel.org,
 Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
 Johan Hedberg <johan.hedberg@gmail.com>,
 Marcel Holtmann <marcel@holtmann.org>,
 chromeos-bluetooth-upstreaming@chromium.org,
 Abhishek Pandit-Subedi <abhishekpandit@google.com>,
 linux-kernel@vger.kernel.org
References: <20240422152413.1.Ib96985e197f3db620a127a84aa20f3f3017aaf57@changeid>
Content-Language: en-US
From: Paul Menzel <pmenzel@molgen.mpg.de>
In-Reply-To: <20240422152413.1.Ib96985e197f3db620a127a84aa20f3f3017aaf57@changeid>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Dear Archie,


Thank you for your patch.

Am 22.04.24 um 09:24 schrieb Archie Pusaka:
> From: Archie Pusaka <apusaka@chromium.org>
> 
> The hardware information surfaced via debugfs might be usable by the
> userspace to set some configuration knobs. This patch sets the hw_info
> for Intel and Realtek chipsets.

Could you please add an example paste to the commit message?

> Signed-off-by: Archie Pusaka <apusaka@chromium.org>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@google.com>
> 
> ---
> 
>   drivers/bluetooth/btintel.c | 9 +++++++++
>   drivers/bluetooth/btrtl.c   | 7 +++++++
>   2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index a19ebe47bd951..dc48352166a52 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2956,6 +2956,11 @@ static int btintel_setup_combined(struct hci_dev *hdev)
>   			err = -EINVAL;
>   		}
>   
> +		hci_set_hw_info(hdev,
> +				"INTEL platform=%u variant=%u revision=%u",
> +				ver.hw_platform, ver.hw_variant,
> +				ver.hw_revision);
> +
>   		goto exit_error;
>   	}
>   
> @@ -3060,6 +3065,10 @@ static int btintel_setup_combined(struct hci_dev *hdev)
>   		break;
>   	}
>   
> +	hci_set_hw_info(hdev, "INTEL platform=%u variant=%u",
> +			INTEL_HW_PLATFORM(ver_tlv.cnvi_bt),
> +			INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
> +

Why does it need to be added at two places?

>   exit_error:
>   	kfree_skb(skb);
>   
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index cc50de69e8dc9..4f1e37b4f7802 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -1339,6 +1339,13 @@ int btrtl_setup_realtek(struct hci_dev *hdev)
>   
>   	btrtl_set_quirks(hdev, btrtl_dev);
>   
> +	hci_set_hw_info(hdev,
> +			"RTL lmp_subver=%u hci_rev=%u hci_ver=%u hci_bus=%u",
> +			btrtl_dev->ic_info->lmp_subver,
> +			btrtl_dev->ic_info->hci_rev,
> +			btrtl_dev->ic_info->hci_ver,
> +			btrtl_dev->ic_info->hci_bus);
> +
>   	btrtl_free(btrtl_dev);
>   	return ret;
>   }


Kind regards,

Paul