rpmsg: core: add support to power domains for devices

Message ID 20180615095939.1331-1-srinivas.kandagatla@linaro.org
State Accepted
Commit fe782affd0f440a4e60e2cc81b8f2eccb2923113
Headers show
Series
  • rpmsg: core: add support to power domains for devices
Related show

Commit Message

Srinivas Kandagatla June 15, 2018, 9:59 a.m.
Some of the rpmsg devices need to switch on power domains to communicate
with remote processor. For example on Qualcomm DB820c platform LPASS
power domain needs to switched on for any kind of audio services.
This patch adds the missing power domain support in rpmsg core.

Without this patch attempting to play audio via QDSP on DB820c would
reboot the system.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

---
 drivers/rpmsg/rpmsg_core.c | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.16.2

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Srinivas Kandagatla Sept. 25, 2018, 8:29 a.m. | #1
Hi Suman,

On 25/09/18 01:25, Suman Anna wrote:
> Hi Srinivas,

> 

> On 06/15/2018 04:59 AM, Srinivas Kandagatla wrote:

>> Some of the rpmsg devices need to switch on power domains to communicate

>> with remote processor. For example on Qualcomm DB820c platform LPASS

>> power domain needs to switched on for any kind of audio services.

>> This patch adds the missing power domain support in rpmsg core.

>>

>> Without this patch attempting to play audio via QDSP on DB820c would

>> reboot the system.

>>

>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

>> ---

>>   drivers/rpmsg/rpmsg_core.c | 7 +++++++

>>   1 file changed, 7 insertions(+)

>>

>> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c

>> index b714a543a91d..8122807db380 100644

>> --- a/drivers/rpmsg/rpmsg_core.c

>> +++ b/drivers/rpmsg/rpmsg_core.c

>> @@ -15,6 +15,7 @@

>>   #include <linux/module.h>

>>   #include <linux/rpmsg.h>

>>   #include <linux/of_device.h>

>> +#include <linux/pm_domain.h>

>>   #include <linux/slab.h>

>>   

>>   #include "rpmsg_internal.h"

>> @@ -449,6 +450,10 @@ static int rpmsg_dev_probe(struct device *dev)

>>   	struct rpmsg_endpoint *ept = NULL;

>>   	int err;

>>   

>> +	err = dev_pm_domain_attach(dev, true);

>> +	if (err)

>> +		goto out;

> 

> This patch has broken the virtio-rpmsg stack based rpmsg devices. These

> devices are non-DT and the rpmsg_dev_probe() is now failing with -19

> (-ENODEV) error code.

> 


There seems to be some dependency on this patch ( 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.19-rc5&id=919b7308fcc452cd4e282bab389c33384a9f3790 
) which seems to rework the return values from acpi_dev_pm_attach() and 
genpd_dev_pm_attach()



--srini


> Loic, Arnaud,

> Can one of you double-confirm this behavior on ST platforms as well?

> The patch came through 4.14.71 stable release and broke our downstream

> kernels.

> 

> regards

> Suman

> 

>> +

>>   	if (rpdrv->callback) {

>>   		strncpy(chinfo.name, rpdev->id.name, RPMSG_NAME_SIZE);

>>   		chinfo.src = rpdev->src;

>> @@ -490,6 +495,8 @@ static int rpmsg_dev_remove(struct device *dev)

>>   

>>   	rpdrv->remove(rpdev);

>>   

>> +	dev_pm_domain_detach(dev, true);

>> +

>>   	if (rpdev->ept)

>>   		rpmsg_destroy_ept(rpdev->ept);

>>   

>>

>
Arnaud Pouliquen Sept. 25, 2018, 5:25 p.m. | #2
Hi Suman

On 09/25/2018 02:25 AM, Suman Anna wrote:
> Hi Srinivas,

> 

> On 06/15/2018 04:59 AM, Srinivas Kandagatla wrote:

>> Some of the rpmsg devices need to switch on power domains to communicate

>> with remote processor. For example on Qualcomm DB820c platform LPASS

>> power domain needs to switched on for any kind of audio services.

>> This patch adds the missing power domain support in rpmsg core.

>>

>> Without this patch attempting to play audio via QDSP on DB820c would

>> reboot the system.

>>

>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

>> ---

>>  drivers/rpmsg/rpmsg_core.c | 7 +++++++

>>  1 file changed, 7 insertions(+)

>>

>> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c

>> index b714a543a91d..8122807db380 100644

>> --- a/drivers/rpmsg/rpmsg_core.c

>> +++ b/drivers/rpmsg/rpmsg_core.c

>> @@ -15,6 +15,7 @@

>>  #include <linux/module.h>

>>  #include <linux/rpmsg.h>

>>  #include <linux/of_device.h>

>> +#include <linux/pm_domain.h>

>>  #include <linux/slab.h>

>>  

>>  #include "rpmsg_internal.h"

>> @@ -449,6 +450,10 @@ static int rpmsg_dev_probe(struct device *dev)

>>  	struct rpmsg_endpoint *ept = NULL;

>>  	int err;

>>  

>> +	err = dev_pm_domain_attach(dev, true);

>> +	if (err)

>> +		goto out;

> 

> This patch has broken the virtio-rpmsg stack based rpmsg devices. These

> devices are non-DT and the rpmsg_dev_probe() is now failing with -19

> (-ENODEV) error code.

> 

> Loic, Arnaud,

> Can one of you double-confirm this behavior on ST platforms as well?

> The patch came through 4.14.71 stable release and broke our downstream

> kernels.

I do not reproduce issue on 4.14.0 + your patch,  on my ST platform (not
able to test on the LTS branch).
By looking the source code, seems that your issue is related to
genpd_dev_pm_attach that returns -ENODEV because you have
no remoteproc device node...
Do you have a rproc node defined in your DT? we have one.

Regards
Arnaud

> 

> regards

> Suman

> 

>> +

>>  	if (rpdrv->callback) {

>>  		strncpy(chinfo.name, rpdev->id.name, RPMSG_NAME_SIZE);

>>  		chinfo.src = rpdev->src;

>> @@ -490,6 +495,8 @@ static int rpmsg_dev_remove(struct device *dev)

>>  

>>  	rpdrv->remove(rpdev);

>>  

>> +	dev_pm_domain_detach(dev, true);

>> +

>>  	if (rpdev->ept)

>>  		rpmsg_destroy_ept(rpdev->ept);

>>  

>>

>
Arnaud Pouliquen Sept. 26, 2018, 12:48 p.m. | #3
Hi Suman,


On 09/25/2018 09:02 PM, Suman Anna wrote:
> Hi Arnaud,

> 

>> On 09/25/2018 02:25 AM, Suman Anna wrote:

>>> Hi Srinivas,

>>>

>>> On 06/15/2018 04:59 AM, Srinivas Kandagatla wrote:

>>>> Some of the rpmsg devices need to switch on power domains to communicate

>>>> with remote processor. For example on Qualcomm DB820c platform LPASS

>>>> power domain needs to switched on for any kind of audio services.

>>>> This patch adds the missing power domain support in rpmsg core.

>>>>

>>>> Without this patch attempting to play audio via QDSP on DB820c would

>>>> reboot the system.

>>>>

>>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

>>>> ---

>>>>  drivers/rpmsg/rpmsg_core.c | 7 +++++++

>>>>  1 file changed, 7 insertions(+)

>>>>

>>>> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c

>>>> index b714a543a91d..8122807db380 100644

>>>> --- a/drivers/rpmsg/rpmsg_core.c

>>>> +++ b/drivers/rpmsg/rpmsg_core.c

>>>> @@ -15,6 +15,7 @@

>>>>  #include <linux/module.h>

>>>>  #include <linux/rpmsg.h>

>>>>  #include <linux/of_device.h>

>>>> +#include <linux/pm_domain.h>

>>>>  #include <linux/slab.h>

>>>>  

>>>>  #include "rpmsg_internal.h"

>>>> @@ -449,6 +450,10 @@ static int rpmsg_dev_probe(struct device *dev)

>>>>  	struct rpmsg_endpoint *ept = NULL;

>>>>  	int err;

>>>>  

>>>> +	err = dev_pm_domain_attach(dev, true);

>>>> +	if (err)

>>>> +		goto out;

>>>

>>> This patch has broken the virtio-rpmsg stack based rpmsg devices. These

>>> devices are non-DT and the rpmsg_dev_probe() is now failing with -19

>>> (-ENODEV) error code.

>>>

>>> Loic, Arnaud,

>>> Can one of you double-confirm this behavior on ST platforms as well?

>>> The patch came through 4.14.71 stable release and broke our downstream

>>> kernels.

> 

>> I do not reproduce issue on 4.14.0 + your patch,  on my ST platform (not

>> able to test on the LTS branch).

>> By looking the source code, seems that your issue is related to

>> genpd_dev_pm_attach that returns -ENODEV because you have

>> no remoteproc device node...

>> Do you have a rproc node defined in your DT? we have one.

> 

> The dev here is not the remoteproc dev but the rpmsg device which does

> not have an OF node, and so I get the ENODEV due to the check on

> dev->of_node in genpd_dev_pm_attach. I do have DT remoteproc nodes.

> 

> Are you publishing an rpmsg device from the remote side and are they

> being probed? Issue is seen only during the probe phase, you will not

> see any errors if the rpmsg devices are just being published and created.

Sorry something was wrong in my fw used yesterday for testing...so
forget my previous status.

I re-tested it and I have the same issue (-ENODEV) with rpmsg device
probed on ns announcement rpmsg from remote side.
So we confirm the issue.

Regards
Arnaud

> 

> regards

> Suman

> 

>>

>> Regards

>> Arnaud

>>

>>>

>>> regards

>>> Suman

>>>

>>>> +

>>>>  	if (rpdrv->callback) {

>>>>  		strncpy(chinfo.name, rpdev->id.name, RPMSG_NAME_SIZE);

>>>>  		chinfo.src = rpdev->src;

>>>> @@ -490,6 +495,8 @@ static int rpmsg_dev_remove(struct device *dev)

>>>>  

>>>>  	rpdrv->remove(rpdev);

>>>>  

>>>> +	dev_pm_domain_detach(dev, true);

>>>> +

>>>>  	if (rpdev->ept)

>>>>  		rpmsg_destroy_ept(rpdev->ept);

>>>>  

>>>>

>>>

>

Patch

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index b714a543a91d..8122807db380 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -15,6 +15,7 @@ 
 #include <linux/module.h>
 #include <linux/rpmsg.h>
 #include <linux/of_device.h>
+#include <linux/pm_domain.h>
 #include <linux/slab.h>
 
 #include "rpmsg_internal.h"
@@ -449,6 +450,10 @@  static int rpmsg_dev_probe(struct device *dev)
 	struct rpmsg_endpoint *ept = NULL;
 	int err;
 
+	err = dev_pm_domain_attach(dev, true);
+	if (err)
+		goto out;
+
 	if (rpdrv->callback) {
 		strncpy(chinfo.name, rpdev->id.name, RPMSG_NAME_SIZE);
 		chinfo.src = rpdev->src;
@@ -490,6 +495,8 @@  static int rpmsg_dev_remove(struct device *dev)
 
 	rpdrv->remove(rpdev);
 
+	dev_pm_domain_detach(dev, true);
+
 	if (rpdev->ept)
 		rpmsg_destroy_ept(rpdev->ept);