| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <963d092c-9288-2d38-f677-6d99d3d0bcce@mediatek.com>
Date: Sat, 14 Sep 2024 02:15:50 +0800
From: Macpaul Lin <macpaul.lin@...iatek.com>
To: Rob Herring <robh@...nel.org>
CC: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Matthias Brugger <matthias.bgg@...il.com>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
<linux-leds@...r.kernel.org>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown
<broonie@...nel.org>, Sebastian Reichel <sre@...nel.org>, Pavel Machek
<pavel@....cz>, Sean Wang <sean.wang@...iatek.com>, Lee Jones
<lee@...nel.org>, Alexandre Mergnat <amergnat@...libre.com>, Flora Fu
<flora.fu@...iatek.com>, Bear Wang <bear.wang@...iatek.com>, Pablo Sun
<pablo.sun@...iatek.com>, Macpaul Lin <macpaul@...il.com>, Sen Chu
<sen.chu@...iatek.com>, Chris-qj chen <chris-qj.chen@...iatek.com>, "MediaTek
Chromebook Upstream" <Project_Global_Chrome_Upstream_Group@...iatek.com>,
<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
<linux-mediatek@...ts.infradead.org>, <linux-pm@...r.kernel.org>,
<linux-rtc@...r.kernel.org>, <linux-sound@...r.kernel.org>,
<devicetree@...r.kernel.org>, Chen-Yu Tsai <wenst@...omium.org>
Subject: Re: [PATCH v2 2/7] dt-bindings: mfd: mediatek: mt6397: Convert to DT
schema format
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@...iatek.com>
>> Signed-off-by: Macpaul Lin <macpaul.lin@...iatek.com>
>> ---
>> .../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@mediatek.com/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@...iatek.com>
>> + - Macpaul Lin <macpaul.lin@...iatek.com>
>> +
>> +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
Powered by blists - more mailing lists