[PATCH u-boot v2019.04-aspeed-openbmc v2] ARM: dts: aspeed: add Qualcomm DC-SCM V1

[Cédric Le Goater]

On 6/8/22 08:20, Joel Stanley wrote:
> On Wed, 8 Jun 2022 at 06:16, Cédric Le Goater <clg at kaod.org> wrote:
>>
>> On 6/8/22 05:05, Joel Stanley wrote:
>>> On Tue, 7 Jun 2022 at 14:03, Jae Hyun Yoo <quic_jaehyoo at quicinc.com> wrote:
>>>>
>>>> On 6/7/2022 12:43 AM, Cédric Le Goater wrote:
>>>>> On 6/7/22 03:05, Joel Stanley wrote:
>>>>>> On Mon, 6 Jun 2022 at 13:56, Jae Hyun Yoo <quic_jaehyoo at quicinc.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Ping
>>>>>>>
>>>>>>> On 5/19/2022 11:53 AM, Jae Hyun Yoo wrote:
>>>>>>>> From: Graeme Gregory <quic_ggregory at quicinc.com>
>>>>>>>>
>>>>>>>> Add initial version of device tree for Qualcomm DC-SCM V1 BMC which is
>>>>>>>> equipped with Aspeed AST2600 BMC SoC.
>>>>>>>>
>>>>>>>> Signed-off-by: Graeme Gregory <quic_ggregory at quicinc.com>
>>>>>>>> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo at quicinc.com>
>>>>>>>> ---
>>>>>>>> Changes in v2:
>>>>>>>> * Changed vendor name from Nuvia to Qualcomm.
>>>>>>>>
>>>>>>>>      arch/arm/dts/Makefile                   |   1 +
>>>>>>>>      arch/arm/dts/ast2600-qcom-dc-scm-v1.dts | 208
>>>>>>>> ++++++++++++++++++++++++
>>>>>>>>      2 files changed, 209 insertions(+)
>>>>>>>>      create mode 100644 arch/arm/dts/ast2600-qcom-dc-scm-v1.dts
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/dts/ast2600-qcom-dc-scm-v1.dts
>>>>>>>> b/arch/arm/dts/ast2600-qcom-dc-scm-v1.dts
>>>>>>>> new file mode 100644
>>>>>>>> index 000000000000..e966f739b708
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/arch/arm/dts/ast2600-qcom-dc-scm-v1.dts
>>>>
>>>> [...]
>>>>
>>>>>>>> +&fmc {
>>>>>>>> +     status = "okay";
>>>>>>>> +
>>>>>>>> +     pinctrl-names = "default";
>>>>>>>> +     pinctrl-0 = <&pinctrl_fmcquad_default>;
>>>>>>>> +
>>>>>>>> +     flash at 0 {
>>>>>>>> +             compatible = "spi-flash", "sst,w25q256";
>>>>>>
>>>>>> Compatible strings should go from the most general to the most
>>>>>> specific, so these are around the wrong way. I see you've probably
>>>>>> just copied the ast2600-evb which makes the same mistake.
>>>>>>
>>>>>> In the dtsi we have jedec,spi-nor which you overwrite here.
>>>>>>
>>>>>> u-boot has spi-flash but Linux uses jedec,spi-nor. I wonder if we
>>>>>> should update the aspeed driver to use that?
>>>>>>
>>>>>> Cédric, do you have thoughts here?
>>>>>
>>>>> Yes. The closer we are to the Linux DT the better.
>>>>>
>>>>> The "spi-flash" and "jedec,spi-nor" compatibles are equivalent for
>>>>> the SF driver in U-Boot. I think we can remove all the :
>>>>>
>>>>>       compatible = "spi-flash", "sst,w25q256";
>>>>>
>>>>> from the Aspeed dts files.
>>>>>
>>>>> This fits what the latest U-Boot does.
>>>>
>>>> I checked that it works without the compatible string. I'll drop the
>>>> line in v3.
>>>
>>> Can you describe your test setup?
>>>
>>> When I tested, using the ast2600-evb.dts, the driver reported:
>>>
>>> ast# sf probe
>>> aspeed_spi: flash-controller at 1e620000 has no flash devices ?!
>>> Invalid bus 0 (err=-19)
>>> Failed to initialize SPI flash at 0:0 (error -19)
>>>
>>> This is because the aspeed_spi.c driver counts the number of "spi-flash" nodes:
>>>
>>> static int aspeed_spi_count_flash_devices(struct udevice *bus)
>>> {
>>>           ofnode node;
>>>           int count = 0;
>>>
>>>           dev_for_each_subnode(node, bus) {
>>>                   if (ofnode_is_available(node) &&
>>>                       ofnode_device_is_compatible(node, "spi-flash"))
>>>                           count++;
>>>           }
>>>
>>>           return count;
>>> }
>>
>> This is only needed for the "sf" command. boot simply uses the mapping on
>> 0x20000000.
> 
> The environment reading code appears to use it too:
> 
> U-Boot 2019.04-astsdk-00073-gccf81eee1b3c-dirty (Jun 08 2022 - 12:32:42 +0930)
> 
> SOC: AST2600-A3
> Secure Boot: Mode_GCM
> eSPI Mode: SIO:Enable : SuperIO-2e
> Eth: MAC0: RGMII, MAC1: RGMII, MAC2: RGMII, MAC3: RGMII
> Model: AST2600 EVB
> DRAM:  1008 MiB (capacity:1024 MiB, VGA:64 MiB, ECC:off)
> RC Bridge phy at 1e6ed200 : Link down
> MMC:
> 
> sdhci_slot0 at 100: 1, sdhci_slot1 at 200: 2, emmc_slot0 at 100: 0
> Loading Environment from SPI Flash... aspeed_spi:
> flash-controller at 1e620000 has no flash devices ?!
> Invalid bus 0 (err=-19)
> *** Warning - spi_flash_probe_bus_cs() failed, using default environment

Ah yes. Interesting. I would have thought that CONFIG_ENV_OFFSET
was enough.

Thanks,

C.

> 
>>
>>
>>> We need to fix that if we're going to remove the "spi-flash"
>>> compatibles from the device tree.
>>
>> Just sent patches for it.
> 
> Thanks