On 25 September 2014 20:57, Jassi Brar <jaswinder.singh@xxxxxxxxxx> wrote: > On 24 September 2014 09:14, Ashwin Chaugule <ashwin.chaugule@xxxxxxxxxx> wrote: >> On 22 September 2014 14:33, Sudeep Holla <sudeep.holla@xxxxxxx> wrote: > >>>>>> +static void poll_txdone(unsigned long data) >>>>>> +{ >>>>>> + struct mbox_controller *mbox = (struct mbox_controller *)data; >>>>>> + bool txdone, resched = false; >>>>>> + int i; >>>>>> + >>>>>> + for (i = 0; i < mbox->num_chans; i++) { >>>>>> + struct mbox_chan *chan = &mbox->chans[i]; >>>>>> + >>>>>> + if (chan->active_req && chan->cl) { >>>>>> + resched = true; >>>>>> + txdone = chan->mbox->ops->last_tx_done(chan); >>>>>> + if (txdone) >>>>>> + tx_tick(chan, 0); >>>>>> + } >>>>>> + } >>>>>> + >>>>>> + if (resched) >>>>>> + mod_timer(&mbox->poll, jiffies + >>>>>> + msecs_to_jiffies(mbox->period)); >>>>> >>>>> >>>>> While preparing a different patch which uses the Mbox framework, I >>>>> noticed that mbox->period might not be initialized anywhere. Also, how >>>>> is mbox->txpoll_period to be used? It appears from the description of >>>>> txpoll_period in mbox_controller.h that you'd want to use that value >>>>> in the mod_timer above, or equate the two somewhere in the controller >>>>> registration or eliminate one of the two. FWIW I also looked at your >>>>> code in [1]. >>>>> >>>> >>>> IIUC the controller needs to set the txpoll_period if it sets >>>> txdone_poll, may be a sanity check for !0 would be good. >>>> >>> >>> Ah, sorry I confused mbox->period to txpoll_period. >>> You are right mbox->period is not set, the header claims it to be >>> private, and hence I assume it needs to be handled only in core mailbox >>> library. Not sure if we need both mbox->period and txpoll_period though. >> >> Right. I dont see the need for having both either. Unless the Mailbox >> maintainer wants to fix this in some other way, I can send a patch for >> replacing mbox->period with mbox->txpoll_period along with my PCC >> work. >> > Yeah, probably we should just get rid of mbox_controller.period I > have updated the same in for-next. Looks like linux-next already has the older version[1]. Your for-next branch seems to have the fix squashed into the previous commit. Wouldnt you need to send this as a separate patch? Or ask for the patch to be reverted in linux-next and then pull again from your branch. Thanks, Ashwin [1] - http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/mailbox/mailbox.c -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html