linux-kernel - Re: [PATCH v6 06/17] dt-bindings: mfd: amd,pensando-elbasr: Add AMD Pensando Elba System Resource chip

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]

Message-ID: <936c5623-07b9-c49d-e113-fc056a4eb9bc@linaro.org>
Date:   Thu, 1 Sep 2022 10:20:12 +0300
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     "Larson, Bradley" <Bradley.Larson@....com>,
        Rob Herring <robh@...nel.org>, Brad Larson <brad@...sando.io>
Cc:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "adrian.hunter@...el.com" <adrian.hunter@...el.com>,
        "alcooperx@...il.com" <alcooperx@...il.com>,
        "andy.shevchenko@...il.com" <andy.shevchenko@...il.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "brijeshkumar.singh@....com" <brijeshkumar.singh@....com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "gsomlo@...il.com" <gsomlo@...il.com>,
        "gerg@...ux-m68k.org" <gerg@...ux-m68k.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "lee.jones@...aro.org" <lee.jones@...aro.org>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "yamada.masahiro@...ionext.com" <yamada.masahiro@...ionext.com>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "piotrs@...ence.com" <piotrs@...ence.com>,
        "p.yadav@...com" <p.yadav@...com>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "samuel@...lland.org" <samuel@...lland.org>,
        "fancer.lancer@...il.com" <fancer.lancer@...il.com>,
        "Suthikulpanit, Suravee" <Suravee.Suthikulpanit@....com>,
        "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "will@...nel.org" <will@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v6 06/17] dt-bindings: mfd: amd,pensando-elbasr: Add AMD
 Pensando Elba System Resource chip

On 01/09/2022 02:01, Larson, Bradley wrote:
> 
>>> +  is implemented by a sub-device reset-controller which accesses
>>> +  a CS0 control register.
>>> +
>>> +maintainers:
>>> +  - Brad Larson <blarson@....com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - amd,pensando-elbasr
>>> +
>>> +  spi-max-frequency:
>>> +    description: Maximum SPI frequency of the device in Hz.
>> No need for generic descriptions of common properties.
> 
> Changed to "spi-max-frequency: true" and moved to end of properties.

Then you should rather reference spi-peripheral-props just like other
SPI devices.

> 
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  '#address-cells':
>>> +    const: 1
>>> +
>>> +  '#size-cells':
>>> +    const: 0
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - spi-max-frequency
>>> +
>>> +patternProperties:
>>> +  '^reset-controller@[a-f0-9]+$':
>>> +    $ref: /schemas/reset/amd,pensando-elbasr-reset.yaml
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +    spi {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +        num-cs = <4>;
>>> +
>>> +        sysc: system-controller@0 {
>>> +            compatible = "amd,pensando-elbasr";
>>> +            reg = <0>;
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>> +            spi-max-frequency = <12000000>;
>>> +
>>> +            rstc: reset-controller@0 {
>>> +                compatible = "amd,pensando-elbasr-reset";
>>> +                reg = <0>;
>> What does 0 represent here? A register address within 'elbasr' device?
> 
> Removed, I recall a check threw a warning or error without reg.
> 
>> Why do you need a child node for this? Are there other sub-devices and
>> your binding is incomplete? Just put '#reset-cells' in the parent.
> 
> Without a reset-controller node and booting the function 
> __of_reset_control_get(...) fails to find a match in the list here

That's not actually the answer to the question. There was no concerns
whether you need or not reset controller. The question was why do you
need a child device instead of elbasr being the reset controller.

Your answer does not cover this at all, so again - why do you need a
child for this?

Best regards,
Krzysztof