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

Groups > linux.kernel > #1694583 > unrolled thread

Re: [PATCH 3/9] firmware: arm_scmi: add basic driver infrastructure for SCMI

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 3/9] firmware: arm_scmi: add basic driver infrastructure  for SCMI Sudeep Holla <sudeep.holla@arm.com> - 2017-07-24 12:00 +0200
    Re: [PATCH 3/9] firmware: arm_scmi: add basic driver infrastructure  for SCMI Jassi Brar <jassisinghbrar@gmail.com> - 2017-07-24 17:50 +0200
      Re: [PATCH 3/9] firmware: arm_scmi: add basic driver infrastructure  for SCMI Sudeep Holla <sudeep.holla@arm.com> - 2017-07-24 18:30 +0200
        Re: [PATCH 3/9] firmware: arm_scmi: add basic driver infrastructure  for SCMI Jassi Brar <jassisinghbrar@gmail.com> - 2017-07-24 19:30 +0200
          Re: [PATCH 3/9] firmware: arm_scmi: add basic driver infrastructure  for SCMI Sudeep Holla <sudeep.holla@arm.com> - 2017-07-24 19:40 +0200

#1694583 — Re: [PATCH 3/9] firmware: arm_scmi: add basic driver infrastructure for SCMI

On 08/07/17 06:32, Jassi Brar wrote:
> Hi Roy, Matt, Nishant, Harb Abdulhamid, Loc,
> 
> I have a gut feeling you guys were part of the SCMI spec committee. If
> so, could you please chime in?
> 

I take complete silence as no objection.

> 
> On Fri, Jul 7, 2017 at 11:09 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 07/07/17 17:52, Jassi Brar wrote:
>>> Hi Arnd, Hi Rob, Hi Mark,
>>>
>>> [CC'ing only those who I have the email id of]
>>>
>>>> +/**
>>>> + * scmi_do_xfer() - Do one transfer
>>>> + *
>>>> + * @info: Pointer to SCMI entity information
>>>> + * @xfer: Transfer to initiate and wait for response
>>>> + *
>>>> + * Return: -ETIMEDOUT in case of no response, if transmit error,
>>>> + *   return corresponding error, else if all goes well,
>>>> + *   return 0.
>>>> + */
>>>> +int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer)
>>>> +{
>>>> +    int ret;
>>>> +    int timeout;
>>>> +    struct scmi_info *info = handle_to_scmi_info(handle);
>>>> +    struct device *dev = info->dev;
>>>> +
>>>> +    ret = mbox_send_message(info->tx_chan, xfer);
>>>> +
>>>>
>>> The api is
>>>
>>>     int mbox_send_message(struct mbox_chan *chan, void *mssg)
>>>
>>> where each controller driver defines its own format in which it accepts
>>> the 'mssg' to be transmitted.
>>>
>>
>> Yes they can continue that, but SCMI just doesn't depend on that.
>>
>>> For example :-
>>> ti_msgmgr_send_data(struct mbox_chan *, struct ti_msgmgr_message *)
>>> rockchip_mbox_send_data(struct mbox_chan *, struct rockchip_mbox_msg *)
>>> ....and so on...  you get the idea.
>>>
>>
>> Yes I am aware of that.
>>
>>> Some controller driver may ignore the 'mssg' because only an interrupt line
>>> is shared with the remote and not some register/fifo.
>>> For example,
>>>   sti_mbox_send_data(struct mbox_chan *, void *ignored)
>>>
>>
>> Exactly, now with SCMI, every controller *can do* that, as we just care
>> about the signaling which in other terms I have so far referred as
>> "doorbell".
>>

You keep combining the 2 changes. Please treat them differently.
I need ARM MHU doorbell changes as I need to support multiple channels
on my platform which are independent and run different protocol.
That doorbell change *is not SCMI* specific. I am not sure how to conver
that message to you. All my attempts have failed so far.

> No, the controllers can not ... unless you clone and adapt the 9
> drivers+bindings to conform to the expectations of SCMI (like you
> attempted with MHU recently). Also, then mandate every future
> controller driver must emulate "doorbell" channels.
> 

Why do you think so ? Please give me your understanding on what SCMI
specification expects as you seem to think we need to clone ?
All I am saying is we don't pass any information in the controller.
The current drivers just need a way to trigger the remote and I believe
all of them already have.

> As the mailbox maintainer, I am open to suggestions that would allow
> every controller to support SCMI.

That's good to know.

> But compared to the options of scmi-as-a-library and
> scmi-as-child-node-of-platform-parent, this does not even qualify as
> an option.
> 

I don't understand this.

> Why? Because SCMI is but one protocol that provides 4 features ATM,
> and certainly can not provide for every whim and quirk of future
> platforms.  Among the sane requirements are watchdog,
> suspend/resume/hibernation and thermal _control_ (not just sensor
> readings) and among the weird are video, network and storage over
> mailbox api. And even a filesystem backed by read/write over mailbox!!
>  And these are only that I have worked on first hand.
> 

Fine. We need to support them I am not saying that we don't. Please
answer my question above which I believe should reduce the confusion.

>   The point is : you can not assume SCMI to be the only protocol
> running over a controller _and_ you can not dictate other protocols to
> not touch certain bits of the signal register/fifo.
> 

NO, I am saying again that I am not assuming that and you are the one
who is assuming MHU *must* work in the way the driver is written
currently and constantly/repeatedly ignoring the fact what MHY
specification states and mixing that change with SCMI here.

> In simplest terms, controller driver can not cater to only a
> particular client. That's the reason we have the controller driver
> define the message format and clients conform to it.

100%. You need to understand the fact that my changes makes it work with
SCMI and other protocol and MHU as it stands only work with whatever
platform you have with whatever client that supports.

-- 
Regards,
Sudeep

[toc] | [next] | [standalone]

#1694845

On Mon, Jul 24, 2017 at 3:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 08/07/17 06:32, Jassi Brar wrote:
>> Hi Roy, Matt, Nishant, Harb Abdulhamid, Loc,
>>
>> I have a gut feeling you guys were part of the SCMI spec committee. If
>> so, could you please chime in?
>>
>
> I take complete silence as no objection.
>
I take it as no self respecting developer/architect would stand by
your FUBAR implementation of SCMI.

SCMI  calls
    mbox_send_message(struct mbox_chan *chan, struct scmi_xfer *xfer);

whereas the API expects
    mbox_send_message(struct mbox_chan *chan, struct controller_specific *xfer);

As the maintainer I have tried to explain it to you in a dozen ways.
If you still can't see how it's broken, you can't be helped.

Checking out until someone else has any point.
NAK.

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

#1694876

On 24/07/17 16:41, Jassi Brar wrote:
> On Mon, Jul 24, 2017 at 3:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 08/07/17 06:32, Jassi Brar wrote:
>>> Hi Roy, Matt, Nishant, Harb Abdulhamid, Loc,
>>>
>>> I have a gut feeling you guys were part of the SCMI spec committee. If
>>> so, could you please chime in?
>>>
>>
>> I take complete silence as no objection.
>>
> I take it as no self respecting developer/architect would stand by
> your FUBAR implementation of SCMI.
>

Wow really ? This is exactly how ACPI PCC works today IIUC.

> SCMI  calls
>     mbox_send_message(struct mbox_chan *chan, struct scmi_xfer *xfer);
> 
> whereas the API expects
>     mbox_send_message(struct mbox_chan *chan, struct controller_specific *xfer);
> 

That's not hard to change but you seem to have combined my ARM MHU
changes into this which is totally wrong and now I say also stupid after
mentioning the same thing so many times. Consider them independently and
try to understand.

> As the maintainer I have tried to explain it to you in a dozen ways.
> If you still can't see how it's broken, you can't be helped.
> 

Sorry but you wanted driver to explain initially. I posted them. But you
haven't answer my specific questions yet.

-- 
Regards,
Sudeep

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

#1694918

On Mon, Jul 24, 2017 at 9:51 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> On 24/07/17 16:41, Jassi Brar wrote:

>> SCMI  calls
>>     mbox_send_message(struct mbox_chan *chan, struct scmi_xfer *xfer);
>>
>> whereas the API expects
>>     mbox_send_message(struct mbox_chan *chan, struct controller_specific *xfer);
>>
>
> That's not hard to change ...

Cool, please change it and lets move on.

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

#1694926

On 24/07/17 18:21, Jassi Brar wrote:
> On Mon, Jul 24, 2017 at 9:51 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> On 24/07/17 16:41, Jassi Brar wrote:
> 
>>> SCMI  calls
>>>     mbox_send_message(struct mbox_chan *chan, struct scmi_xfer *xfer);
>>>
>>> whereas the API expects
>>>     mbox_send_message(struct mbox_chan *chan, struct controller_specific *xfer);
>>>
>>
>> That's not hard to change ...
> 
> Cool, please change it and lets move on.
> 

But tell me why first ? You said it needs to work with any controller.
And now you say I need to change it to some controller_specific ptr,
won't that make it controller specific protocol ?

You responded on something you wanted ignoring all the other parts,
still waiting for those response.

1. Tell me how my changes to MHU and SCMI are related ?
2. How my MHU changes and PCC are different ? Why do you think
   this SCMI is FUBAR design which the clients using PCC are not ?

-- 
Regards,
Sudeep

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web