Re: [PATCH v4 01/19] dt-bindings: mfd: mediatek: mt6397: Add accdet subnode

[Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx>]

On Fri, Mar 07, 2025 at 08:11:26AM +0100, Krzysztof Kozlowski wrote:
> On 06/03/2025 13:19, Nícolas F. R. A. Prado wrote:
> >>>    It is interfaced to host controller using SPI interface by a proprietary hardware
> >>>    called PMIC wrapper or pwrap. MT6397/MT6323 PMIC is a child device of pwrap.
> >>> @@ -224,6 +225,30 @@ properties:
> >>>      description:
> >>>        Pin controller
> >>>  
> >>> +  accdet:
> >>> +    type: object
> >>> +    additionalProperties: false
> >>> +    description:
> >>> +      The Accessory Detection module found on the PMIC allows detecting audio
> >>> +      jack insertion and removal, as well as identifying the type of events
> >>> +      connected to the jack.
> >>> +
> >>> +    properties:
> >>> +      compatible:
> >>> +        const: mediatek,mt6359-accdet
> >>
> >> You just removed the other file, no folding happened here. Drop the
> >> accdet node and fold this into parent.
> > 
> > Sorry, I'm still not sure what you mean by folding here then. Right now the
> > accdet is a subnode of the PMIC. If you want me to remove the accdet node, where
> 
> Yes
> 
> > would its compatible and property go?
> 
> compatible: nowhere, because it is close to redundancy.
> 
> property: to the parent pmic node.
> 
>     pmic {
>         compatible = "mediatek,mt6359";
>         interrupt-controller;
>         #interrupt-cells = <2>;
> 
>         mediatek,hp-eint-high;
>     };

I'm not sure that's right. The ACCDET submodule does have some resources, IRQs,
that it registers in its mfd cell, see patch 2 of this series [1]. It also has
its own driver (sound/soc/codecs/mt6359-accdet.c) that probes based on this
compatible and handles those interrupts. Why would it not get its own node like
the other MFD cells?

[1] https://lore.kernel.org/all/20250305-mt6359-accdet-dts-v4-2-e5ffa5ee9991@xxxxxxxxxxxxx

Thanks,
Nícolas