Hi Greg,
Recently runtime pm testing on slimbus reveled that its
totally broken on SlimBus ngd drivers.
Below are the fixes to get it back working.
- One of the reason begin incorrect device pointer used for
runtime pm and in some places
- Second one was to do with unable to validate transaction id
which resulted in negative runtime pm count.
- Other fix was to do with resetting dma addresses once ngd
controller is power-cycled.
With all these fixed runtime pm is now fully functional on NGD
controllers.
Currently I marked them all with Cc: <[email protected]>
as these all patches are required to fix runtime pm on SlimBus
NGD controller.
Can you please queue them up for 5.15.
thanks for you help,
srini
Srinivas Kandagatla (4):
slimbus: messaging: start transaction ids from 1 instead of zero
slimbus: messaging: check for valid transaction id
slimbus: ngd: set correct device for pm
slimbus: ngd: reset dma setup during runtime pm
drivers/slimbus/messaging.c | 7 ++++---
drivers/slimbus/qcom-ngd-ctrl.c | 22 +++++++++++++---------
2 files changed, 17 insertions(+), 12 deletions(-)
--
2.21.0
In some usecases transaction ids are dynamically allocated inside
the controller driver after sending the messages which have generic
acknowledge responses. So check for this before refcounting pm_runtime.
Without this we would end up imbalancing runtime pm count by
doing pm_runtime_put() in both slim_do_transfer() and slim_msg_response()
for a single pm_runtime_get() in slim_do_transfer()
Fixes: d3062a210930 ("slimbus: messaging: add slim_alloc/free_txn_tid()")
Cc: <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/slimbus/messaging.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/slimbus/messaging.c b/drivers/slimbus/messaging.c
index 6097ddc43a35..e5ae26227bdb 100644
--- a/drivers/slimbus/messaging.c
+++ b/drivers/slimbus/messaging.c
@@ -131,7 +131,8 @@ int slim_do_transfer(struct slim_controller *ctrl, struct slim_msg_txn *txn)
goto slim_xfer_err;
}
}
-
+ /* Initialize tid to invalid value */
+ txn->tid = 0;
need_tid = slim_tid_txn(txn->mt, txn->mc);
if (need_tid) {
@@ -163,7 +164,7 @@ int slim_do_transfer(struct slim_controller *ctrl, struct slim_msg_txn *txn)
txn->mt, txn->mc, txn->la, ret);
slim_xfer_err:
- if (!clk_pause_msg && (!need_tid || ret == -ETIMEDOUT)) {
+ if (!clk_pause_msg && (txn->tid == 0 || ret == -ETIMEDOUT)) {
/*
* remove runtime-pm vote if this was TX only, or
* if there was error during this transaction
--
2.21.0
During suspend/resume NGD remote instance is power cycled along
with remotely controlled bam dma engine.
So Reset the dma configuration during this suspend resume path
so that we are not dealing with any stale dma setup.
Without this transactions timeout after first suspend resume path.
Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
Cc: <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/slimbus/qcom-ngd-ctrl.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index f3ee8e036372..7040293c2ee8 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -1080,7 +1080,8 @@ static void qcom_slim_ngd_setup(struct qcom_slim_ngd_ctrl *ctrl)
{
u32 cfg = readl_relaxed(ctrl->ngd->base);
- if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN)
+ if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN ||
+ ctrl->state == QCOM_SLIM_NGD_CTRL_ASLEEP)
qcom_slim_ngd_init_dma(ctrl);
/* By default enable message queues */
@@ -1131,6 +1132,7 @@ static int qcom_slim_ngd_power_up(struct qcom_slim_ngd_ctrl *ctrl)
dev_info(ctrl->dev, "Subsys restart: ADSP active framer\n");
return 0;
}
+ qcom_slim_ngd_setup(ctrl);
return 0;
}
@@ -1618,6 +1620,7 @@ static int __maybe_unused qcom_slim_ngd_runtime_suspend(struct device *dev)
struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(dev);
int ret = 0;
+ qcom_slim_ngd_exit_dma(ctrl);
if (!ctrl->qmi.handle)
return 0;
--
2.21.0
As tid is unsigned its hard to figure out if the tid is valid or
invalid. So Start the transaction ids from 1 instead of zero
so that we could differentiate between a valid tid and invalid tids
This is useful in cases where controller would add a tid for controller
specific transfers.
Fixes: d3062a210930 ("slimbus: messaging: add slim_alloc/free_txn_tid()")
Cc: <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/slimbus/messaging.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/slimbus/messaging.c b/drivers/slimbus/messaging.c
index f2b5d347d227..6097ddc43a35 100644
--- a/drivers/slimbus/messaging.c
+++ b/drivers/slimbus/messaging.c
@@ -66,7 +66,7 @@ int slim_alloc_txn_tid(struct slim_controller *ctrl, struct slim_msg_txn *txn)
int ret = 0;
spin_lock_irqsave(&ctrl->txn_lock, flags);
- ret = idr_alloc_cyclic(&ctrl->tid_idr, txn, 0,
+ ret = idr_alloc_cyclic(&ctrl->tid_idr, txn, 1,
SLIM_MAX_TIDS, GFP_ATOMIC);
if (ret < 0) {
spin_unlock_irqrestore(&ctrl->txn_lock, flags);
--
2.21.0
For some reason we ended up using wrong device in some places for pm_runtime calls.
Fix this so that NGG driver can do runtime pm correctly.
Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
Cc: <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/slimbus/qcom-ngd-ctrl.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index c054e83ab636..f3ee8e036372 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -618,7 +618,7 @@ static void qcom_slim_ngd_rx(struct qcom_slim_ngd_ctrl *ctrl, u8 *buf)
(mc == SLIM_USR_MC_GENERIC_ACK &&
mt == SLIM_MSG_MT_SRC_REFERRED_USER)) {
slim_msg_response(&ctrl->ctrl, &buf[4], buf[3], len - 4);
- pm_runtime_mark_last_busy(ctrl->dev);
+ pm_runtime_mark_last_busy(ctrl->ctrl.dev);
}
}
@@ -1257,13 +1257,14 @@ static int qcom_slim_ngd_enable(struct qcom_slim_ngd_ctrl *ctrl, bool enable)
}
/* controller state should be in sync with framework state */
complete(&ctrl->qmi.qmi_comp);
- if (!pm_runtime_enabled(ctrl->dev) ||
- !pm_runtime_suspended(ctrl->dev))
- qcom_slim_ngd_runtime_resume(ctrl->dev);
+ if (!pm_runtime_enabled(ctrl->ctrl.dev) ||
+ !pm_runtime_suspended(ctrl->ctrl.dev))
+ qcom_slim_ngd_runtime_resume(ctrl->ctrl.dev);
else
- pm_runtime_resume(ctrl->dev);
- pm_runtime_mark_last_busy(ctrl->dev);
- pm_runtime_put(ctrl->dev);
+ pm_runtime_resume(ctrl->ctrl.dev);
+
+ pm_runtime_mark_last_busy(ctrl->ctrl.dev);
+ pm_runtime_put(ctrl->ctrl.dev);
ret = slim_register_controller(&ctrl->ctrl);
if (ret) {
@@ -1389,7 +1390,7 @@ static int qcom_slim_ngd_ssr_pdr_notify(struct qcom_slim_ngd_ctrl *ctrl,
/* Make sure the last dma xfer is finished */
mutex_lock(&ctrl->tx_lock);
if (ctrl->state != QCOM_SLIM_NGD_CTRL_DOWN) {
- pm_runtime_get_noresume(ctrl->dev);
+ pm_runtime_get_noresume(ctrl->ctrl.dev);
ctrl->state = QCOM_SLIM_NGD_CTRL_DOWN;
qcom_slim_ngd_down(ctrl);
qcom_slim_ngd_exit_dma(ctrl);
--
2.21.0
On Fri, Aug 06, 2021 at 10:16:35AM +0100, Srinivas Kandagatla wrote:
> Hi Greg,
>
> Recently runtime pm testing on slimbus reveled that its
> totally broken on SlimBus ngd drivers.
>
> Below are the fixes to get it back working.
>
> - One of the reason begin incorrect device pointer used for
> runtime pm and in some places
>
> - Second one was to do with unable to validate transaction id
> which resulted in negative runtime pm count.
>
> - Other fix was to do with resetting dma addresses once ngd
> controller is power-cycled.
>
> With all these fixed runtime pm is now fully functional on NGD
> controllers.
>
> Currently I marked them all with Cc: <[email protected]>
> as these all patches are required to fix runtime pm on SlimBus
> NGD controller.
>
> Can you please queue them up for 5.15.
Why do you want these for 5.15-rc1 when you need them to fix problems in
5.14? Shouldn't they go into 5.14-final?
confused,
greg k-h
On 06/08/2021 14:20, Greg KH wrote:
> On Fri, Aug 06, 2021 at 10:16:35AM +0100, Srinivas Kandagatla wrote:
>> Hi Greg,
>>
>> Recently runtime pm testing on slimbus reveled that its
>> totally broken on SlimBus ngd drivers.
>>
>> Below are the fixes to get it back working.
>>
>> - One of the reason begin incorrect device pointer used for
>> runtime pm and in some places
>>
>> - Second one was to do with unable to validate transaction id
>> which resulted in negative runtime pm count.
>>
>> - Other fix was to do with resetting dma addresses once ngd
>> controller is power-cycled.
>>
>> With all these fixed runtime pm is now fully functional on NGD
>> controllers.
>>
>> Currently I marked them all with Cc: <[email protected]>
>> as these all patches are required to fix runtime pm on SlimBus
>> NGD controller.
>>
>> Can you please queue them up for 5.15.
>
> Why do you want these for 5.15-rc1 when you need them to fix problems in
> 5.14? Shouldn't they go into 5.14-final?
Yes, these should go to other stable trees aswell.
I assumed that Fixes tag will automatically backport those patches.
>
> confused,
Sorry if I confused you.
--srini
>
> greg k-h
>
On Sat, Aug 07, 2021 at 11:48:48AM +0100, Srinivas Kandagatla wrote:
>
>
> On 06/08/2021 14:20, Greg KH wrote:
> > On Fri, Aug 06, 2021 at 10:16:35AM +0100, Srinivas Kandagatla wrote:
> > > Hi Greg,
> > >
> > > Recently runtime pm testing on slimbus reveled that its
> > > totally broken on SlimBus ngd drivers.
> > >
> > > Below are the fixes to get it back working.
> > >
> > > - One of the reason begin incorrect device pointer used for
> > > runtime pm and in some places
> > >
> > > - Second one was to do with unable to validate transaction id
> > > which resulted in negative runtime pm count.
> > >
> > > - Other fix was to do with resetting dma addresses once ngd
> > > controller is power-cycled.
> > >
> > > With all these fixed runtime pm is now fully functional on NGD
> > > controllers.
> > >
> > > Currently I marked them all with Cc: <[email protected]>
> > > as these all patches are required to fix runtime pm on SlimBus
> > > NGD controller.
> > >
> > > Can you please queue them up for 5.15.
> >
> > Why do you want these for 5.15-rc1 when you need them to fix problems in
> > 5.14? Shouldn't they go into 5.14-final?
>
> Yes, these should go to other stable trees aswell.
> I assumed that Fixes tag will automatically backport those patches.
Yes, but that can not happen until they hit Linus's tree, which would
not be until 5.15-rc1. Do you want to delay until that long from now?
How about splitting this into 2 patch series, one that you want to see
get into 5.14-final, and one for 5.15-rc1.
thanks,
greg k-h
On 07/08/2021 14:01, Greg KH wrote:
> On Sat, Aug 07, 2021 at 11:48:48AM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 06/08/2021 14:20, Greg KH wrote:
>>> On Fri, Aug 06, 2021 at 10:16:35AM +0100, Srinivas Kandagatla wrote:
>>>> Hi Greg,
>>>>
>>>> Recently runtime pm testing on slimbus reveled that its
>>>> totally broken on SlimBus ngd drivers.
>>>>
>>>> Below are the fixes to get it back working.
>>>>
>>>> - One of the reason begin incorrect device pointer used for
>>>> runtime pm and in some places
>>>>
>>>> - Second one was to do with unable to validate transaction id
>>>> which resulted in negative runtime pm count.
>>>>
>>>> - Other fix was to do with resetting dma addresses once ngd
>>>> controller is power-cycled.
>>>>
>>>> With all these fixed runtime pm is now fully functional on NGD
>>>> controllers.
>>>>
>>>> Currently I marked them all with Cc: <[email protected]>
>>>> as these all patches are required to fix runtime pm on SlimBus
>>>> NGD controller.
>>>>
>>>> Can you please queue them up for 5.15.
>>>
>>> Why do you want these for 5.15-rc1 when you need them to fix problems in
>>> 5.14? Shouldn't they go into 5.14-final?
>>
>> Yes, these should go to other stable trees aswell.
>> I assumed that Fixes tag will automatically backport those patches.
>
> Yes, but that can not happen until they hit Linus's tree, which would
> not be until 5.15-rc1. Do you want to delay until that long from now?
>
> How about splitting this into 2 patch series, one that you want to see
> get into 5.14-final, and one for 5.15-rc1.
All the patches in these series are fixes so the can go to 5.14-final.
--srini
>
> thanks,
>
> greg k-h
>
On Sat, Aug 07, 2021 at 02:04:39PM +0100, Srinivas Kandagatla wrote:
>
>
> On 07/08/2021 14:01, Greg KH wrote:
> > On Sat, Aug 07, 2021 at 11:48:48AM +0100, Srinivas Kandagatla wrote:
> > >
> > >
> > > On 06/08/2021 14:20, Greg KH wrote:
> > > > On Fri, Aug 06, 2021 at 10:16:35AM +0100, Srinivas Kandagatla wrote:
> > > > > Hi Greg,
> > > > >
> > > > > Recently runtime pm testing on slimbus reveled that its
> > > > > totally broken on SlimBus ngd drivers.
> > > > >
> > > > > Below are the fixes to get it back working.
> > > > >
> > > > > - One of the reason begin incorrect device pointer used for
> > > > > runtime pm and in some places
> > > > >
> > > > > - Second one was to do with unable to validate transaction id
> > > > > which resulted in negative runtime pm count.
> > > > >
> > > > > - Other fix was to do with resetting dma addresses once ngd
> > > > > controller is power-cycled.
> > > > >
> > > > > With all these fixed runtime pm is now fully functional on NGD
> > > > > controllers.
> > > > >
> > > > > Currently I marked them all with Cc: <[email protected]>
> > > > > as these all patches are required to fix runtime pm on SlimBus
> > > > > NGD controller.
> > > > >
> > > > > Can you please queue them up for 5.15.
> > > >
> > > > Why do you want these for 5.15-rc1 when you need them to fix problems in
> > > > 5.14? Shouldn't they go into 5.14-final?
> > >
> > > Yes, these should go to other stable trees aswell.
> > > I assumed that Fixes tag will automatically backport those patches.
> >
> > Yes, but that can not happen until they hit Linus's tree, which would
> > not be until 5.15-rc1. Do you want to delay until that long from now?
> >
> > How about splitting this into 2 patch series, one that you want to see
> > get into 5.14-final, and one for 5.15-rc1.
>
> All the patches in these series are fixes so the can go to 5.14-final.
Then why did you say originally that you wanted them in 5.15?
confused,
greg k-h
On 07/08/2021 14:08, Greg KH wrote:
> On Sat, Aug 07, 2021 at 02:04:39PM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 07/08/2021 14:01, Greg KH wrote:
>>> On Sat, Aug 07, 2021 at 11:48:48AM +0100, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 06/08/2021 14:20, Greg KH wrote:
>>>>> On Fri, Aug 06, 2021 at 10:16:35AM +0100, Srinivas Kandagatla wrote:
>>>>>> Hi Greg,
>>>>>>
>>>>>> Recently runtime pm testing on slimbus reveled that its
>>>>>> totally broken on SlimBus ngd drivers.
>>>>>>
>>>>>> Below are the fixes to get it back working.
>>>>>>
>>>>>> - One of the reason begin incorrect device pointer used for
>>>>>> runtime pm and in some places
>>>>>>
>>>>>> - Second one was to do with unable to validate transaction id
>>>>>> which resulted in negative runtime pm count.
>>>>>>
>>>>>> - Other fix was to do with resetting dma addresses once ngd
>>>>>> controller is power-cycled.
>>>>>>
>>>>>> With all these fixed runtime pm is now fully functional on NGD
>>>>>> controllers.
>>>>>>
>>>>>> Currently I marked them all with Cc: <[email protected]>
>>>>>> as these all patches are required to fix runtime pm on SlimBus
>>>>>> NGD controller.
>>>>>>
>>>>>> Can you please queue them up for 5.15.
>>>>>
>>>>> Why do you want these for 5.15-rc1 when you need them to fix problems in
>>>>> 5.14? Shouldn't they go into 5.14-final?
>>>>
>>>> Yes, these should go to other stable trees aswell.
>>>> I assumed that Fixes tag will automatically backport those patches.
>>>
>>> Yes, but that can not happen until they hit Linus's tree, which would
>>> not be until 5.15-rc1. Do you want to delay until that long from now?
>>>
>>> How about splitting this into 2 patch series, one that you want to see
>>> get into 5.14-final, and one for 5.15-rc1.
>>
>> All the patches in these series are fixes so the can go to 5.14-final.
>
> Then why did you say originally that you wanted them in 5.15?
TBH, I tend to send out SlimBus and nvmem patches only once around
rc5-rc6 time which also includes some minor fixes, and you normally
apply them for next rc1 release.
In this particular case I should have explicitly said to pick them up
for 5.14 next rc.
Do you want me to resend them with proper cover letter? or are you okay
to take them as they are?
> > confused,
My Bad, I will be careful with my wording next time around.
thanks,
--srini
>
> greg k-h
>
On Sat, Aug 07, 2021 at 02:26:11PM +0100, Srinivas Kandagatla wrote:
>
>
> On 07/08/2021 14:08, Greg KH wrote:
> > On Sat, Aug 07, 2021 at 02:04:39PM +0100, Srinivas Kandagatla wrote:
> > >
> > >
> > > On 07/08/2021 14:01, Greg KH wrote:
> > > > On Sat, Aug 07, 2021 at 11:48:48AM +0100, Srinivas Kandagatla wrote:
> > > > >
> > > > >
> > > > > On 06/08/2021 14:20, Greg KH wrote:
> > > > > > On Fri, Aug 06, 2021 at 10:16:35AM +0100, Srinivas Kandagatla wrote:
> > > > > > > Hi Greg,
> > > > > > >
> > > > > > > Recently runtime pm testing on slimbus reveled that its
> > > > > > > totally broken on SlimBus ngd drivers.
> > > > > > >
> > > > > > > Below are the fixes to get it back working.
> > > > > > >
> > > > > > > - One of the reason begin incorrect device pointer used for
> > > > > > > runtime pm and in some places
> > > > > > >
> > > > > > > - Second one was to do with unable to validate transaction id
> > > > > > > which resulted in negative runtime pm count.
> > > > > > >
> > > > > > > - Other fix was to do with resetting dma addresses once ngd
> > > > > > > controller is power-cycled.
> > > > > > >
> > > > > > > With all these fixed runtime pm is now fully functional on NGD
> > > > > > > controllers.
> > > > > > >
> > > > > > > Currently I marked them all with Cc: <[email protected]>
> > > > > > > as these all patches are required to fix runtime pm on SlimBus
> > > > > > > NGD controller.
> > > > > > >
> > > > > > > Can you please queue them up for 5.15.
> > > > > >
> > > > > > Why do you want these for 5.15-rc1 when you need them to fix problems in
> > > > > > 5.14? Shouldn't they go into 5.14-final?
> > > > >
> > > > > Yes, these should go to other stable trees aswell.
> > > > > I assumed that Fixes tag will automatically backport those patches.
> > > >
> > > > Yes, but that can not happen until they hit Linus's tree, which would
> > > > not be until 5.15-rc1. Do you want to delay until that long from now?
> > > >
> > > > How about splitting this into 2 patch series, one that you want to see
> > > > get into 5.14-final, and one for 5.15-rc1.
> > >
> > > All the patches in these series are fixes so the can go to 5.14-final.
> >
> > Then why did you say originally that you wanted them in 5.15?
>
> TBH, I tend to send out SlimBus and nvmem patches only once around rc5-rc6
> time which also includes some minor fixes, and you normally apply them for
> next rc1 release.
>
> In this particular case I should have explicitly said to pick them up for
> 5.14 next rc.
>
> Do you want me to resend them with proper cover letter? or are you okay to
> take them as they are?
Yes, please resend the series, for some reason I didn't think the first
one needed to go to 5.14-final, but I might have been thinking of a
different patch series...
thanks,
greg k-h