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

[Jae Hyun Yoo]

On 6/7/2022 11:16 PM, Cédric Le Goater 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.
> 
> 
>> 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!

This dts works with the patch Cedric submitted:
https://lore.kernel.org/openbmc/20220608061455.365123-1-clg@kaod.org/T/#t

-Jae