linux-kernel - Re: [PATCH v1 3/6] dt-bindings: hwmon: Add hpe,gxp-fan-ctrl

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:   Mon, 7 Nov 2022 22:36:48 +0000
From:   "Hawkins, Nick" <nick.hawkins@....com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        "jdelvare@...e.com" <jdelvare@...e.com>,
        "linux@...ck-us.net" <linux@...ck-us.net>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "Verdun, Jean-Marie" <verdun@....com>,
        "corbet@....net" <corbet@....net>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v1 3/6] dt-bindings: hwmon: Add hpe,gxp-fan-ctrl


    > > This provides the base registers address, programmable logic registers
    > > address, and the function 2 registers to allow control access of the HPE
    > > fans on the GXP SoC.

    > What is "This"? If "This patch", then drop it.
    > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

    > If "This hardware" then please instead describe the hardware, not it
    components. What are its features? If it controls the fan, then why
    there are no PWM-related cells? How do you set the speed?

Greetings Krzysztof,

    Thank you for the feedback. The intention was this binding.. however, that was an error on my part, and I will correct it to reflect the hardware situation of the GXP with the fan controller and how each of the mapped registers provide control to the system. To answer your questions: The fans speeds are controlled through an external CPLD device which we provide a PWM value (0-255) using the "base" register to the CIF interface. This interface provides access to the CPLD. The CPLD then drives the fan. The CPLD can generate up to 8 unique different PWMs to multiple fans. The CPLD monitors the fans and reports the status back to the SoC through the CIF interface to the "plreg base". The plreg includes the installation, failed, and identification statuses. The function 2 register base is used to check the power state of the system as that influences the PWM values read back.

As the PWM generation happens outside the SoC do we still need pwm-cells? If so, should we have a custom compatible for that?

Thanks,

-Nick