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
+
+ 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;
+ };
+ };
+ };
--
2.48.1
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
© 2016 - 2025 Red Hat, Inc.