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

[Xianwei Zhao <xianwei.zhao@amlogic.com>]

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@amlogic.com>
>>
>> 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@amlogic.com>
>> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
>> ---
>>   .../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@amlogic.com>
>> +  - Feng Chen <feng.chen@amlogic.com>
>> +  - Xianwei Zhao <xianwei.zhao@amlogic.com>
>> +
>> +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
> 

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic