Uh oh!
There was an error while loading. Please reload this page.
ASOC: SOF: mediatek: Add ipc support for mt8195 - #3258
Conversation
plbossart
left a comment
There was a problem hiding this comment.
I don't fully follow what you are trying to do with a platform device here...
| priv->ipc_dev = platform_device_register_data(&pdev->dev, "mtk-adsp-ipc", | ||
| PLATFORM_DEVID_NONE, | ||
| pdev, sizeof(*pdev)); |
There was a problem hiding this comment.
where is the platform_device_unregister() done?
There was a problem hiding this comment.
Sorry, I miss this part, will fix this. thanks.
| goto err_adsp_sram_power_off; | ||
| } | ||
| adsp_ipc_set_data(priv->dsp_ipc, priv); |
There was a problem hiding this comment.
i am not sure I get what you are trying to do here, but the drvdata is typically private stuff handled in the scope of a device. Here you are using it at the parent level. is this intentional and by-design?
There was a problem hiding this comment.
YES, this is intentional and by-design. private data is used in mtk adsp IPC IRQ callback func.
There was a problem hiding this comment.
imx design uses a separate driver for IPC communication using the mailbox framework (see drivers/mailbox/imx-mailbox.c and drivers/firmware/imx/imx-dsp.c).
We create a child platform device that will be handled by imx-dsp.c mailbox driver. with ipc_set_data call we pass some information from parent to the child.
I'm not sure if MTK has the mailbox driver though because I havent seen one.
Later edit: Oh, now I have seen the link for the mailbox driver in @kuanhsuncheng comment.
@kuanhsuncheng Please Cc me for this kind of patches or at least sound open firmware mailing list. sound-open-firmware@alsa-project.org
There was a problem hiding this comment.
yes,
There are three patches are waiting for reviewer reply.
mailbox controller driver
https://patchwork.kernel.org/project/linux-mediatek/patch/20211028132756.8251-1-allen-kh.cheng@mediatek.com/
adsp ipc protocol driver (mailbox client interface)
https://patchwork.kernel.org/project/linux-mediatek/patch/20211028132822.8306-1-allen-kh.cheng@mediatek.com/
This PR should wait upstream finished.
There was a problem hiding this comment.
@kuanhsuncheng please mark this PR as draft so that it doesnt get accidentally merged. https://github.blog/changelog/2020-04-08-convert-pull-request-to-draft/
There was a problem hiding this comment.
I resend patches again.
(https://lore.kernel.org/linux-arm-kernel/20211117131222.30541-4-allen-kh.cheng@mediatek.com/T/)
Also cc sound-open-firmware@alsa-project.org
Thanks.
kuanhsuncheng
left a comment
There was a problem hiding this comment.
I update the platform_device_unregister part.
For this patch, I refer to imx design.
Note:
Wait upstream done. this patch should not cause compile error
https://lore.kernel.org/linux-arm-kernel/20211028132655.8136-1-allen-kh.cheng@mediatek.com/t/
| priv->ipc_dev = platform_device_register_data(&pdev->dev, "mtk-adsp-ipc", | ||
| PLATFORM_DEVID_NONE, | ||
| pdev, sizeof(*pdev)); |
There was a problem hiding this comment.
Sorry, I miss this part, will fix this. thanks.
| goto err_adsp_sram_power_off; | ||
| } | ||
| adsp_ipc_set_data(priv->dsp_ipc, priv); |
There was a problem hiding this comment.
YES, this is intentional and by-design. private data is used in mtk adsp IPC IRQ callback func.
cujomalainey
commented
Nov 17, 2021
Did the patch stall getting sent upstream? |
kuanhsuncheng
commented
Nov 18, 2021
I resend the related patches again. (https://lore.kernel.org/linux-arm-kernel/20211117131222.30541-4-allen-kh.cheng@mediatek.com/T/) I think this PR should wait upstream finished. |
lyakh
left a comment
There was a problem hiding this comment.
feel free to discard my comment after you've addressed it if I'm still on vacation at the time
Uh oh!
There was an error while loading. Please reload this page.
kuanhsuncheng
commented
Dec 10, 2021
Hi sirs, The below is the upstream list for other related patches. https://patchwork.kernel.org/project/linux-mediatek/list/?series=593601 I had upstreamed for a while. Could someone help me review those patches ? I would appreciate your help. Many thanks. |
plbossart
commented
Dec 10, 2021
@kuanhsuncheng it's a bit confusing since this PR is still a 'draft'. It's not clear if reviewers should spent time on this if you don't update the status? |
kuanhsuncheng
commented
Dec 13, 2021
Hi @plbossart , This PR is still a draft because I am waiting the below patch list upstream done. https://patchwork.kernel.org/project/linux-mediatek/list/?series=593601 I wonder maybe there is a way can speed up. |
@lgirdwood , @dbaluta@plbossart Hi sirs, I wonder is that ok I take a another PR for below series. https://patchwork.kernel.org/project/linux-mediatek/list/?series=605114 and merge this current PR to sof-dev ? There are other patches for MT8195 needed to merge also. However, they have a ipc API dependency and cannot directly merge. I think maintainer may not have time to rely me. (Upstream series=589151) please kindly give me advice, thanks. |
This patch adds IPC support for mt8195 using mbox. On mt8195 resource, there are two mboxes are used to handle ipc request and reply. We create a mtk-adsp-ipc client device to request mbox controllers. Signed-off-by: Allen-KH Cheng <Allen-KH.Cheng@mediatek.com>
plbossart
commented
May 3, 2022
@kuanhsuncheng is this PR still relevant or can it be closed? |
Hi sirs, We can close this PR. Mark has applied to https://urldefense.com/v3/__https:/git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git__;!!CTRNKA9wMg0ARbw!wU0fgArK_q_tKNK7UqFxsnd2931eAre0YciHrlAxzzQaER8C2tTATfYaiNuALVi85laWXg$ for-next [2/3] ASoC: SOF: mediatek: Add ipc support for mt8195 Thanks, |
This patch adds IPC support for mt8195 using mbox. On mt8195 resource, there are two mboxes are used to handle ipc request and reply. We create a mtk-adsp-ipc client device to request mbox controllers. Signed-off-by: Allen-KH Cheng <Allen-KH.Cheng@mediatek.com> (cherry picked from commit e2a85136b16761f18c181c0483f3d81fe97d7acf https://github.com/kuanhsuncheng/linux.git 8195_dev) PR: thesofproject/linux#3258 BUG=b:189238342 TEST=emerge-cherry sys-kernel/chromeos-kernel-5_10 Signed-off-by: Allen-kh Cheng <allen-kh.cheng@mediatek.corp-partner.google.com> Change-Id: Ie51ff4272eb65940583d1383eea30a036dff50a9 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3271540 Reviewed-by: Li-Yu Yu <aaronyu@google.com> Commit-Queue: Li-Yu Yu <aaronyu@google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3592206 Reviewed-by: YC Hung <yc.hung@mediatek.corp-partner.google.com> Tested-by: Justin Yeh <justin.yeh@mediatek.corp-partner.google.com>
This patch adds IPC support for mt8195 using mbox.
On mt8195 resource, there are two mboxes are used to handle
ipc request and reply.
We create a mtk-adsp-ipc client device to request mbox
controllers.
Signed-off-by: Allen-KH Cheng Allen-KH.Cheng@mediatek.com
Note: there are three patches delivered upstream
https://lore.kernel.org/linux-arm-kernel/20211028132655.8136-1-allen-kh.cheng@mediatek.com/t/