| 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
| ||
|
Message-ID: <20240213222808.GA2490812-robh@kernel.org>
Date: Tue, 13 Feb 2024 16:28:08 -0600
From: Rob Herring <robh@...nel.org>
To: Manojkiran Eda <manojkiran.eda@...il.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Joel Stanley <joel@....id.au>,
Andrew Jeffery <andrew@...econstruct.com.au>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Richard Weinberger <richard@....at>,
Vignesh Raghavendra <vigneshr@...com>, jk@...econstruct.com.au,
Patrick Rudolph <patrick.rudolph@...ements.com>,
Chia-Wei Wang <chiawei_wang@...eedtech.com>,
Ryan Chen <ryan_chen@...eedtech.com>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-aspeed@...ts.ozlabs.org,
linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org,
openbmc@...ts.ozlabs.org, zev@...ilderbeest.net
Subject: Re: [PATCH] Add eSPI device driver (flash channel)
On Tue, Feb 13, 2024 at 08:06:08PM +0530, Manojkiran Eda wrote:
> This patch adds the driver support for the eSPI controller of
> Aspeed 5/6th generation SoCs. This controller is a slave device
> communicating with a master over Enhanced Serial Peripheral
> Interface (eSPI).
>
> eSPI supports 4 channels, namely peripheral, virtual wire,
> out-of-band, and flash, and operates at max frequency of 66MHz.
>
> But at the moment, this patch set only supports the flash channel.
You're not going to need binding changes to add support for those,
right?
>
> Signed-off-by: Manojkiran Eda <manojkiran.eda@...il.com>
> ---
> Hello everyone,
>
> I'm presenting a revised version of the eSPI device driver patch series found at the following link:
>
> https://lore.kernel.org/openbmc/20220516005412.4844-1-chiawei_wang@aspeedtech.com/
>
> This update addresses the issues identified during the review process.
>
> While the previous patch series attempted to incorporate support for all four different channels of eSPI,
> this new series focuses on upstreaming the flash channel initially, ensuring that all review comments are
> duly addressed, before progressing further.
>
> Results:
>
> Successfully conducted a flash update via eSPI.
>
> Note:
>
> This marks my inaugural endeavor in contributing code to the kernel subsystem. I kindly request reviewers
> to incorporate as many details as possible in the review comments.
Please start with submitting-patches.rst and the DT specific version of
that.
> ---
> .../devicetree/bindings/soc/aspeed/espi.yaml | 125 ++++++
This should be a separate patch. checkpatch.pl will tell you this and
other things.
Filename should match compatible.
> arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 16 +-
This is another patch.
> drivers/mtd/mtdcore.c | 2 +-
Yet another patch. But really, this one will be rejected most likely
unless you can justify why it is needed.
> drivers/soc/aspeed/Kconfig | 10 +
> drivers/soc/aspeed/Makefile | 3 +
> drivers/soc/aspeed/aspeed-espi-ctrl.c | 197 +++++++++
> drivers/soc/aspeed/aspeed-espi-ctrl.h | 169 ++++++++
> drivers/soc/aspeed/aspeed-espi-flash.c | 466 +++++++++++++++++++++
> drivers/soc/aspeed/aspeed-espi-flash.h | 45 ++
> include/uapi/linux/espi/aspeed-espi-ioc.h | 103 +++++
Your own interface to userspace is probably not going to be accepted
either.
> 10 files changed, 1134 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/soc/aspeed/espi.yaml b/Documentation/devicetree/bindings/soc/aspeed/espi.yaml
> new file mode 100644
> index 000000000000..6521a351d18d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/aspeed/espi.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# # Copyright (c) 2021 Aspeed Technology Inc.
It's 2024 now.
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/soc/aspeed/espi.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Aspeed eSPI Controller
> +
> +maintainers:
> + - Manojkiran Eda <manojkiran.eda@...il.com>
> + - Patrick Rudolph <patrick.rudolph@...ements.com>
> + - Chia-Wei Wang <chiawei_wang@...eedtech.com>
> + - Ryan Chen <ryan_chen@...eedtech.com>
> +
> +description:
> + Aspeed eSPI controller implements a slave side eSPI endpoint device
s/slave/device/
> + supporting the four eSPI channels, namely peripheral, virtual wire,
> + out-of-band, and flash.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - aspeed,ast2500-espi
> + - aspeed,ast2600-espi
> + - const: simple-mfd
> + - const: syscon
> +
> + reg:
> + maxItems: 1
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 1
> +
> + ranges: true
> +
> +patternProperties:
> + "^espi-ctrl@[0-9a-f]+$":
> + type: object
Is this really a separate sub-block? As in could it be reused somewhere
else or in a different combination of blocks?
> +
> + description: Control of the four basic eSPI channels
> +
> + properties:
> + compatible:
> + items:
> + - enum:
> + - aspeed,ast2500-espi-ctrl
> + - aspeed,ast2600-espi-ctrl
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + flash,dma-mode:
> + type: boolean
> + description: Enable DMA support for eSPI flash channel
> +
> + flash,safs-mode:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 0, 1, 2 ]
> + default: 0
> + description: Slave-Attached-Sharing-Flash mode, 0->Mix, 1->SW, 2->HW
> +
> + required:
> + - compatible
> + - interrupts
> + - clocks
> +
> + "^espi-mmbi@[0-9a-f]+$":
> + type: object
Is this really a separate sub-block?
> +
> + description: Control of the PCH-BMC data exchange over eSPI peripheral memory cycle
> +
> + properties:
> + compatible:
> + const: aspeed,ast2600-espi-mmbi
> +
> + interrupts:
> + maxItems: 1
> +
> + required:
> + - compatible
> + - interrupts
> +
> +required:
> + - compatible
> + - reg
> + - "#address-cells"
> + - "#size-cells"
> + - ranges
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/ast2600-clock.h>
> +
> + espi: espi@...ee000 {
> + compatible = "aspeed,ast2600-espi", "simple-mfd", "syscon";
> + reg = <0x1e6ee000 0x1000>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x0 0x1e6ee000 0x1000>;
> +
> + espi_ctrl: espi-ctrl@0 {
> + compatible = "aspeed,ast2600-espi-ctrl";
> + reg = <0x0 0x800>;
> + interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_GATE_ESPICLK>;
> + };
> +
> + espi_mmbi: espi-mmbi@800 {
> + compatible = "aspeed,ast2600-espi-mmbi";
> + reg = <0x800 0x50>;
> + interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> + };
> + };
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> index c4d1faade8be..08d7a2689086 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> @@ -453,7 +453,21 @@ video: video@...00000 {
> interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> status = "disabled";
> };
> -
> + espi: espi@...ee000 {
> + compatible = "aspeed,ast2600-espi", "simple-mfd", "syscon";
> + reg = <0x1e6ee000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x0 0x1e6ee000 0x1000>;
> + espi_ctrl: espi-ctrl@0 {
> + compatible = "aspeed,ast2600-espi-ctrl";
> + reg = <0x0 0x800>,<0x0 0x4000000>;
> + reg-names = "espi_ctrl","espi_flash";
> + interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_GATE_ESPICLK>;
> + status = "disabled";
> + };
Wrong indentation.
> + };
> gpio0: gpio@...80000 {
> #gpio-cells = <2>;
> gpio-controller;
Powered by blists - more mailing lists