Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 14:45, Jassi Brar wrote:

On Fri, Jul 31, 2015 at 6:38 PM, Sudeep Holla  wrote:



On 31/07/15 11:43, Sudeep Holla wrote:




On 31/07/15 11:38, Jassi Brar wrote:


On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla 
wrote:



I forgot to mention, we have a the following description in
mbox_client_txdone which is misleading:

"The client/protocol had received some 'ACK' packet and it notifies the
API that the last packet was sent successfully. This only works if the
controller can't sense TX-Done."

which is clearly not the case in SCPI. IMO we may have to reword that.


Yes. And also see whether it could race with polling driven tx_tick.



Yes I am also looking at that now while I am trying to check if
TXDONE_BY_ACK works on Juno, will keep you posted.



OK, I recollect the racy condition now which I had in my mind from the
beginning convincing myself why we can't use that option. I was not good
in words to explain it so far but let me try with the ASCII art this
time. Note Tx ACK below means the remote setting the register flag and
not to be confused with the ACK packet. For simplicity Rx can be assumed
to be Tx ACK packet

  Time   MHU/SCPIRemote SCP
||
 1  | Tx1 -->|
||
 2  |<--- Tx1 ACK ---|
||
 3  | Tx2 -->|
||
 4  |<--- Rx1 ---|
||
 5  |<--- Tx2 ACK ---|
||
 6  | Tx3 -->|
||
 7  |<--- Rx2 ---|

Now lets consider the above scenario, suppose we have TXDONE_BY_ACK
and use mbox_client_txdone in Rx interrupt(i.e. response packet), we end
up in the race easily IIUC.

E.g. A client would have sent Tx2(3) before Rx1 interrupt(4) and if we
ACK Tx1 now in (3), we will end up acknowledging Tx2(5) as the mailbox
core assumes only one Tx request at a time which is clearly not the case
in our setup. The client can then go ahead and send Tx3(6) overwriting
the payload while remote was processing or even result in remote missing
the request completely. Does that make sense or am I missing something ?


Yeah, that could happen. But the race can be fixed by checking
last_tx_done if the controller provides that. If last_tx_done is not
implemented, polling won't race.

Please try the following...



Looks good to me. Sometimes it takes very long time(days) to hit race
conditions(esp. in firmware), so I need some time to think before
I cook up a patch to start stress test this on Juno so that I don't
waste time waiting for result.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 14:45, Jassi Brar wrote:

On Fri, Jul 31, 2015 at 6:38 PM, Sudeep Holla sudeep.ho...@arm.com wrote:



On 31/07/15 11:43, Sudeep Holla wrote:




On 31/07/15 11:38, Jassi Brar wrote:


On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla sudeep.ho...@arm.com
wrote:



I forgot to mention, we have a the following description in
mbox_client_txdone which is misleading:

The client/protocol had received some 'ACK' packet and it notifies the
API that the last packet was sent successfully. This only works if the
controller can't sense TX-Done.

which is clearly not the case in SCPI. IMO we may have to reword that.


Yes. And also see whether it could race with polling driven tx_tick.



Yes I am also looking at that now while I am trying to check if
TXDONE_BY_ACK works on Juno, will keep you posted.



OK, I recollect the racy condition now which I had in my mind from the
beginning convincing myself why we can't use that option. I was not good
in words to explain it so far but let me try with the ASCII art this
time. Note Tx ACK below means the remote setting the register flag and
not to be confused with the ACK packet. For simplicity Rx can be assumed
to be Tx ACK packet

  Time   MHU/SCPIRemote SCP
||
 1  | Tx1 --|
||
 2  |--- Tx1 ACK ---|
||
 3  | Tx2 --|
||
 4  |--- Rx1 ---|
||
 5  |--- Tx2 ACK ---|
||
 6  | Tx3 --|
||
 7  |--- Rx2 ---|

Now lets consider the above scenario, suppose we have TXDONE_BY_ACK
and use mbox_client_txdone in Rx interrupt(i.e. response packet), we end
up in the race easily IIUC.

E.g. A client would have sent Tx2(3) before Rx1 interrupt(4) and if we
ACK Tx1 now in (3), we will end up acknowledging Tx2(5) as the mailbox
core assumes only one Tx request at a time which is clearly not the case
in our setup. The client can then go ahead and send Tx3(6) overwriting
the payload while remote was processing or even result in remote missing
the request completely. Does that make sense or am I missing something ?


Yeah, that could happen. But the race can be fixed by checking
last_tx_done if the controller provides that. If last_tx_done is not
implemented, polling won't race.

Please try the following...



Looks good to me. Sometimes it takes very long time(days) to hit race
conditions(esp. in firmware), so I need some time to think before
I cook up a patch to start stress test this on Juno so that I don't
waste time waiting for result.

Regards,
Sudeep
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Fri, Jul 31, 2015 at 6:38 PM, Sudeep Holla  wrote:
>
>
> On 31/07/15 11:43, Sudeep Holla wrote:
>>
>>
>>
>> On 31/07/15 11:38, Jassi Brar wrote:
>>>
>>> On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla 
>>> wrote:


 I forgot to mention, we have a the following description in
 mbox_client_txdone which is misleading:

 "The client/protocol had received some 'ACK' packet and it notifies the
 API that the last packet was sent successfully. This only works if the
 controller can't sense TX-Done."

 which is clearly not the case in SCPI. IMO we may have to reword that.

>>> Yes. And also see whether it could race with polling driven tx_tick.
>>>
>>
>> Yes I am also looking at that now while I am trying to check if
>> TXDONE_BY_ACK works on Juno, will keep you posted.
>>
>
> OK, I recollect the racy condition now which I had in my mind from the
> beginning convincing myself why we can't use that option. I was not good
> in words to explain it so far but let me try with the ASCII art this
> time. Note Tx ACK below means the remote setting the register flag and
> not to be confused with the ACK packet. For simplicity Rx can be assumed
> to be Tx ACK packet
>
>  Time   MHU/SCPIRemote SCP
>||
> 1  | Tx1 -->|
>||
> 2  |<--- Tx1 ACK ---|
>||
> 3  | Tx2 -->|
>||
> 4  |<--- Rx1 ---|
>||
> 5  |<--- Tx2 ACK ---|
>||
> 6  | Tx3 -->|
>||
> 7  |<--- Rx2 ---|
>
> Now lets consider the above scenario, suppose we have TXDONE_BY_ACK
> and use mbox_client_txdone in Rx interrupt(i.e. response packet), we end
> up in the race easily IIUC.
>
> E.g. A client would have sent Tx2(3) before Rx1 interrupt(4) and if we
> ACK Tx1 now in (3), we will end up acknowledging Tx2(5) as the mailbox
> core assumes only one Tx request at a time which is clearly not the case
> in our setup. The client can then go ahead and send Tx3(6) overwriting
> the payload while remote was processing or even result in remote missing
> the request completely. Does that make sense or am I missing something ?
>
Yeah, that could happen. But the race can be fixed by checking
last_tx_done if the controller provides that. If last_tx_done is not
implemented, polling won't race.

Please try the following...

==>8=
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 07fd507..c973c2c 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -181,17 +181,23 @@ EXPORT_SYMBOL_GPL(mbox_chan_txdone);
  * @r: Success status of last transmission.
  *
  * The client/protocol had received some 'ACK' packet and it notifies
- * the API that the last packet was sent successfully. This only works
- * if the controller can't sense TX-Done.
+ * the API that the last packet was sent successfully.
  */
 void mbox_client_txdone(struct mbox_chan *chan, int r)
 {
+   bool txdone = true;
+
if (unlikely(!(chan->txdone_method & TXDONE_BY_ACK))) {
dev_err(chan->mbox->dev, "Client can't run the TX ticker\n");
return;
}

-   tx_tick(chan, r);
+   /* if h/w can check for tx-done, make sure its done */
+   if (chan->mbox->ops->last_tx_done)
+   txdone = chan->mbox->ops->last_tx_done(chan);
+
+   if (txdone)
+   tx_tick(chan, 0);
 }
 EXPORT_SYMBOL_GPL(mbox_client_txdone);

==8<=

we might also have to looking into protecting last_tx_done and tx_tick
together. Anyways, those changes are warranted in order to get the
expected functionality.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 11:43, Sudeep Holla wrote:



On 31/07/15 11:38, Jassi Brar wrote:

On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla  wrote:


I forgot to mention, we have a the following description in
mbox_client_txdone which is misleading:

"The client/protocol had received some 'ACK' packet and it notifies the
API that the last packet was sent successfully. This only works if the
controller can't sense TX-Done."

which is clearly not the case in SCPI. IMO we may have to reword that.


Yes. And also see whether it could race with polling driven tx_tick.



Yes I am also looking at that now while I am trying to check if
TXDONE_BY_ACK works on Juno, will keep you posted.



OK, I recollect the racy condition now which I had in my mind from the
beginning convincing myself why we can't use that option. I was not good
in words to explain it so far but let me try with the ASCII art this
time. Note Tx ACK below means the remote setting the register flag and
not to be confused with the ACK packet. For simplicity Rx can be assumed
to be Tx ACK packet

 Time   MHU/SCPIRemote SCP
   ||
1  | Tx1 -->|
   ||
2  |<--- Tx1 ACK ---|
   ||
3  | Tx2 -->|
   ||
4  |<--- Rx1 ---|
   ||
5  |<--- Tx2 ACK ---|
   ||
6  | Tx3 -->|
   ||
7  |<--- Rx2 ---|

Now lets consider the above scenario, suppose we have TXDONE_BY_ACK
and use mbox_client_txdone in Rx interrupt(i.e. response packet), we end
up in the race easily IIUC.

E.g. A client would have sent Tx2(3) before Rx1 interrupt(4) and if we
ACK Tx1 now in (3), we will end up acknowledging Tx2(5) as the mailbox
core assumes only one Tx request at a time which is clearly not the case
in our setup. The client can then go ahead and send Tx3(6) overwriting
the payload while remote was processing or even result in remote missing
the request completely. Does that make sense or am I missing something ?

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 11:38, Jassi Brar wrote:

On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla  wrote:


I forgot to mention, we have a the following description in
mbox_client_txdone which is misleading:

"The client/protocol had received some 'ACK' packet and it notifies the
API that the last packet was sent successfully. This only works if the
controller can't sense TX-Done."

which is clearly not the case in SCPI. IMO we may have to reword that.


Yes. And also see whether it could race with polling driven tx_tick.



Yes I am also looking at that now while I am trying to check if
TXDONE_BY_ACK works on Juno, will keep you posted.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla  wrote:
>
> I forgot to mention, we have a the following description in
> mbox_client_txdone which is misleading:
>
> "The client/protocol had received some 'ACK' packet and it notifies the
> API that the last packet was sent successfully. This only works if the
> controller can't sense TX-Done."
>
> which is clearly not the case in SCPI. IMO we may have to reword that.
>
Yes. And also see whether it could race with polling driven tx_tick.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 30/07/15 18:56, Jassi Brar wrote:

On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla  wrote:

On 29/07/15 12:19, Jassi Brar wrote:


Assuming the former, let me explain. When a client receives a
response, it can be sure that the request has already been read by the
remote.


Waiting for the response would be too late for few expensive commands
(e.g setting up external regulators). The remote firmware acknowledges
Tx by setting status flags and will be ready to accept new commands.


No. Polling still happens. If anything, mbox_client_txdone() should
only speed up things.


If the protocol specifies every request has some response, the


Not always true there can be few commands without response. The protocol
specifies that we need check the status flag before sending the new
command as it's bidirectional, hence polling is recommended (Section 2.2
Communication flow in the SCPI specification)


mbox_client_txdone() will only be called for commands that has some
response. Commands that don't have a response would be completed by
polling.



I forgot to mention, we have a the following description in
mbox_client_txdone which is misleading:

"The client/protocol had received some 'ACK' packet and it notifies the
API that the last packet was sent successfully. This only works if the
controller can't sense TX-Done."

which is clearly not the case in SCPI. IMO we may have to reword that.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 30/07/15 18:56, Jassi Brar wrote:

On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla  wrote:

On 29/07/15 12:19, Jassi Brar wrote:


Assuming the former, let me explain. When a client receives a
response, it can be sure that the request has already been read by the
remote.


Waiting for the response would be too late for few expensive commands
(e.g setting up external regulators). The remote firmware acknowledges
Tx by setting status flags and will be ready to accept new commands.


No. Polling still happens. If anything, mbox_client_txdone() should
only speed up things.



Yes I understand and that's good.


If the protocol specifies every request has some response, the


Not always true there can be few commands without response. The protocol
specifies that we need check the status flag before sending the new
command as it's bidirectional, hence polling is recommended (Section 2.2
Communication flow in the SCPI specification)


mbox_client_txdone() will only be called for commands that has some
response. Commands that don't have a response would be completed by
polling.



OK, got it


client should assert 'knows_txdone' and call mbox_client_txdone() upon
receiving a reply packet.


Since this is not always true and not recommended in the specification,
I am hesitant to use this option as the firmware can always change their
internal mechanics without breaking the protocol. We need to ensure we are
compliant to the spec.


I don't see how it could break compliance.



While I agree it shouldn't, the firmware guys won't support if we
deviate from the spec. I won't get support for firmware bug fixes in
that case.

Having said that, I don't rule out the usage of TX_BY_ACK, I will need
more time for testing(usually we stress test firmware using Linux for
few of days continuously as we have hit issues after that :)) and
getting things fixed if anything breaks.


So I said,  cl->knows_txdone = false;   is the root of problems you


It could be and won't rule that out. I would prefer using knows_txdone
and use mbox_client_txdone if feasible, but I can't as the without
violating the specification.

FYI, I had tried it and ended up with issues in the firmware. The
argument from the firmware is that we aren't specification compliant,
so I had to use polling.


I am sure you would have copy of that discarded code. Care to share? I
can't imagine how we handle completions locally could affect the
remote. The mbox_client_txdone() is untested so I don't rule out bugs,
otherwise it should only make things better.



I tested it with very old firmware almost 4-5 months back. I don't have
the patch on top of this series handy, but will dig it out and give it a
try with latest firmware. I will let you know the results.

For now, I would keep this just polling and unblock others who are
waiting on this series.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 11:43, Sudeep Holla wrote:



On 31/07/15 11:38, Jassi Brar wrote:

On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla sudeep.ho...@arm.com wrote:


I forgot to mention, we have a the following description in
mbox_client_txdone which is misleading:

The client/protocol had received some 'ACK' packet and it notifies the
API that the last packet was sent successfully. This only works if the
controller can't sense TX-Done.

which is clearly not the case in SCPI. IMO we may have to reword that.


Yes. And also see whether it could race with polling driven tx_tick.



Yes I am also looking at that now while I am trying to check if
TXDONE_BY_ACK works on Juno, will keep you posted.



OK, I recollect the racy condition now which I had in my mind from the
beginning convincing myself why we can't use that option. I was not good
in words to explain it so far but let me try with the ASCII art this
time. Note Tx ACK below means the remote setting the register flag and
not to be confused with the ACK packet. For simplicity Rx can be assumed
to be Tx ACK packet

 Time   MHU/SCPIRemote SCP
   ||
1  | Tx1 --|
   ||
2  |--- Tx1 ACK ---|
   ||
3  | Tx2 --|
   ||
4  |--- Rx1 ---|
   ||
5  |--- Tx2 ACK ---|
   ||
6  | Tx3 --|
   ||
7  |--- Rx2 ---|

Now lets consider the above scenario, suppose we have TXDONE_BY_ACK
and use mbox_client_txdone in Rx interrupt(i.e. response packet), we end
up in the race easily IIUC.

E.g. A client would have sent Tx2(3) before Rx1 interrupt(4) and if we
ACK Tx1 now in (3), we will end up acknowledging Tx2(5) as the mailbox
core assumes only one Tx request at a time which is clearly not the case
in our setup. The client can then go ahead and send Tx3(6) overwriting
the payload while remote was processing or even result in remote missing
the request completely. Does that make sense or am I missing something ?

Regards,
Sudeep
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Fri, Jul 31, 2015 at 6:38 PM, Sudeep Holla sudeep.ho...@arm.com wrote:


 On 31/07/15 11:43, Sudeep Holla wrote:



 On 31/07/15 11:38, Jassi Brar wrote:

 On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla sudeep.ho...@arm.com
 wrote:


 I forgot to mention, we have a the following description in
 mbox_client_txdone which is misleading:

 The client/protocol had received some 'ACK' packet and it notifies the
 API that the last packet was sent successfully. This only works if the
 controller can't sense TX-Done.

 which is clearly not the case in SCPI. IMO we may have to reword that.

 Yes. And also see whether it could race with polling driven tx_tick.


 Yes I am also looking at that now while I am trying to check if
 TXDONE_BY_ACK works on Juno, will keep you posted.


 OK, I recollect the racy condition now which I had in my mind from the
 beginning convincing myself why we can't use that option. I was not good
 in words to explain it so far but let me try with the ASCII art this
 time. Note Tx ACK below means the remote setting the register flag and
 not to be confused with the ACK packet. For simplicity Rx can be assumed
 to be Tx ACK packet

  Time   MHU/SCPIRemote SCP
||
 1  | Tx1 --|
||
 2  |--- Tx1 ACK ---|
||
 3  | Tx2 --|
||
 4  |--- Rx1 ---|
||
 5  |--- Tx2 ACK ---|
||
 6  | Tx3 --|
||
 7  |--- Rx2 ---|

 Now lets consider the above scenario, suppose we have TXDONE_BY_ACK
 and use mbox_client_txdone in Rx interrupt(i.e. response packet), we end
 up in the race easily IIUC.

 E.g. A client would have sent Tx2(3) before Rx1 interrupt(4) and if we
 ACK Tx1 now in (3), we will end up acknowledging Tx2(5) as the mailbox
 core assumes only one Tx request at a time which is clearly not the case
 in our setup. The client can then go ahead and send Tx3(6) overwriting
 the payload while remote was processing or even result in remote missing
 the request completely. Does that make sense or am I missing something ?

Yeah, that could happen. But the race can be fixed by checking
last_tx_done if the controller provides that. If last_tx_done is not
implemented, polling won't race.

Please try the following...

==8=
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 07fd507..c973c2c 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -181,17 +181,23 @@ EXPORT_SYMBOL_GPL(mbox_chan_txdone);
  * @r: Success status of last transmission.
  *
  * The client/protocol had received some 'ACK' packet and it notifies
- * the API that the last packet was sent successfully. This only works
- * if the controller can't sense TX-Done.
+ * the API that the last packet was sent successfully.
  */
 void mbox_client_txdone(struct mbox_chan *chan, int r)
 {
+   bool txdone = true;
+
if (unlikely(!(chan-txdone_method  TXDONE_BY_ACK))) {
dev_err(chan-mbox-dev, Client can't run the TX ticker\n);
return;
}

-   tx_tick(chan, r);
+   /* if h/w can check for tx-done, make sure its done */
+   if (chan-mbox-ops-last_tx_done)
+   txdone = chan-mbox-ops-last_tx_done(chan);
+
+   if (txdone)
+   tx_tick(chan, 0);
 }
 EXPORT_SYMBOL_GPL(mbox_client_txdone);

==8=

we might also have to looking into protecting last_tx_done and tx_tick
together. Anyways, those changes are warranted in order to get the
expected functionality.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 30/07/15 18:56, Jassi Brar wrote:

On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla sudeep.ho...@arm.com wrote:

On 29/07/15 12:19, Jassi Brar wrote:


Assuming the former, let me explain. When a client receives a
response, it can be sure that the request has already been read by the
remote.


Waiting for the response would be too late for few expensive commands
(e.g setting up external regulators). The remote firmware acknowledges
Tx by setting status flags and will be ready to accept new commands.


No. Polling still happens. If anything, mbox_client_txdone() should
only speed up things.



Yes I understand and that's good.


If the protocol specifies every request has some response, the


Not always true there can be few commands without response. The protocol
specifies that we need check the status flag before sending the new
command as it's bidirectional, hence polling is recommended (Section 2.2
Communication flow in the SCPI specification)


mbox_client_txdone() will only be called for commands that has some
response. Commands that don't have a response would be completed by
polling.



OK, got it


client should assert 'knows_txdone' and call mbox_client_txdone() upon
receiving a reply packet.


Since this is not always true and not recommended in the specification,
I am hesitant to use this option as the firmware can always change their
internal mechanics without breaking the protocol. We need to ensure we are
compliant to the spec.


I don't see how it could break compliance.



While I agree it shouldn't, the firmware guys won't support if we
deviate from the spec. I won't get support for firmware bug fixes in
that case.

Having said that, I don't rule out the usage of TX_BY_ACK, I will need
more time for testing(usually we stress test firmware using Linux for
few of days continuously as we have hit issues after that :)) and
getting things fixed if anything breaks.


So I said,  cl-knows_txdone = false;   is the root of problems you


It could be and won't rule that out. I would prefer using knows_txdone
and use mbox_client_txdone if feasible, but I can't as the without
violating the specification.

FYI, I had tried it and ended up with issues in the firmware. The
argument from the firmware is that we aren't specification compliant,
so I had to use polling.


I am sure you would have copy of that discarded code. Care to share? I
can't imagine how we handle completions locally could affect the
remote. The mbox_client_txdone() is untested so I don't rule out bugs,
otherwise it should only make things better.



I tested it with very old firmware almost 4-5 months back. I don't have
the patch on top of this series handy, but will dig it out and give it a
try with latest firmware. I will let you know the results.

For now, I would keep this just polling and unblock others who are
waiting on this series.

Regards,
Sudeep
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 30/07/15 18:56, Jassi Brar wrote:

On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla sudeep.ho...@arm.com wrote:

On 29/07/15 12:19, Jassi Brar wrote:


Assuming the former, let me explain. When a client receives a
response, it can be sure that the request has already been read by the
remote.


Waiting for the response would be too late for few expensive commands
(e.g setting up external regulators). The remote firmware acknowledges
Tx by setting status flags and will be ready to accept new commands.


No. Polling still happens. If anything, mbox_client_txdone() should
only speed up things.


If the protocol specifies every request has some response, the


Not always true there can be few commands without response. The protocol
specifies that we need check the status flag before sending the new
command as it's bidirectional, hence polling is recommended (Section 2.2
Communication flow in the SCPI specification)


mbox_client_txdone() will only be called for commands that has some
response. Commands that don't have a response would be completed by
polling.



I forgot to mention, we have a the following description in
mbox_client_txdone which is misleading:

The client/protocol had received some 'ACK' packet and it notifies the
API that the last packet was sent successfully. This only works if the
controller can't sense TX-Done.

which is clearly not the case in SCPI. IMO we may have to reword that.

Regards,
Sudeep
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla sudeep.ho...@arm.com wrote:

 I forgot to mention, we have a the following description in
 mbox_client_txdone which is misleading:

 The client/protocol had received some 'ACK' packet and it notifies the
 API that the last packet was sent successfully. This only works if the
 controller can't sense TX-Done.

 which is clearly not the case in SCPI. IMO we may have to reword that.

Yes. And also see whether it could race with polling driven tx_tick.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 11:38, Jassi Brar wrote:

On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla sudeep.ho...@arm.com wrote:


I forgot to mention, we have a the following description in
mbox_client_txdone which is misleading:

The client/protocol had received some 'ACK' packet and it notifies the
API that the last packet was sent successfully. This only works if the
controller can't sense TX-Done.

which is clearly not the case in SCPI. IMO we may have to reword that.


Yes. And also see whether it could race with polling driven tx_tick.



Yes I am also looking at that now while I am trying to check if
TXDONE_BY_ACK works on Juno, will keep you posted.

Regards,
Sudeep
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla  wrote:
> On 29/07/15 12:19, Jassi Brar wrote:
>
>> Assuming the former, let me explain. When a client receives a
>> response, it can be sure that the request has already been read by the
>> remote.
>
> Waiting for the response would be too late for few expensive commands
> (e.g setting up external regulators). The remote firmware acknowledges
> Tx by setting status flags and will be ready to accept new commands.
>
No. Polling still happens. If anything, mbox_client_txdone() should
only speed up things.

>> If the protocol specifies every request has some response, the
>
> Not always true there can be few commands without response. The protocol
> specifies that we need check the status flag before sending the new
> command as it's bidirectional, hence polling is recommended (Section 2.2
> Communication flow in the SCPI specification)
>
mbox_client_txdone() will only be called for commands that has some
response. Commands that don't have a response would be completed by
polling.

>> client should assert 'knows_txdone' and call mbox_client_txdone() upon
>> receiving a reply packet.
>
> Since this is not always true and not recommended in the specification,
> I am hesitant to use this option as the firmware can always change their
> internal mechanics without breaking the protocol. We need to ensure we are
> compliant to the spec.
>
I don't see how it could break compliance.

>> So I said,  cl->knows_txdone = false;   is the root of problems you
>
> It could be and won't rule that out. I would prefer using knows_txdone
> and use mbox_client_txdone if feasible, but I can't as the without
> violating the specification.
>
> FYI, I had tried it and ended up with issues in the firmware. The
> argument from the firmware is that we aren't specification compliant,
> so I had to use polling.
>
I am sure you would have copy of that discarded code. Care to share? I
can't imagine how we handle completions locally could affect the
remote. The mbox_client_txdone() is untested so I don't rule out bugs,
otherwise it should only make things better.

Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla sudeep.ho...@arm.com wrote:
 On 29/07/15 12:19, Jassi Brar wrote:

 Assuming the former, let me explain. When a client receives a
 response, it can be sure that the request has already been read by the
 remote.

 Waiting for the response would be too late for few expensive commands
 (e.g setting up external regulators). The remote firmware acknowledges
 Tx by setting status flags and will be ready to accept new commands.

No. Polling still happens. If anything, mbox_client_txdone() should
only speed up things.

 If the protocol specifies every request has some response, the

 Not always true there can be few commands without response. The protocol
 specifies that we need check the status flag before sending the new
 command as it's bidirectional, hence polling is recommended (Section 2.2
 Communication flow in the SCPI specification)

mbox_client_txdone() will only be called for commands that has some
response. Commands that don't have a response would be completed by
polling.

 client should assert 'knows_txdone' and call mbox_client_txdone() upon
 receiving a reply packet.

 Since this is not always true and not recommended in the specification,
 I am hesitant to use this option as the firmware can always change their
 internal mechanics without breaking the protocol. We need to ensure we are
 compliant to the spec.

I don't see how it could break compliance.

 So I said,  cl-knows_txdone = false;   is the root of problems you

 It could be and won't rule that out. I would prefer using knows_txdone
 and use mbox_client_txdone if feasible, but I can't as the without
 violating the specification.

 FYI, I had tried it and ended up with issues in the firmware. The
 argument from the firmware is that we aren't specification compliant,
 so I had to use polling.

I am sure you would have copy of that discarded code. Care to share? I
can't imagine how we handle completions locally could affect the
remote. The mbox_client_txdone() is untested so I don't rule out bugs,
otherwise it should only make things better.

Jassi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 29/07/15 12:19, Jassi Brar wrote:

On Wed, Jul 29, 2015 at 2:08 PM, Sudeep Holla  wrote:

On 29/07/15 09:05, Jassi Brar wrote:




+static int scpi_probe(struct platform_device *pdev)
+{
+   int count, idx, ret;
+   struct resource res;
+   struct scpi_chan *scpi_chan;
+   struct device *dev = >dev;
+   struct device_node *np = dev->of_node;
+
+   scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
+   if (!scpi_info)
+   return -ENOMEM;
+
+   count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
+   if (count < 0) {
+   dev_err(dev, "no mboxes property in '%s'\n",
np->full_name);
+   return -ENODEV;
+   }
+
+   scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan),
GFP_KERNEL);
+   if (!scpi_chan)
+   return -ENOMEM;
+
+   for (idx = 0; idx < count; idx++) {
+   resource_size_t size;
+   struct scpi_chan *pchan = scpi_chan + idx;
+   struct mbox_client *cl = >cl;
+   struct device_node *shmem = of_parse_phandle(np, "shmem",
idx);
+
+   if (of_address_to_resource(shmem, 0, )) {
+   dev_err(dev, "failed to get SCPI payload mem
resource\n");
+   ret = -EINVAL;
+   goto err;
+   }
+
+   size = resource_size();
+   pchan->rx_payload = devm_ioremap(dev, res.start, size);
+   if (!pchan->rx_payload) {
+   dev_err(dev, "failed to ioremap SCPI payload\n");
+   ret = -EADDRNOTAVAIL;
+   goto err;
+   }
+   pchan->tx_payload = pchan->rx_payload + (size >> 1);
+
+   cl->dev = dev;
+   cl->rx_callback = scpi_handle_remote_msg;
+   cl->tx_prepare = scpi_tx_prepare;
+   cl->tx_block = true;
+   cl->tx_tout = 50;
+   cl->knows_txdone = false; /* controller can ack */


   This is the cause of your problems that you think should be solved by
using hrtimer.



Ah sorry, it's stupid mistake on my part while writing the comment. It
should have been controller can't ack, fixed locally now thanks for
pointing it out.


No, I am talking about code, not the comment.


   Controller may or may not (like MHU) set txdone_irq. However every
scpi command (struct scpi_ops members) is replied to as a response
packet reporting success or failure.



No that's not true, I have already mentioned that couple of times in the
other thread. It's just wrong comment here which went unnoticed from
day#1, sorry for that.


So the client should set 'knows_txdone' to be true unless it is told
the controller on that platform supports txdone_irq (what you call
'ack').


I got the concept but SCP can't ack via protocol, protocol has no such
provision and it sets flags in MHU status register.


You either don't get the concept of TXDONE_BY_ACK  or deliberately
overlook my point.



No I do understand the concept and didn't overlook the points you made.


Assuming the former, let me explain. When a client receives a
response, it can be sure that the request has already been read by the
remote.


Waiting for the response would be too late for few expensive commands
(e.g setting up external regulators). The remote firmware acknowledges
Tx by setting status flags and will be ready to accept new commands.


If the protocol specifies every request has some response, the


Not always true there can be few commands without response. The protocol
specifies that we need check the status flag before sending the new
command as it's bidirectional, hence polling is recommended (Section 2.2
Communication flow in the SCPI specification)


client should assert 'knows_txdone' and call mbox_client_txdone() upon
receiving a reply packet.


Since this is not always true and not recommended in the specification,
I am hesitant to use this option as the firmware can always change their
internal mechanics without breaking the protocol. We need to ensure we 
are compliant to the spec.



So I said,  cl->knows_txdone = false;   is the root of problems you


It could be and won't rule that out. I would prefer using knows_txdone
and use mbox_client_txdone if feasible, but I can't as the without
violating the specification.

FYI, I had tried it and ended up with issues in the firmware. The
argument from the firmware is that we aren't specification compliant,
so I had to use polling.


report. If you fix that, the performance should be even better than
using hrtimer.



That would have been ideal and much better but I can't use that for
above mentioned reason.

Though you had valid concerns here and I hope I clarified them all
sucessfully, none were related to hrtimers.

Can I assume you are fine with hrtimers ? If so, can you review this patch ?

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" 

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Wed, Jul 29, 2015 at 2:08 PM, Sudeep Holla  wrote:
> On 29/07/15 09:05, Jassi Brar wrote:
>
>>
>>> +static int scpi_probe(struct platform_device *pdev)
>>> +{
>>> +   int count, idx, ret;
>>> +   struct resource res;
>>> +   struct scpi_chan *scpi_chan;
>>> +   struct device *dev = >dev;
>>> +   struct device_node *np = dev->of_node;
>>> +
>>> +   scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
>>> +   if (!scpi_info)
>>> +   return -ENOMEM;
>>> +
>>> +   count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
>>> +   if (count < 0) {
>>> +   dev_err(dev, "no mboxes property in '%s'\n",
>>> np->full_name);
>>> +   return -ENODEV;
>>> +   }
>>> +
>>> +   scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan),
>>> GFP_KERNEL);
>>> +   if (!scpi_chan)
>>> +   return -ENOMEM;
>>> +
>>> +   for (idx = 0; idx < count; idx++) {
>>> +   resource_size_t size;
>>> +   struct scpi_chan *pchan = scpi_chan + idx;
>>> +   struct mbox_client *cl = >cl;
>>> +   struct device_node *shmem = of_parse_phandle(np, "shmem",
>>> idx);
>>> +
>>> +   if (of_address_to_resource(shmem, 0, )) {
>>> +   dev_err(dev, "failed to get SCPI payload mem
>>> resource\n");
>>> +   ret = -EINVAL;
>>> +   goto err;
>>> +   }
>>> +
>>> +   size = resource_size();
>>> +   pchan->rx_payload = devm_ioremap(dev, res.start, size);
>>> +   if (!pchan->rx_payload) {
>>> +   dev_err(dev, "failed to ioremap SCPI payload\n");
>>> +   ret = -EADDRNOTAVAIL;
>>> +   goto err;
>>> +   }
>>> +   pchan->tx_payload = pchan->rx_payload + (size >> 1);
>>> +
>>> +   cl->dev = dev;
>>> +   cl->rx_callback = scpi_handle_remote_msg;
>>> +   cl->tx_prepare = scpi_tx_prepare;
>>> +   cl->tx_block = true;
>>> +   cl->tx_tout = 50;
>>> +   cl->knows_txdone = false; /* controller can ack */
>>>
>>   This is the cause of your problems that you think should be solved by
>> using hrtimer.
>>
>
> Ah sorry, it's stupid mistake on my part while writing the comment. It
> should have been controller can't ack, fixed locally now thanks for
> pointing it out.
>
No, I am talking about code, not the comment.

>>   Controller may or may not (like MHU) set txdone_irq. However every
>> scpi command (struct scpi_ops members) is replied to as a response
>> packet reporting success or failure.
>
>
> No that's not true, I have already mentioned that couple of times in the
> other thread. It's just wrong comment here which went unnoticed from
> day#1, sorry for that.
>
>> So the client should set 'knows_txdone' to be true unless it is told
>> the controller on that platform supports txdone_irq (what you call
>> 'ack').
>>
> I got the concept but SCP can't ack via protocol, protocol has no such
> provision and it sets flags in MHU status register.
>
You either don't get the concept of TXDONE_BY_ACK  or deliberately
overlook my point.

Assuming the former, let me explain. When a client receives a
response, it can be sure that the request has already been read by the
remote. If the protocol specifies every request has some response, the
client should assert 'knows_txdone' and call mbox_client_txdone() upon
receiving a reply packet.
   So I said,  cl->knows_txdone = false;   is the root of problems you
report. If you fix that, the performance should be even better than
using hrtimer.

-Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 29/07/15 09:05, Jassi Brar wrote:

On Thu, Jul 23, 2015 at 4:40 PM, Sudeep Holla  wrote:

...


+static int scpi_probe(struct platform_device *pdev)
+{
+   int count, idx, ret;
+   struct resource res;
+   struct scpi_chan *scpi_chan;
+   struct device *dev = >dev;
+   struct device_node *np = dev->of_node;
+
+   scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
+   if (!scpi_info)
+   return -ENOMEM;
+
+   count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
+   if (count < 0) {
+   dev_err(dev, "no mboxes property in '%s'\n", np->full_name);
+   return -ENODEV;
+   }
+
+   scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan), GFP_KERNEL);
+   if (!scpi_chan)
+   return -ENOMEM;
+
+   for (idx = 0; idx < count; idx++) {
+   resource_size_t size;
+   struct scpi_chan *pchan = scpi_chan + idx;
+   struct mbox_client *cl = >cl;
+   struct device_node *shmem = of_parse_phandle(np, "shmem", idx);
+
+   if (of_address_to_resource(shmem, 0, )) {
+   dev_err(dev, "failed to get SCPI payload mem 
resource\n");
+   ret = -EINVAL;
+   goto err;
+   }
+
+   size = resource_size();
+   pchan->rx_payload = devm_ioremap(dev, res.start, size);
+   if (!pchan->rx_payload) {
+   dev_err(dev, "failed to ioremap SCPI payload\n");
+   ret = -EADDRNOTAVAIL;
+   goto err;
+   }
+   pchan->tx_payload = pchan->rx_payload + (size >> 1);
+
+   cl->dev = dev;
+   cl->rx_callback = scpi_handle_remote_msg;
+   cl->tx_prepare = scpi_tx_prepare;
+   cl->tx_block = true;
+   cl->tx_tout = 50;
+   cl->knows_txdone = false; /* controller can ack */


  This is the cause of your problems that you think should be solved by
using hrtimer.



Ah sorry, it's stupid mistake on my part while writing the comment. It
should have been controller can't ack, fixed locally now thanks for
pointing it out.


  Controller may or may not (like MHU) set txdone_irq. However every
scpi command (struct scpi_ops members) is replied to as a response
packet reporting success or failure.


No that's not true, I have already mentioned that couple of times in the
other thread. It's just wrong comment here which went unnoticed from
day#1, sorry for that.


So the client should set 'knows_txdone' to be true unless it is told
the controller on that platform supports txdone_irq (what you call
'ack').



I got the concept but SCP can't ack via protocol, protocol has no such
provision and it sets flags in MHU status register.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Thu, Jul 23, 2015 at 4:40 PM, Sudeep Holla  wrote:

...

> +static int scpi_probe(struct platform_device *pdev)
> +{
> +   int count, idx, ret;
> +   struct resource res;
> +   struct scpi_chan *scpi_chan;
> +   struct device *dev = >dev;
> +   struct device_node *np = dev->of_node;
> +
> +   scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
> +   if (!scpi_info)
> +   return -ENOMEM;
> +
> +   count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
> +   if (count < 0) {
> +   dev_err(dev, "no mboxes property in '%s'\n", np->full_name);
> +   return -ENODEV;
> +   }
> +
> +   scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan), GFP_KERNEL);
> +   if (!scpi_chan)
> +   return -ENOMEM;
> +
> +   for (idx = 0; idx < count; idx++) {
> +   resource_size_t size;
> +   struct scpi_chan *pchan = scpi_chan + idx;
> +   struct mbox_client *cl = >cl;
> +   struct device_node *shmem = of_parse_phandle(np, "shmem", 
> idx);
> +
> +   if (of_address_to_resource(shmem, 0, )) {
> +   dev_err(dev, "failed to get SCPI payload mem 
> resource\n");
> +   ret = -EINVAL;
> +   goto err;
> +   }
> +
> +   size = resource_size();
> +   pchan->rx_payload = devm_ioremap(dev, res.start, size);
> +   if (!pchan->rx_payload) {
> +   dev_err(dev, "failed to ioremap SCPI payload\n");
> +   ret = -EADDRNOTAVAIL;
> +   goto err;
> +   }
> +   pchan->tx_payload = pchan->rx_payload + (size >> 1);
> +
> +   cl->dev = dev;
> +   cl->rx_callback = scpi_handle_remote_msg;
> +   cl->tx_prepare = scpi_tx_prepare;
> +   cl->tx_block = true;
> +   cl->tx_tout = 50;
> +   cl->knows_txdone = false; /* controller can ack */
>
 This is the cause of your problems that you think should be solved by
using hrtimer.

 Controller may or may not (like MHU) set txdone_irq. However every
scpi command (struct scpi_ops members) is replied to as a response
packet reporting success or failure.
So the client should set 'knows_txdone' to be true unless it is told
the controller on that platform supports txdone_irq (what you call
'ack').

-Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Thu, Jul 23, 2015 at 4:40 PM, Sudeep Holla sudeep.ho...@arm.com wrote:

...

 +static int scpi_probe(struct platform_device *pdev)
 +{
 +   int count, idx, ret;
 +   struct resource res;
 +   struct scpi_chan *scpi_chan;
 +   struct device *dev = pdev-dev;
 +   struct device_node *np = dev-of_node;
 +
 +   scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
 +   if (!scpi_info)
 +   return -ENOMEM;
 +
 +   count = of_count_phandle_with_args(np, mboxes, #mbox-cells);
 +   if (count  0) {
 +   dev_err(dev, no mboxes property in '%s'\n, np-full_name);
 +   return -ENODEV;
 +   }
 +
 +   scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan), GFP_KERNEL);
 +   if (!scpi_chan)
 +   return -ENOMEM;
 +
 +   for (idx = 0; idx  count; idx++) {
 +   resource_size_t size;
 +   struct scpi_chan *pchan = scpi_chan + idx;
 +   struct mbox_client *cl = pchan-cl;
 +   struct device_node *shmem = of_parse_phandle(np, shmem, 
 idx);
 +
 +   if (of_address_to_resource(shmem, 0, res)) {
 +   dev_err(dev, failed to get SCPI payload mem 
 resource\n);
 +   ret = -EINVAL;
 +   goto err;
 +   }
 +
 +   size = resource_size(res);
 +   pchan-rx_payload = devm_ioremap(dev, res.start, size);
 +   if (!pchan-rx_payload) {
 +   dev_err(dev, failed to ioremap SCPI payload\n);
 +   ret = -EADDRNOTAVAIL;
 +   goto err;
 +   }
 +   pchan-tx_payload = pchan-rx_payload + (size  1);
 +
 +   cl-dev = dev;
 +   cl-rx_callback = scpi_handle_remote_msg;
 +   cl-tx_prepare = scpi_tx_prepare;
 +   cl-tx_block = true;
 +   cl-tx_tout = 50;
 +   cl-knows_txdone = false; /* controller can ack */

 This is the cause of your problems that you think should be solved by
using hrtimer.

 Controller may or may not (like MHU) set txdone_irq. However every
scpi command (struct scpi_ops members) is replied to as a response
packet reporting success or failure.
So the client should set 'knows_txdone' to be true unless it is told
the controller on that platform supports txdone_irq (what you call
'ack').

-Jassi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 29/07/15 09:05, Jassi Brar wrote:

On Thu, Jul 23, 2015 at 4:40 PM, Sudeep Holla sudeep.ho...@arm.com wrote:

...


+static int scpi_probe(struct platform_device *pdev)
+{
+   int count, idx, ret;
+   struct resource res;
+   struct scpi_chan *scpi_chan;
+   struct device *dev = pdev-dev;
+   struct device_node *np = dev-of_node;
+
+   scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
+   if (!scpi_info)
+   return -ENOMEM;
+
+   count = of_count_phandle_with_args(np, mboxes, #mbox-cells);
+   if (count  0) {
+   dev_err(dev, no mboxes property in '%s'\n, np-full_name);
+   return -ENODEV;
+   }
+
+   scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan), GFP_KERNEL);
+   if (!scpi_chan)
+   return -ENOMEM;
+
+   for (idx = 0; idx  count; idx++) {
+   resource_size_t size;
+   struct scpi_chan *pchan = scpi_chan + idx;
+   struct mbox_client *cl = pchan-cl;
+   struct device_node *shmem = of_parse_phandle(np, shmem, idx);
+
+   if (of_address_to_resource(shmem, 0, res)) {
+   dev_err(dev, failed to get SCPI payload mem 
resource\n);
+   ret = -EINVAL;
+   goto err;
+   }
+
+   size = resource_size(res);
+   pchan-rx_payload = devm_ioremap(dev, res.start, size);
+   if (!pchan-rx_payload) {
+   dev_err(dev, failed to ioremap SCPI payload\n);
+   ret = -EADDRNOTAVAIL;
+   goto err;
+   }
+   pchan-tx_payload = pchan-rx_payload + (size  1);
+
+   cl-dev = dev;
+   cl-rx_callback = scpi_handle_remote_msg;
+   cl-tx_prepare = scpi_tx_prepare;
+   cl-tx_block = true;
+   cl-tx_tout = 50;
+   cl-knows_txdone = false; /* controller can ack */


  This is the cause of your problems that you think should be solved by
using hrtimer.



Ah sorry, it's stupid mistake on my part while writing the comment. It
should have been controller can't ack, fixed locally now thanks for
pointing it out.


  Controller may or may not (like MHU) set txdone_irq. However every
scpi command (struct scpi_ops members) is replied to as a response
packet reporting success or failure.


No that's not true, I have already mentioned that couple of times in the
other thread. It's just wrong comment here which went unnoticed from
day#1, sorry for that.


So the client should set 'knows_txdone' to be true unless it is told
the controller on that platform supports txdone_irq (what you call
'ack').



I got the concept but SCP can't ack via protocol, protocol has no such
provision and it sets flags in MHU status register.

Regards,
Sudeep
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Wed, Jul 29, 2015 at 2:08 PM, Sudeep Holla sudeep.ho...@arm.com wrote:
 On 29/07/15 09:05, Jassi Brar wrote:


 +static int scpi_probe(struct platform_device *pdev)
 +{
 +   int count, idx, ret;
 +   struct resource res;
 +   struct scpi_chan *scpi_chan;
 +   struct device *dev = pdev-dev;
 +   struct device_node *np = dev-of_node;
 +
 +   scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
 +   if (!scpi_info)
 +   return -ENOMEM;
 +
 +   count = of_count_phandle_with_args(np, mboxes, #mbox-cells);
 +   if (count  0) {
 +   dev_err(dev, no mboxes property in '%s'\n,
 np-full_name);
 +   return -ENODEV;
 +   }
 +
 +   scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan),
 GFP_KERNEL);
 +   if (!scpi_chan)
 +   return -ENOMEM;
 +
 +   for (idx = 0; idx  count; idx++) {
 +   resource_size_t size;
 +   struct scpi_chan *pchan = scpi_chan + idx;
 +   struct mbox_client *cl = pchan-cl;
 +   struct device_node *shmem = of_parse_phandle(np, shmem,
 idx);
 +
 +   if (of_address_to_resource(shmem, 0, res)) {
 +   dev_err(dev, failed to get SCPI payload mem
 resource\n);
 +   ret = -EINVAL;
 +   goto err;
 +   }
 +
 +   size = resource_size(res);
 +   pchan-rx_payload = devm_ioremap(dev, res.start, size);
 +   if (!pchan-rx_payload) {
 +   dev_err(dev, failed to ioremap SCPI payload\n);
 +   ret = -EADDRNOTAVAIL;
 +   goto err;
 +   }
 +   pchan-tx_payload = pchan-rx_payload + (size  1);
 +
 +   cl-dev = dev;
 +   cl-rx_callback = scpi_handle_remote_msg;
 +   cl-tx_prepare = scpi_tx_prepare;
 +   cl-tx_block = true;
 +   cl-tx_tout = 50;
 +   cl-knows_txdone = false; /* controller can ack */

   This is the cause of your problems that you think should be solved by
 using hrtimer.


 Ah sorry, it's stupid mistake on my part while writing the comment. It
 should have been controller can't ack, fixed locally now thanks for
 pointing it out.

No, I am talking about code, not the comment.

   Controller may or may not (like MHU) set txdone_irq. However every
 scpi command (struct scpi_ops members) is replied to as a response
 packet reporting success or failure.


 No that's not true, I have already mentioned that couple of times in the
 other thread. It's just wrong comment here which went unnoticed from
 day#1, sorry for that.

 So the client should set 'knows_txdone' to be true unless it is told
 the controller on that platform supports txdone_irq (what you call
 'ack').

 I got the concept but SCP can't ack via protocol, protocol has no such
 provision and it sets flags in MHU status register.

You either don't get the concept of TXDONE_BY_ACK  or deliberately
overlook my point.

Assuming the former, let me explain. When a client receives a
response, it can be sure that the request has already been read by the
remote. If the protocol specifies every request has some response, the
client should assert 'knows_txdone' and call mbox_client_txdone() upon
receiving a reply packet.
   So I said,  cl-knows_txdone = false;   is the root of problems you
report. If you fix that, the performance should be even better than
using hrtimer.

-Jassi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 29/07/15 12:19, Jassi Brar wrote:

On Wed, Jul 29, 2015 at 2:08 PM, Sudeep Holla sudeep.ho...@arm.com wrote:

On 29/07/15 09:05, Jassi Brar wrote:




+static int scpi_probe(struct platform_device *pdev)
+{
+   int count, idx, ret;
+   struct resource res;
+   struct scpi_chan *scpi_chan;
+   struct device *dev = pdev-dev;
+   struct device_node *np = dev-of_node;
+
+   scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
+   if (!scpi_info)
+   return -ENOMEM;
+
+   count = of_count_phandle_with_args(np, mboxes, #mbox-cells);
+   if (count  0) {
+   dev_err(dev, no mboxes property in '%s'\n,
np-full_name);
+   return -ENODEV;
+   }
+
+   scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan),
GFP_KERNEL);
+   if (!scpi_chan)
+   return -ENOMEM;
+
+   for (idx = 0; idx  count; idx++) {
+   resource_size_t size;
+   struct scpi_chan *pchan = scpi_chan + idx;
+   struct mbox_client *cl = pchan-cl;
+   struct device_node *shmem = of_parse_phandle(np, shmem,
idx);
+
+   if (of_address_to_resource(shmem, 0, res)) {
+   dev_err(dev, failed to get SCPI payload mem
resource\n);
+   ret = -EINVAL;
+   goto err;
+   }
+
+   size = resource_size(res);
+   pchan-rx_payload = devm_ioremap(dev, res.start, size);
+   if (!pchan-rx_payload) {
+   dev_err(dev, failed to ioremap SCPI payload\n);
+   ret = -EADDRNOTAVAIL;
+   goto err;
+   }
+   pchan-tx_payload = pchan-rx_payload + (size  1);
+
+   cl-dev = dev;
+   cl-rx_callback = scpi_handle_remote_msg;
+   cl-tx_prepare = scpi_tx_prepare;
+   cl-tx_block = true;
+   cl-tx_tout = 50;
+   cl-knows_txdone = false; /* controller can ack */


   This is the cause of your problems that you think should be solved by
using hrtimer.



Ah sorry, it's stupid mistake on my part while writing the comment. It
should have been controller can't ack, fixed locally now thanks for
pointing it out.


No, I am talking about code, not the comment.


   Controller may or may not (like MHU) set txdone_irq. However every
scpi command (struct scpi_ops members) is replied to as a response
packet reporting success or failure.



No that's not true, I have already mentioned that couple of times in the
other thread. It's just wrong comment here which went unnoticed from
day#1, sorry for that.


So the client should set 'knows_txdone' to be true unless it is told
the controller on that platform supports txdone_irq (what you call
'ack').


I got the concept but SCP can't ack via protocol, protocol has no such
provision and it sets flags in MHU status register.


You either don't get the concept of TXDONE_BY_ACK  or deliberately
overlook my point.



No I do understand the concept and didn't overlook the points you made.


Assuming the former, let me explain. When a client receives a
response, it can be sure that the request has already been read by the
remote.


Waiting for the response would be too late for few expensive commands
(e.g setting up external regulators). The remote firmware acknowledges
Tx by setting status flags and will be ready to accept new commands.


If the protocol specifies every request has some response, the


Not always true there can be few commands without response. The protocol
specifies that we need check the status flag before sending the new
command as it's bidirectional, hence polling is recommended (Section 2.2
Communication flow in the SCPI specification)


client should assert 'knows_txdone' and call mbox_client_txdone() upon
receiving a reply packet.


Since this is not always true and not recommended in the specification,
I am hesitant to use this option as the firmware can always change their
internal mechanics without breaking the protocol. We need to ensure we 
are compliant to the spec.



So I said,  cl-knows_txdone = false;   is the root of problems you


It could be and won't rule that out. I would prefer using knows_txdone
and use mbox_client_txdone if feasible, but I can't as the without
violating the specification.

FYI, I had tried it and ended up with issues in the firmware. The
argument from the firmware is that we aren't specification compliant,
so I had to use polling.


report. If you fix that, the performance should be even better than
using hrtimer.



That would have been ideal and much better but I can't use that for
above mentioned reason.

Though you had valid concerns here and I hope I clarified them all
sucessfully, none were related to hrtimers.

Can I assume you are fine with hrtimers ? If so, can you review this patch ?

Regards,
Sudeep
--
To unsubscribe from this list: send the line unsubscribe 

[PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

This patch adds support for System Control and Power Interface (SCPI)
Message Protocol used between the Application Cores(AP) and the System
Control Processor(SCP). The MHU peripheral provides a mechanism for
inter-processor communication between SCP's M3 processor and AP.

SCP offers control and management of the core/cluster power states,
various power domain DVFS including the core/cluster, certain system
clocks configuration, thermal sensors and many others.

This protocol driver provides interface for all the client drivers using
SCPI to make use of the features offered by the SCP.

Signed-off-by: Sudeep Holla 
Reviewed-by: Jon Medhurst (Tixy) 
Cc: Jassi Brar 
Cc: Liviu Dudau 
Cc: Lorenzo Pieralisi 
---
 MAINTAINERS   |   2 +
 drivers/firmware/Kconfig  |  19 ++
 drivers/firmware/Makefile |   1 +
 drivers/firmware/arm_scpi.c   | 711 ++
 include/linux/scpi_protocol.h |  61 
 5 files changed, 794 insertions(+)
 create mode 100644 drivers/firmware/arm_scpi.c
 create mode 100644 include/linux/scpi_protocol.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 9351b62dbbd7..be511dd69ae4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8960,6 +8960,8 @@ M:Sudeep Holla 
 L: linux-arm-ker...@lists.infradead.org
 S: Maintained
 F: Documentation/devicetree/bindings/arm/arm,scpi.txt
+F: drivers/firmware/arm_scpi.c
+F: include/linux/scpi_protocol.h
 
 SCSI CDROM DRIVER
 M: Jens Axboe 
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 99c69a3205c4..fc40751a1a1f 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -136,6 +136,25 @@ config QCOM_SCM
bool
depends on ARM || ARM64
 
+config ARM_SCPI_PROTOCOL
+   tristate "ARM System Control and Power Interface (SCPI) Message 
Protocol"
+   depends on ARM_MHU
+   help
+ System Control and Power Interface (SCPI) Message Protocol is
+ defined for the purpose of communication between the Application
+ Cores(AP) and the System Control Processor(SCP). The MHU peripheral
+ provides a mechanism for inter-processor communication between SCP
+ and AP.
+
+ SCP controls most of the power managament on the Application
+ Processors. It offers control and management of: the core/cluster
+ power states, various power domain DVFS including the core/cluster,
+ certain system clocks configuration, thermal sensors and many
+ others.
+
+ This protocol library provides interface for all the client drivers
+ making use of the features offered by the SCP.
+
 source "drivers/firmware/broadcom/Kconfig"
 source "drivers/firmware/google/Kconfig"
 source "drivers/firmware/efi/Kconfig"
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 4a4b897f9314..85f1032c8898 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -1,6 +1,7 @@
 #
 # Makefile for the linux kernel.
 #
+obj-$(CONFIG_ARM_SCPI_PROTOCOL)+= arm_scpi.o
 obj-$(CONFIG_DMI)  += dmi_scan.o
 obj-$(CONFIG_DMI_SYSFS)+= dmi-sysfs.o
 obj-$(CONFIG_EDD)  += edd.o
diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
new file mode 100644
index ..5e3898f24b96
--- /dev/null
+++ b/drivers/firmware/arm_scpi.c
@@ -0,0 +1,711 @@
+/*
+ * System Control and Power Interface (SCPI) Message Protocol driver
+ *
+ * SCPI Message Protocol is used between the System Control Processor(SCP)
+ * and the Application Processors(AP). The Message Handling Unit(MHU)
+ * provides a mechanism for inter-processor communication between SCP's
+ * Cortex M3 and AP.
+ *
+ * SCP offers control and management of the core/cluster power states,
+ * various power domain DVFS including the core/cluster, certain system
+ * clocks configuration, thermal sensors and many others.
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program. If not, see .
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define CMD_ID_SHIFT   0
+#define CMD_ID_MASK0x7f
+#define CMD_TOKEN_ID_SHIFT 8
+#define CMD_TOKEN_ID_MASK  0xff
+#define CMD_DATA_SIZE_SHIFT16
+#define 

[PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

This patch adds support for System Control and Power Interface (SCPI)
Message Protocol used between the Application Cores(AP) and the System
Control Processor(SCP). The MHU peripheral provides a mechanism for
inter-processor communication between SCP's M3 processor and AP.

SCP offers control and management of the core/cluster power states,
various power domain DVFS including the core/cluster, certain system
clocks configuration, thermal sensors and many others.

This protocol driver provides interface for all the client drivers using
SCPI to make use of the features offered by the SCP.

Signed-off-by: Sudeep Holla sudeep.ho...@arm.com
Reviewed-by: Jon Medhurst (Tixy) t...@linaro.org
Cc: Jassi Brar jassisinghb...@gmail.com
Cc: Liviu Dudau liviu.du...@arm.com
Cc: Lorenzo Pieralisi lorenzo.pieral...@arm.com
---
 MAINTAINERS   |   2 +
 drivers/firmware/Kconfig  |  19 ++
 drivers/firmware/Makefile |   1 +
 drivers/firmware/arm_scpi.c   | 711 ++
 include/linux/scpi_protocol.h |  61 
 5 files changed, 794 insertions(+)
 create mode 100644 drivers/firmware/arm_scpi.c
 create mode 100644 include/linux/scpi_protocol.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 9351b62dbbd7..be511dd69ae4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8960,6 +8960,8 @@ M:Sudeep Holla sudeep.ho...@arm.com
 L: linux-arm-ker...@lists.infradead.org
 S: Maintained
 F: Documentation/devicetree/bindings/arm/arm,scpi.txt
+F: drivers/firmware/arm_scpi.c
+F: include/linux/scpi_protocol.h
 
 SCSI CDROM DRIVER
 M: Jens Axboe ax...@kernel.dk
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 99c69a3205c4..fc40751a1a1f 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -136,6 +136,25 @@ config QCOM_SCM
bool
depends on ARM || ARM64
 
+config ARM_SCPI_PROTOCOL
+   tristate ARM System Control and Power Interface (SCPI) Message 
Protocol
+   depends on ARM_MHU
+   help
+ System Control and Power Interface (SCPI) Message Protocol is
+ defined for the purpose of communication between the Application
+ Cores(AP) and the System Control Processor(SCP). The MHU peripheral
+ provides a mechanism for inter-processor communication between SCP
+ and AP.
+
+ SCP controls most of the power managament on the Application
+ Processors. It offers control and management of: the core/cluster
+ power states, various power domain DVFS including the core/cluster,
+ certain system clocks configuration, thermal sensors and many
+ others.
+
+ This protocol library provides interface for all the client drivers
+ making use of the features offered by the SCP.
+
 source drivers/firmware/broadcom/Kconfig
 source drivers/firmware/google/Kconfig
 source drivers/firmware/efi/Kconfig
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 4a4b897f9314..85f1032c8898 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -1,6 +1,7 @@
 #
 # Makefile for the linux kernel.
 #
+obj-$(CONFIG_ARM_SCPI_PROTOCOL)+= arm_scpi.o
 obj-$(CONFIG_DMI)  += dmi_scan.o
 obj-$(CONFIG_DMI_SYSFS)+= dmi-sysfs.o
 obj-$(CONFIG_EDD)  += edd.o
diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
new file mode 100644
index ..5e3898f24b96
--- /dev/null
+++ b/drivers/firmware/arm_scpi.c
@@ -0,0 +1,711 @@
+/*
+ * System Control and Power Interface (SCPI) Message Protocol driver
+ *
+ * SCPI Message Protocol is used between the System Control Processor(SCP)
+ * and the Application Processors(AP). The Message Handling Unit(MHU)
+ * provides a mechanism for inter-processor communication between SCP's
+ * Cortex M3 and AP.
+ *
+ * SCP offers control and management of the core/cluster power states,
+ * various power domain DVFS including the core/cluster, certain system
+ * clocks configuration, thermal sensors and many others.
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program. If not, see http://www.gnu.org/licenses/.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
+
+#include linux/bitmap.h
+#include linux/device.h
+#include linux/err.h
+#include linux/export.h
+#include linux/io.h
+#include linux/kernel.h
+#include linux/list.h
+#include linux/mailbox_client.h
+#include linux/module.h
+#include