On 02/06/2022 13:53, Tinghan Shen wrote: > Hi Krzysztof, > > On Thu, 2022-06-02 at 12:45 +0200, Krzysztof Kozlowski wrote: >> On 02/06/2022 12:19, Tinghan Shen wrote: >>> Hi Krzysztof, >>> >>> On Thu, 2022-06-02 at 09:40 +0200, Krzysztof Kozlowski wrote: >>>> On 02/06/2022 08:44, Tinghan Shen wrote: >>>>>>> + mbox-names: >>>>>>> + items: >>>>>>> + - const: mbox0 >>>>>>> + - const: mbox1 >>>>>> >>>>>> These should be rather some meaningful names, e.g. "rx" and "tx". >>>>> >>>>> The mbox name has to align with the adsp ipc driver. >>>>> The adsp ipc driver is using 'mbox%d' for mailbox channels. >>>>> >>>>> >>>>> >>> >>> > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?id=9db69df4bdd37eb1f65b6931ee067fb15b9a4d5c__;!!CTRNKA9wMg0ARbw!1TmempNkQhC5QuLBhyfWo_AC97MoLuWipsGV-LPaW9RKNPheU7Bgc-eboNi1JA1nC5I$ >>>>> >>>>> >>>>> chan_name = kasprintf(GFP_KERNEL, "mbox%d", i); >>>>> >>>>> /* ...snip... */ >>>>> >>>>> adsp_chan->ch = mbox_request_channel_byname(cl, chan_name); >>>>> >>>>> Is it ok to continue using these names? >>>> >>>> It is a bit confusing... how did that driver got merged recently without >>>> bindings? Why bindings are separate? >>>> >>>> The bindings always come together in one patchset with the driver >>>> implementing them. Bindings are though a separate patch, yet still >>>> followed by the driver which uses them. >>>> >>>> I do not see any compatibles in that driver, which suggests there is no >>>> other binding using it. If that's correct, then you need to change the >>>> driver. >>>> >>> >>> The mtk-adsp-ipc driver's sole function is to encapsulate the operations >>> of mailbox framework from adsp ipc users. The mtk-adsp-ipc is not defined >>> in the dts file and we don't need it to be defined. The creation of mtk-adsp-ipc >>> device is requested by adsp ipc users via the use of 'platform_device_register_data'[1]. >>> >>> the driver implemented the mailbox framework is 'mtk-adsp-mailbox'[2]. it has >>> corresponding hardwares and a yaml file[3] to describe it. >> >> I don't understand how is this related. We talk here about the >> mbox-names for this bindings file. You replied, that these bindings are >> already used by something, but now you say that they are not? So why do >> you need to change anything in any driver? >> >> Simple question - do the bindings here "add mt8186 dsp document" are >> used by any specific Linux driver already? > > This bindings, 'add mt8186 dsp document', are used by the SOF sound driver of MT8186[1]. > > I'm sorry for miss leading you in previous reply. I was thought that you're > asking why the mtk-adsp-ipc driver got merged without bindings. So, I tried > to explain why mtk-adsp-ipc doesn't have bindings. Then my question is kind of still valid: How did "mt8186 SOF" driver got merged recently without bindings? Why bindings are separate? You cannot just sneak in usage of bindings in a driver, then submit bindings and say "we already have an user!". No, the bindings come with the driver. Always. Linked patch [1] brings undocumented compatible mediatek,mt8186-dsp, so you should see big fat warning when running checkpatch. So this points that you did not run checkpatch which is another not acceptable submission. :( [1] https://lore.kernel.org/all/20220422055659.8738-2-tinghan.shen@xxxxxxxxxxxx/ Best regards, Krzysztof
