Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

On 08/31/18 13:55, Bjorn Andersson wrote:
> On Fri 31 Aug 13:41 PDT 2018, Frank Rowand wrote:
> 
>> On 08/30/18 21:07, Bjorn Andersson wrote:
>>> On Thu 30 Aug 20:57 PDT 2018, Frank Rowand wrote:
>>>
 Hi Bjorn,


 On 04/19/18 18:17, Bjorn Andersson wrote:
> Attempt to acquire the APCS IPC through the mailbox framework and fall
> back to the old syscon based approach, to allow us to move away from
> using the syscon.
>
> Reviewed-by: Arun Kumar Neelakantam 
> Signed-off-by: Bjorn Andersson 
> ---
>
> Changes since v2:
> - Added comment about mbox_send_message() return value.
>
>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>  drivers/rpmsg/Kconfig  |  1 +
>  drivers/rpmsg/qcom_smd.c   | 67 
> --
>  3 files changed, 56 insertions(+), 20 deletions(-)

 This patch in the mainline Linux kernel as commit 
 ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
 causes a problem with the APQ8074 Dragonboard.  The mmc device is not set 
 up
 with the patch applied, thus I do not have the block device my root file 
 system
 is located on.

 Testing on v4.18, if I revert this commit the mmc device is available.

 I'll reply to this email with the console messages for 4.18 and for 4.18 
 with
 this commit reverted.

>>>
>>> The mmc device would fail to come up if the regulators didn't come up,
>>> which would be the result of smd not working. But it should fallback to
>>> the old mechanism if no mailbox is specified.
>>>
>>> Can you double check that CONFIG_RPMSG_QCOM_SMD is still set in your
>>> .config after applying and building with this commit included? And if
>>> not, try to enable CONFIG_MAILBOX.
>>
>> Thank you!
>>
>> That is indeed the cause.  ab460a2e72da added a "depends on MAILBOX" to
>> CONFIG_RPMSG_QCOM_SMD, so CONFIG_RPMSG_QCOM_SMD becomes unset since
>> CONFIG_MAILBOX is not enabled in qcom_defconfig and is not otherwise
>> selected for the dragonboard.
>>
> 
> Thanks for verifying this!
> 
>> Is there a config variable that should be selecting MAILBOX for a class
>> of systems that would include the APQ8074 Dragonboard?  For my testing
>> I added the "select MAILBOX" to CONFIG_ARCH_MSM8974, but I do not know
>> what systems that includes, and whether it is appropriate to do the
>> select for all of them.
>>
> 
> We typically don't use the CONFIG_ARCH_* to enable user selectable
> config options, even if they serve a critical role in the system.
> 
> So minimum change would be to add CONFIG_MAILBOX to the qcom_defconfig
> (and multi_v7_defconfig I presume). The fuller solution would be to add
> qcom,msm8974-apcs-kpss-global (.data is 8) to the qcom-apcs-ipc-mailbox
> list of compatibles and replace the syscon currently used, and then
> enable these in the defconfigs.

I would be happy with either solution.  Can I twist your arm to create
the patch?


> 
> Regards,
> Bjorn
> 

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

On 08/31/18 13:55, Bjorn Andersson wrote:
> On Fri 31 Aug 13:41 PDT 2018, Frank Rowand wrote:
> 
>> On 08/30/18 21:07, Bjorn Andersson wrote:
>>> On Thu 30 Aug 20:57 PDT 2018, Frank Rowand wrote:
>>>
 Hi Bjorn,


 On 04/19/18 18:17, Bjorn Andersson wrote:
> Attempt to acquire the APCS IPC through the mailbox framework and fall
> back to the old syscon based approach, to allow us to move away from
> using the syscon.
>
> Reviewed-by: Arun Kumar Neelakantam 
> Signed-off-by: Bjorn Andersson 
> ---
>
> Changes since v2:
> - Added comment about mbox_send_message() return value.
>
>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>  drivers/rpmsg/Kconfig  |  1 +
>  drivers/rpmsg/qcom_smd.c   | 67 
> --
>  3 files changed, 56 insertions(+), 20 deletions(-)

 This patch in the mainline Linux kernel as commit 
 ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
 causes a problem with the APQ8074 Dragonboard.  The mmc device is not set 
 up
 with the patch applied, thus I do not have the block device my root file 
 system
 is located on.

 Testing on v4.18, if I revert this commit the mmc device is available.

 I'll reply to this email with the console messages for 4.18 and for 4.18 
 with
 this commit reverted.

>>>
>>> The mmc device would fail to come up if the regulators didn't come up,
>>> which would be the result of smd not working. But it should fallback to
>>> the old mechanism if no mailbox is specified.
>>>
>>> Can you double check that CONFIG_RPMSG_QCOM_SMD is still set in your
>>> .config after applying and building with this commit included? And if
>>> not, try to enable CONFIG_MAILBOX.
>>
>> Thank you!
>>
>> That is indeed the cause.  ab460a2e72da added a "depends on MAILBOX" to
>> CONFIG_RPMSG_QCOM_SMD, so CONFIG_RPMSG_QCOM_SMD becomes unset since
>> CONFIG_MAILBOX is not enabled in qcom_defconfig and is not otherwise
>> selected for the dragonboard.
>>
> 
> Thanks for verifying this!
> 
>> Is there a config variable that should be selecting MAILBOX for a class
>> of systems that would include the APQ8074 Dragonboard?  For my testing
>> I added the "select MAILBOX" to CONFIG_ARCH_MSM8974, but I do not know
>> what systems that includes, and whether it is appropriate to do the
>> select for all of them.
>>
> 
> We typically don't use the CONFIG_ARCH_* to enable user selectable
> config options, even if they serve a critical role in the system.
> 
> So minimum change would be to add CONFIG_MAILBOX to the qcom_defconfig
> (and multi_v7_defconfig I presume). The fuller solution would be to add
> qcom,msm8974-apcs-kpss-global (.data is 8) to the qcom-apcs-ipc-mailbox
> list of compatibles and replace the syscon currently used, and then
> enable these in the defconfigs.

I would be happy with either solution.  Can I twist your arm to create
the patch?


> 
> Regards,
> Bjorn
> 

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Bjorn Andersson]

On Fri 31 Aug 13:41 PDT 2018, Frank Rowand wrote:

> On 08/30/18 21:07, Bjorn Andersson wrote:
> > On Thu 30 Aug 20:57 PDT 2018, Frank Rowand wrote:
> > 
> >> Hi Bjorn,
> >>
> >>
> >> On 04/19/18 18:17, Bjorn Andersson wrote:
> >>> Attempt to acquire the APCS IPC through the mailbox framework and fall
> >>> back to the old syscon based approach, to allow us to move away from
> >>> using the syscon.
> >>>
> >>> Reviewed-by: Arun Kumar Neelakantam 
> >>> Signed-off-by: Bjorn Andersson 
> >>> ---
> >>>
> >>> Changes since v2:
> >>> - Added comment about mbox_send_message() return value.
> >>>
> >>>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
> >>>  drivers/rpmsg/Kconfig  |  1 +
> >>>  drivers/rpmsg/qcom_smd.c   | 67 
> >>> --
> >>>  3 files changed, 56 insertions(+), 20 deletions(-)
> >>
> >> This patch in the mainline Linux kernel as commit 
> >> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
> >> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set 
> >> up
> >> with the patch applied, thus I do not have the block device my root file 
> >> system
> >> is located on.
> >>
> >> Testing on v4.18, if I revert this commit the mmc device is available.
> >>
> >> I'll reply to this email with the console messages for 4.18 and for 4.18 
> >> with
> >> this commit reverted.
> >>
> > 
> > The mmc device would fail to come up if the regulators didn't come up,
> > which would be the result of smd not working. But it should fallback to
> > the old mechanism if no mailbox is specified.
> > 
> > Can you double check that CONFIG_RPMSG_QCOM_SMD is still set in your
> > .config after applying and building with this commit included? And if
> > not, try to enable CONFIG_MAILBOX.
> 
> Thank you!
> 
> That is indeed the cause.  ab460a2e72da added a "depends on MAILBOX" to
> CONFIG_RPMSG_QCOM_SMD, so CONFIG_RPMSG_QCOM_SMD becomes unset since
> CONFIG_MAILBOX is not enabled in qcom_defconfig and is not otherwise
> selected for the dragonboard.
> 

Thanks for verifying this!

> Is there a config variable that should be selecting MAILBOX for a class
> of systems that would include the APQ8074 Dragonboard?  For my testing
> I added the "select MAILBOX" to CONFIG_ARCH_MSM8974, but I do not know
> what systems that includes, and whether it is appropriate to do the
> select for all of them.
> 

We typically don't use the CONFIG_ARCH_* to enable user selectable
config options, even if they serve a critical role in the system.

So minimum change would be to add CONFIG_MAILBOX to the qcom_defconfig
(and multi_v7_defconfig I presume). The fuller solution would be to add
qcom,msm8974-apcs-kpss-global (.data is 8) to the qcom-apcs-ipc-mailbox
list of compatibles and replace the syscon currently used, and then
enable these in the defconfigs.

Regards,
Bjorn

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Bjorn Andersson]

On Fri 31 Aug 13:41 PDT 2018, Frank Rowand wrote:

> On 08/30/18 21:07, Bjorn Andersson wrote:
> > On Thu 30 Aug 20:57 PDT 2018, Frank Rowand wrote:
> > 
> >> Hi Bjorn,
> >>
> >>
> >> On 04/19/18 18:17, Bjorn Andersson wrote:
> >>> Attempt to acquire the APCS IPC through the mailbox framework and fall
> >>> back to the old syscon based approach, to allow us to move away from
> >>> using the syscon.
> >>>
> >>> Reviewed-by: Arun Kumar Neelakantam 
> >>> Signed-off-by: Bjorn Andersson 
> >>> ---
> >>>
> >>> Changes since v2:
> >>> - Added comment about mbox_send_message() return value.
> >>>
> >>>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
> >>>  drivers/rpmsg/Kconfig  |  1 +
> >>>  drivers/rpmsg/qcom_smd.c   | 67 
> >>> --
> >>>  3 files changed, 56 insertions(+), 20 deletions(-)
> >>
> >> This patch in the mainline Linux kernel as commit 
> >> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
> >> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set 
> >> up
> >> with the patch applied, thus I do not have the block device my root file 
> >> system
> >> is located on.
> >>
> >> Testing on v4.18, if I revert this commit the mmc device is available.
> >>
> >> I'll reply to this email with the console messages for 4.18 and for 4.18 
> >> with
> >> this commit reverted.
> >>
> > 
> > The mmc device would fail to come up if the regulators didn't come up,
> > which would be the result of smd not working. But it should fallback to
> > the old mechanism if no mailbox is specified.
> > 
> > Can you double check that CONFIG_RPMSG_QCOM_SMD is still set in your
> > .config after applying and building with this commit included? And if
> > not, try to enable CONFIG_MAILBOX.
> 
> Thank you!
> 
> That is indeed the cause.  ab460a2e72da added a "depends on MAILBOX" to
> CONFIG_RPMSG_QCOM_SMD, so CONFIG_RPMSG_QCOM_SMD becomes unset since
> CONFIG_MAILBOX is not enabled in qcom_defconfig and is not otherwise
> selected for the dragonboard.
> 

Thanks for verifying this!

> Is there a config variable that should be selecting MAILBOX for a class
> of systems that would include the APQ8074 Dragonboard?  For my testing
> I added the "select MAILBOX" to CONFIG_ARCH_MSM8974, but I do not know
> what systems that includes, and whether it is appropriate to do the
> select for all of them.
> 

We typically don't use the CONFIG_ARCH_* to enable user selectable
config options, even if they serve a critical role in the system.

So minimum change would be to add CONFIG_MAILBOX to the qcom_defconfig
(and multi_v7_defconfig I presume). The fuller solution would be to add
qcom,msm8974-apcs-kpss-global (.data is 8) to the qcom-apcs-ipc-mailbox
list of compatibles and replace the syscon currently used, and then
enable these in the defconfigs.

Regards,
Bjorn

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

On 08/30/18 21:07, Bjorn Andersson wrote:
> On Thu 30 Aug 20:57 PDT 2018, Frank Rowand wrote:
> 
>> Hi Bjorn,
>>
>>
>> On 04/19/18 18:17, Bjorn Andersson wrote:
>>> Attempt to acquire the APCS IPC through the mailbox framework and fall
>>> back to the old syscon based approach, to allow us to move away from
>>> using the syscon.
>>>
>>> Reviewed-by: Arun Kumar Neelakantam 
>>> Signed-off-by: Bjorn Andersson 
>>> ---
>>>
>>> Changes since v2:
>>> - Added comment about mbox_send_message() return value.
>>>
>>>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>>>  drivers/rpmsg/Kconfig  |  1 +
>>>  drivers/rpmsg/qcom_smd.c   | 67 
>>> --
>>>  3 files changed, 56 insertions(+), 20 deletions(-)
>>
>> This patch in the mainline Linux kernel as commit 
>> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
>> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
>> with the patch applied, thus I do not have the block device my root file 
>> system
>> is located on.
>>
>> Testing on v4.18, if I revert this commit the mmc device is available.
>>
>> I'll reply to this email with the console messages for 4.18 and for 4.18 with
>> this commit reverted.
>>
> 
> The mmc device would fail to come up if the regulators didn't come up,
> which would be the result of smd not working. But it should fallback to
> the old mechanism if no mailbox is specified.
> 
> Can you double check that CONFIG_RPMSG_QCOM_SMD is still set in your
> .config after applying and building with this commit included? And if
> not, try to enable CONFIG_MAILBOX.

Thank you!

That is indeed the cause.  ab460a2e72da added a "depends on MAILBOX" to
CONFIG_RPMSG_QCOM_SMD, so CONFIG_RPMSG_QCOM_SMD becomes unset since
CONFIG_MAILBOX is not enabled in qcom_defconfig and is not otherwise
selected for the dragonboard.

Is there a config variable that should be selecting MAILBOX for a class
of systems that would include the APQ8074 Dragonboard?  For my testing
I added the "select MAILBOX" to CONFIG_ARCH_MSM8974, but I do not know
what systems that includes, and whether it is appropriate to do the
select for all of them.

-Frank

> 
> Regards,
> Bjorn
> 
>> -Frank
>>
>>
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt 
>>> b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
>>> index ea1dc75ec9ea..234ae2256501 100644
>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
>>> @@ -22,9 +22,15 @@ The edge is described by the following properties:
>>> Definition: should specify the IRQ used by the remote processor to
>>> signal this processor about communication related updates
>>>  
>>> -- qcom,ipc:
>>> +- mboxes:
>>> Usage: required
>>> Value type: 
>>> +   Definition: reference to the associated doorbell in APCS, as described
>>> +   in mailbox/mailbox.txt
>>> +
>>> +- qcom,ipc:
>>> +   Usage: required, unless mboxes is specified
>>> +   Value type: 
>>> Definition: three entries specifying the outgoing ipc bit used for
>>> signaling the remote processor:
>>> - phandle to a syscon node representing the apcs registers
>>> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
>>> index 0fe6eac46512..2e4fb4ffd562 100644
>>> --- a/drivers/rpmsg/Kconfig
>>> +++ b/drivers/rpmsg/Kconfig
>>> @@ -39,6 +39,7 @@ config RPMSG_QCOM_GLINK_SMEM
>>>  
>>>  config RPMSG_QCOM_SMD
>>> tristate "Qualcomm Shared Memory Driver (SMD)"
>>> +   depends on MAILBOX
>>> depends on QCOM_SMEM
>>> select RPMSG
>>> help
>>> diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
>>> index bc0b30657230..3ff271a44bef 100644
>>> --- a/drivers/rpmsg/qcom_smd.c
>>> +++ b/drivers/rpmsg/qcom_smd.c
>>> @@ -14,6 +14,7 @@
>>>  
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -107,6 +108,8 @@ static const struct {
>>>   * @ipc_regmap:regmap handle holding the outgoing ipc register
>>>   * @ipc_offset:offset within @ipc_regmap of the register for 
>>> ipc
>>>   * @ipc_bit:   bit in the register at @ipc_offset of 
>>> @ipc_regmap
>>> + * @mbox_client:   mailbox client handle
>>> + * @mbox_chan: apcs ipc mailbox channel handle
>>>   * @channels:  list of all channels detected on this edge
>>>   * @channels_lock: guard for modifications of @channels
>>>   * @allocated: array of bitmaps representing already allocated 
>>> channels
>>> @@ -129,6 +132,9 @@ struct qcom_smd_edge {
>>> int ipc_offset;
>>> int ipc_bit;
>>>  
>>> +   struct mbox_client mbox_client;
>>> +   struct mbox_chan *mbox_chan;
>>> +
>>> struct list_head channels;
>>> spinlock_t channels_lock;
>>>  
>>> @@ -366,7 +372,17 @@ static void qcom_smd_signal_channel(struct 
>>> qcom_smd_channel *channel)
>>> 

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

On 08/30/18 21:07, Bjorn Andersson wrote:
> On Thu 30 Aug 20:57 PDT 2018, Frank Rowand wrote:
> 
>> Hi Bjorn,
>>
>>
>> On 04/19/18 18:17, Bjorn Andersson wrote:
>>> Attempt to acquire the APCS IPC through the mailbox framework and fall
>>> back to the old syscon based approach, to allow us to move away from
>>> using the syscon.
>>>
>>> Reviewed-by: Arun Kumar Neelakantam 
>>> Signed-off-by: Bjorn Andersson 
>>> ---
>>>
>>> Changes since v2:
>>> - Added comment about mbox_send_message() return value.
>>>
>>>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>>>  drivers/rpmsg/Kconfig  |  1 +
>>>  drivers/rpmsg/qcom_smd.c   | 67 
>>> --
>>>  3 files changed, 56 insertions(+), 20 deletions(-)
>>
>> This patch in the mainline Linux kernel as commit 
>> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
>> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
>> with the patch applied, thus I do not have the block device my root file 
>> system
>> is located on.
>>
>> Testing on v4.18, if I revert this commit the mmc device is available.
>>
>> I'll reply to this email with the console messages for 4.18 and for 4.18 with
>> this commit reverted.
>>
> 
> The mmc device would fail to come up if the regulators didn't come up,
> which would be the result of smd not working. But it should fallback to
> the old mechanism if no mailbox is specified.
> 
> Can you double check that CONFIG_RPMSG_QCOM_SMD is still set in your
> .config after applying and building with this commit included? And if
> not, try to enable CONFIG_MAILBOX.

Thank you!

That is indeed the cause.  ab460a2e72da added a "depends on MAILBOX" to
CONFIG_RPMSG_QCOM_SMD, so CONFIG_RPMSG_QCOM_SMD becomes unset since
CONFIG_MAILBOX is not enabled in qcom_defconfig and is not otherwise
selected for the dragonboard.

Is there a config variable that should be selecting MAILBOX for a class
of systems that would include the APQ8074 Dragonboard?  For my testing
I added the "select MAILBOX" to CONFIG_ARCH_MSM8974, but I do not know
what systems that includes, and whether it is appropriate to do the
select for all of them.

-Frank

> 
> Regards,
> Bjorn
> 
>> -Frank
>>
>>
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt 
>>> b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
>>> index ea1dc75ec9ea..234ae2256501 100644
>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
>>> @@ -22,9 +22,15 @@ The edge is described by the following properties:
>>> Definition: should specify the IRQ used by the remote processor to
>>> signal this processor about communication related updates
>>>  
>>> -- qcom,ipc:
>>> +- mboxes:
>>> Usage: required
>>> Value type: 
>>> +   Definition: reference to the associated doorbell in APCS, as described
>>> +   in mailbox/mailbox.txt
>>> +
>>> +- qcom,ipc:
>>> +   Usage: required, unless mboxes is specified
>>> +   Value type: 
>>> Definition: three entries specifying the outgoing ipc bit used for
>>> signaling the remote processor:
>>> - phandle to a syscon node representing the apcs registers
>>> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
>>> index 0fe6eac46512..2e4fb4ffd562 100644
>>> --- a/drivers/rpmsg/Kconfig
>>> +++ b/drivers/rpmsg/Kconfig
>>> @@ -39,6 +39,7 @@ config RPMSG_QCOM_GLINK_SMEM
>>>  
>>>  config RPMSG_QCOM_SMD
>>> tristate "Qualcomm Shared Memory Driver (SMD)"
>>> +   depends on MAILBOX
>>> depends on QCOM_SMEM
>>> select RPMSG
>>> help
>>> diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
>>> index bc0b30657230..3ff271a44bef 100644
>>> --- a/drivers/rpmsg/qcom_smd.c
>>> +++ b/drivers/rpmsg/qcom_smd.c
>>> @@ -14,6 +14,7 @@
>>>  
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -107,6 +108,8 @@ static const struct {
>>>   * @ipc_regmap:regmap handle holding the outgoing ipc register
>>>   * @ipc_offset:offset within @ipc_regmap of the register for 
>>> ipc
>>>   * @ipc_bit:   bit in the register at @ipc_offset of 
>>> @ipc_regmap
>>> + * @mbox_client:   mailbox client handle
>>> + * @mbox_chan: apcs ipc mailbox channel handle
>>>   * @channels:  list of all channels detected on this edge
>>>   * @channels_lock: guard for modifications of @channels
>>>   * @allocated: array of bitmaps representing already allocated 
>>> channels
>>> @@ -129,6 +132,9 @@ struct qcom_smd_edge {
>>> int ipc_offset;
>>> int ipc_bit;
>>>  
>>> +   struct mbox_client mbox_client;
>>> +   struct mbox_chan *mbox_chan;
>>> +
>>> struct list_head channels;
>>> spinlock_t channels_lock;
>>>  
>>> @@ -366,7 +372,17 @@ static void qcom_smd_signal_channel(struct 
>>> qcom_smd_channel *channel)
>>> 

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Bjorn Andersson]

On Thu 30 Aug 20:57 PDT 2018, Frank Rowand wrote:

> Hi Bjorn,
> 
> 
> On 04/19/18 18:17, Bjorn Andersson wrote:
> > Attempt to acquire the APCS IPC through the mailbox framework and fall
> > back to the old syscon based approach, to allow us to move away from
> > using the syscon.
> > 
> > Reviewed-by: Arun Kumar Neelakantam 
> > Signed-off-by: Bjorn Andersson 
> > ---
> > 
> > Changes since v2:
> > - Added comment about mbox_send_message() return value.
> > 
> >  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
> >  drivers/rpmsg/Kconfig  |  1 +
> >  drivers/rpmsg/qcom_smd.c   | 67 
> > --
> >  3 files changed, 56 insertions(+), 20 deletions(-)
> 
> This patch in the mainline Linux kernel as commit 
> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
> with the patch applied, thus I do not have the block device my root file 
> system
> is located on.
> 
> Testing on v4.18, if I revert this commit the mmc device is available.
> 
> I'll reply to this email with the console messages for 4.18 and for 4.18 with
> this commit reverted.
> 

The mmc device would fail to come up if the regulators didn't come up,
which would be the result of smd not working. But it should fallback to
the old mechanism if no mailbox is specified.

Can you double check that CONFIG_RPMSG_QCOM_SMD is still set in your
.config after applying and building with this commit included? And if
not, try to enable CONFIG_MAILBOX.

Regards,
Bjorn

> -Frank
> 
> 
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt 
> > b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> > index ea1dc75ec9ea..234ae2256501 100644
> > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> > @@ -22,9 +22,15 @@ The edge is described by the following properties:
> > Definition: should specify the IRQ used by the remote processor to
> > signal this processor about communication related updates
> >  
> > -- qcom,ipc:
> > +- mboxes:
> > Usage: required
> > Value type: 
> > +   Definition: reference to the associated doorbell in APCS, as described
> > +   in mailbox/mailbox.txt
> > +
> > +- qcom,ipc:
> > +   Usage: required, unless mboxes is specified
> > +   Value type: 
> > Definition: three entries specifying the outgoing ipc bit used for
> > signaling the remote processor:
> > - phandle to a syscon node representing the apcs registers
> > diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
> > index 0fe6eac46512..2e4fb4ffd562 100644
> > --- a/drivers/rpmsg/Kconfig
> > +++ b/drivers/rpmsg/Kconfig
> > @@ -39,6 +39,7 @@ config RPMSG_QCOM_GLINK_SMEM
> >  
> >  config RPMSG_QCOM_SMD
> > tristate "Qualcomm Shared Memory Driver (SMD)"
> > +   depends on MAILBOX
> > depends on QCOM_SMEM
> > select RPMSG
> > help
> > diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
> > index bc0b30657230..3ff271a44bef 100644
> > --- a/drivers/rpmsg/qcom_smd.c
> > +++ b/drivers/rpmsg/qcom_smd.c
> > @@ -14,6 +14,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -107,6 +108,8 @@ static const struct {
> >   * @ipc_regmap:regmap handle holding the outgoing ipc register
> >   * @ipc_offset:offset within @ipc_regmap of the register for 
> > ipc
> >   * @ipc_bit:   bit in the register at @ipc_offset of 
> > @ipc_regmap
> > + * @mbox_client:   mailbox client handle
> > + * @mbox_chan: apcs ipc mailbox channel handle
> >   * @channels:  list of all channels detected on this edge
> >   * @channels_lock: guard for modifications of @channels
> >   * @allocated: array of bitmaps representing already allocated 
> > channels
> > @@ -129,6 +132,9 @@ struct qcom_smd_edge {
> > int ipc_offset;
> > int ipc_bit;
> >  
> > +   struct mbox_client mbox_client;
> > +   struct mbox_chan *mbox_chan;
> > +
> > struct list_head channels;
> > spinlock_t channels_lock;
> >  
> > @@ -366,7 +372,17 @@ static void qcom_smd_signal_channel(struct 
> > qcom_smd_channel *channel)
> >  {
> > struct qcom_smd_edge *edge = channel->edge;
> >  
> > -   regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));
> > +   if (edge->mbox_chan) {
> > +   /*
> > +* We can ignore a failing mbox_send_message() as the only
> > +* possible cause is that the FIFO in the framework is full of
> > +* other writes to the same bit.
> > +*/
> > +   mbox_send_message(edge->mbox_chan, NULL);
> > +   mbox_client_txdone(edge->mbox_chan, 0);
> > +   } else {
> > +   regmap_write(edge->ipc_regmap, edge->ipc_offset, 
> > BIT(edge->ipc_bit));
> > +   }
> 

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Bjorn Andersson]

On Thu 30 Aug 20:57 PDT 2018, Frank Rowand wrote:

> Hi Bjorn,
> 
> 
> On 04/19/18 18:17, Bjorn Andersson wrote:
> > Attempt to acquire the APCS IPC through the mailbox framework and fall
> > back to the old syscon based approach, to allow us to move away from
> > using the syscon.
> > 
> > Reviewed-by: Arun Kumar Neelakantam 
> > Signed-off-by: Bjorn Andersson 
> > ---
> > 
> > Changes since v2:
> > - Added comment about mbox_send_message() return value.
> > 
> >  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
> >  drivers/rpmsg/Kconfig  |  1 +
> >  drivers/rpmsg/qcom_smd.c   | 67 
> > --
> >  3 files changed, 56 insertions(+), 20 deletions(-)
> 
> This patch in the mainline Linux kernel as commit 
> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
> with the patch applied, thus I do not have the block device my root file 
> system
> is located on.
> 
> Testing on v4.18, if I revert this commit the mmc device is available.
> 
> I'll reply to this email with the console messages for 4.18 and for 4.18 with
> this commit reverted.
> 

The mmc device would fail to come up if the regulators didn't come up,
which would be the result of smd not working. But it should fallback to
the old mechanism if no mailbox is specified.

Can you double check that CONFIG_RPMSG_QCOM_SMD is still set in your
.config after applying and building with this commit included? And if
not, try to enable CONFIG_MAILBOX.

Regards,
Bjorn

> -Frank
> 
> 
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt 
> > b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> > index ea1dc75ec9ea..234ae2256501 100644
> > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> > @@ -22,9 +22,15 @@ The edge is described by the following properties:
> > Definition: should specify the IRQ used by the remote processor to
> > signal this processor about communication related updates
> >  
> > -- qcom,ipc:
> > +- mboxes:
> > Usage: required
> > Value type: 
> > +   Definition: reference to the associated doorbell in APCS, as described
> > +   in mailbox/mailbox.txt
> > +
> > +- qcom,ipc:
> > +   Usage: required, unless mboxes is specified
> > +   Value type: 
> > Definition: three entries specifying the outgoing ipc bit used for
> > signaling the remote processor:
> > - phandle to a syscon node representing the apcs registers
> > diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
> > index 0fe6eac46512..2e4fb4ffd562 100644
> > --- a/drivers/rpmsg/Kconfig
> > +++ b/drivers/rpmsg/Kconfig
> > @@ -39,6 +39,7 @@ config RPMSG_QCOM_GLINK_SMEM
> >  
> >  config RPMSG_QCOM_SMD
> > tristate "Qualcomm Shared Memory Driver (SMD)"
> > +   depends on MAILBOX
> > depends on QCOM_SMEM
> > select RPMSG
> > help
> > diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
> > index bc0b30657230..3ff271a44bef 100644
> > --- a/drivers/rpmsg/qcom_smd.c
> > +++ b/drivers/rpmsg/qcom_smd.c
> > @@ -14,6 +14,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -107,6 +108,8 @@ static const struct {
> >   * @ipc_regmap:regmap handle holding the outgoing ipc register
> >   * @ipc_offset:offset within @ipc_regmap of the register for 
> > ipc
> >   * @ipc_bit:   bit in the register at @ipc_offset of 
> > @ipc_regmap
> > + * @mbox_client:   mailbox client handle
> > + * @mbox_chan: apcs ipc mailbox channel handle
> >   * @channels:  list of all channels detected on this edge
> >   * @channels_lock: guard for modifications of @channels
> >   * @allocated: array of bitmaps representing already allocated 
> > channels
> > @@ -129,6 +132,9 @@ struct qcom_smd_edge {
> > int ipc_offset;
> > int ipc_bit;
> >  
> > +   struct mbox_client mbox_client;
> > +   struct mbox_chan *mbox_chan;
> > +
> > struct list_head channels;
> > spinlock_t channels_lock;
> >  
> > @@ -366,7 +372,17 @@ static void qcom_smd_signal_channel(struct 
> > qcom_smd_channel *channel)
> >  {
> > struct qcom_smd_edge *edge = channel->edge;
> >  
> > -   regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));
> > +   if (edge->mbox_chan) {
> > +   /*
> > +* We can ignore a failing mbox_send_message() as the only
> > +* possible cause is that the FIFO in the framework is full of
> > +* other writes to the same bit.
> > +*/
> > +   mbox_send_message(edge->mbox_chan, NULL);
> > +   mbox_client_txdone(edge->mbox_chan, 0);
> > +   } else {
> > +   regmap_write(edge->ipc_regmap, edge->ipc_offset, 
> > BIT(edge->ipc_bit));
> > +   }
> 

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

On 08/30/18 20:57, Frank Rowand wrote:
> Hi Bjorn,
> 
> 
> On 04/19/18 18:17, Bjorn Andersson wrote:
>> Attempt to acquire the APCS IPC through the mailbox framework and fall
>> back to the old syscon based approach, to allow us to move away from
>> using the syscon.
>>
>> Reviewed-by: Arun Kumar Neelakantam 
>> Signed-off-by: Bjorn Andersson 
>> ---
>>
>> Changes since v2:
>> - Added comment about mbox_send_message() return value.
>>
>>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>>  drivers/rpmsg/Kconfig  |  1 +
>>  drivers/rpmsg/qcom_smd.c   | 67 
>> --
>>  3 files changed, 56 insertions(+), 20 deletions(-)
> 
> This patch in the mainline Linux kernel as commit 
> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
> with the patch applied, thus I do not have the block device my root file 
> system
> is located on.
> 
> Testing on v4.18, if I revert this commit the mmc device is available.
> 
> I'll reply to this email with the console messages for 4.18 and for 4.18 with
> this commit reverted.



< snip >

Note that I have commented out the WARN_ON() in gic_irq_domain_translate()
at drivers/irqchip/irq-gic.c:1016 to remove a lot of noise from the boot
messages.

Here are the console messages for 4.18, with ab460a2e72da reverted:

Android Bootloader - UART_DM Initialized!!!
[0] welcome to lk

[10] platform_init()
[10] target_init()
[10] Display Init: Start
[10] display_init(),target_id=10.
[30] Config MIPI_VIDEO_PANEL.
[30] Turn on MIPI_VIDEO_PANEL.
[50] Video lane tested successfully
[50] Display Init: Done
[80] Loading keystore failed status 5 [80] ERROR: scm_protect_keystore 
Failed[200] USB init ept @ 0xf96b000
[220] fastboot_init()
[220] udc_start()
[350] -- reset --
[350] -- portchange --
[460] -- reset --
[460] -- portchange --
[650] fastboot: processing commands
[760] fastboot: download:00ed6000
[1250] fastboot: boot
[1270] Found Appeneded Flattened Device tree
[1270] cmdline: console=ttyMSM0,115200,n8 androidboot.hardware=qcom 
user_debug=31 maxcpus=2 msm_rtb.filter=0x37 ehci-hcd.park=3 
kgdboc=ttyMSM0,115200 debug and
roidboot.emmc=true androidboot.serialno=40081c41 androidboot.baseband=apq
[1290] Updating device tree: start
[1300] Updating device tree: done
[1300] booting linux @ 0x8000, ramdisk @ 0x200 (9533134), tags/device tree 
@ 0x1e0
[1310] Turn off MIPI_VIDEO_PANEL.
[1310] Continuous splash enabled, keeping panel alive.
Booting Linux on physical CPU 0x0
Linux version 4.18.0-dirty (frowand@xps8900) (gcc version 4.6.x-google 20120106 
(prerelease) (GCC)) #2 SMP PREEMPT Thu Aug 30 20:26:18 PDT 2018
CPU: ARMv7 Processor [512f06f0] revision 0 (ARMv7), cr=10c5787d
CPU: div instructions available: patching division code
CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
OF: fdt: Machine model: Qualcomm APQ8074 Dragonboard
Memory policy: Data cache writealloc
On node 0 totalpages: 491776
  Normal zone: 1536 pages used for memmap
  Normal zone: 0 pages reserved
  Normal zone: 164096 pages, LIFO batch:31
  HighMem zone: 327680 pages, LIFO batch:31
random: get_random_bytes called from start_kernel+0x80/0x47c with crng_init=0
percpu: Embedded 17 pages/cpu @(ptrval) s39616 r8192 d21824 u69632
pcpu-alloc: s39616 r8192 d21824 u69632 alloc=17*4096
pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 
Built 1 zonelists, mobility grouping on.  Total pages: 490240
Kernel command line: console=ttyMSM0,115200,n8 androidboot.hardware=qcom 
user_debug=31 maxcpus=2 msm_rtb.filter=0x37 ehci-hcd.park=3 kgdboc=ttyM
SM0,115200 debug androidboot.emmc=true androidboot.serialno=40081c41 
androidboot.baseband=apq
Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
Memory: 1926832K/1967104K available (8192K kernel code, 793K rwdata, 3260K 
rodata, 1024K init, 267K bss, 40272K reserved, 0K cma-reserved, 13107
20K highmem)
Virtual kernel memory layout:
vector  : 0x - 0x1000   (   4 kB)
fixmap  : 0xffc0 - 0xfff0   (3072 kB)
vmalloc : 0xf080 - 0xff80   ( 240 MB)
lowmem  : 0xc000 - 0xf000   ( 768 MB)
pkmap   : 0xbfe0 - 0xc000   (   2 MB)
modules : 0xbf00 - 0xbfe0   (  14 MB)
  .text : 0x(ptrval) - 0x(ptrval)   (9184 kB)
  .init : 0x(ptrval) - 0x(ptrval)   (1024 kB)
  .data : 0x(ptrval) - 0x(ptrval)   ( 794 kB)
   .bss : 0x(ptrval) - 0x(ptrval)   ( 268 kB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
Preemptible hierarchical RCU implementation.
 Tasks RCU enabled.
NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
arch_timer: cp15 and mmio timer(s) running at 19.20MHz (virt/virt).
clocksource: arch_sys_counter: mask: 0xff max_cycles: 0x46d987e47, 
max_idle_ns: 440795202767 ns
sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
Switching to timer-based 

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

On 08/30/18 20:57, Frank Rowand wrote:
> Hi Bjorn,
> 
> 
> On 04/19/18 18:17, Bjorn Andersson wrote:
>> Attempt to acquire the APCS IPC through the mailbox framework and fall
>> back to the old syscon based approach, to allow us to move away from
>> using the syscon.
>>
>> Reviewed-by: Arun Kumar Neelakantam 
>> Signed-off-by: Bjorn Andersson 
>> ---
>>
>> Changes since v2:
>> - Added comment about mbox_send_message() return value.
>>
>>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>>  drivers/rpmsg/Kconfig  |  1 +
>>  drivers/rpmsg/qcom_smd.c   | 67 
>> --
>>  3 files changed, 56 insertions(+), 20 deletions(-)
> 
> This patch in the mainline Linux kernel as commit 
> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
> with the patch applied, thus I do not have the block device my root file 
> system
> is located on.
> 
> Testing on v4.18, if I revert this commit the mmc device is available.
> 
> I'll reply to this email with the console messages for 4.18 and for 4.18 with
> this commit reverted.



< snip >

Note that I have commented out the WARN_ON() in gic_irq_domain_translate()
at drivers/irqchip/irq-gic.c:1016 to remove a lot of noise from the boot
messages.

Here are the console messages for 4.18, with ab460a2e72da reverted:

Android Bootloader - UART_DM Initialized!!!
[0] welcome to lk

[10] platform_init()
[10] target_init()
[10] Display Init: Start
[10] display_init(),target_id=10.
[30] Config MIPI_VIDEO_PANEL.
[30] Turn on MIPI_VIDEO_PANEL.
[50] Video lane tested successfully
[50] Display Init: Done
[80] Loading keystore failed status 5 [80] ERROR: scm_protect_keystore 
Failed[200] USB init ept @ 0xf96b000
[220] fastboot_init()
[220] udc_start()
[350] -- reset --
[350] -- portchange --
[460] -- reset --
[460] -- portchange --
[650] fastboot: processing commands
[760] fastboot: download:00ed6000
[1250] fastboot: boot
[1270] Found Appeneded Flattened Device tree
[1270] cmdline: console=ttyMSM0,115200,n8 androidboot.hardware=qcom 
user_debug=31 maxcpus=2 msm_rtb.filter=0x37 ehci-hcd.park=3 
kgdboc=ttyMSM0,115200 debug and
roidboot.emmc=true androidboot.serialno=40081c41 androidboot.baseband=apq
[1290] Updating device tree: start
[1300] Updating device tree: done
[1300] booting linux @ 0x8000, ramdisk @ 0x200 (9533134), tags/device tree 
@ 0x1e0
[1310] Turn off MIPI_VIDEO_PANEL.
[1310] Continuous splash enabled, keeping panel alive.
Booting Linux on physical CPU 0x0
Linux version 4.18.0-dirty (frowand@xps8900) (gcc version 4.6.x-google 20120106 
(prerelease) (GCC)) #2 SMP PREEMPT Thu Aug 30 20:26:18 PDT 2018
CPU: ARMv7 Processor [512f06f0] revision 0 (ARMv7), cr=10c5787d
CPU: div instructions available: patching division code
CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
OF: fdt: Machine model: Qualcomm APQ8074 Dragonboard
Memory policy: Data cache writealloc
On node 0 totalpages: 491776
  Normal zone: 1536 pages used for memmap
  Normal zone: 0 pages reserved
  Normal zone: 164096 pages, LIFO batch:31
  HighMem zone: 327680 pages, LIFO batch:31
random: get_random_bytes called from start_kernel+0x80/0x47c with crng_init=0
percpu: Embedded 17 pages/cpu @(ptrval) s39616 r8192 d21824 u69632
pcpu-alloc: s39616 r8192 d21824 u69632 alloc=17*4096
pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 
Built 1 zonelists, mobility grouping on.  Total pages: 490240
Kernel command line: console=ttyMSM0,115200,n8 androidboot.hardware=qcom 
user_debug=31 maxcpus=2 msm_rtb.filter=0x37 ehci-hcd.park=3 kgdboc=ttyM
SM0,115200 debug androidboot.emmc=true androidboot.serialno=40081c41 
androidboot.baseband=apq
Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
Memory: 1926832K/1967104K available (8192K kernel code, 793K rwdata, 3260K 
rodata, 1024K init, 267K bss, 40272K reserved, 0K cma-reserved, 13107
20K highmem)
Virtual kernel memory layout:
vector  : 0x - 0x1000   (   4 kB)
fixmap  : 0xffc0 - 0xfff0   (3072 kB)
vmalloc : 0xf080 - 0xff80   ( 240 MB)
lowmem  : 0xc000 - 0xf000   ( 768 MB)
pkmap   : 0xbfe0 - 0xc000   (   2 MB)
modules : 0xbf00 - 0xbfe0   (  14 MB)
  .text : 0x(ptrval) - 0x(ptrval)   (9184 kB)
  .init : 0x(ptrval) - 0x(ptrval)   (1024 kB)
  .data : 0x(ptrval) - 0x(ptrval)   ( 794 kB)
   .bss : 0x(ptrval) - 0x(ptrval)   ( 268 kB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
Preemptible hierarchical RCU implementation.
 Tasks RCU enabled.
NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
arch_timer: cp15 and mmio timer(s) running at 19.20MHz (virt/virt).
clocksource: arch_sys_counter: mask: 0xff max_cycles: 0x46d987e47, 
max_idle_ns: 440795202767 ns
sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
Switching to timer-based 

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

On 08/30/18 20:57, Frank Rowand wrote:
> Hi Bjorn,
> 
> 
> On 04/19/18 18:17, Bjorn Andersson wrote:
>> Attempt to acquire the APCS IPC through the mailbox framework and fall
>> back to the old syscon based approach, to allow us to move away from
>> using the syscon.
>>
>> Reviewed-by: Arun Kumar Neelakantam 
>> Signed-off-by: Bjorn Andersson 
>> ---
>>
>> Changes since v2:
>> - Added comment about mbox_send_message() return value.
>>
>>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>>  drivers/rpmsg/Kconfig  |  1 +
>>  drivers/rpmsg/qcom_smd.c   | 67 
>> --
>>  3 files changed, 56 insertions(+), 20 deletions(-)
> 
> This patch in the mainline Linux kernel as commit 
> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
> with the patch applied, thus I do not have the block device my root file 
> system
> is located on.
> 
> Testing on v4.18, if I revert this commit the mmc device is available.
> 
> I'll reply to this email with the console messages for 4.18 and for 4.18 with
> this commit reverted.

< snip >

Note that I have commented out the WARN_ON() in gic_irq_domain_translate()
at drivers/irqchip/irq-gic.c:1016 to remove a lot of noise from the boot
messages.

Here are the console messages for 4.18:

Android Bootloader - UART_DM Initialized!!!
[0] welcome to lk

[10] platform_init()
[10] target_init()
[10] Display Init: Start
[10] display_init(),target_id=10.
[30] Config MIPI_VIDEO_PANEL.
[30] Turn on MIPI_VIDEO_PANEL.
[50] Video lane tested successfully
[50] Display Init: Done
[80] Loading keystore failed status 5 [80] ERROR: scm_protect_keystore 
Failed[200] USB init ept @ 0xf96b000
[220] fastboot_init()
[220] udc_start()
[350] -- reset --
[350] -- portchange --
[460] -- reset --
[460] -- portchange --
[650] fastboot: processing commands
[750] fastboot: download:00ece000
[1220] fastboot: boot
[1240] Found Appeneded Flattened Device tree
[1240] cmdline: console=ttyMSM0,115200,n8 androidboot.hardware=qcom 
user_debug=31 maxcpus=2 msm_rtb.filter=0x37 ehci-hcd.park=3 
kgdboc=ttyMSM0,115200 debug and
roidboot.emmc=true androidboot.serialno=40081c41 androidboot.baseband=apq
[1270] Updating device tree: start
[1270] Updating device tree: done
[1270] booting linux @ 0x8000, ramdisk @ 0x200 (9533134), tags/device tree 
@ 0x1e0
[1280] Turn off MIPI_VIDEO_PANEL.
[1280] Continuous splash enabled, keeping panel alive.
Booting Linux on physical CPU 0x0
Linux version 4.18.0-dirty (frowand@xps8900) (gcc version 4.6.x-google 20120106 
(prerelease) (GCC)) #3 SMP PREEMPT Thu Aug 30 20:32:21 PDT 2018
CPU: ARMv7 Processor [512f06f0] revision 0 (ARMv7), cr=10c5787d
CPU: div instructions available: patching division code
CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
OF: fdt: Machine model: Qualcomm APQ8074 Dragonboard
Memory policy: Data cache writealloc
On node 0 totalpages: 491776
  Normal zone: 1536 pages used for memmap
  Normal zone: 0 pages reserved
  Normal zone: 164096 pages, LIFO batch:31
  HighMem zone: 327680 pages, LIFO batch:31
random: get_random_bytes called from start_kernel+0x80/0x47c with crng_init=0
percpu: Embedded 17 pages/cpu @(ptrval) s39616 r8192 d21824 u69632
pcpu-alloc: s39616 r8192 d21824 u69632 alloc=17*4096
pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 
Built 1 zonelists, mobility grouping on.  Total pages: 490240
Kernel command line: console=ttyMSM0,115200,n8 androidboot.hardware=qcom 
user_debug=31 maxcpus=2 msm_rtb.filter=0x37 ehci-hcd.park=3 kgdboc=ttyM
SM0,115200 debug androidboot.emmc=true androidboot.serialno=40081c41 
androidboot.baseband=apq
Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
Memory: 1926844K/1967104K available (8192K kernel code, 782K rwdata, 3212K 
rodata, 1024K init, 267K bss, 40260K reserved, 0K cma-reserved, 13107
20K highmem)
Virtual kernel memory layout:
vector  : 0x - 0x1000   (   4 kB)
fixmap  : 0xffc0 - 0xfff0   (3072 kB)
vmalloc : 0xf080 - 0xff80   ( 240 MB)
lowmem  : 0xc000 - 0xf000   ( 768 MB)
pkmap   : 0xbfe0 - 0xc000   (   2 MB)
modules : 0xbf00 - 0xbfe0   (  14 MB)
  .text : 0x(ptrval) - 0x(ptrval)   (9184 kB)
  .init : 0x(ptrval) - 0x(ptrval)   (1024 kB)
  .data : 0x(ptrval) - 0x(ptrval)   ( 783 kB)
   .bss : 0x(ptrval) - 0x(ptrval)   ( 268 kB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
Preemptible hierarchical RCU implementation.
 Tasks RCU enabled.
NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
arch_timer: cp15 and mmio timer(s) running at 19.20MHz (virt/virt).
clocksource: arch_sys_counter: mask: 0xff max_cycles: 0x46d987e47, 
max_idle_ns: 440795202767 ns
sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
Switching to timer-based delay loop, resolution 52ns

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

On 08/30/18 20:57, Frank Rowand wrote:
> Hi Bjorn,
> 
> 
> On 04/19/18 18:17, Bjorn Andersson wrote:
>> Attempt to acquire the APCS IPC through the mailbox framework and fall
>> back to the old syscon based approach, to allow us to move away from
>> using the syscon.
>>
>> Reviewed-by: Arun Kumar Neelakantam 
>> Signed-off-by: Bjorn Andersson 
>> ---
>>
>> Changes since v2:
>> - Added comment about mbox_send_message() return value.
>>
>>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>>  drivers/rpmsg/Kconfig  |  1 +
>>  drivers/rpmsg/qcom_smd.c   | 67 
>> --
>>  3 files changed, 56 insertions(+), 20 deletions(-)
> 
> This patch in the mainline Linux kernel as commit 
> ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
> causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
> with the patch applied, thus I do not have the block device my root file 
> system
> is located on.
> 
> Testing on v4.18, if I revert this commit the mmc device is available.
> 
> I'll reply to this email with the console messages for 4.18 and for 4.18 with
> this commit reverted.

< snip >

Note that I have commented out the WARN_ON() in gic_irq_domain_translate()
at drivers/irqchip/irq-gic.c:1016 to remove a lot of noise from the boot
messages.

Here are the console messages for 4.18:

Android Bootloader - UART_DM Initialized!!!
[0] welcome to lk

[10] platform_init()
[10] target_init()
[10] Display Init: Start
[10] display_init(),target_id=10.
[30] Config MIPI_VIDEO_PANEL.
[30] Turn on MIPI_VIDEO_PANEL.
[50] Video lane tested successfully
[50] Display Init: Done
[80] Loading keystore failed status 5 [80] ERROR: scm_protect_keystore 
Failed[200] USB init ept @ 0xf96b000
[220] fastboot_init()
[220] udc_start()
[350] -- reset --
[350] -- portchange --
[460] -- reset --
[460] -- portchange --
[650] fastboot: processing commands
[750] fastboot: download:00ece000
[1220] fastboot: boot
[1240] Found Appeneded Flattened Device tree
[1240] cmdline: console=ttyMSM0,115200,n8 androidboot.hardware=qcom 
user_debug=31 maxcpus=2 msm_rtb.filter=0x37 ehci-hcd.park=3 
kgdboc=ttyMSM0,115200 debug and
roidboot.emmc=true androidboot.serialno=40081c41 androidboot.baseband=apq
[1270] Updating device tree: start
[1270] Updating device tree: done
[1270] booting linux @ 0x8000, ramdisk @ 0x200 (9533134), tags/device tree 
@ 0x1e0
[1280] Turn off MIPI_VIDEO_PANEL.
[1280] Continuous splash enabled, keeping panel alive.
Booting Linux on physical CPU 0x0
Linux version 4.18.0-dirty (frowand@xps8900) (gcc version 4.6.x-google 20120106 
(prerelease) (GCC)) #3 SMP PREEMPT Thu Aug 30 20:32:21 PDT 2018
CPU: ARMv7 Processor [512f06f0] revision 0 (ARMv7), cr=10c5787d
CPU: div instructions available: patching division code
CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
OF: fdt: Machine model: Qualcomm APQ8074 Dragonboard
Memory policy: Data cache writealloc
On node 0 totalpages: 491776
  Normal zone: 1536 pages used for memmap
  Normal zone: 0 pages reserved
  Normal zone: 164096 pages, LIFO batch:31
  HighMem zone: 327680 pages, LIFO batch:31
random: get_random_bytes called from start_kernel+0x80/0x47c with crng_init=0
percpu: Embedded 17 pages/cpu @(ptrval) s39616 r8192 d21824 u69632
pcpu-alloc: s39616 r8192 d21824 u69632 alloc=17*4096
pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 
Built 1 zonelists, mobility grouping on.  Total pages: 490240
Kernel command line: console=ttyMSM0,115200,n8 androidboot.hardware=qcom 
user_debug=31 maxcpus=2 msm_rtb.filter=0x37 ehci-hcd.park=3 kgdboc=ttyM
SM0,115200 debug androidboot.emmc=true androidboot.serialno=40081c41 
androidboot.baseband=apq
Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
Memory: 1926844K/1967104K available (8192K kernel code, 782K rwdata, 3212K 
rodata, 1024K init, 267K bss, 40260K reserved, 0K cma-reserved, 13107
20K highmem)
Virtual kernel memory layout:
vector  : 0x - 0x1000   (   4 kB)
fixmap  : 0xffc0 - 0xfff0   (3072 kB)
vmalloc : 0xf080 - 0xff80   ( 240 MB)
lowmem  : 0xc000 - 0xf000   ( 768 MB)
pkmap   : 0xbfe0 - 0xc000   (   2 MB)
modules : 0xbf00 - 0xbfe0   (  14 MB)
  .text : 0x(ptrval) - 0x(ptrval)   (9184 kB)
  .init : 0x(ptrval) - 0x(ptrval)   (1024 kB)
  .data : 0x(ptrval) - 0x(ptrval)   ( 783 kB)
   .bss : 0x(ptrval) - 0x(ptrval)   ( 268 kB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
Preemptible hierarchical RCU implementation.
 Tasks RCU enabled.
NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
arch_timer: cp15 and mmio timer(s) running at 19.20MHz (virt/virt).
clocksource: arch_sys_counter: mask: 0xff max_cycles: 0x46d987e47, 
max_idle_ns: 440795202767 ns
sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
Switching to timer-based delay loop, resolution 52ns

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

Hi Bjorn,


On 04/19/18 18:17, Bjorn Andersson wrote:
> Attempt to acquire the APCS IPC through the mailbox framework and fall
> back to the old syscon based approach, to allow us to move away from
> using the syscon.
> 
> Reviewed-by: Arun Kumar Neelakantam 
> Signed-off-by: Bjorn Andersson 
> ---
> 
> Changes since v2:
> - Added comment about mbox_send_message() return value.
> 
>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>  drivers/rpmsg/Kconfig  |  1 +
>  drivers/rpmsg/qcom_smd.c   | 67 
> --
>  3 files changed, 56 insertions(+), 20 deletions(-)

This patch in the mainline Linux kernel as commit 
ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
with the patch applied, thus I do not have the block device my root file system
is located on.

Testing on v4.18, if I revert this commit the mmc device is available.

I'll reply to this email with the console messages for 4.18 and for 4.18 with
this commit reverted.

-Frank


> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt 
> b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> index ea1dc75ec9ea..234ae2256501 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> @@ -22,9 +22,15 @@ The edge is described by the following properties:
>   Definition: should specify the IRQ used by the remote processor to
>   signal this processor about communication related updates
>  
> -- qcom,ipc:
> +- mboxes:
>   Usage: required
>   Value type: 
> + Definition: reference to the associated doorbell in APCS, as described
> + in mailbox/mailbox.txt
> +
> +- qcom,ipc:
> + Usage: required, unless mboxes is specified
> + Value type: 
>   Definition: three entries specifying the outgoing ipc bit used for
>   signaling the remote processor:
>   - phandle to a syscon node representing the apcs registers
> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
> index 0fe6eac46512..2e4fb4ffd562 100644
> --- a/drivers/rpmsg/Kconfig
> +++ b/drivers/rpmsg/Kconfig
> @@ -39,6 +39,7 @@ config RPMSG_QCOM_GLINK_SMEM
>  
>  config RPMSG_QCOM_SMD
>   tristate "Qualcomm Shared Memory Driver (SMD)"
> + depends on MAILBOX
>   depends on QCOM_SMEM
>   select RPMSG
>   help
> diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
> index bc0b30657230..3ff271a44bef 100644
> --- a/drivers/rpmsg/qcom_smd.c
> +++ b/drivers/rpmsg/qcom_smd.c
> @@ -14,6 +14,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -107,6 +108,8 @@ static const struct {
>   * @ipc_regmap:  regmap handle holding the outgoing ipc register
>   * @ipc_offset:  offset within @ipc_regmap of the register for 
> ipc
>   * @ipc_bit: bit in the register at @ipc_offset of @ipc_regmap
> + * @mbox_client: mailbox client handle
> + * @mbox_chan:   apcs ipc mailbox channel handle
>   * @channels:list of all channels detected on this edge
>   * @channels_lock:   guard for modifications of @channels
>   * @allocated:   array of bitmaps representing already allocated 
> channels
> @@ -129,6 +132,9 @@ struct qcom_smd_edge {
>   int ipc_offset;
>   int ipc_bit;
>  
> + struct mbox_client mbox_client;
> + struct mbox_chan *mbox_chan;
> +
>   struct list_head channels;
>   spinlock_t channels_lock;
>  
> @@ -366,7 +372,17 @@ static void qcom_smd_signal_channel(struct 
> qcom_smd_channel *channel)
>  {
>   struct qcom_smd_edge *edge = channel->edge;
>  
> - regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));
> + if (edge->mbox_chan) {
> + /*
> +  * We can ignore a failing mbox_send_message() as the only
> +  * possible cause is that the FIFO in the framework is full of
> +  * other writes to the same bit.
> +  */
> + mbox_send_message(edge->mbox_chan, NULL);
> + mbox_client_txdone(edge->mbox_chan, 0);
> + } else {
> + regmap_write(edge->ipc_regmap, edge->ipc_offset, 
> BIT(edge->ipc_bit));
> + }
>  }
>  
>  /*
> @@ -1326,27 +1342,37 @@ static int qcom_smd_parse_edge(struct device *dev,
>   key = "qcom,remote-pid";
>   of_property_read_u32(node, key, >remote_pid);
>  
> - syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
> - if (!syscon_np) {
> - dev_err(dev, "no qcom,ipc node\n");
> - return -ENODEV;
> - }
> + edge->mbox_client.dev = dev;
> + edge->mbox_client.knows_txdone = true;
> + edge->mbox_chan = mbox_request_channel(>mbox_client, 0);
> + if (IS_ERR(edge->mbox_chan)) {
> + if (PTR_ERR(edge->mbox_chan) != -ENODEV)

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Frank Rowand]

Hi Bjorn,


On 04/19/18 18:17, Bjorn Andersson wrote:
> Attempt to acquire the APCS IPC through the mailbox framework and fall
> back to the old syscon based approach, to allow us to move away from
> using the syscon.
> 
> Reviewed-by: Arun Kumar Neelakantam 
> Signed-off-by: Bjorn Andersson 
> ---
> 
> Changes since v2:
> - Added comment about mbox_send_message() return value.
> 
>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>  drivers/rpmsg/Kconfig  |  1 +
>  drivers/rpmsg/qcom_smd.c   | 67 
> --
>  3 files changed, 56 insertions(+), 20 deletions(-)

This patch in the mainline Linux kernel as commit 
ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
with the patch applied, thus I do not have the block device my root file system
is located on.

Testing on v4.18, if I revert this commit the mmc device is available.

I'll reply to this email with the console messages for 4.18 and for 4.18 with
this commit reverted.

-Frank


> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt 
> b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> index ea1dc75ec9ea..234ae2256501 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> @@ -22,9 +22,15 @@ The edge is described by the following properties:
>   Definition: should specify the IRQ used by the remote processor to
>   signal this processor about communication related updates
>  
> -- qcom,ipc:
> +- mboxes:
>   Usage: required
>   Value type: 
> + Definition: reference to the associated doorbell in APCS, as described
> + in mailbox/mailbox.txt
> +
> +- qcom,ipc:
> + Usage: required, unless mboxes is specified
> + Value type: 
>   Definition: three entries specifying the outgoing ipc bit used for
>   signaling the remote processor:
>   - phandle to a syscon node representing the apcs registers
> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
> index 0fe6eac46512..2e4fb4ffd562 100644
> --- a/drivers/rpmsg/Kconfig
> +++ b/drivers/rpmsg/Kconfig
> @@ -39,6 +39,7 @@ config RPMSG_QCOM_GLINK_SMEM
>  
>  config RPMSG_QCOM_SMD
>   tristate "Qualcomm Shared Memory Driver (SMD)"
> + depends on MAILBOX
>   depends on QCOM_SMEM
>   select RPMSG
>   help
> diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
> index bc0b30657230..3ff271a44bef 100644
> --- a/drivers/rpmsg/qcom_smd.c
> +++ b/drivers/rpmsg/qcom_smd.c
> @@ -14,6 +14,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -107,6 +108,8 @@ static const struct {
>   * @ipc_regmap:  regmap handle holding the outgoing ipc register
>   * @ipc_offset:  offset within @ipc_regmap of the register for 
> ipc
>   * @ipc_bit: bit in the register at @ipc_offset of @ipc_regmap
> + * @mbox_client: mailbox client handle
> + * @mbox_chan:   apcs ipc mailbox channel handle
>   * @channels:list of all channels detected on this edge
>   * @channels_lock:   guard for modifications of @channels
>   * @allocated:   array of bitmaps representing already allocated 
> channels
> @@ -129,6 +132,9 @@ struct qcom_smd_edge {
>   int ipc_offset;
>   int ipc_bit;
>  
> + struct mbox_client mbox_client;
> + struct mbox_chan *mbox_chan;
> +
>   struct list_head channels;
>   spinlock_t channels_lock;
>  
> @@ -366,7 +372,17 @@ static void qcom_smd_signal_channel(struct 
> qcom_smd_channel *channel)
>  {
>   struct qcom_smd_edge *edge = channel->edge;
>  
> - regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));
> + if (edge->mbox_chan) {
> + /*
> +  * We can ignore a failing mbox_send_message() as the only
> +  * possible cause is that the FIFO in the framework is full of
> +  * other writes to the same bit.
> +  */
> + mbox_send_message(edge->mbox_chan, NULL);
> + mbox_client_txdone(edge->mbox_chan, 0);
> + } else {
> + regmap_write(edge->ipc_regmap, edge->ipc_offset, 
> BIT(edge->ipc_bit));
> + }
>  }
>  
>  /*
> @@ -1326,27 +1342,37 @@ static int qcom_smd_parse_edge(struct device *dev,
>   key = "qcom,remote-pid";
>   of_property_read_u32(node, key, >remote_pid);
>  
> - syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
> - if (!syscon_np) {
> - dev_err(dev, "no qcom,ipc node\n");
> - return -ENODEV;
> - }
> + edge->mbox_client.dev = dev;
> + edge->mbox_client.knows_txdone = true;
> + edge->mbox_chan = mbox_request_channel(>mbox_client, 0);
> + if (IS_ERR(edge->mbox_chan)) {
> + if (PTR_ERR(edge->mbox_chan) != -ENODEV)

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Rob Herring]

On Thu, Apr 19, 2018 at 06:17:57PM -0700, Bjorn Andersson wrote:
> Attempt to acquire the APCS IPC through the mailbox framework and fall
> back to the old syscon based approach, to allow us to move away from
> using the syscon.
> 
> Reviewed-by: Arun Kumar Neelakantam 
> Signed-off-by: Bjorn Andersson 
> ---
> 
> Changes since v2:
> - Added comment about mbox_send_message() return value.
> 
>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>  drivers/rpmsg/Kconfig  |  1 +
>  drivers/rpmsg/qcom_smd.c   | 67 
> --
>  3 files changed, 56 insertions(+), 20 deletions(-)

Reviewed-by: Rob Herring 

Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Rob Herring]

On Thu, Apr 19, 2018 at 06:17:57PM -0700, Bjorn Andersson wrote:
> Attempt to acquire the APCS IPC through the mailbox framework and fall
> back to the old syscon based approach, to allow us to move away from
> using the syscon.
> 
> Reviewed-by: Arun Kumar Neelakantam 
> Signed-off-by: Bjorn Andersson 
> ---
> 
> Changes since v2:
> - Added comment about mbox_send_message() return value.
> 
>  .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
>  drivers/rpmsg/Kconfig  |  1 +
>  drivers/rpmsg/qcom_smd.c   | 67 
> --
>  3 files changed, 56 insertions(+), 20 deletions(-)

Reviewed-by: Rob Herring 

[PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Bjorn Andersson]

Attempt to acquire the APCS IPC through the mailbox framework and fall
back to the old syscon based approach, to allow us to move away from
using the syscon.

Reviewed-by: Arun Kumar Neelakantam 
Signed-off-by: Bjorn Andersson 
---

Changes since v2:
- Added comment about mbox_send_message() return value.

 .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
 drivers/rpmsg/Kconfig  |  1 +
 drivers/rpmsg/qcom_smd.c   | 67 --
 3 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
index ea1dc75ec9ea..234ae2256501 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
@@ -22,9 +22,15 @@ The edge is described by the following properties:
Definition: should specify the IRQ used by the remote processor to
signal this processor about communication related updates
 
-- qcom,ipc:
+- mboxes:
Usage: required
Value type: 
+   Definition: reference to the associated doorbell in APCS, as described
+   in mailbox/mailbox.txt
+
+- qcom,ipc:
+   Usage: required, unless mboxes is specified
+   Value type: 
Definition: three entries specifying the outgoing ipc bit used for
signaling the remote processor:
- phandle to a syscon node representing the apcs registers
diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
index 0fe6eac46512..2e4fb4ffd562 100644
--- a/drivers/rpmsg/Kconfig
+++ b/drivers/rpmsg/Kconfig
@@ -39,6 +39,7 @@ config RPMSG_QCOM_GLINK_SMEM
 
 config RPMSG_QCOM_SMD
tristate "Qualcomm Shared Memory Driver (SMD)"
+   depends on MAILBOX
depends on QCOM_SMEM
select RPMSG
help
diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
index bc0b30657230..3ff271a44bef 100644
--- a/drivers/rpmsg/qcom_smd.c
+++ b/drivers/rpmsg/qcom_smd.c
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -107,6 +108,8 @@ static const struct {
  * @ipc_regmap:regmap handle holding the outgoing ipc register
  * @ipc_offset:offset within @ipc_regmap of the register for 
ipc
  * @ipc_bit:   bit in the register at @ipc_offset of @ipc_regmap
+ * @mbox_client:   mailbox client handle
+ * @mbox_chan: apcs ipc mailbox channel handle
  * @channels:  list of all channels detected on this edge
  * @channels_lock: guard for modifications of @channels
  * @allocated: array of bitmaps representing already allocated channels
@@ -129,6 +132,9 @@ struct qcom_smd_edge {
int ipc_offset;
int ipc_bit;
 
+   struct mbox_client mbox_client;
+   struct mbox_chan *mbox_chan;
+
struct list_head channels;
spinlock_t channels_lock;
 
@@ -366,7 +372,17 @@ static void qcom_smd_signal_channel(struct 
qcom_smd_channel *channel)
 {
struct qcom_smd_edge *edge = channel->edge;
 
-   regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));
+   if (edge->mbox_chan) {
+   /*
+* We can ignore a failing mbox_send_message() as the only
+* possible cause is that the FIFO in the framework is full of
+* other writes to the same bit.
+*/
+   mbox_send_message(edge->mbox_chan, NULL);
+   mbox_client_txdone(edge->mbox_chan, 0);
+   } else {
+   regmap_write(edge->ipc_regmap, edge->ipc_offset, 
BIT(edge->ipc_bit));
+   }
 }
 
 /*
@@ -1326,27 +1342,37 @@ static int qcom_smd_parse_edge(struct device *dev,
key = "qcom,remote-pid";
of_property_read_u32(node, key, >remote_pid);
 
-   syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
-   if (!syscon_np) {
-   dev_err(dev, "no qcom,ipc node\n");
-   return -ENODEV;
-   }
+   edge->mbox_client.dev = dev;
+   edge->mbox_client.knows_txdone = true;
+   edge->mbox_chan = mbox_request_channel(>mbox_client, 0);
+   if (IS_ERR(edge->mbox_chan)) {
+   if (PTR_ERR(edge->mbox_chan) != -ENODEV)
+   return PTR_ERR(edge->mbox_chan);
 
-   edge->ipc_regmap = syscon_node_to_regmap(syscon_np);
-   if (IS_ERR(edge->ipc_regmap))
-   return PTR_ERR(edge->ipc_regmap);
+   edge->mbox_chan = NULL;
 
-   key = "qcom,ipc";
-   ret = of_property_read_u32_index(node, key, 1, >ipc_offset);
-   if (ret < 0) {
-   dev_err(dev, "no offset in %s\n", key);
-   return -EINVAL;
-   }
+   syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
+   if (!syscon_np) {
+   dev_err(dev, 

[PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework

[Bjorn Andersson]

Attempt to acquire the APCS IPC through the mailbox framework and fall
back to the old syscon based approach, to allow us to move away from
using the syscon.

Reviewed-by: Arun Kumar Neelakantam 
Signed-off-by: Bjorn Andersson 
---

Changes since v2:
- Added comment about mbox_send_message() return value.

 .../devicetree/bindings/soc/qcom/qcom,smd.txt  |  8 ++-
 drivers/rpmsg/Kconfig  |  1 +
 drivers/rpmsg/qcom_smd.c   | 67 --
 3 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
index ea1dc75ec9ea..234ae2256501 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
@@ -22,9 +22,15 @@ The edge is described by the following properties:
Definition: should specify the IRQ used by the remote processor to
signal this processor about communication related updates
 
-- qcom,ipc:
+- mboxes:
Usage: required
Value type: 
+   Definition: reference to the associated doorbell in APCS, as described
+   in mailbox/mailbox.txt
+
+- qcom,ipc:
+   Usage: required, unless mboxes is specified
+   Value type: 
Definition: three entries specifying the outgoing ipc bit used for
signaling the remote processor:
- phandle to a syscon node representing the apcs registers
diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
index 0fe6eac46512..2e4fb4ffd562 100644
--- a/drivers/rpmsg/Kconfig
+++ b/drivers/rpmsg/Kconfig
@@ -39,6 +39,7 @@ config RPMSG_QCOM_GLINK_SMEM
 
 config RPMSG_QCOM_SMD
tristate "Qualcomm Shared Memory Driver (SMD)"
+   depends on MAILBOX
depends on QCOM_SMEM
select RPMSG
help
diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
index bc0b30657230..3ff271a44bef 100644
--- a/drivers/rpmsg/qcom_smd.c
+++ b/drivers/rpmsg/qcom_smd.c
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -107,6 +108,8 @@ static const struct {
  * @ipc_regmap:regmap handle holding the outgoing ipc register
  * @ipc_offset:offset within @ipc_regmap of the register for 
ipc
  * @ipc_bit:   bit in the register at @ipc_offset of @ipc_regmap
+ * @mbox_client:   mailbox client handle
+ * @mbox_chan: apcs ipc mailbox channel handle
  * @channels:  list of all channels detected on this edge
  * @channels_lock: guard for modifications of @channels
  * @allocated: array of bitmaps representing already allocated channels
@@ -129,6 +132,9 @@ struct qcom_smd_edge {
int ipc_offset;
int ipc_bit;
 
+   struct mbox_client mbox_client;
+   struct mbox_chan *mbox_chan;
+
struct list_head channels;
spinlock_t channels_lock;
 
@@ -366,7 +372,17 @@ static void qcom_smd_signal_channel(struct 
qcom_smd_channel *channel)
 {
struct qcom_smd_edge *edge = channel->edge;
 
-   regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));
+   if (edge->mbox_chan) {
+   /*
+* We can ignore a failing mbox_send_message() as the only
+* possible cause is that the FIFO in the framework is full of
+* other writes to the same bit.
+*/
+   mbox_send_message(edge->mbox_chan, NULL);
+   mbox_client_txdone(edge->mbox_chan, 0);
+   } else {
+   regmap_write(edge->ipc_regmap, edge->ipc_offset, 
BIT(edge->ipc_bit));
+   }
 }
 
 /*
@@ -1326,27 +1342,37 @@ static int qcom_smd_parse_edge(struct device *dev,
key = "qcom,remote-pid";
of_property_read_u32(node, key, >remote_pid);
 
-   syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
-   if (!syscon_np) {
-   dev_err(dev, "no qcom,ipc node\n");
-   return -ENODEV;
-   }
+   edge->mbox_client.dev = dev;
+   edge->mbox_client.knows_txdone = true;
+   edge->mbox_chan = mbox_request_channel(>mbox_client, 0);
+   if (IS_ERR(edge->mbox_chan)) {
+   if (PTR_ERR(edge->mbox_chan) != -ENODEV)
+   return PTR_ERR(edge->mbox_chan);
 
-   edge->ipc_regmap = syscon_node_to_regmap(syscon_np);
-   if (IS_ERR(edge->ipc_regmap))
-   return PTR_ERR(edge->ipc_regmap);
+   edge->mbox_chan = NULL;
 
-   key = "qcom,ipc";
-   ret = of_property_read_u32_index(node, key, 1, >ipc_offset);
-   if (ret < 0) {
-   dev_err(dev, "no offset in %s\n", key);
-   return -EINVAL;
-   }
+   syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
+   if (!syscon_np) {
+   dev_err(dev, "no qcom,ipc node\n");
+