Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1650409 > unrolled thread

Re: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode

Started bySudeep Holla <sudeep.holla@arm.com>
First post2017-05-25 13:40 +0200
Last post2017-05-25 16:20 +0200
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode Sudeep Holla <sudeep.holla@arm.com> - 2017-05-25 13:40 +0200
    Re: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-25 15:30 +0200
      Re: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode Sudeep Holla <sudeep.holla@arm.com> - 2017-05-25 15:40 +0200
        Re: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-25 15:50 +0200
          Re: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode Sudeep Holla <sudeep.holla@arm.com> - 2017-05-25 16:00 +0200
            Re: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-25 16:10 +0200
              Re: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode Sudeep Holla <sudeep.holla@arm.com> - 2017-05-25 16:20 +0200

#1650409 — Re: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode

FromSudeep Holla <sudeep.holla@arm.com>
Date2017-05-25 13:40 +0200
SubjectRe: [PATCH v2 0/6] mailbox: arm_mhu: add support for doorbell mode
Message-ID<tKZhy-7An-79@gated-at.bofh.it>

On 24/05/17 11:56, Jassi Brar wrote:
> On Wed, May 24, 2017 at 3:46 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>> Hi,
>>
>> This series adds doorbell support to ARM MHU mailbox controller driver.
>> Since we need to callback the different client based on the doorbel bits
>> triggered from the remote, we can manage with single channel for the set
>> of 32 doorbells.
>>
>> Regards,
>> Sudeep
>>
>> v1->v2:
>>         - Removed the notion od subchannels
>>         - Treat each bit in the MHU register as a doorbell and hence
>>           different channel with respect to mailbox framework
>>
> Whatever happened to the endless explanations I gave you, how the MHU
> driver already supports your usecase?
> 

Yes but you didn't respond to my queries:
1. The client driver is generic and expects it to be doorbell like
   mailbox controller. I am referring to SCMI which will be released
   soon. We can't embed ARM MHU or any other mailbox controller info
   into that.

2. How do we call multiple clients from mhu_irq ? I have Slot/bit 0
   being used by SCPI protocol(already in mainline) and slot 1/2 or more
   will be used by SCMI ?

3. We already have mailbox-sti.c which implements exactly the same logic
   of doorbell. Why did you not push back to implement something like
   arm_mhu.c then ? I am confused as why you are so particular in this
   case ?

Few more things to note here:

1. Just because the platform you worked used MHU to pass the command
   doesn't mean that's the only one use-case and others have to
   workaround in the client drivers.

2. Read the specification again. It's clear that it's designed for
   doorbell kind of usage. All I am asking is to support that both in
   the binding and implementation. And lets not assume or make it work
   with one protocol. It's generic IP and can be used in either
   doorbell way or the way it's currently supported.

3. That's one of the reason for just have 2 set's of registers as it's
   possible to use them as 32 different doorbells. Otherwise 2 channels
   is too limited for any platform.

Please address my queries instead of claiming that you can workout a
solution. I simply want the mailbox and protocol independent and hence
the binding. I don't like the idea of you proposed(i.e. 32-bit data to
be written to the controller register).

-- 
Regards,
Sudeep

[toc] | [next] | [standalone]

#1650500

FromJassi Brar <jassisinghbrar@gmail.com>
Date2017-05-25 15:30 +0200
Message-ID<tL0ZY-iP-5@gated-at.bofh.it>
In reply to#1650409
On Thu, May 25, 2017 at 5:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 24/05/17 11:56, Jassi Brar wrote:
>> On Wed, May 24, 2017 at 3:46 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>
>>> Hi,
>>>
>>> This series adds doorbell support to ARM MHU mailbox controller driver.
>>> Since we need to callback the different client based on the doorbel bits
>>> triggered from the remote, we can manage with single channel for the set
>>> of 32 doorbells.
>>>
>>> Regards,
>>> Sudeep
>>>
>>> v1->v2:
>>>         - Removed the notion od subchannels
>>>         - Treat each bit in the MHU register as a doorbell and hence
>>>           different channel with respect to mailbox framework
>>>
>> Whatever happened to the endless explanations I gave you, how the MHU
>> driver already supports your usecase?
>>
>
> Yes but you didn't respond to my queries:
> 1. The client driver is generic and expects it to be doorbell like
>    mailbox controller. I am referring to SCMI which will be released
>    soon. We can't embed ARM MHU or any other mailbox controller info
>    into that.
>
If SCMI is to be usable over different platforms, there has to be 2
sub-parts of the SCMI - one platform agnostic high level protocol
implementation, and the other platform specific 'transport' layer
where actual message xfer is done.

For the Nth time:-
    The 'mssg' in mbox_send_message(struct mbox_chan *chan, void
*mssg) is platform specific. For MHU it is simple u32*, whereas for
other platform it will be like 'struct my_protocol_message *'

I can't make it any clearer.

> 2. How do we call multiple clients from mhu_irq ? I have Slot/bit 0
>    being used by SCPI protocol(already in mainline) and slot 1/2 or more
>    will be used by SCMI ?
>
Like other platforms do, have a common client that manages messages
to/from clients working on same channel.

> 3. We already have mailbox-sti.c which implements exactly the same logic
>    of doorbell. Why did you not push back to implement something like
>    arm_mhu.c then ? I am confused as why you are so particular in this
>    case ?
>
The way STI's controller works (or as I was told), it warrants that
design. I know MHU very well and I bet it needs no modification.

 I explained in more than one way how to use the current driver, but
you refuse to acknowledge. Then I offered to modify your code for you,
but you don't agree to that either. I am running out of ways to
respond and point you back to my old posts.

[toc] | [prev] | [next] | [standalone]

#1650514

FromSudeep Holla <sudeep.holla@arm.com>
Date2017-05-25 15:40 +0200
Message-ID<tL19E-me-25@gated-at.bofh.it>
In reply to#1650500

On 25/05/17 14:20, Jassi Brar wrote:
> On Thu, May 25, 2017 at 5:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 24/05/17 11:56, Jassi Brar wrote:
>>> On Wed, May 24, 2017 at 3:46 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> This series adds doorbell support to ARM MHU mailbox controller driver.
>>>> Since we need to callback the different client based on the doorbel bits
>>>> triggered from the remote, we can manage with single channel for the set
>>>> of 32 doorbells.
>>>>
>>>> Regards,
>>>> Sudeep
>>>>
>>>> v1->v2:
>>>>         - Removed the notion od subchannels
>>>>         - Treat each bit in the MHU register as a doorbell and hence
>>>>           different channel with respect to mailbox framework
>>>>
>>> Whatever happened to the endless explanations I gave you, how the MHU
>>> driver already supports your usecase?
>>>
>>
>> Yes but you didn't respond to my queries:
>> 1. The client driver is generic and expects it to be doorbell like
>>    mailbox controller. I am referring to SCMI which will be released
>>    soon. We can't embed ARM MHU or any other mailbox controller info
>>    into that.
>>
> If SCMI is to be usable over different platforms, there has to be 2
> sub-parts of the SCMI - one platform agnostic high level protocol
> implementation, and the other platform specific 'transport' layer
> where actual message xfer is done.
> 

It recommends doorbell kind of interface for the transport.

> For the Nth time:-
>     The 'mssg' in mbox_send_message(struct mbox_chan *chan, void
> *mssg) is platform specific. For MHU it is simple u32*, whereas for
> other platform it will be like 'struct my_protocol_message *'
> 
> I can't make it any clearer.
> 

Why is that ? Just because it was used on your platform like that ?
Sorry that's not a valid reason. I too repeat for the nth time that the
MHU is designed to have 32 doorbells which can be used *independent* of
each other as *clearly* stated in the specification. So don't make it
platform issue. It just happened that *your* platform chose to write
some 32bit data as a whole doesn't mean that's the only use.

>> 2. How do we call multiple clients from mhu_irq ? I have Slot/bit 0
>>    being used by SCPI protocol(already in mainline) and slot 1/2 or more
>>    will be used by SCMI ?
>>
> Like other platforms do, have a common client that manages messages
> to/from clients working on same channel.
> 

Not possible as the protocols are not related. Please accept the fact
that protocol are not just platform specific. SCMI is all about generic
protocol layer that can be used on any transport.

Also each slot or bit has a different shared memory, how do you
represent that ? You are simply missing my point when I say it's proper
channel with a bit in MHU register as doorbell.

>> 3. We already have mailbox-sti.c which implements exactly the same logic
>>    of doorbell. Why did you not push back to implement something like
>>    arm_mhu.c then ? I am confused as why you are so particular in this
>>    case ?
>>
> The way STI's controller works (or as I was told), it warrants that
> design. I know MHU very well and I bet it needs no modification.
> 

It's exactly the same. I bet not better than the MHU hardware designer
I spoke to.

>  I explained in more than one way how to use the current driver, but
> you refuse to acknowledge. Then I offered to modify your code for you,
> but you don't agree to that either. I am running out of ways to
> respond and point you back to my old posts.
> 

Yes, you are simply missing the point that both ARM MHU and whatever
protocol sits on it has to be platform agnostic.

OK, you can stop pointing me back and simply propose alternate binding.
The binding needs to be changed to handle my use-case. I will then see
what need to be done.

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]

#1650520

FromJassi Brar <jassisinghbrar@gmail.com>
Date2017-05-25 15:50 +0200
Message-ID<tL1jj-pv-13@gated-at.bofh.it>
In reply to#1650514
On Thu, May 25, 2017 at 7:05 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>> 1. The client driver is generic and expects it to be doorbell like
>>>    mailbox controller. I am referring to SCMI which will be released
>>>    soon. We can't embed ARM MHU or any other mailbox controller info
>>>    into that.
>>>
>> If SCMI is to be usable over different platforms, there has to be 2
>> sub-parts of the SCMI - one platform agnostic high level protocol
>> implementation, and the other platform specific 'transport' layer
>> where actual message xfer is done.
>>
>
> It recommends doorbell kind of interface for the transport.
>
>> For the Nth time:-
>>     The 'mssg' in mbox_send_message(struct mbox_chan *chan, void
>> *mssg) is platform specific. For MHU it is simple u32*, whereas for
>> other platform it will be like 'struct my_protocol_message *'
>>
>> I can't make it any clearer.
>>
>
> Why is that ? Just because it was used on your platform like that ?
> Sorry that's not a valid reason.
>
To be clear, by "other platform" I mean platforms with mailbox
controller other than MHU.  (Not to mean SCMI can't run on MHU as of
today).

Do you intend SCMI to run only on platforms that have MHU controller?
I hope not.

Now re-read my last post until you get it.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1650527

FromSudeep Holla <sudeep.holla@arm.com>
Date2017-05-25 16:00 +0200
Message-ID<tL1t0-sX-11@gated-at.bofh.it>
In reply to#1650520

On 25/05/17 14:44, Jassi Brar wrote:
> On Thu, May 25, 2017 at 7:05 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>> 1. The client driver is generic and expects it to be doorbell like
>>>>    mailbox controller. I am referring to SCMI which will be released
>>>>    soon. We can't embed ARM MHU or any other mailbox controller info
>>>>    into that.
>>>>
>>> If SCMI is to be usable over different platforms, there has to be 2
>>> sub-parts of the SCMI - one platform agnostic high level protocol
>>> implementation, and the other platform specific 'transport' layer
>>> where actual message xfer is done.
>>>
>>
>> It recommends doorbell kind of interface for the transport.
>>
>>> For the Nth time:-
>>>     The 'mssg' in mbox_send_message(struct mbox_chan *chan, void
>>> *mssg) is platform specific. For MHU it is simple u32*, whereas for
>>> other platform it will be like 'struct my_protocol_message *'
>>>
>>> I can't make it any clearer.
>>>
>>
>> Why is that ? Just because it was used on your platform like that ?
>> Sorry that's not a valid reason.
>>
> To be clear, by "other platform" I mean platforms with mailbox
> controller other than MHU.  (Not to mean SCMI can't run on MHU as of
> today).
> 

It can't run along with existing SCPI without some hacking. You need to
add a layer that is platform specific which is *hacky*.

> Do you intend SCMI to run only on platforms that have MHU controller?
> I hope not.
> 

Definitely not, that's the whole point. It should work with any
controller that supports doorbell mode.

> Now re-read my last post until you get it.
> 
Just propose the binding yourself.

On juno, say we will use low priority channel

BIT(0) - SCPI
BIT(1) - SCMI (general)
BIT(2) - SCMI (notification)
BIT(3) - A totally new protocol
:
:
with each of the above with specific shared memory reserved for them.

Beware we don't need any of these info in either of the driver as it
may change with another platform. So this info has to come from DT.
So I am requesting you to propose the binding if you think you can
better than the one I have.

I can base my code on that.

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]

#1650531

FromJassi Brar <jassisinghbrar@gmail.com>
Date2017-05-25 16:10 +0200
Message-ID<tL1CF-La-5@gated-at.bofh.it>
In reply to#1650527
On Thu, May 25, 2017 at 7:23 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:

> Just propose the binding yourself.
>
My proposed binding is what already exists. Zero change.
I offer to adapt your driver for you, if you are too busy to do it yourself.

[toc] | [prev] | [next] | [standalone]

#1650538

FromSudeep Holla <sudeep.holla@arm.com>
Date2017-05-25 16:20 +0200
Message-ID<tL1Ml-OP-5@gated-at.bofh.it>
In reply to#1650531

On 25/05/17 15:07, Jassi Brar wrote:
> On Thu, May 25, 2017 at 7:23 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
>> Just propose the binding yourself.
>>
> My proposed binding is what already exists. Zero change.
> I offer to adapt your driver for you, if you are too busy to do it yourself.
> 

Care to explain how with the example I gave ?

BIT(0) - SCPI
BIT(1) - SCMI (general)
BIT(2) - SCMI (notification)
BIT(3) - A totally new protocol
:
:

The protocols drivers *has* to be independent as they are used with
other mailbox controllers. I can do the changes, but I would like to
know how.

-- 
Regards,
Sudeep

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web