[v2] mt8395/mt6359: Fix several dtb-check errors

[PATCH v2 1/3] dt-bindings: mfd: mediatek: mt6397: Add #sound-dai-cells property

Posted by Julien Massot 9 months ago

The 'mt6359.dtsi' file already uses the '#sound-dai-cells' property.
Add the corresponding property to the binding to fix the following
dtb-check error:

mediatek/mt8395-radxa-nio-12l.dtb: pmic: '#sound-dai-cells', 'mt6359rtc' do not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml#

Fixes: 9bc8353be720 ("arm64: dts: mt6359: Add #sound-dai-cells property")
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
index 6a89b479d10fad3c8b61cab5a3af1453baca4d1a..9580c4ec1ae00f1dd1182357d8b0a5035a1b7f82 100644
--- a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
+++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
@@ -30,6 +30,9 @@ description: |
   See the following for pwrap node definitions:
   Documentation/devicetree/bindings/soc/mediatek/mediatek,pwrap.yaml
 
+allOf:
+  - $ref: /schemas/sound/dai-common.yaml#
+
 properties:
   compatible:
     oneOf:
@@ -53,6 +56,9 @@ properties:
   "#interrupt-cells":
     const: 2
 
+  '#sound-dai-cells':
+    const: 1
+
   rtc:
     type: object
     $ref: /schemas/rtc/rtc.yaml#

-- 
2.49.0

Re: [PATCH v2 1/3] dt-bindings: mfd: mediatek: mt6397: Add #sound-dai-cells property

Posted by Krzysztof Kozlowski 8 months, 3 weeks ago

On Wed, May 14, 2025 at 10:19:56AM GMT, Julien Massot wrote:
> The 'mt6359.dtsi' file already uses the '#sound-dai-cells' property.
> Add the corresponding property to the binding to fix the following
> dtb-check error:
> 
> mediatek/mt8395-radxa-nio-12l.dtb: pmic: '#sound-dai-cells', 'mt6359rtc' do not match any of the regexes: 'pinctrl-[0-9]+'
> from schema $id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml#

If this is a random drive-by, would be fine, but if that's your platform
which you should know, then I expect this to be a real reason instead
something which can easily be rejected with: what if DTS is wrong?

I could not find the ASoC driver for that compatible and quick glance to
MFD shown me no usage of dai cells, so you need proper explanation here.

Especially, that there is a subnode audio-codec, so adding dai cells to
the parent node feels just wrong. One is wrong - either subnode or
parent is a codec.

> 
> Fixes: 9bc8353be720 ("arm64: dts: mt6359: Add #sound-dai-cells property")
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
>  Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> index 6a89b479d10fad3c8b61cab5a3af1453baca4d1a..9580c4ec1ae00f1dd1182357d8b0a5035a1b7f82 100644
> --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> @@ -30,6 +30,9 @@ description: |
>    See the following for pwrap node definitions:
>    Documentation/devicetree/bindings/soc/mediatek/mediatek,pwrap.yaml
>  
> +allOf:
> +  - $ref: /schemas/sound/dai-common.yaml#
> +
>  properties:
>    compatible:
>      oneOf:
> @@ -53,6 +56,9 @@ properties:
>    "#interrupt-cells":
>      const: 2
>  
> +  '#sound-dai-cells':
> +    const: 1

Also extend the example, if there is one for such device.

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: mfd: mediatek: mt6397: Add #sound-dai-cells property

Posted by Nícolas F. R. A. Prado 8 months, 2 weeks ago

On Thu, May 22, 2025 at 09:52:07AM +0200, Krzysztof Kozlowski wrote:
> On Wed, May 14, 2025 at 10:19:56AM GMT, Julien Massot wrote:
> > The 'mt6359.dtsi' file already uses the '#sound-dai-cells' property.
> > Add the corresponding property to the binding to fix the following
> > dtb-check error:
> > 
> > mediatek/mt8395-radxa-nio-12l.dtb: pmic: '#sound-dai-cells', 'mt6359rtc' do not match any of the regexes: 'pinctrl-[0-9]+'
> > from schema $id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml#
> 
> If this is a random drive-by, would be fine, but if that's your platform
> which you should know, then I expect this to be a real reason instead
> something which can easily be rejected with: what if DTS is wrong?
> 
> I could not find the ASoC driver for that compatible and quick glance to
> MFD shown me no usage of dai cells, so you need proper explanation here.
> 
> Especially, that there is a subnode audio-codec, so adding dai cells to
> the parent node feels just wrong. One is wrong - either subnode or
> parent is a codec.

The driver is sound/soc/codecs/mt6359.c, which defines 2 DAIs. It's currently
probed by the MFD through the driver name, hence the parent MFD device is the
one used to register the audio component.

The right fix would probably be to add the compatible both to the ASoC and the
MFD drivers, and then move the dai-cells to the sound subnode in DT.

Thanks,
Nícolas

Re: [PATCH v2 1/3] dt-bindings: mfd: mediatek: mt6397: Add #sound-dai-cells property

Posted by Krzysztof Kozlowski 8 months, 1 week ago

On 29/05/2025 17:53, Nícolas F. R. A. Prado wrote:
> On Thu, May 22, 2025 at 09:52:07AM +0200, Krzysztof Kozlowski wrote:
>> On Wed, May 14, 2025 at 10:19:56AM GMT, Julien Massot wrote:
>>> The 'mt6359.dtsi' file already uses the '#sound-dai-cells' property.
>>> Add the corresponding property to the binding to fix the following
>>> dtb-check error:
>>>
>>> mediatek/mt8395-radxa-nio-12l.dtb: pmic: '#sound-dai-cells', 'mt6359rtc' do not match any of the regexes: 'pinctrl-[0-9]+'
>>> from schema $id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml#
>>
>> If this is a random drive-by, would be fine, but if that's your platform
>> which you should know, then I expect this to be a real reason instead
>> something which can easily be rejected with: what if DTS is wrong?
>>
>> I could not find the ASoC driver for that compatible and quick glance to
>> MFD shown me no usage of dai cells, so you need proper explanation here.
>>
>> Especially, that there is a subnode audio-codec, so adding dai cells to
>> the parent node feels just wrong. One is wrong - either subnode or
>> parent is a codec.
> 
> The driver is sound/soc/codecs/mt6359.c, which defines 2 DAIs. It's currently
> probed by the MFD through the driver name, hence the parent MFD device is the

It does not matter whether MFD or simple bus instantiates a device. Really.

> one used to register the audio component.

I see codecs/mt6359.c registering the component, not the parent MFD
device. This change does not look right but maybe the binding needs
fixes as well.



Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: mfd: mediatek: mt6397: Add #sound-dai-cells property

Posted by AngeloGioacchino Del Regno 9 months ago

Il 14/05/25 10:19, Julien Massot ha scritto:
> The 'mt6359.dtsi' file already uses the '#sound-dai-cells' property.
> Add the corresponding property to the binding to fix the following
> dtb-check error:
> 
> mediatek/mt8395-radxa-nio-12l.dtb: pmic: '#sound-dai-cells', 'mt6359rtc' do not match any of the regexes: 'pinctrl-[0-9]+'
> from schema $id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml#
> 
> Fixes: 9bc8353be720 ("arm64: dts: mt6359: Add #sound-dai-cells property")
> Signed-off-by: Julien Massot <julien.massot@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>