Re: [PATCH v1] dt-bindings: dsp: mediatek: add mt8186 dsp document

From: Krzysztof Kozlowski
Date: Thu Jun 02 2022 - 06:46:02 EST


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?



Best regards,
Krzysztof