| Message ID | 20220922103502.49981-1-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: mediatek: mt8195-mt6359: Use snd_soc_pm_ops instead of custom ops | expand |
On Thu, Sep 22, 2022 at 12:35:02PM +0200, AngeloGioacchino Del Regno wrote: > -static const struct dev_pm_ops mt8195_mt6359_pm_ops = { > - .poweroff = snd_soc_poweroff, > - .restore = snd_soc_resume, > -}; > - > static struct platform_driver mt8195_mt6359_driver = { > .driver = { > .name = "mt8195_mt6359", > .of_match_table = mt8195_mt6359_dt_match, > - .pm = &mt8195_mt6359_pm_ops, > + .pm = &snd_soc_pm_ops, snd_soc_pm_ops[1] sets some more fields. I'm not quite sure whether it would introduce any side effect or not. Perhaps Trevor could provide some suggestions. [1]: https://elixir.bootlin.com/linux/v5.19.10/source/sound/soc/soc-core.c#L2150
On Thu, Sep 22, 2022 at 07:26:21PM +0800, Tzung-Bi Shih wrote: > snd_soc_pm_ops[1] sets some more fields. I'm not quite sure whether > it would introduce any side effect or not. Perhaps Trevor could > provide some suggestions. If it does it should be to fix isues rather than introduce new problems - I suspect the other operations just don't work currently.
Il 22/09/22 15:30, Mark Brown ha scritto: > On Thu, Sep 22, 2022 at 07:26:21PM +0800, Tzung-Bi Shih wrote: > >> snd_soc_pm_ops[1] sets some more fields. I'm not quite sure whether >> it would introduce any side effect or not. Perhaps Trevor could >> provide some suggestions. > > If it does it should be to fix isues rather than introduce new > problems - I suspect the other operations just don't work > currently. From my upstream tests, this didn't introduce any issues, that's why I've sent this patch. In any case, let's check with Trevor, just as to be extremely sure, but please use an upstream kernel for eventual tests, as there are quite a bit of changes between 5.10 and current upstream. Cheers, Angelo
On Thu, 2022-09-22 at 16:10 +0200, AngeloGioacchino Del Regno wrote: > Il 22/09/22 15:30, Mark Brown ha scritto: > > On Thu, Sep 22, 2022 at 07:26:21PM +0800, Tzung-Bi Shih wrote: > > > > > snd_soc_pm_ops[1] sets some more fields. I'm not quite sure > > > whether > > > it would introduce any side effect or not. Perhaps Trevor could > > > provide some suggestions. > > > > If it does it should be to fix isues rather than introduce new > > problems - I suspect the other operations just don't work > > currently. > > From my upstream tests, this didn't introduce any issues, that's why > I've > sent this patch. > > In any case, let's check with Trevor, just as to be extremely sure, > but please > use an upstream kernel for eventual tests, as there are quite a bit > of changes > between 5.10 and current upstream. > > Cheers, > Angelo I think it's not a big risk if Angelo already did the test and snd_soc_pm_ops is also used in MT8186. I can help do more tests on 5.10 when the patch is back to chromium in the future. MTK also have a plan to support complete suspend/resume functionality in MT8195. If Tzung-bi has concerns about the patch, I can help submit the patch at the time. Thanks, Trevor
On Fri, Sep 23, 2022 at 12:39:54PM +0800, Trevor Wu wrote: > On Thu, 2022-09-22 at 16:10 +0200, AngeloGioacchino Del Regno wrote: > > Il 22/09/22 15:30, Mark Brown ha scritto: > > > On Thu, Sep 22, 2022 at 07:26:21PM +0800, Tzung-Bi Shih wrote: > > > > > > > snd_soc_pm_ops[1] sets some more fields. I'm not quite sure > > > > whether > > > > it would introduce any side effect or not. Perhaps Trevor could > > > > provide some suggestions. > > > > > > If it does it should be to fix isues rather than introduce new > > > problems - I suspect the other operations just don't work > > > currently. > > > > From my upstream tests, this didn't introduce any issues, that's why > > I've > > sent this patch. > > > > In any case, let's check with Trevor, just as to be extremely sure, > > but please > > use an upstream kernel for eventual tests, as there are quite a bit > > of changes > > between 5.10 and current upstream. > > > > Cheers, > > Angelo > > I think it's not a big risk if Angelo already did the test and > snd_soc_pm_ops is also used in MT8186. > I can help do more tests on 5.10 when the patch is back to chromium in > the future. > MTK also have a plan to support complete suspend/resume functionality > in MT8195. If Tzung-bi has concerns about the patch, I can help submit > the patch at the time. I have no further concern as long as you guys are also aware of the change.
On Thu, 22 Sep 2022 12:35:02 +0200, AngeloGioacchino Del Regno wrote: > It is possible to use the standard snd_soc_pm_ops for this card: > remove the custom mt8195_mt6359_pm_ops. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: mediatek: mt8195-mt6359: Use snd_soc_pm_ops instead of custom ops commit: 14ed837b9740cc6ec25910980d67c22894b4ff56 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/sound/soc/mediatek/mt8195/mt8195-mt6359.c b/sound/soc/mediatek/mt8195/mt8195-mt6359.c index 961e769602d6..23bdde6acd1c 100644 --- a/sound/soc/mediatek/mt8195/mt8195-mt6359.c +++ b/sound/soc/mediatek/mt8195/mt8195-mt6359.c @@ -1532,16 +1532,11 @@ static const struct of_device_id mt8195_mt6359_dt_match[] = { {}, }; -static const struct dev_pm_ops mt8195_mt6359_pm_ops = { - .poweroff = snd_soc_poweroff, - .restore = snd_soc_resume, -}; - static struct platform_driver mt8195_mt6359_driver = { .driver = { .name = "mt8195_mt6359", .of_match_table = mt8195_mt6359_dt_match, - .pm = &mt8195_mt6359_pm_ops, + .pm = &snd_soc_pm_ops, }, .probe = mt8195_mt6359_dev_probe, };
It is possible to use the standard snd_soc_pm_ops for this card: remove the custom mt8195_mt6359_pm_ops. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- sound/soc/mediatek/mt8195/mt8195-mt6359.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)