Re: [PATCH v11 01/14] dt-bindings: Add PECI subsystem document

[Rob Herring <robh@kernel.org>]

On Wed, Dec 11, 2019 at 11:46:11AM -0800, Jae Hyun Yoo wrote:
> This commit adds PECI subsystem document.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
> Changes since v10:
> - Changed documents format to DT schema format so I dropped all review tags.
>   Please review it again.
> 
>  .../devicetree/bindings/peci/peci-bus.yaml    | 129 ++++++++++++++++++
>  .../devicetree/bindings/peci/peci-client.yaml |  54 ++++++++
>  2 files changed, 183 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/peci/peci-bus.yaml
>  create mode 100644 Documentation/devicetree/bindings/peci/peci-client.yaml
> 
> diff --git a/Documentation/devicetree/bindings/peci/peci-bus.yaml b/Documentation/devicetree/bindings/peci/peci-bus.yaml
> new file mode 100644
> index 000000000000..b085e67089cf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/peci/peci-bus.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/peci/peci-bus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic Device Tree Bindings for PECI bus
> +
> +maintainers:
> +  - Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> +
> +description: |
> +  PECI (Platform Environment Control Interface) is a one-wire bus interface that
> +  provides a communication channel from Intel processors and chipset components
> +  to external monitoring or control devices. PECI is designed to support the
> +  following sideband functions:
> +
> +  * Processor and DRAM thermal management
> +    - Processor fan speed control is managed by comparing Digital Thermal
> +      Sensor (DTS) thermal readings acquired via PECI against the
> +      processor-specific fan speed control reference point, or TCONTROL. Both
> +      TCONTROL and DTS thermal readings are accessible via the processor PECI
> +      client. These variables are referenced to a common temperature, the TCC
> +      activation point, and are both defined as negative offsets from that
> +      reference.
> +    - PECI based access to the processor package configuration space provides
> +      a means for Baseboard Management Controllers (BMC) or other platform
> +      management devices to actively manage the processor and memory power
> +      and thermal features.
> +
> +  * Platform Manageability
> +    - Platform manageability functions including thermal, power, and error
> +      monitoring. Note that platform 'power' management includes monitoring
> +      and control for both the processor and DRAM subsystem to assist with
> +      data center power limiting.
> +    - PECI allows read access to certain error registers in the processor MSR
> +      space and status monitoring registers in the PCI configuration space
> +      within the processor and downstream devices.
> +    - PECI permits writes to certain registers in the processor PCI
> +      configuration space.
> +
> +  * Processor Interface Tuning and Diagnostics
> +    - Processor interface tuning and diagnostics capabilities
> +      (Intel Interconnect BIST). The processors Intel Interconnect Built In
> +      Self Test (Intel IBIST) allows for infield diagnostic capabilities in
> +      the Intel UPI and memory controller interfaces. PECI provides a port to
> +      execute these diagnostics via its PCI Configuration read and write
> +      capabilities.
> +
> +  * Failure Analysis
> +    - Output the state of the processor after a failure for analysis via
> +      Crashdump.
> +
> +  PECI uses a single wire for self-clocking and data transfer. The bus
> +  requires no additional control lines. The physical layer is a self-clocked
> +  one-wire bus that begins each bit with a driven, rising edge from an idle
> +  level near zero volts. The duration of the signal driven high depends on
> +  whether the bit value is a logic '0' or logic '1'. PECI also includes
> +  variable data transfer rate established with every message. In this way, it
> +  is highly flexible even though underlying logic is simple.
> +
> +  The interface design was optimized for interfacing between an Intel
> +  processor and chipset components in both single processor and multiple
> +  processor environments. The single wire interface provides low board
> +  routing overhead for the multiple load connections in the congested routing
> +  area near the processor and chipset components. Bus speed, error checking,
> +  and low protocol overhead provides adequate link bandwidth and reliability
> +  to transfer critical device operating conditions and configuration
> +  information.
> +
> +  PECI subsystem provides single or multiple bus nodes support so each bus can
> +  have one adapter node and multiple device specific client nodes that can be
> +  attached to the PECI bus so each processor client's features can be supported
> +  by the client node through an adapter connection in the bus.
> +
> +properties:
> +  compatible:
> +    const: simple-bus

This is wrong. We already have a schema for this.

What's needed is a peci-bus schema that defines the bus node structure 
and then schemas for the specific controllers and child devices. See 
i2c-controller.yaml for an example.

> +
> +  "#address-cells":
> +    # Required to define bus device control resource address.
> +    const: 1
> +
> +  "#size-cells":
> +    # Required to define bus device control resource address.
> +    const: 1
> +
> +  ranges: true
> +
> +required:
> +  - compatible
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/ast2600-clock.h>
> +    peci: bus@1e78b000 {
> +        compatible = "simple-bus";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0x0 0x1e78b000 0x200>;
> +
> +        peci0: peci-bus@0 {
> +            compatible = "aspeed,ast2600-peci";
> +            reg = <0x0 0x100>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&syscon ASPEED_CLK_GATE_REF0CLK>;
> +            resets = <&syscon ASPEED_RESET_PECI>;
> +            clock-frequency = <24000000>;
> +        };
> +
> +        // Just an example. ast2600 doesn't have a second PECI module actually.
> +        peci1: peci-bus@100 {
> +            compatible = "aspeed,ast2600-peci";
> +            reg = <0x100 0x100>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&syscon ASPEED_CLK_GATE_REF0CLK>;
> +            resets = <&syscon ASPEED_RESET_PECI>;
> +            clock-frequency = <24000000>;
> +        };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/peci/peci-client.yaml b/Documentation/devicetree/bindings/peci/peci-client.yaml
> new file mode 100644
> index 000000000000..fc7c4110e929
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/peci/peci-client.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/peci/peci-client.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic Device Tree Bindings for PECI clients
> +
> +maintainers:
> +  - Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - intel,peci-client
> +
> +  reg:
> +    description: |
> +      Address of a client CPU. According to the PECI specification, client
> +      addresses start from 0x30.

0x30 being the min should be a constraint in the bus schema.

> +    maxItems: 1
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/ast2600-clock.h>
> +    peci: bus@1e78b000 {
> +        compatible = "simple-bus";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0x0 0x1e78b000 0x60>;
> +
> +        peci0: peci-bus@0 {
> +            compatible = "aspeed,ast2600-peci";
> +            reg = <0x0 0x100>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&syscon ASPEED_CLK_GATE_REF0CLK>;
> +            resets = <&syscon ASPEED_RESET_PECI>;
> +            clock-frequency = <24000000>;
> +
> +            peci-client@30 {
> +                compatible = "intel,peci-client";
> +                reg = <0x30>;
> +            };
> +
> +            peci-client@31 {
> +                compatible = "intel,peci-client";
> +                reg = <0x31>;
> +            };
> +        };
> +    };
> +...
> -- 
> 2.17.1
>