Re: [PATCH 1/3] spi: dt-bindings: add doc for Amlogic A113L2 SFC

From: Xianwei Zhao
Date: Wed Aug 13 2025 - 02:15:36 EST


Hi Krzysztof,
Thanks for your reply.

On 2025/8/8 16:03, Krzysztof Kozlowski wrote:
[ EXTERNAL EMAIL ]

On Fri, Aug 08, 2025 at 10:00:34AM +0800, Xianwei Zhao wrote:
From: Feng Chen <feng.chen@xxxxxxxxxxx>

The Flash Controller is derived by adding an SPI path to the original
raw NAND controller. This controller supports two modes: raw mode and
SPI mode. The raw mode has already been implemented in the community,
and the SPI mode is described here.


Subject: drop doc, so just "Add Amlogic foo ..."


Will do.

Add bindings for Amlogic A113L2 SPI Flash Controller.

Signed-off-by: Feng Chen <feng.chen@xxxxxxxxxxx>
Signed-off-by: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
---
.../devicetree/bindings/spi/amlogic,a4-spifc.yaml | 104 +++++++++++++++++++++
1 file changed, 104 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/amlogic,a4-spifc.yaml b/Documentation/devicetree/bindings/spi/amlogic,a4-spifc.yaml
new file mode 100644
index 000000000000..712a17a4b117
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/amlogic,a4-spifc.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2025 Amlogic, Inc. All rights reserved
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/amlogic,a4-spifc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SPI flash controller for Amlogic ARM SoCs
+
+maintainers:
+ - Liang Yang <liang.yang@xxxxxxxxxxx>
+ - Feng Chen <feng.chen@xxxxxxxxxxx>
+ - Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
+
+description:
+ The Amlogic SPI flash controller is an extended version
+ of the Amlogic NAND flash controller. It supports SPI Nor
+ Flash and SPI NAND Flash(where the Host ECC HW engine could
+ be enabled).

Wrap at coding style, 80.


Will do.

+
+allOf:
+ - $ref: /schemas/spi/spi-controller.yaml#
+
+properties:
+ compatible:
+ const: amlogic,a4-spifc
+
+ reg:
+ items:
+ - description: core registers
+ - description: parent clk control registers

Why are you poking to parent node or to clock registers? This looks like
mixing up device address spaces.


The SPIFC bus clock multiplexes EMMC modules, so the corresponding frequency division register is also in EMMC module. The SPIFC and the EMMC modules cannot be used simultaneously.

+
+ reg-names:
+ items:
+ - const: core
+ - const: pclk
+
+ clocks:
+ items:
+ - description: clock gate

Are you sure this is separate clock input to this device?


This clock is used by the APB interface to access the SPIFC registers.

+ - description: clock used for the controller
+ - description: clock used for the SPI bus
+
+ clock-names:
+ items:
+ - const: gate
+ - const: core
+ - const: device
+
+ interrupts:
+ maxItems: 1
+
+ amlogic,sfc-enable-random:
+ description: Enable data scrambling

Why would this be a property of the board?

Will get rid of it.
+ type: boolean
+
+ amlogic,sfc-no-hwecc:
+ description: Disable ECC HW engine

Same question, why not having ECC always?


Will get rid of it.

+ type: boolean
+
+ amlogic,rx-adj:
+ description:
+ Adjust sample timing for RX, Sampling time
+ move later by 1 bus clock.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2, 3]
+ default: 0
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ sfc0: spi@fe08d000 {
+ compatible = "amlogic,a4-spifc";
+ reg = <0xfe08d000 0x800>, <0xfe08c000 0x4>;

One register for the parent clock? This really does not look like part
of this device.


EMMC bus clock modules were reused.

Best regards,
Krzysztof