From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sudeep Holla Subject: Re: [PATCHv10 2/4] mailbox: Introduce framework for mailbox Date: Mon, 22 Sep 2014 19:15:56 +0100 Message-ID: <5420675C.6040805@arm.com> References: <1406896194-4667-1-git-send-email-jaswinder.singh@linaro.org> <1406896296-4863-1-git-send-email-jaswinder.singh@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Ashwin Chaugule , Jassi Brar Cc: Sudeep Holla , Devicetree List , lkml , "ks.giri-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org" , Arnd Bergmann , "ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org" , Mark Rutland , Rob Herring , Pawel Moll , Courtney Cavin , Matt Porter , Craig McGeachie , LeyFoon Tan , Loic Pallardy , "Anna, Suman" , Bjorn Andersson , Patch Tracking , "mollie.wu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" , Tetsuya Takinishi , "broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" , Kevin Hilman , "lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" List-Id: devicetree@vger.kernel.org On 22/09/14 19:01, Ashwin Chaugule wrote: > Hi Jassi, > > On 1 August 2014 08:31, Jassi Brar wrote: >> Introduce common framework for client/protocol drivers and >> controller drivers of Inter-Processor-Communication (IPC). >> >> Client driver developers should have a look at >> include/linux/mailbox_client.h to understand the part of >> the API exposed to client drivers. >> Similarly controller driver developers should have a look >> at include/linux/mailbox_controller.h >> >> Signed-off-by: Jassi Brar >> --- >> MAINTAINERS | 8 + >> drivers/mailbox/Makefile | 4 + >> drivers/mailbox/mailbox.c | 466 +++++++++++++++++++++++++++++++++++++ >> include/linux/mailbox_client.h | 46 ++++ >> include/linux/mailbox_controller.h | 135 +++++++++++ >> 5 files changed, 659 insertions(+) >> create mode 100644 drivers/mailbox/mailbox.c >> create mode 100644 include/linux/mailbox_client.h >> create mode 100644 include/linux/mailbox_controller.h >> > > [..] > >> + >> +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. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754369AbaIVSQF (ORCPT ); Mon, 22 Sep 2014 14:16:05 -0400 Received: from service87.mimecast.com ([91.220.42.44]:46234 "EHLO service87.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753525AbaIVSQC convert rfc822-to-8bit (ORCPT ); Mon, 22 Sep 2014 14:16:02 -0400 Message-ID: <5420675C.6040805@arm.com> Date: Mon, 22 Sep 2014 19:15:56 +0100 From: Sudeep Holla User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: Ashwin Chaugule , Jassi Brar CC: Sudeep Holla , Devicetree List , lkml , "ks.giri@samsung.com" , Arnd Bergmann , "ijc+devicetree@hellion.org.uk" , Mark Rutland , Rob Herring , Pawel Moll , Courtney Cavin , Matt Porter , Craig McGeachie , LeyFoon Tan , Loic Pallardy , "Anna, Suman" , Bjorn Andersson , Patch Tracking , "mollie.wu@linaro.org" , Tetsuya Takinishi , "broonie@linaro.org" , Kevin Hilman , "lee.jones@linaro.org" , "andy.green@linaro.org" Subject: Re: [PATCHv10 2/4] mailbox: Introduce framework for mailbox References: <1406896194-4667-1-git-send-email-jaswinder.singh@linaro.org> <1406896296-4863-1-git-send-email-jaswinder.singh@linaro.org> In-Reply-To: X-OriginalArrivalTime: 22 Sep 2014 18:15:56.0710 (UTC) FILETIME=[410E6C60:01CFD691] X-MC-Unique: 114092219155905601 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 22/09/14 19:01, Ashwin Chaugule wrote: > Hi Jassi, > > On 1 August 2014 08:31, Jassi Brar wrote: >> Introduce common framework for client/protocol drivers and >> controller drivers of Inter-Processor-Communication (IPC). >> >> Client driver developers should have a look at >> include/linux/mailbox_client.h to understand the part of >> the API exposed to client drivers. >> Similarly controller driver developers should have a look >> at include/linux/mailbox_controller.h >> >> Signed-off-by: Jassi Brar >> --- >> MAINTAINERS | 8 + >> drivers/mailbox/Makefile | 4 + >> drivers/mailbox/mailbox.c | 466 +++++++++++++++++++++++++++++++++++++ >> include/linux/mailbox_client.h | 46 ++++ >> include/linux/mailbox_controller.h | 135 +++++++++++ >> 5 files changed, 659 insertions(+) >> create mode 100644 drivers/mailbox/mailbox.c >> create mode 100644 include/linux/mailbox_client.h >> create mode 100644 include/linux/mailbox_controller.h >> > > [..] > >> + >> +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. Regards, Sudeep