Messages in this thread | | | Date | Tue, 22 Aug 2023 10:14:39 +0200 | Subject | Re: [PATCH 1/2] dt-bindings: spi: Convert sprd spi bindings to yaml | From | Krzysztof Kozlowski <> |
| |
On 22/08/2023 09:51, Jiansheng Wu wrote: > Convert sprd,sc27xx-pmic.txt to yaml, and add UMP962x series PMICs. > > Signed-off-by: Jiansheng Wu <jiansheng.wu@unisoc.com>
Subject is bogus. This is not SPI.
> --- > .../bindings/mfd/sprd,sc27xx-pmic.txt | 40 --------- > .../bindings/mfd/sprd,sc27xx-pmic.yaml | 84 +++++++++++++++++++ > 2 files changed, 84 insertions(+), 40 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt > create mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml > > diff --git a/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt b/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt > deleted file mode 100644 > index 21b9a897fca5..000000000000 > --- a/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt > +++ /dev/null > @@ -1,40 +0,0 @@ > -Spreadtrum SC27xx Power Management Integrated Circuit (PMIC) > - > -The Spreadtrum SC27xx series PMICs contain SC2720, SC2721, SC2723, SC2730 > -and SC2731. The Spreadtrum PMIC belonging to SC27xx series integrates all > -mobile handset power management, audio codec, battery management and user > -interface support function in a single chip. It has 6 major functional > -blocks: > -- DCDCs to support CPU, memory. > -- LDOs to support both internal and external requirement. > -- Battery management system, such as charger, fuel gauge. > -- Audio codec. > -- User interface function, such as indicator, flash LED and so on. > -- IC level interface, such as power on/off control, RTC and typec and so on. > - > -Required properties: > -- compatible: Should be one of the following: > - "sprd,sc2720" > - "sprd,sc2721" > - "sprd,sc2723" > - "sprd,sc2730" > - "sprd,sc2731" > -- reg: The address of the device chip select, should be 0. > -- spi-max-frequency: Typically set to 26000000. > -- interrupts: The interrupt line the device is connected to. > -- interrupt-controller: Marks the device node as an interrupt controller. > -- #interrupt-cells: The number of cells to describe an PMIC IRQ, must be 2. > -- #address-cells: Child device offset number of cells, must be 1. > -- #size-cells: Child device size number of cells, must be 0. > - > -Example: > -pmic@0 { > - compatible = "sprd,sc2731"; > - reg = <0>; > - spi-max-frequency = <26000000>; > - interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>; > - interrupt-controller; > - #interrupt-cells = <2>; > - #address-cells = <1>; > - #size-cells = <0>; > -}; > diff --git a/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml b/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml > new file mode 100644 > index 000000000000..590970a17143 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml > @@ -0,0 +1,84 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/sprd,sc27xx-pmic.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: | > + Spreadtrum SC27xx and UMP96xx Power Management Integrated Circuit (PMIC) > + > +maintainers: > + - Orson Zhai <orsonzhai@gmail.com> > + - Baolin Wang <baolin.wang7@gmail.com> > + - Chunyan Zhang <zhang.lyra@gmail.com> > + > +description: | > + The Spreadtrum SC27xx series PMICs contain SC2720, SC2721, SC2723, SC2730, SC2731 > + and UMP96xx series PMICs contain ump9620, ump962 and ump9622. The Spreadtrum PMIC > + belonging to SC27xx series and ump962x series integrates all mobile handset power > + management, audio codec, battery management and user interface support function in > + a single chip. It has 6 major functional. > + > +blocks:
Please don't use confusing YAML syntax.
> + - DCDCs to support CPU, memory. > + - LDOs to support both internal and external requirement. > + - Battery management system, such as charger, fuel gauge. > + - Audio codec. > + - User interface function, such as indicator, flash LED and so on. > + - IC level interface, such as power on/off control, RTC and typec and so on. > + > +allOf: > + - $ref: /schemas/spi/sprd,spi-adi.yaml#
This is confusing. How is this device a SPI controller? You have entirely different compatibles, so no, it's wrong. Missing ref to spi-peripheral properties. See other bindings.
> + > +properties: > + compatible: > + enum: > + - sprd,sc2720 > + - sprd,sc2721 > + - sprd,sc2723 > + - sprd,sc2730 > + - sprd,sc2731 > + - sprd,ump9620 > + - sprd,ump9621 > + - sprd,ump9622
This does not match your previous bindings and nothing is explained in commit msg.
> + > + reg: > + maxItems: 7 > + description: The address of the device chip select
7 items? No way.
> + > + spi-max-frequency: > + default: 26000000 > + > + interrupts: true
You need constraints.
> + > + interrupt-controller: > + description: Marks the device node as an interrupt controller.
Drop description. ": true" instead
> + > + interrupt-cells: > + const: 2 > + description: The number of cells to describe an PMIC IRQ, must be 2.
You did not test it.
> + > +required: > + - compatible > + - reg > + - spi-max-frequency > + - '#address-cells' # Child device offset number of cells, must be 1.
??
> + - '#size-cells' # Child device size number of cells, must be 0.
?? No way. You do not have any children. Drop bogus properties and explain it in commit msg.
> + > +unevaluatedProperties: false > + > +Example: > + - | > + adi_bus {
No underscores in node names, generic node names, whatever this supposed to be. I guess spi...
> + pmic@0 { > + compatible = "sprd,sc2731"; > + reg = <0>; > + spi-max-frequency = <26000000>; > + interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-controller; > + #interrupt-cells = <2>;
Test your bindings...
> +...
Best regards, Krzysztof
| |