fixup! ASoC: rt711-sdca: fix IO transfer timed out when unloading the module. by shumingfan · Pull Request #2642 · thesofproject/linux

shumingfan · 2020-12-17T06:14:47Z

Due to the SCP_SDCA_INTMASK will be cleared by any reset,
the codec driver wants to set the setting back while the device attached again.
However, the codec driver may go to the suspend mode already at this moment.
The previous version tries to set cache_only=N and bypass the cache, then writes the registers.
There is a problem here that the cache_only feature doesn't set back to 'Y' if the codec suspended.
With this condition, the driver will try to access the registers when unloading the module.
But, the master suspended already and doesn't work for register access properly.
Therefore, this patch uses sdw_write_no_pm instead of regmap_write in the rt711_sdca_update_status().

The second patch also changed API from the regmap to sdw_write/read_no_pm.
Address 0x5c and 0x5d are related to the SDCA interrupt masks.
Regmap should be limited to vendor-specific registers.

bardliao · 2020-12-17T06:50:23Z

+ sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INTMASK1,
 SDW_SCP_SDCA_INTMASK_SDCA_0);
-regmap_write(rt711->regmap, SDW_SCP_SDCA_INTMASK2,
+sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INTMASK2,


We should be really careful to access a register by regmap and the IO function directly. The code looks fine to me since we set the same value here as rt711_sdca_jack_init(), and we don't read it. But I will wait for @plbossart 's comment.

indeed we should not have the INTMASK1/2 registers in regmap, so what's missing is the removal of those registers from the regmap definition.
I think I added this in another PR some time back?

bardliao · 2020-12-22T05:09:29Z

SOFCI TEST

plbossart · 2021-01-11T20:55:25Z

+ sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INTMASK1,
 SDW_SCP_SDCA_INTMASK_SDCA_0);
-regmap_write(rt711->regmap, SDW_SCP_SDCA_INTMASK2,
+sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INTMASK2,


indeed we should not have the INTMASK1/2 registers in regmap, so what's missing is the removal of those registers from the regmap definition.
I think I added this in another PR some time back?

shumingfan · 2021-01-14T08:37:43Z

@plbossart
I modified your patch (923fec7) and added it to this PR.

bardliao

LGTM, thanks @shumingfan Should we explain why we don't use regmap functions for SDW interrupt registers in the commit message when sending these sdca codec driver upstream?

shumingfan · 2021-01-20T06:56:39Z

@plbossart
I modified your patch (923fec7) and added it to this PR.

@plbossart Do you have any comments?

plbossart · 2021-01-20T13:57:38Z

@shumingfan I wasn't sure if you wanted this PR to be merged, the commit messages need work especially the second one that makes a reference to another commit in the SOF tree without explanations.

shumingfan · 2021-01-21T02:12:01Z

@shumingfan I wasn't sure if you wanted this PR to be merged, the commit messages need work especially the second one that makes a reference to another commit in the SOF tree without explanations.

Sorry about that. I will update the commit messages and update this PR.

Due to the SCP_SDCA_INTMASK will be cleared by any reset, the codec driver wants to set the setting back while the device attached again. However, the codec driver may go to the suspend mode already at this moment. The previous version tries to set cache_only=N and bypass the cache, then writes the registers. There is a problem here that the cache_only feature doesn't set back to 'Y' if the codec suspended. With this condition, the driver will try to access the registers when unloading the module. But, the master suspended already and doesn't work for register access properly. Therefore, this patch uses sdw_write_no_pm instead of regmap_write in the rt711_sdca_update_status(). Signed-off-by: Shuming Fan <shumingf@realtek.com>

This patch rewrote from the original commit. thesofproject@923fec7 Address 0x5c and 0x5d are related to the SDCA interrupt masks. Regmap should be limited to vendor-specific registers. There are some differences from the original patch. It did the minor changes in rt711_sdca_interrupt_callback(). When the suspend/resume test, we saw that the driver will not re-do the io_init(). That's why the rt711_sdca_update_status() still set 0x5c/0x5d settings back if the rt711_sdca_jack_init() was called before. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Shuming Fan <shumingf@realtek.com>

shumingfan · 2021-01-21T06:23:21Z

@shumingfan I wasn't sure if you wanted this PR to be merged, the commit messages need work especially the second one that makes a reference to another commit in the SOF tree without explanations.

@plbossart I had updated the commits.

bardliao · 2021-01-21T08:11:25Z

@shumingfan I wasn't sure if you wanted this PR to be merged, the commit messages need work especially the second one that makes a reference to another commit in the SOF tree without explanations.
@plbossart I had updated the commits.

Also, note that the SDCA codec drivers are not upstream yet. So we will squash all these fixups when sending the patch upstream. So the commit message will disappear or we have to rewrite the commit message when squashing

bardliao · 2021-01-22T02:47:45Z

SOFCI TEST

bardliao

LGTM. Let's wait for the CI test result.

plbossart · 2021-01-26T20:36:07Z

@bardliao should we merge? not sure what we are missing. I'd like the SDCA codecs to be submitted upstream, we are near the end of the 5.11 cycle.

bardliao · 2021-01-27T00:38:03Z

The CI pr-device-test failure is NotTested and SKIP. Let's merge it.

bardliao · 2021-01-27T09:21:48Z

@bardliao should we merge? not sure what we are missing. I'd like the SDCA codecs to be submitted upstream, we are near the end of the 5.11 cycle.

@plbossart soundwire: export sdw_write/read_no_pm functions is required for the sdca codec drivers. It is still not merged on upstream yet.

plbossart · 2021-01-27T12:25:33Z

@bardliao can you ping Vinod and Mark on this? was there any objection?

bardliao · 2021-01-27T13:58:07Z

@bardliao can you ping Vinod and Mark on this? was there any objection?

I didn't see any objection. Vinod has pinged Mark on Jan. 6 Re: regmap: sdw: use _no_pm functions in regmap_read/write and I resent the series on 22 Jan [RESEND PATCH v2 0/9] soundwire/regmap: use _no_pm routines

shumingfan requested review from RanderWang, jack-cy-yu and oder-chiou as code owners December 17, 2020 06:14

shumingfan requested review from bardliao and plbossart December 17, 2020 06:15

shumingfan force-pushed the topic/sof-dev branch from db71c8b to 8372f69Compare December 17, 2020 06:37

bardliao reviewed Dec 17, 2020
View reviewed changes

plbossart requested changes Jan 11, 2021
View reviewed changes

shumingfan force-pushed the topic/sof-dev branch from 8372f69 to 797bb07Compare January 14, 2021 08:31

bardliao previously approved these changes Jan 14, 2021
View reviewed changes

RanderWang previously approved these changes Jan 18, 2021
View reviewed changes

shumingfan added 2 commits January 21, 2021 11:16

shumingfan dismissed stale reviews from RanderWang and bardliao via 2915466January 21, 2021 06:15

shumingfan force-pushed the topic/sof-dev branch from 797bb07 to 2915466Compare January 21, 2021 06:15

plbossart approved these changes Jan 21, 2021
View reviewed changes

bardliao approved these changes Jan 22, 2021
View reviewed changes

bardliao merged commit ba56e4d into thesofproject:topic/sof-devJan 27, 2021

bardliao mentioned this pull request Jan 28, 2021
[BUG] IO transfer timed out in kmod test on TGLU-SDW #2723
Closed

Uh oh!

Conversation

shumingfan commented Dec 17, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bardliaoDec 17, 2020

Choose a reason for hiding this comment

Uh oh!

plbossartJan 11, 2021

Choose a reason for hiding this comment

Uh oh!

shumingfanJan 12, 2021

Choose a reason for hiding this comment

Uh oh!

bardliao commented Dec 22, 2020

Uh oh!

plbossartJan 11, 2021

Choose a reason for hiding this comment

Uh oh!

shumingfan commented Jan 14, 2021

Uh oh!

bardliao left a comment

Choose a reason for hiding this comment

Uh oh!

shumingfan commented Jan 20, 2021

Uh oh!

plbossart commented Jan 20, 2021

Uh oh!

shumingfan commented Jan 21, 2021

Uh oh!

shumingfan commented Jan 21, 2021

Uh oh!

bardliao commented Jan 21, 2021

Uh oh!

bardliao commented Jan 22, 2021

Uh oh!

bardliao left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart commented Jan 26, 2021

Uh oh!

bardliao commented Jan 27, 2021

Uh oh!

bardliao commented Jan 27, 2021

Uh oh!

plbossart commented Jan 27, 2021

Uh oh!

bardliao commented Jan 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shumingfan commented Dec 17, 2020•
edited
Loading