Re: [PATCH v10] Bluetooth: btwilink driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 9, 2011 at 3:49 PM, Ville Tervo <ville.tervo@xxxxxxxxx> wrote:
> Hi Pavan,
>
> On Wed, Feb 09, 2011 at 01:39:37AM -0600, ext pavan_savoy@xxxxxx wrote:
>> From: Pavan Savoy <pavan_savoy@xxxxxx>
>>
>> Gustavo,
>>
>> v10,
>> fixing the really long goto label.
>>
>> Find below the patch version v9.
>> Have taken care of Ville's comments.
>> In addition fixed the issue seen when built as module, with
>> repeated probe/remove of the platform driver.
>>
>> patch v8 comments taken care,
>>
>> As suggested, the modifications to TI ST driver to remove the bluetooth
>> references have been made and the changes have been merged to the
>> linux-next tree.
>>
>> Find below the patch for the btwilink driver,
>> I have taken care of your following comments,
>> 1. channel IDs and the offsets are now being taken from the
>> header file hci.h.
>> 2. removed the un-necessary BT_ERR.
>> 3. changed the order during return from st_register.
>> 4. continue to unregister even if 1st unregister fails.
>>
>> Please review and provide comments,
>
> One more comment.
>
>> +/* Called from HCI core to initialize the device */
>> +static int ti_st_open(struct hci_dev *hdev)
>> +{
>> + Â Â unsigned long timeleft;
>> + Â Â struct ti_st *hst;
>> + Â Â int err, i;
>> +
>> + Â Â BT_DBG("%s %p", hdev->name, hdev);
>> +
>> + Â Â if (test_and_set_bit(HCI_RUNNING, &hdev->flags))
>> + Â Â Â Â Â Â return -EBUSY;
>> +
>> + Â Â /* provide contexts for callbacks from ST */
>> + Â Â hst = hdev->driver_data;
>> +
>> + Â Â for (i = 0; i < MAX_BT_CHNL_IDS; i++) {
>> + Â Â Â Â Â Â ti_st_proto[i].priv_data = hst;
>> + Â Â Â Â Â Â ti_st_proto[i].max_frame_size = HCI_MAX_FRAME_SIZE;
>> + Â Â Â Â Â Â ti_st_proto[i].recv = st_receive;
>> + Â Â Â Â Â Â ti_st_proto[i].reg_complete_cb = st_reg_completion_cb;
>> +
>> + Â Â Â Â Â Â /* Prepare wait-for-completion handler */
>> + Â Â Â Â Â Â init_completion(&hst->wait_reg_completion);
>> +
>> + Â Â Â Â Â Â err = st_register(&ti_st_proto[i]);
>> + Â Â Â Â Â Â if (!err)
>> + Â Â Â Â Â Â Â Â Â Â goto done;
>> +
>> + Â Â Â Â Â Â if (err != -EINPROGRESS) {
>> + Â Â Â Â Â Â Â Â Â Â clear_bit(HCI_RUNNING, &hdev->flags);
>> + Â Â Â Â Â Â Â Â Â Â BT_ERR("st_register failed %d", err);
>> + Â Â Â Â Â Â Â Â Â Â return err;
>> + Â Â Â Â Â Â }
>> +
>> + Â Â Â Â Â Â /* ST is busy with either protocol
>> + Â Â Â Â Â Â Â* registration or firmware download.
>> + Â Â Â Â Â Â Â*/
>> +
>> + Â Â Â Â Â Â /* Reset ST registration callback status flag,
>> + Â Â Â Â Â Â Â* this value will be updated in
>> + Â Â Â Â Â Â Â* st_reg_completion_cb()
>> + Â Â Â Â Â Â Â* function whenever it called from ST driver.
>> + Â Â Â Â Â Â Â*/
>> + Â Â Â Â Â Â hst->reg_status = -EINPROGRESS;
>
> reg_status initialization should be moved also before st_register(). Reason is
> again the same as for init_completion.

You under-estimate the time taken for firmware download :)
Yes alright, I agree it makes more sense to put it up there.

Will fix up and resend....


> --
> Ville
> --
> 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
>
--
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


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux