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

Groups > linux.kernel > #1638101

Re: [PATCH 2/6] Documentation: devicetree: add bindings to support ARM MHU subchannels

Show all headers | View raw

On 09/05/17 12:55, Jassi Brar wrote:
> On Tue, May 9, 2017 at 4:23 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 09/05/17 11:31, Jassi Brar wrote:
>>> On Tue, May 9, 2017 at 3:28 PM, Sudeep Holla <sudeep.holla@arm.com>
>>> wrote:
>>>
>>>>>
>>>>> If it is still not clear, please share your client driver. I
>>>>> will adapt that to work with existing MHU driver & bindings.
>>>>>
>>>>
>>>> Just take example of SCPI in the mainline. Assume there's another
>>>> protocol SCMI which uses few more bits in the same channel and the
>>>> remote firmware implements both but both are totally independent
>>>> and not related/linked. Also be keep in mind that SCPI is used by
>>>> other platforms and so will be the new protocol. We simply make
>>>> SCPI or SCMI bindings aligned to ARM MHU. That's ruled out.
>>>>
>>> Not sure what you mean by "that's ruled out".
>>
>> 1. The mailbox client bindings should be independent of this ARM MHU
>>    mailbox bindings
>> 2. All we need in client is a mailbox to point at and not any meta data
>>    That's what I meant by ruled-out as both client and MHU can be used
>>    independent of each other and *should not* be linked.
>>
> I am shocked at this coming from you.
> 
> You design SCMI based upon MHU assumption of single bit "doorbell" and
> then you say a client should be independent of the underlying
> controller? Do you intend SCMI to work only over MHU?
> 

No, I never said that. What I said is SCMI protocol will be on doorbell
based.

>  What if some controller does not support the simple "doorbell" and
> expects detailed info? For example, apart from SCMI, the remote also
> supports platform specific functions like thermal, watchdog, wakeup
> etc. The SCMI's would just be a subset of the full command set.
> You/SCMI can not dictate what numerical value the platform assigns to
> SCMI commands... 

What ? That's the whole point of specification. The command set is
*fixed* and can be implemented on any platform and have generic driver
for that.

> all that is required is the remote firmware should
> uniquely identify which command is it and implement what the SCMI
> protocol expects.
> 

Yes, not just uniquely but exactly as the specification.

> Have a look at  mbox_send_message(struct mbox_chan *chan, void *mssg)
> The 'mssg' is the pointer to _controller-specified_ structure. The
> client driver (SCMI) _must_ know what the underlying controller
> expects and pass that info. For example of 'mssg', look at "struct
> brcm_message" in include/linux/mailbox/brcm-message.h   The client
> driver must use that platform specific knowledge to send a message
> i.e, you can not make a mailbox client driver 100% provider agnostic.
> 

I disagree. The whole idea is to make it generic with doorbell kind of
logic. We don't pass any command in the doorbell register, everything is
part of shared memory and all standardized.

> You need to divide SCMI into two parts - one that manages the SCMI
> protocol and the other platform specific glue that talks to the
> mailbox controller over the mailbox api.
> 

Why ? That's totally unnecessary IMO.

> 
>>> You have already shared this "v2" MHU driver, now please also share
>>> your client driver. I'll make it work with original MHU driver and
>>> that should settle your confusion.
>>
>> It should first work with SCPI in the mainline. Then we will add another
>> similar protocol soon. So I think you have all you need in the mainline.
>> Today we have hack in the SCPI driver to pass bit 0 set in data. But
>> that's broken as we may want different slot on some other platform.
>> Basically SCPI is designed with the use of doorbell and it should not
>> have any details on how to write that into a particular register as
>> along as we just choose the right channel.
>>

Check arm_scpi.c, the aim is to eliminate the need to send the data with
BIT(0) set as that's platform specific and the protocol has to be generic.

>> On digging more about different mailbox controllers, I found
>> mailbox-sti.c has exactly similar logic as what I have done in this series.
>>

Did you look at this driver ?

>> Also don't mix implementation with the binding. I need a simple answer
>> in this binding. How do I represent specific bits if each bit is
>> implemented as a doorbell ? That's all. First let's agree on that when
>> we use this mailbox independently and please *don't mix* with any
>> client here. It's simple, this controller has 2-3 sets of 32 doorbell
>> bits. And I am aiming to come up with the binding for that as your
>> initial bindings didn't consider that.
>>
> Please send in whatever changes you plan to do, and I'll modify it so
> we don't have to bloat the MHU driver and add bindings for a software
> feature. Until then ... Cheers!
> 

Changes to what ? arm_mhu.c ? This series is complete and implements
doorbell completely.

You seem to have missed to answer some of my questions above. Please do.

-- 
Regards,
Sudeep

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/6] mailbox: arm_mhu: add support for subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-02 16:00 +0200
  [PATCH 5/6] mailbox: arm_mhu: add full support for sub-channels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-02 16:00 +0200
  [PATCH 4/6] mailbox: arm_mhu: re-factor data structure to add subchannel support Sudeep Holla <sudeep.holla@arm.com> - 2017-05-02 16:00 +0200
  [PATCH 3/6] mailbox: arm_mhu: migrate to threaded irq handler Sudeep Holla <sudeep.holla@arm.com> - 2017-05-02 16:00 +0200
  [PATCH 6/6] mailbox: arm_mhu: add name support to record mbox-name Sudeep Holla <sudeep.holla@arm.com> - 2017-05-02 16:00 +0200
  [PATCH 2/6] Documentation: devicetree: add bindings to support ARM MHU subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-02 16:00 +0200
    Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Rob Herring <robh@kernel.org> - 2017-05-08 18:20 +0200
      Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-08 18:50 +0200
        Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-08 19:10 +0200
          Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-08 20:00 +0200
            Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-09 11:40 +0200
          Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-09 05:00 +0200
            Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-09 12:00 +0200
              Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-09 12:40 +0200
                Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-09 13:00 +0200
                Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-09 14:00 +0200
                Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-09 14:50 +0200
                Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-09 15:30 +0200
                Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-09 16:30 +0200
      Re: [PATCH 2/6] Documentation: devicetree: add bindings to support  ARM MHU subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-08 19:00 +0200
  [PATCH 1/6] mailbox: arm_mhu: reorder header inclusion and drop unneeded ones Sudeep Holla <sudeep.holla@arm.com> - 2017-05-02 16:00 +0200
  Re: [PATCH 0/6] mailbox: arm_mhu: add support for subchannels Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-03 05:20 +0200
    Re: [PATCH 0/6] mailbox: arm_mhu: add support for subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-03 11:30 +0200
      Re: [PATCH 0/6] mailbox: arm_mhu: add support for subchannels Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-05 13:20 +0200
        Re: [PATCH 0/6] mailbox: arm_mhu: add support for subchannels Sudeep Holla <sudeep.holla@arm.com> - 2017-05-05 13:30 +0200

csiph-web