Patchwork [GIT,PULL] mailbox driver framework for v3.10 merge window

login
register
mail settings
Submitter Anna, Suman
Date April 4, 2013, 9:48 p.m.
Message ID <37C860A02101E749A747FA2D3C1E3C504A1971@DLEE11.ent.ti.com>
Download mbox
Permalink /patch/233956/
State New
Headers show

Pull-request

git://github.com/sumananna/mailbox.git dbx500-prcmu-mailbox

Comments

Anna, Suman - April 4, 2013, 9:48 p.m.
The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9: 

        Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)

are available in the git repository at:

        git://github.com/sumananna/mailbox.git dbx500-prcmu-mailbox

for you to fetch changes up to c497eba5247728c67ba0e0de0907723dd114134a:

        mailbox: fix invalid use of sizeof in mailbox_msg_send() (2013-03-23 15:04:14 +0800)

------------------------------------------------------------------------------
OMAP and ST-Ericsson platforms are both using mailbox to communicate with some coprocessors. This series creates a consolidated framework, living under drivers/mailbox.
The changes mainly contain:
- create a mailbox framework independent from OMAP h/w
- creates dbx500 mailbox driver for ST-Ericsson platforms
- move the omap mailbox out of plat-omap/mach-omapX  adapting to the new framework.
- minor bug fixes in mailbox code

This series is needed for the work currently being done on the PRCMU driver.
------------------------------------------------------------------------------

Loic Pallardy (7):
  mailbox: rename omap_mbox in mailbox
  mailbox: create opened message type
  mailbox: change protection mechanisms
  mailbox: add shared memory mailbox type
  mailbox: add IRQF_NO_SUSPEND flag
  mailbox: add no_irq send message
  mailbox: create dbx500 mailbox driver

Omar Ramirez Luna (2):
  mailbox: OMAP: introduce mailbox framework
  mailbox: split internal header from API header

Suman Anna (5):
  mailbox: rename pl320-ipc specific mailbox.h
  ARM: OMAP2+: mbox: remove dependencies with soc.h
  mailbox/omap: check iomem resource before dereferencing it
  mailbox: check for NULL nb in mailbox_put
  mailbox: call request_irq after mbox queues are allocated

Wei Yongjun (1):
  mailbox: fix invalid use of sizeof in mailbox_msg_send()

 .../devicetree/bindings/mailbox/dbx500-mailbox.txt |  27 +
 arch/arm/configs/omap1_defconfig                   |   3 +-
 arch/arm/mach-omap1/Makefile                       |   4 -
 arch/arm/mach-omap1/mailbox.c                      | 199 -------
 arch/arm/mach-omap2/Makefile                       |   3 -
 arch/arm/mach-omap2/devices.c                      |  13 +-
 arch/arm/mach-omap2/mailbox.c                      | 430 --------------
 arch/arm/mach-omap2/omap_hwmod_2420_data.c         |  12 +
 arch/arm/mach-omap2/omap_hwmod_2430_data.c         |  11 +
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |  11 +
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |  13 +
 arch/arm/plat-omap/Kconfig                         |  16 -
 arch/arm/plat-omap/Makefile                        |   3 -
 arch/arm/plat-omap/include/plat/mailbox.h          | 105 ----
 arch/arm/plat-omap/mailbox.c                       | 435 --------------
 drivers/cpufreq/highbank-cpufreq.c                 |   2 +-
 drivers/mailbox/Kconfig                            |  41 ++
 drivers/mailbox/Makefile                           |   5 +
 drivers/mailbox/mailbox-dbx500.c                   | 648 +++++++++++++++++++++
 drivers/mailbox/mailbox-omap1.c                    | 229 ++++++++
 drivers/mailbox/mailbox-omap2.c                    | 370 ++++++++++++
 drivers/mailbox/mailbox.c                          | 552 ++++++++++++++++++
 drivers/mailbox/mailbox_internal.h                 |  70 +++
 drivers/mailbox/pl320-ipc.c                        |   2 +-
 drivers/remoteproc/Kconfig                         |   3 +-
 drivers/remoteproc/omap_remoteproc.c               |  36 +-
 drivers/staging/tidspbridge/Kconfig                |   3 +-
 drivers/staging/tidspbridge/core/_tiomap.h         |   2 +-
 drivers/staging/tidspbridge/core/chnl_sm.c         |   8 +-
 drivers/staging/tidspbridge/core/io_sm.c           |   5 +-
 drivers/staging/tidspbridge/core/tiomap3430.c      |   6 +-
 drivers/staging/tidspbridge/core/tiomap3430_pwr.c  |   6 +-
 drivers/staging/tidspbridge/core/tiomap_io.c       |   9 +-
 .../tidspbridge/include/dspbridge/host_os.h        |   2 +-
 include/linux/mailbox.h                            |  52 +-
 include/linux/pl320-ipc.h                          |  17 +
 include/linux/platform_data/mailbox-dbx500.h       |  12 +
 include/linux/platform_data/mailbox-omap.h         |  53 ++
 38 files changed, 2170 insertions(+), 1248 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/dbx500-mailbox.txt
 delete mode 100644 arch/arm/mach-omap1/mailbox.c
 delete mode 100644 arch/arm/mach-omap2/mailbox.c
 delete mode 100644 arch/arm/plat-omap/include/plat/mailbox.h
 delete mode 100644 arch/arm/plat-omap/mailbox.c
 create mode 100644 drivers/mailbox/mailbox-dbx500.c
 create mode 100644 drivers/mailbox/mailbox-omap1.c
 create mode 100644 drivers/mailbox/mailbox-omap2.c
 create mode 100644 drivers/mailbox/mailbox.c
 create mode 100644 drivers/mailbox/mailbox_internal.h
 create mode 100644 include/linux/pl320-ipc.h
 create mode 100644 include/linux/platform_data/mailbox-dbx500.h
 create mode 100644 include/linux/platform_data/mailbox-omap.h

--
Arnd Bergmann - April 9, 2013, 10:55 a.m.
On Thursday 04 April 2013, Anna, Suman wrote:
> OMAP and ST-Ericsson platforms are both using mailbox to communicate with some coprocessors. This series creates a consolidated framework, living under drivers/mailbox.
> The changes mainly contain:
> - create a mailbox framework independent from OMAP h/w
> - creates dbx500 mailbox driver for ST-Ericsson platforms
> - move the omap mailbox out of plat-omap/mach-omapX  adapting to the new framework.
> - minor bug fixes in mailbox code

Pulled into a new next/mailbox branch, to keep it separate from the
existing subsystems.

As a note for you: when you send a pull request, please make sure that you
use a tag that includes the changeset text (your description above), so I
don't have to copy it from the email. I noticed that you do have a tag
"mailbox-for-v3.10" in your tree, but the pull request was for the branch
with the same contents.

	Arnd
Anna, Suman - April 9, 2013, 3:33 p.m.
On Thursday 04 April 2013, Anna, Suman wrote:
> > OMAP and ST-Ericsson platforms are both using mailbox to communicate with
> some coprocessors. This series creates a consolidated framework, living under
> drivers/mailbox.
> > The changes mainly contain:
> > - create a mailbox framework independent from OMAP h/w
> > - creates dbx500 mailbox driver for ST-Ericsson platforms
> > - move the omap mailbox out of plat-omap/mach-omapX  adapting to the new
> framework.
> > - minor bug fixes in mailbox code
> 
> Pulled into a new next/mailbox branch, to keep it separate from the existing
> subsystems.
> 
> As a note for you: when you send a pull request, please make sure that you use a
> tag that includes the changeset text (your description above), so I don't have to
> copy it from the email. I noticed that you do have a tag "mailbox-for-v3.10" in
> your tree, but the pull request was for the branch with the same contents.
> 

Thanks Arnd. Yes, the tag is for the same SHA, and has the same comments as above. I understood the process only a bit later.

Regards
Suman
Jassi Brar - April 29, 2013, 4:07 a.m.
Hello Arnd,

On 9 April 2013 16:25, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 04 April 2013, Anna, Suman wrote:
>> OMAP and ST-Ericsson platforms are both using mailbox to communicate with some coprocessors. This series creates a consolidated framework, living under drivers/mailbox.
>> The changes mainly contain:
>> - create a mailbox framework independent from OMAP h/w
>> - creates dbx500 mailbox driver for ST-Ericsson platforms
>> - move the omap mailbox out of plat-omap/mach-omapX  adapting to the new framework.
>> - minor bug fixes in mailbox code
>
> Pulled into a new next/mailbox branch, to keep it separate from the
> existing subsystems.
>
I am going to be a heavy user of the Mailbox API. And I have reviewed
this API quite in detail. I pointed out many aspects that might have
worked for TI's usage but are not going to be work on many platforms
(including one of mine).  Suman and Loic also acknowledged most (if
not all) from 'generic' POV.  Here is the thread ...
   http://www.spinics.net/lists/arm-kernel/msg239433.html

Not to mean only talk and no do, I prepared another proposal that
addressed all the concerns shared so far in the RFC
http://www.spinics.net/lists/kernel/msg1523873.html (its not ready for
primetime yet)  And also converted the PL320 driver to the new API,
unlike the pulled patchset which leaves that out in the cold.
http://www.spinics.net/lists/kernel/msg1523874.html

Now, we could either call it (effectively the TI's framework with
quirks for STE) as the "Common API" and then dismantle and convert it
patch by patch (authors and I seem to agree many things need to be
changed and implemented).
  OR we do it reasonably right the first time even if that means yet
another release. IMHO we should go for slow but steady thing.

Hi Suman, Hi Loic,
   Please feel free to object to anything you think I might have
misrepresented. I am OK if you are not sure about my implementation
either - we could co-work on a new one.

Regards,
-Jassi
Anna, Suman - May 2, 2013, 10:09 p.m.
Hi Arnd,

On 04/28/2013 11:07 PM, Jassi Brar wrote:
> Hello Arnd,
> 
> On 9 April 2013 16:25, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Thursday 04 April 2013, Anna, Suman wrote:
>>> OMAP and ST-Ericsson platforms are both using mailbox to communicate with some coprocessors. This series creates a consolidated framework, living under drivers/mailbox.
>>> The changes mainly contain:
>>> - create a mailbox framework independent from OMAP h/w
>>> - creates dbx500 mailbox driver for ST-Ericsson platforms
>>> - move the omap mailbox out of plat-omap/mach-omapX  adapting to the new framework.
>>> - minor bug fixes in mailbox code
>>
>> Pulled into a new next/mailbox branch, to keep it separate from the
>> existing subsystems.
>>
> I am going to be a heavy user of the Mailbox API. And I have reviewed
> this API quite in detail. I pointed out many aspects that might have
> worked for TI's usage but are not going to be work on many platforms
> (including one of mine).  Suman and Loic also acknowledged most (if
> not all) from 'generic' POV.  Here is the thread ...
>    http://www.spinics.net/lists/arm-kernel/msg239433.html
> 
> Not to mean only talk and no do, I prepared another proposal that
> addressed all the concerns shared so far in the RFC
> http://www.spinics.net/lists/kernel/msg1523873.html (its not ready for
> primetime yet)  And also converted the PL320 driver to the new API,
> unlike the pulled patchset which leaves that out in the cold.
> http://www.spinics.net/lists/kernel/msg1523874.html

Yes, the current code is mainly interrupt-driven and supports only
non-atomic context. We have a need to support atomic contexts and ipc
controllers that do not have interrupt-based triggering. As Jassi
pointed out, the RFC is not ready yet and there are still some
contention points that needs to be sorted out (especially to maintain
OMAP mailbox functionality).

> 
> Now, we could either call it (effectively the TI's framework with
> quirks for STE) as the "Common API" and then dismantle and convert it
> patch by patch (authors and I seem to agree many things need to be
> changed and implemented).
>   OR we do it reasonably right the first time even if that means yet
> another release. IMHO we should go for slow but steady thing.

I think it is your call here, either of the above approaches would
definitely involve some rework on the framework as well as both the OMAP
& ST mailboxes. Atleast for OMAP, the code exists in kernel but disabled
currently due to the multi-platform support. It is pending on the move
to drivers/mailbox folder, and can be enabled just with the first 3
patches (and another one for renaming generic mailbox.c/.h back to
omap_mailbox.c/.h files if we go the RFC approach) in the series
(irrespective of the framework). TI DSP/Bridge would remain broken
because of the omap dmtimer api dependencies on multi-platform.

I do not know how much of an impact it is for the ST driver as the
series adds the driver, and would have to wait until the RFC is sorted
out otherwise.

regards
Suman
Jassi Brar - May 3, 2013, 2:37 a.m.
On 3 May 2013 03:39, Suman Anna <s-anna@ti.com> wrote:
> Hi Arnd,
>
> On 04/28/2013 11:07 PM, Jassi Brar wrote:
>>
>> Not to mean only talk and no do, I prepared another proposal that
>> addressed all the concerns shared so far in the RFC
>> http://www.spinics.net/lists/kernel/msg1523873.html (its not ready for
>> primetime yet)  And also converted the PL320 driver to the new API,
>> unlike the pulled patchset which leaves that out in the cold.
>> http://www.spinics.net/lists/kernel/msg1523874.html
>
> Yes, the current code is mainly interrupt-driven and supports only
> non-atomic context. We have a need to support atomic contexts and ipc
> controllers that do not have interrupt-based triggering.
>
Supporting poll and client driven TX and atomic context is going to
need big chunks of changes which we can avoid by doing them already.
Plus a bottleneck with PL320, as Mark pointed out they can't afford
any bigger latency, which will come from RX via notifier path.

> As Jassi
> pointed out, the RFC is not ready yet and there are still some
> contention points that needs to be sorted out (especially to maintain
> OMAP mailbox functionality).
>
Apart from a few checkpatch fixes, a missing timer delete call and
some testing with dummy client and controller drivers for various
usecases, it's ready from my side. It worked at least as good as your
API in our internal testing.

Please do let me know which OMAP functionality are you worried about,
I believe it could all still be done above this api.

>>
>> Now, we could either call it (effectively the TI's framework with
>> quirks for STE) as the "Common API" and then dismantle and convert it
>> patch by patch (authors and I seem to agree many things need to be
>> changed and implemented).
>>   OR we do it reasonably right the first time even if that means yet
>> another release. IMHO we should go for slow but steady thing.
>
> I think it is your call here, either of the above approaches would
> definitely involve some rework on the framework as well as both the OMAP
> & ST mailboxes.

I am interested to know what changes do you anticipate in my proposed
API. Not to mean it's perfect, but I thought I provided for all
practical use-cases.

Yes, TI and STE would need re-work, but then as of now they are their
own APIs upstream. And even with your proposal they would still need
to be changed if we are to implement the desired features. What about
PL320?

Thanks.
Arnd Bergmann - May 3, 2013, 1:25 p.m.
On Thursday 02 May 2013 17:09:07 Suman Anna wrote:
> On 04/28/2013 11:07 PM, Jassi Brar wrote:
> > Now, we could either call it (effectively the TI's framework with
> > quirks for STE) as the "Common API" and then dismantle and convert it
> > patch by patch (authors and I seem to agree many things need to be
> > changed and implemented).
> >   OR we do it reasonably right the first time even if that means yet
> > another release. IMHO we should go for slow but steady thing.
> 
> I think it is your call here, either of the above approaches would
> definitely involve some rework on the framework as well as both the OMAP
> & ST mailboxes. Atleast for OMAP, the code exists in kernel but disabled
> currently due to the multi-platform support. It is pending on the move
> to drivers/mailbox folder, and can be enabled just with the first 3
> patches (and another one for renaming generic mailbox.c/.h back to
> omap_mailbox.c/.h files if we go the RFC approach) in the series
> (irrespective of the framework). TI DSP/Bridge would remain broken
> because of the omap dmtimer api dependencies on multi-platform.
> 
> I do not know how much of an impact it is for the ST driver as the
> series adds the driver, and would have to wait until the RFC is sorted
> out otherwise.

I think I'd prefer to drop the branch from the 3.10 queue then
and let you all work out a common approach for 3.11. Olof, any
other input?

	Arnd
Linus Walleij - May 3, 2013, 1:39 p.m.
On Fri, May 3, 2013 at 3:25 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 02 May 2013 17:09:07 Suman Anna wrote:
>>
>> I do not know how much of an impact it is for the ST driver as the
>> series adds the driver, and would have to wait until the RFC is sorted
>> out otherwise.
>
> I think I'd prefer to drop the branch from the 3.10 queue then
> and let you all work out a common approach for 3.11. Olof, any
> other input?

This will block all refactoring of the PRCMU driver, which Loic
is working on, and also Ulf Hansson's clock driver. It is basically
the key to breaking that driver apart and distributing it out into
the proper subsystems. Loic has a big patch series for that
for next merge window which will then have to be postponed,
or queued on top of the mailbox work when finished.

But maybe it's the right thing to do if the subsystem needs more
work? I have no clear opinion on this, Loic, Ulf?

Yours,
Linus Walleij
Arnd Bergmann - May 3, 2013, 1:49 p.m.
On Friday 03 May 2013 15:39:42 Linus Walleij wrote:
> On Fri, May 3, 2013 at 3:25 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 02 May 2013 17:09:07 Suman Anna wrote:
> >>
> >> I do not know how much of an impact it is for the ST driver as the
> >> series adds the driver, and would have to wait until the RFC is sorted
> >> out otherwise.
> >
> > I think I'd prefer to drop the branch from the 3.10 queue then
> > and let you all work out a common approach for 3.11. Olof, any
> > other input?
> 
> This will block all refactoring of the PRCMU driver, which Loic
> is working on, and also Ulf Hansson's clock driver. It is basically
> the key to breaking that driver apart and distributing it out into
> the proper subsystems. Loic has a big patch series for that
> for next merge window which will then have to be postponed,
> or queued on top of the mailbox work when finished.
> 
> But maybe it's the right thing to do if the subsystem needs more
> work? I have no clear opinion on this, Loic, Ulf?

I think we can queue them together. I'm certainly fine with the
mailbox subsystem getting merged through both the mfd and arm-soc
trees to avoid conflicts.

	Arnd
Anna, Suman - May 3, 2013, 4:29 p.m.
On 05/02/2013 09:37 PM, Jassi Brar wrote:
> On 3 May 2013 03:39, Suman Anna <s-anna@ti.com> wrote:
>> Hi Arnd,
>>
>> On 04/28/2013 11:07 PM, Jassi Brar wrote:
>>>
>>> Not to mean only talk and no do, I prepared another proposal that
>>> addressed all the concerns shared so far in the RFC
>>> http://www.spinics.net/lists/kernel/msg1523873.html (its not ready for
>>> primetime yet)  And also converted the PL320 driver to the new API,
>>> unlike the pulled patchset which leaves that out in the cold.
>>> http://www.spinics.net/lists/kernel/msg1523874.html
>>
>> Yes, the current code is mainly interrupt-driven and supports only
>> non-atomic context. We have a need to support atomic contexts and ipc
>> controllers that do not have interrupt-based triggering.
>>
> Supporting poll and client driven TX and atomic context is going to
> need big chunks of changes which we can avoid by doing them already.
> Plus a bottleneck with PL320, as Mark pointed out they can't afford
> any bigger latency, which will come from RX via notifier path.
> 
>> As Jassi
>> pointed out, the RFC is not ready yet and there are still some
>> contention points that needs to be sorted out (especially to maintain
>> OMAP mailbox functionality).
>>
> Apart from a few checkpatch fixes, a missing timer delete call and
> some testing with dummy client and controller drivers for various
> usecases, it's ready from my side. It worked at least as good as your
> API in our internal testing.
> 
> Please do let me know which OMAP functionality are you worried about,
> I believe it could all still be done above this api.

Mainly the splitting of bottom-half responsibility of the controller
driver between mailbox and the client (or another in-between layer
between my existing client (remoteproc driver), because of the support
for only atomic context.

> 
>>>
>>> Now, we could either call it (effectively the TI's framework with
>>> quirks for STE) as the "Common API" and then dismantle and convert it
>>> patch by patch (authors and I seem to agree many things need to be
>>> changed and implemented).
>>>   OR we do it reasonably right the first time even if that means yet
>>> another release. IMHO we should go for slow but steady thing.
>>
>> I think it is your call here, either of the above approaches would
>> definitely involve some rework on the framework as well as both the OMAP
>> & ST mailboxes.
> 
> I am interested to know what changes do you anticipate in my proposed
> API. Not to mean it's perfect, but I thought I provided for all
> practical use-cases.

I will provide feedback on the RFC thread, and we can continue the
discussion on that thread. I will also share the link to my current
work-in-progress branch so that you can see the design approach that I
have taken.

> 
> Yes, TI and STE would need re-work, but then as of now they are their
> own APIs upstream. And even with your proposal they would still need
> to be changed if we are to implement the desired features. What about
> PL320?

The pl320_transmit function is identical to the present
mailbox_msg_send_receive_no_irq, but that is a non-factor anyway since
we would have changed the API.

regards
Suman