From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754704AbdEDFp1 (ORCPT ); Thu, 4 May 2017 01:45:27 -0400 Received: from mail-pg0-f46.google.com ([74.125.83.46]:33211 "EHLO mail-pg0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754630AbdEDFpU (ORCPT ); Thu, 4 May 2017 01:45:20 -0400 Date: Wed, 3 May 2017 22:45:16 -0700 From: Bjorn Andersson To: Jassi Brar Cc: Loic PALLARDY , Andy Gross , Rob Herring , Mark Rutland , Ohad Ben-Cohen , "linux-arm-msm@vger.kernel.org" , "linux-soc@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-remoteproc@vger.kernel.org" Subject: Re: [PATCH v3 2/4] soc: qcom: Introduce APCS IPC driver Message-ID: <20170504054516.GB15143@minitux> References: <20170503052929.17422-1-bjorn.andersson@linaro.org> <20170503052929.17422-2-bjorn.andersson@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 03 May 02:55 PDT 2017, Jassi Brar wrote: > Loic, thanks for adding me. > > On Wed, May 3, 2017 at 2:58 PM, Loic PALLARDY wrote: > > > > > >> -----Original Message----- > >> From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc- > >> owner@vger.kernel.org] On Behalf Of Bjorn Andersson > >> Sent: Wednesday, May 03, 2017 7:29 AM > >> To: Andy Gross ; Rob Herring > >> ; Mark Rutland ; Ohad Ben- > >> Cohen > >> Cc: linux-arm-msm@vger.kernel.org; linux-soc@vger.kernel.org; > >> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > >> remoteproc@vger.kernel.org > >> Subject: [PATCH v3 2/4] soc: qcom: Introduce APCS IPC driver > >> > >> This implements a driver that exposes the IPC bits found in the APCS Global > >> block in various Qualcomm platforms. The bits are used to signal inter- > >> processor communication signals from the application CPU to other masters. > >> > >> The driver implements the "doorbell" binding and could be used as basis for a > >> new Linux framework, if found useful outside Qualcomm. > >> > > Hi Bjorn, > > > > Even if Qualcom APCS IPC is limited, why don't you rely on existing mailbox framework. > > It is there to gather all IPC management under the same interface. > > No need to create a new one from my pov. > > If you don't provide message data, mailbox framework behaves as doorbell. > > > QCOM RPM reinvented the wheel for what mailbox framework already did, > despite my pointing it out => > http://lkml.iu.edu/hypermail//linux/kernel/1406.2/03918.html > The RPM interface works by writing various information in shared DRAM and then invoking an interrupt on the remote processor. What this patch does is introduce an abstraction for the invocation of that interrupt. My argumentation against using the mailbox framework for the RPM case was that the message model is a software construct and doesn't fit the mailbox framework. But the single step of invoking the remote interrupt is essentially a non-message mailbox. Perhaps this part of the RPM driver is what you're referring to in your comments on the RPM. Before "inventing" the function for acquiring a handle to a doorbell I did look at making this a mailbox controller, but as each mailbox channel related to a single bit and 1 is the only value we'll ever write it didn't feel like it matches the mailbox expectations. But as you seem to object I'll attempt to rewrite it using the mailbox framework instead. But which one of these would be appropriate for a "mailbox channel" that doesn't have any actual messages? mbox_send_message(chan, NULL) or const int one = 1; mbox_send_message(chan, (void*)&one); > The driver bypassed mailbox framework and was pushed via another tree. > Same is being attempted now, only now it is more expensive to switch > to generic mailbox framework having spent so much time on QCOM > specific implementation of controller and protocol drivers inside > drivers/soc/qcom/ I'm not sure I follow this, there's no extensive rework going on here - all I'm trying to do is abstract the "writel(BIT(x), ipc_reg)" line of the RPM driver, because it's being used in other clients as well. Regards, Bjorn