| Message ID | 20250305-mt6359-accdet-dts-v4-1-e5ffa5ee9991@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Get mt6359-accdet ready for usage in Devicetree | expand |
On Wed, Mar 05, 2025 at 03:58:16PM -0300, Nícolas F. R. A. Prado wrote: > Describe the accessory detection (accdet) module as a possible subnode > of the MT6359 PMIC. > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > --- > .../devicetree/bindings/mfd/mediatek,mt6397.yaml | 51 ++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > index 6a89b479d10fad3c8b61cab5a3af1453baca4d1a..51012b8bbfaef3df7bdb619a4f8d828d6f9cc15a 100644 > --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > @@ -24,6 +24,7 @@ description: | > - LED > - Keys > - Power controller > + - Accessory Detection > > 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. Best regards, Krzysztof
On Thu, Mar 06, 2025 at 08:57:27AM +0100, Krzysztof Kozlowski wrote: > On Wed, Mar 05, 2025 at 03:58:16PM -0300, Nícolas F. R. A. Prado wrote: > > Describe the accessory detection (accdet) module as a possible subnode > > of the MT6359 PMIC. > > > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > --- > > .../devicetree/bindings/mfd/mediatek,mt6397.yaml | 51 ++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > > index 6a89b479d10fad3c8b61cab5a3af1453baca4d1a..51012b8bbfaef3df7bdb619a4f8d828d6f9cc15a 100644 > > --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > > +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > > @@ -24,6 +24,7 @@ description: | > > - LED > > - Keys > > - Power controller > > + - Accessory Detection > > > > 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 would its compatible and property go? pmic { compatible = "mediatek,mt6359"; interrupt-controller; #interrupt-cells = <2>; accdet { compatible = "mediatek,mt6359-accdet"; mediatek,hp-eint-high; }; }; Thanks, Nícolas
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; }; Best regards, Krzysztof
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@collabora.com Thanks, Nícolas
On 07/03/2025 14:22, Nícolas F. R. A. Prado wrote: > 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 Binding is supposed to be complete, so why suddenly we have here some resources which you did not add? Post complete binding, so you will get proper review. > its own driver (sound/soc/codecs/mt6359-accdet.c) that probes based on this Drivers do not define bindings. > compatible and handles those interrupts. Why would it not get its own node like Sorry, cannot go. You cannot document binding post factum and claim "I have a driver which uses that compatible". This would be a nice way to bypass review. > the other MFD cells? I explained why. I gave you the exact reason. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml index 6a89b479d10fad3c8b61cab5a3af1453baca4d1a..51012b8bbfaef3df7bdb619a4f8d828d6f9cc15a 100644 --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml @@ -24,6 +24,7 @@ description: | - LED - Keys - Power controller + - Accessory Detection 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 + + mediatek,hp-eint-high: + type: boolean + description: + By default, MT6359's HP_EINT pin is assumed to be pulled high and + connected to a normally open 3.5mm jack. Plug insertion is detected + when the pin is brought low in that case. Add this property if the + behavior should be inverted, for example if a normally closed 3.5mm + jack is used, or if the line is pulled low on open. + + required: + - compatible + required: - compatible - regulators @@ -598,3 +623,29 @@ examples: compatible = "mediatek,mt6397-rtc"; }; }; + - | + #include <dt-bindings/input/input.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + pmic { + compatible = "mediatek,mt6359"; + interrupt-controller; + #interrupt-cells = <2>; + + accdet { + compatible = "mediatek,mt6359-accdet"; + mediatek,hp-eint-high; + }; + + regulators { + compatible = "mediatek,mt6359-regulator"; + + buck_vs1 { + regulator-name = "vs1"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <2200000>; + regulator-enable-ramp-delay = <0>; + regulator-always-on; + }; + }; + };
Describe the accessory detection (accdet) module as a possible subnode of the MT6359 PMIC. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- .../devicetree/bindings/mfd/mediatek,mt6397.yaml | 51 ++++++++++++++++++++++ 1 file changed, 51 insertions(+)