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