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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 9 Sep 2020 19:03:34 +0300
From:   Cristian Ciocaltea <cristian.ciocaltea@...il.com>
To:     Rob Herring <robh@...nel.org>
Cc:     Lee Jones <lee.jones@...aro.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Sebastian Reichel <sre@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        Andreas Färber <afaerber@...e.de>,
        linux-actions@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
        linux-pm@...r.kernel.org
Subject: Re: [PATCH v2 1/6] dt-bindings: mfd: Add Actions Semi ATC260x PMIC
 binding

Hi Rob,

Thanks for reviewing!

On Tue, Sep 08, 2020 at 03:47:24PM -0600, Rob Herring wrote:
> On Sat, Aug 22, 2020 at 01:19:47AM +0300, Cristian Ciocaltea wrote:
> > Add devicetree binding for Actions Semi ATC260x PMICs.
> > 
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@...il.com>
> > ---
> >  .../bindings/mfd/actions,atc260x.yaml         | 221 ++++++++++++++++++
> >  1 file changed, 221 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/actions,atc260x.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/actions,atc260x.yaml b/Documentation/devicetree/bindings/mfd/actions,atc260x.yaml
> > new file mode 100644
> > index 000000000000..4a55bbe1306e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/actions,atc260x.yaml
> > @@ -0,0 +1,221 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/actions,atc260x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Actions Semi ATC260x Power Management IC bindings
> > +
> > +maintainers:
> > +  - Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > +  - Cristian Ciocaltea <cristian.ciocaltea@...il.com>
> > +
> > +description: |
> > +  ATC260x series PMICs integrates Audio Codec, Power Management, RTC, IR
> > +  and GPIO controller blocks. Currently only the PM related functionalities
> > +  (i.e. regulators and system power-off/reboot) for the ATC2603C and ATC2609A
> > +  chip variants are supported.
> > +  ATC2603C includes 3 programmable DC-DC converters and 9 LDO regulators.
> > +  ATC2609A includes 5 programmable DC-DC converters and 10 LDO regulators.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - actions,atc2603c
> > +      - actions,atc2609a
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  pwrc:
> > +    type: object
> > +    description: |
> > +      The power controller integrated in ATC260x provides system power-off
> > +      and reboot operations.
> 
> No need for this node as there are no properties, just instantiate 
> what's needed in the MFD driver.

My intention was to allow the user specify what functionality in the MFD
shall be enabled. For this particular case, if the 'pwrc' node is not
provided, the power-off/reboot functions will be disabled.

> > +
> > +    properties:
> > +      compatible:
> > +        enum:
> > +          - actions,atc2603c-pwrc
> > +          - actions,atc2609a-pwrc
> > +
> > +    required:
> > +      - compatible
> > +
> > +    additionalProperties: false
> > +
> > +  onkey:
> > +    type: object
> > +    description: |
> > +      Use the ONKEY built into ATC260x PMICs as an input device reporting
> > +      power button status. ONKEY can be used to wakeup from low power
> > +      modes and force a reset on long press.
> > +
> > +    properties:
> > +      compatible:
> > +        enum:
> > +          - actions,atc2603c-onkey
> > +          - actions,atc2609a-onkey
> > +
> > +      actions,reset-time-sec:
> > +        description: |
> > +          Duration in seconds which the key should be kept pressed for device
> > +          to reset automatically. The hardware default is 8. Use 0 to disable
> > +          this functionality.
> > +        enum: [0, 6, 8, 10, 12]
> 
> We already have 'power-off-time-sec' in input.yaml. How about adding 
> 'reset-time-sec' there.

Thanks for the suggestion, I was actually looking for this before
choosing the custom property and just assumed this was not added for
a particular reason.

So I'm going to handle it. Would you like me to send a separate patch
(not part of this series)?

> This could really just be a property in the parent node.

This is similar with 'pwrc': if 'onkey' node is not present, the ONKEY
functionality will not be enabled.

Is there a better/recommended approach to manage this MFD feature
selection?

> > +
> > +    required:
> > +      - compatible
> > +
> > +    additionalProperties: false
> > +
> > +  regulators:
> > +    type: object
> > +    description: |
> > +      List of child nodes specifying the regulators, depending on chip variant:
> > +      * ATC2603C: dcdc[1-3], ldo[1-3,5-8,11], switchldo1
> > +      * ATC2609A: dcdc[0-4], ldo[0-9]
> > +
> > +    properties:
> > +      compatible:
> > +        enum:
> > +          - actions,atc2603c-regulator
> > +          - actions,atc2609a-regulator
> > +
> > +      switchldo1:
> > +        type: object
> > +        $ref: ../regulator/regulator.yaml
> > +
> > +        properties:
> > +          regulator-name: true
> > +          regulator-boot-on: true
> > +          regulator-always-on: true
> > +          regulator-min-microvolt: true
> > +          regulator-max-microvolt: true
> > +          regulator-allow-bypass: true
> > +          regulator-active-discharge: true
> > +
> > +        additionalProperties: false
> > +
> > +    patternProperties:
> > +      "^(dcdc[0-4]|ldo[0-9]|ldo11|switchldo1)-supply$":
> > +        description: ATC260x voltage regulators supplies
> > +
> > +      "^(dcdc[0-4]|ldo[0-9]|ldo11)$":
> > +        type: object
> > +        $ref: ../regulator/regulator.yaml
> > +
> > +        properties:
> > +          regulator-name: true
> > +          regulator-boot-on: true
> > +          regulator-always-on: true
> > +          regulator-min-microvolt: true
> > +          regulator-max-microvolt: true
> > +          regulator-allow-bypass: true
> > +
> > +        additionalProperties: false
> > +
> > +    allOf:
> > +      - if:
> > +          properties:
> > +            compatible:
> > +              contains:
> > +                const: actions,atc2603c-regulator
> > +        then:
> > +          patternProperties:
> > +            "^(dcdc[0,4]|ldo[0,4,9])(-supply)?$": false
> > +
> > +            "^(ldo|dcdc)":
> > +              properties:
> > +                regulator-allow-bypass: false
> > +      - if:
> > +          properties:
> > +            compatible:
> > +              contains:
> > +                const: actions,atc2609a-regulator
> > +        then:
> > +          patternProperties:
> > +            "^(ldo11|switchldo1)(-supply)?$": false
> > +
> > +            "^(dcdc|ldo[3-9])":
> > +              properties:
> > +                regulator-allow-bypass: false
> > +
> > +    required:
> > +      - compatible
> > +
> > +    additionalProperties: false
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    i2c0 {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      pmic@65 {
> > +        compatible = "actions,atc2603c";
> > +        reg = <0x65>;
> > +        interrupt-parent = <&sirq>;
> > +        interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +        pwrc {
> > +          compatible = "actions,atc2603c-pwrc";
> > +        };
> > +
> > +        onkey {
> > +          compatible = "actions,atc2603c-onkey";
> > +          actions,reset-time-sec = <6>;
> > +        };
> > +
> > +        regulators {
> > +          compatible = "actions,atc2603c-regulator";
> > +
> > +          dcdc1-supply = <&reg_5v0>;
> > +          dcdc3-supply = <&reg_5v0>;
> > +          ldo5-supply = <&reg_5v0>;
> > +          switchldo1-supply = <&vcc>;
> > +
> > +          vdd_cpu: dcdc1 {
> > +            regulator-name = "VDD_CPU";
> > +            regulator-min-microvolt = <700000>;
> > +            regulator-max-microvolt = <1400000>;
> > +            regulator-always-on;
> > +          };
> > +
> > +          vcc: dcdc3 {
> > +            regulator-name = "VCC";
> > +            regulator-min-microvolt = <2600000>;
> > +            regulator-max-microvolt = <3300000>;
> > +            regulator-always-on;
> > +          };
> > +
> > +          vcc_3v1: ldo5 {
> > +            regulator-name = "VCC_3V1";
> > +            regulator-min-microvolt = <2600000>;
> > +            regulator-max-microvolt = <3300000>;
> > +          };
> > +
> > +          sd_vcc: switchldo1 {
> > +            regulator-name = "SD_VCC";
> > +            regulator-min-microvolt = <3000000>;
> > +            regulator-max-microvolt = <3300000>;
> > +            regulator-always-on;
> > +            regulator-boot-on;
> > +          };
> > +        };
> > +      };
> > +    };
> > +
> > +...
> > -- 
> > 2.28.0
> > 

Kind regards,
Cristi

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ