Re: [PATCH v3 4/4] arm64: dts: rockchip: Add devicetree for Pine64 PineTab2

Jonas Karlman
2024-01-03T14:19:50Z

Hi Manuel,

On 2024-01-03 14:40, Manuel Traut wrote:
> Hi Jonas and Ondřej,
>
>>>> +&sfc {
>>>> + pinctrl-names = "default";
>>>> + pinctrl-0 = <&fspi_dual_io_pins>;
>>>> + status = "okay";
>>>> + #address-cells = <1>;
>>>> + #size-cells = <0>;
>>>> +
>>>> + flash@0 {
>>>> + compatible = "jedec,spi-nor";
>>>> + reg = <0>;
>>>> + spi-max-frequency = <24000000>;
>>>
>>> That's a bit on the low side. The flash chip should work for all commands up to
>>> 80MHz https://megous.com/dl/tmp/b428ad9b85ac4633.png and SGM3157YC6 switch
>>> for the FSPI-CLK should have high enough bandwidth, too.
>>
>> I agree that this is a little bit on the low side, it was a safe rate
>> that I used for U-Boot. U-Boot required an exact rate of the supported
>> sfc clk rates: 24, 50, 75, 100, 125 or 150 MHz.
>>
>> Please also note that the SPI NOR flash chip used in PineTab2 is not a
>> GigaDevice GD25LQ128E, it should be a SiliconKaiser SK25LP128, same as
>> found in the Pine64 PinePhone Pro.
>
> The schematics for v2.0 reference a GD25LQ128EWIGR. I never checked the jedec
> id. How did you retrieve this information, or is it maybe a difference in v0.1
> and 2.0?

This was when working on mainline U-Boot for the PineTab2 (and other
rk356x devices). See [1] for a pending U-Boot patch that is waiting on a
proper mainline linux devicetree for the PT2.

The JEDEC ID is reported as 0x257018 on my v2.0 production unit, and
does not match the JEDEC ID for GD25LQ128E (0xc86018) referenced in
the schematics.

I found that the JEDEC ID 0x257018 was referenced in prior patches
related to the SK25LP128 SPI NOR flash chip used in Pine64 PinePhone Pro.

I have only ever tested the 24 MHz rate, but I am expecting that e.g.
100 MHz also should work. Will not be able to test on my PT2 until at
earliest next week.

[1] https://github.com/Kwiboo/u-boot-rockchip/commit/66562d6eaf2c11a9f97fcdba379d3ceda8aa70ef

Regards,
Jonas

>
>>>> + spi-rx-bus-width = <2>;
>>>
>>> GD25LQ128E supports quad I/O. Maybe try 4 if it will work.
>>
>> The schematic only shows fspi D0 and D1 connected, and use the D2 line
>> for eMMC_RSTn, so spi-rx-bus-width = <2> should be correct.
>
> ack
>
> Since it is only needed for bootloader updates and environment its maybe better
> to stay on the safe side?
>
> But I can test faster frequency if you want me to do..
>
> Regards
> Manuel