Hi Marcel,
On 12/8/14, 9:42 PM, "Marcel Holtmann" <marcel@xxxxxxxxxxxx> wrote:
>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@xxxxxxxxx>
>> ---
>> 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
>
I updated my patch and will send out v3.
Regards,
Tedd
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
