* [RFC v2 2/4] Bluetooth: Set the firmware file extension based on firmware type
@ 2014-12-08 23:24 Tedd Ho-Jeong An
2014-12-09 5:42 ` Marcel Holtmann
0 siblings, 1 reply; 3+ messages in thread
From: Tedd Ho-Jeong An @ 2014-12-08 23:24 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org; +Cc: Johan Hedberg, An, Tedd, Marcel Holtmann
From: Tedd Ho-Jeong An <tedd.an@intel.com>
This patch sets the extension of Intel Bluetooth firmware file based on
firmware type.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btusb.c | 43 +++++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 9ab396b..b3ce85f 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1334,12 +1334,18 @@ struct intel_version {
u8 fw_patch_num;
} __packed;
-static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev)
+enum {
+ INTEL_FW_MODE_OP,
+};
+
+static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
+ int fw_mode)
{
const struct firmware *fw;
struct sk_buff *skb;
struct intel_version *ver;
char fwname[64];
+ char ext[6] = {0};
int ret;
BT_DBG("%s", hdev->name);
@@ -1377,23 +1383,32 @@ static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev)
ver->fw_build_num, ver->fw_build_ww, ver->fw_build_yy,
ver->fw_patch_num);
- /* fw_patch_num indicates the version of patch the device currently
- * have. If there is no patch data in the device, it is always 0x00.
- * So, if it is other than 0x00, no need to patch the device again.
- */
- if (ver->fw_patch_num) {
- BT_INFO("%s: Intel device is already patched. patch num: %02x",
- hdev->name, ver->fw_patch_num);
- btusb_check_bdaddr_intel(hdev);
+ /* set the extension of firmware file based on firmware type */
+ if (fw_mode == INTEL_FW_MODE_OP) {
+ /* fw_patch_num indicates the version of patch the device
+ * currently have. If there is no patch data in the device,
+ * it is always 0x00. So, if it is other than 0x00, no need
+ * to patch the device again.
+ */
+ if (ver->fw_patch_num) {
+ BT_INFO("%s: Intel device is already patched",
+ hdev->name);
+ btusb_check_bdaddr_intel(hdev);
+ return NULL;
+ }
+ strncpy(ext, "bseq", 4);
+ } else {
+ BT_ERR("%s: Invalid Intel firmware type: %d",
+ hdev->name, fw_mode);
return NULL;
}
/* Open the firwmare file if it exists */
snprintf(fwname, sizeof(fwname),
- "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
+ "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.%s",
ver->hw_platform, ver->hw_variant, ver->hw_revision,
ver->fw_variant, ver->fw_revision, ver->fw_build_num,
- ver->fw_build_ww, ver->fw_build_yy);
+ ver->fw_build_ww, ver->fw_build_yy, ext);
ret = request_firmware(&fw, fwname, &hdev->dev);
if (ret < 0) {
@@ -1409,8 +1424,8 @@ static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev)
/* If the correct firmware patch file is not found, use the
* default firmware patch file instead
*/
- snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%x.%x.bseq",
- ver->hw_platform, ver->hw_variant);
+ snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%x.%x.%s",
+ ver->hw_platform, ver->hw_variant, ext);
if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
BT_ERR("%s failed to open default Intel fw file: %s",
hdev->name, fwname);
@@ -1574,7 +1589,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
* If no patch file is found, allow the device to operate without
* a patch.
*/
- fw = btusb_setup_intel_get_fw(hdev);
+ fw = btusb_setup_intel_get_fw(hdev, INTEL_FW_MODE_OP);
if (!fw) {
btusb_check_bdaddr_intel(hdev);
return 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC v2 2/4] Bluetooth: Set the firmware file extension based on firmware type
2014-12-08 23:24 [RFC v2 2/4] Bluetooth: Set the firmware file extension based on firmware type Tedd Ho-Jeong An
@ 2014-12-09 5:42 ` Marcel Holtmann
2014-12-09 23:16 ` An, Tedd
0 siblings, 1 reply; 3+ messages in thread
From: Marcel Holtmann @ 2014-12-09 5:42 UTC (permalink / raw)
To: Tedd Ho-Jeong An; +Cc: linux-bluetooth@vger.kernel.org, Johan Hedberg
Hi Tedd,
> This patch sets the extension of Intel Bluetooth firmware file based on
> firmware type.
>
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
> drivers/bluetooth/btusb.c | 43 +++++++++++++++++++++++++++++--------------
> 1 file changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 9ab396b..b3ce85f 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -1334,12 +1334,18 @@ struct intel_version {
> u8 fw_patch_num;
> } __packed;
>
> -static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev)
> +enum {
> + INTEL_FW_MODE_OP,
> +};
lets name this enum.
> +
> +static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
> + int fw_mode)
And then use the enum here instead of just int.
> {
> const struct firmware *fw;
> struct sk_buff *skb;
> struct intel_version *ver;
> char fwname[64];
> + char ext[6] = {0};
Do not initialize this one. And actually make it const char *ext. Actually naming it file_ext might make the code a lot easier to read and get what this is for.
> int ret;
>
> BT_DBG("%s", hdev->name);
> @@ -1377,23 +1383,32 @@ static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev)
> ver->fw_build_num, ver->fw_build_ww, ver->fw_build_yy,
> ver->fw_patch_num);
>
> - /* fw_patch_num indicates the version of patch the device currently
> - * have. If there is no patch data in the device, it is always 0x00.
> - * So, if it is other than 0x00, no need to patch the device again.
> - */
> - if (ver->fw_patch_num) {
> - BT_INFO("%s: Intel device is already patched. patch num: %02x",
> - hdev->name, ver->fw_patch_num);
> - btusb_check_bdaddr_intel(hdev);
> + /* set the extension of firmware file based on firmware type */
> + if (fw_mode == INTEL_FW_MODE_OP) {
> + /* fw_patch_num indicates the version of patch the device
> + * currently have. If there is no patch data in the device,
> + * it is always 0x00. So, if it is other than 0x00, no need
> + * to patch the device again.
> + */
> + if (ver->fw_patch_num) {
> + BT_INFO("%s: Intel device is already patched",
> + hdev->name);
> + btusb_check_bdaddr_intel(hdev);
As in the previous patch, this btusb_check_bdaddr_intel is not needed. It would cause double execution actually.
> + return NULL;
> + }
> + strncpy(ext, "bseq", 4);
Using ext = "bseq"; works just fine if this is a const pointer. It is all in the same function and thus does not need extra variable space to copy things into.
> + } else {
> + BT_ERR("%s: Invalid Intel firmware type: %d",
> + hdev->name, fw_mode);
> return NULL;
> }
Since we know that it will be an enum, lets use a switch statement here.
>
> /* Open the firwmare file if it exists */
> snprintf(fwname, sizeof(fwname),
> - "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
> + "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.%s",
> ver->hw_platform, ver->hw_variant, ver->hw_revision,
> ver->fw_variant, ver->fw_revision, ver->fw_build_num,
> - ver->fw_build_ww, ver->fw_build_yy);
> + ver->fw_build_ww, ver->fw_build_yy, ext);
>
> ret = request_firmware(&fw, fwname, &hdev->dev);
> if (ret < 0) {
> @@ -1409,8 +1424,8 @@ static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev)
> /* If the correct firmware patch file is not found, use the
> * default firmware patch file instead
> */
> - snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%x.%x.bseq",
> - ver->hw_platform, ver->hw_variant);
> + snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%x.%x.%s",
> + ver->hw_platform, ver->hw_variant, ext);
> if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
> BT_ERR("%s failed to open default Intel fw file: %s",
> hdev->name, fwname);
> @@ -1574,7 +1589,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
> * If no patch file is found, allow the device to operate without
> * a patch.
> */
> - fw = btusb_setup_intel_get_fw(hdev);
> + fw = btusb_setup_intel_get_fw(hdev, INTEL_FW_MODE_OP);
> if (!fw) {
> btusb_check_bdaddr_intel(hdev);
> return 0;
Regards
Marcel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC v2 2/4] Bluetooth: Set the firmware file extension based on firmware type
2014-12-09 5:42 ` Marcel Holtmann
@ 2014-12-09 23:16 ` An, Tedd
0 siblings, 0 replies; 3+ messages in thread
From: An, Tedd @ 2014-12-09 23:16 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org, Johan Hedberg
Hi Marcel,
On 12/8/14, 9:42 PM, "Marcel Holtmann" <marcel@holtmann.org> wrote:
>Hi Tedd,
>
>> This patch sets the extension of Intel Bluetooth firmware file based on
>> firmware type.
>>=20
>> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
>> ---
>> drivers/bluetooth/btusb.c | 43
>>+++++++++++++++++++++++++++++--------------
>> 1 file changed, 29 insertions(+), 14 deletions(-)
>>=20
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index 9ab396b..b3ce85f 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -1334,12 +1334,18 @@ struct intel_version {
>> u8 fw_patch_num;
>> } __packed;
>>=20
>> -static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev
>>*hdev)
>> +enum {
>> + INTEL_FW_MODE_OP,
>> +};
>
>lets name this enum.
>
>> +
>> +static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev
>>*hdev,
>> + int fw_mode)
>
>And then use the enum here instead of just int.
>
>> {
>> const struct firmware *fw;
>> struct sk_buff *skb;
>> struct intel_version *ver;
>> char fwname[64];
>> + char ext[6] =3D {0};
>
>Do not initialize this one. And actually make it const char *ext.
>Actually naming it file_ext might make the code a lot easier to read and
>get what this is for.
>
>> int ret;
>>=20
>> BT_DBG("%s", hdev->name);
>> @@ -1377,23 +1383,32 @@ static const struct firmware
>>*btusb_setup_intel_get_fw(struct hci_dev *hdev)
>> ver->fw_build_num, ver->fw_build_ww, ver->fw_build_yy,
>> ver->fw_patch_num);
>>=20
>> - /* fw_patch_num indicates the version of patch the device currently
>> - * have. If there is no patch data in the device, it is always 0x00.
>> - * So, if it is other than 0x00, no need to patch the device again.
>> - */
>> - if (ver->fw_patch_num) {
>> - BT_INFO("%s: Intel device is already patched. patch num: %02x",
>> - hdev->name, ver->fw_patch_num);
>> - btusb_check_bdaddr_intel(hdev);
>> + /* set the extension of firmware file based on firmware type */
>> + if (fw_mode =3D=3D INTEL_FW_MODE_OP) {
>> + /* fw_patch_num indicates the version of patch the device
>> + * currently have. If there is no patch data in the device,
>> + * it is always 0x00. So, if it is other than 0x00, no need
>> + * to patch the device again.
>> + */
>> + if (ver->fw_patch_num) {
>> + BT_INFO("%s: Intel device is already patched",
>> + hdev->name);
>> + btusb_check_bdaddr_intel(hdev);
>
>As in the previous patch, this btusb_check_bdaddr_intel is not needed. It
>would cause double execution actually.
>
>> + return NULL;
>> + }
>> + strncpy(ext, "bseq", 4);
>
>Using ext =3D "bseq"; works just fine if this is a const pointer. It is al=
l
>in the same function and thus does not need extra variable space to copy
>things into.
>
>> + } else {
>> + BT_ERR("%s: Invalid Intel firmware type: %d",
>> + hdev->name, fw_mode);
>> return NULL;
>> }
>
>Since we know that it will be an enum, lets use a switch statement here.
>
>>=20
>> /* Open the firwmare file if it exists */
>> snprintf(fwname, sizeof(fwname),
>> - "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
>> + "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.%s",
>> ver->hw_platform, ver->hw_variant, ver->hw_revision,
>> ver->fw_variant, ver->fw_revision, ver->fw_build_num,
>> - ver->fw_build_ww, ver->fw_build_yy);
>> + ver->fw_build_ww, ver->fw_build_yy, ext);
>>=20
>> ret =3D request_firmware(&fw, fwname, &hdev->dev);
>> if (ret < 0) {
>> @@ -1409,8 +1424,8 @@ static const struct firmware
>>*btusb_setup_intel_get_fw(struct hci_dev *hdev)
>> /* If the correct firmware patch file is not found, use the
>> * default firmware patch file instead
>> */
>> - snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%x.%x.bseq",
>> - ver->hw_platform, ver->hw_variant);
>> + snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%x.%x.%s",
>> + ver->hw_platform, ver->hw_variant, ext);
>> if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
>> BT_ERR("%s failed to open default Intel fw file: %s",
>> hdev->name, fwname);
>> @@ -1574,7 +1589,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
>> * If no patch file is found, allow the device to operate without
>> * a patch.
>> */
>> - fw =3D btusb_setup_intel_get_fw(hdev);
>> + fw =3D btusb_setup_intel_get_fw(hdev, INTEL_FW_MODE_OP);
>> if (!fw) {
>> btusb_check_bdaddr_intel(hdev);
>> return 0;
>
>Regards
>
>Marcel
>
I updated my patch and will send out v3.
Regards,
Tedd
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-12-09 23:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-08 23:24 [RFC v2 2/4] Bluetooth: Set the firmware file extension based on firmware type Tedd Ho-Jeong An
2014-12-09 5:42 ` Marcel Holtmann
2014-12-09 23:16 ` An, Tedd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).