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

[Cédric Le Goater]

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.

Cheers,
C.