Re: [PATCH v2 2/7] dt-bindings: mfd: mediatek: mt6397: Convert to DT schema format

From: Macpaul Lin
Date: Fri Sep 13 2024 - 14:16:24 EST


On 8/30/24 23:33, Rob Herring wrote:


External email : Please do not click links or open attachments until you have verified the sender or the content.

On Fri, Aug 30, 2024 at 07:07:27PM +0800, Macpaul Lin wrote:
Convert the mfd: mediatek: mt6397 binding to DT schema format.

MT6323/MT6358/MT6397 are PMIC devices with multiple function of
subdevices. They have some variant of the combinations of subdevices
but share a common PMIC design.

New updates in this conversion:
- RTC:
- Convert rtc-mt6397.txt and add it into parent's mt6397 PMIC DT schema.
- regulators:
- Align generic names "regulators" instead of origin names.
- mt6323-regulator: Replace "txt" reference with mt6323-regulaotr.yaml
- mt6358-regulator: Replace "txt" reference with mt6358-regulator.yaml
- mt6397-regulator: Replace "txt" reference with mt6397-reuglator.yaml
- audio-codec:
- Align generic name "audio-codec" for codec and sound subdevices.
- Add "mediatek,dmic-mode" and "Avdd-supply".
- clocks:
- Align generic name "clocks" for clockbuffer subdevices.
- leds:
- Convert leds-mt6323.txt and add it into parent's mt6397 PMIC DT schema.
- keys:
- Add more specific descriptions for power and home keys.
- Add compatible: mediatek,mt6358-keys
- power-controller:
- Add property #power-domain-cells for fixing dt-binding check error.
- Add "Baseband power up" as the explaination of abbrevitation "BBPU".
- pinctrl:
- Align generic name "pinctrl" instead of "pin-controller".

Signed-off-by: Sen Chu <sen.chu@xxxxxxxxxxxx>
Signed-off-by: Macpaul Lin <macpaul.lin@xxxxxxxxxxxx>
---
.../bindings/mfd/mediatek,mt6397.yaml | 1026 +++++++++++++++++
.../devicetree/bindings/mfd/mt6397.txt | 110 --
2 files changed, 1026 insertions(+), 110 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/mt6397.txt

Changes for v1:
- This patch depends on conversion of mediatek,mt6397-regulator.yaml
[1] https://lore.kernel.org/lkml/20240807091738.18387-1-macpaul.lin@xxxxxxxxxxxx/T/

Changes for v2:
- This patch has been made base on linux-next/master git repo.
- Keep the parent and child relationship with mediatek,pwrap in description.
[2] https://lore.kernel.org/all/20240826-slurp-earphone-0d5173923ae8@spud/
- Keep the $ref for regulators since dt_binding_check didn't report any issue
based on linux-next/master repo. - Fix description of mt6397/mt6323 devices, use "power management chip"
instead of "multifunction device"
- Drop unnecessary comments or description according to the review.
- Convert sub-modules to DT Schema:
- RTC, LEDs, power-controllers, regulators
- Drop duplicate sub node name and description for sub-modules
- RTC, Keys
- examples: - drop parent pwrap node
- Add examples from mediatek,mt6323-regulator.yaml
- Add examples from mediatek,mt6358-regulator.yaml
- Add examples from mediatek,mt6397-regulator.yaml
- Complete the examples as could as possible.

diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
new file mode 100644
index 0000000..f5bea33
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
@@ -0,0 +1,1026 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek MT6397/MT6323 Multifunction Device (PMIC)
+
+maintainers:
+ - Sen Chu <sen.chu@xxxxxxxxxxxx>
+ - Macpaul Lin <macpaul.lin@xxxxxxxxxxxx>
+
+description: |
+ MT6397/MT6323 is a power management system chip.
+ Please see the sub-modules below for supported features.
+
+ MT6397/MT6323 is a multifunction device with the following sub modules:
+ - Regulators
+ - RTC
+ - Audio codec
+ - GPIO
+ - Clock
+ - LED
+ - Keys
+ - Power controller
+
+ 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.
+ See the following for pwrap node definitions:
+ Documentation/devicetree/bindings/soc/mediatek/mediatek,pwrap.yaml

[snip]

+ rtc:
+ type: object
+ $ref: /schemas/rtc/rtc.yaml#
+ unevaluatedProperties: false
+ description:
+ MT6397 Real Time Clock.

Blank line

Will fix this in v3 version.


+ properties:
+ compatible:
+ oneOf:
+ - enum:
+ - mediatek,mt6323-rtc
+ - mediatek,mt6331-rtc
+ - mediatek,mt6358-rtc
+ - mediatek,mt6397-rtc
+ - items:
+ - enum:
+ - mediatek,mt6366-rtc
+ - const: mediatek,mt6358-rtc

Blank line between DT properties

Will fix this in v3 version.

+ start-year: true
+ required:
+ - compatible
+
+ regulators:
+ type: object
+ oneOf:
+ - $ref: /schemas/regulator/mediatek,mt6323-regulator.yaml
+ - $ref: /schemas/regulator/mediatek,mt6358-regulator.yaml
+ - $ref: /schemas/regulator/mediatek,mt6397-regulator.yaml
+ unevaluatedProperties: false
+ description:
+ List of child nodes that specify the regulators.
+ properties:
+ compatible:
+ oneOf:
+ - enum:
+ - mediatek,mt6323-regulator
+ - mediatek,mt6358-regulator
+ - mediatek,mt6397-regulator
+ - items:
+ - enum:
+ - mediatek,mt6366-regulator
+ - const: mediatek,mt6358-regulator

You need the references or compatible, but not both. It's more efficient
if you list the compatibles along with a 'additionalProperties: true'.
Otherwise, the referenced schemas have to all be applied and the
matching one will be applied twice.

Also, for compatible here, just use 'contains' and list all possible
compatibles. The exact combinations are enforced in the regulator
schemas.

Both 'addtionalProperties: true' and 'contains' will be added to
v3 version. Since there are different regulator nodes in these
DT Schema but seems no other common nodes, $ref will be added
to if..then.. match rulesfor each compatible.


[snip]

+
+ leds:
+ type: object
+ additionalProperties: false
+ description:

You need '|' or '>' to preserve line breaks.

Will be fixed in v3 version.


+
+ properties:
+ compatible:
+ oneOf:

Only 1 entry, don't need oneOf.


Will be fixed in v3 version.

+ - enum:
+ - mediatek,mt6323-led
+ - mediatek,mt6331-led
+ - mediatek,mt6332-led
+ "#address-cells":
+ const: 1

blank line

Will be fixed in v3 version.

+ "#size-cells":
+ const: 0

blank line. And so on...

Will be fixed in v3 version.

+ reg:
+ description:
+ LED channel number (0..3)
+ minimum: 0
+ maximum: 3

Doesn't use the led binding?

Will be fixed in v3 version: using led bindings.

[snip]

+

+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ mt6323_pmic: pmic {

Drop unused labels.

Will be fixed in v3 version.

+ compatible = "mediatek,mt6323";
+ interrupt-parent = <&pio>;
+ interrupts = <150 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ mt6323_leds: leds {
+ compatible = "mediatek,mt6323-led";
+ #address-cells = <1>;
+ status = "disabled";

Examples shouldn't be disabled.

Will be fixed in v3 version.

[snip]

Thanks for the review.

Regards,
Macpaul Lin