Re: [PATCH 28/31] rockchip: rk3399-rock-pi-4: Sync device tree from linux v6.8

[Dragan Simic <dsimic@manjaro.org>]

On 2024-04-01 01:14, Jonas Karlman wrote:
> On 2024-04-01 00:53, Dragan Simic wrote:
>> Please see my comments below.
>> 
>> On 2024-03-31 22:28, Jonas Karlman wrote:
>>> Sync rk3399-rock-pi-4 related device tree from linux v6.8.
>>> 
>>> Add SPI flash related options to support booting from SPI flash.
>>> 
>>> Add AHCI=y, SCSI_AHCI=y, AHCI_PCI=y and SCSI=y to support PCIe SATA
>>> boot.
>> 
>> As we know, these boards have no standard connectors for PCI Express
>> expansion cards, which makes me wonder how many users actually use M.2
>> PCI Express modules with SATA controllers on them with these boards,
>> and need support for them in U-Boot?
>> 
>> I mean, it can't hurt, but frankly, I'm not 100% sure about it.
> 
> With the M.2 Extension Board [1] and a M.2 to PCIe X4 adapter it is 
> very
> easy to convert the M.2 slot into a standard PCIe X4 slot and use it
> with a PCIe SATA card. At least that is how I have tested and used 
> this.

I was aware of the M.2 extension board sold by Radxa, but frankly,
using it together with another adapter (M.2 to standard PCIe x4 slot)
is a cumbersome setup.  The whole thing becomes at least three times
the size of the SBC, with cables and a dangling PCIe card. :)

I had in mind an M.2 module with a PCIe SATA controller. [2]  Something
like that, used together with the M.2 extension board, would actually
make a rather neat setup.

[2] 
https://www.reddit.com/r/selfhosted/comments/s0bf1d/m2_sata_expansion_anyone_use_something_like_this/

> I do think there will be many users of PCIe AHCI/SATA, but with this
> series I try to get all boards to have same feature set as long as 
> there
> is some way to use a feature.

I guess it can't hurt in the end.

> [1] https://radxa.com/products/accessories/m2-extension-board
> 
>>> Change to SPL_MAX_SIZE=0x40000, SPL can be up to 256 KiB.
>>> 
>>> Add ROCKCHIP_IODOMAIN=y to configure io-domain voltage.
>>> 
>>> Add PHY_REALTEK=y and DM_ETH_PHY=y to support ethernet PHY.
>>> 
>>> Remove SPL_TINY_MEMSET=y to use full memset in SPL.
>>> 
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> 
>> Otherwise, looking good to me.
>> 
>> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> 
> Thanks!
> 
> Regards,
> Jonas
> 
>>> ---
>>>  arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi | 12 ++++++++++
>>>  arch/arm/dts/rk3399-rock-4c-plus.dts         |  1 +
>>>  arch/arm/dts/rk3399-rock-4se-u-boot.dtsi     | 12 ++++++++++
>>>  arch/arm/dts/rk3399-rock-pi-4.dtsi           |  4 +++-
>>>  arch/arm/dts/rk3399-rock-pi-4c-u-boot.dtsi   |  7 ++++++
>>>  arch/arm/dts/rk3399-rock-pi-4c.dts           | 10 ++++++++
>>>  configs/rock-4c-plus-rk3399_defconfig        | 24 
>>> +++++++++++++++-----
>>>  configs/rock-4se-rk3399_defconfig            | 23 
>>> +++++++++++++++++--
>>>  configs/rock-pi-4-rk3399_defconfig           |  8 +++++++
>>>  configs/rock-pi-4c-rk3399_defconfig          | 24 
>>> ++++++++++++++++++--
>>>  10 files changed, 114 insertions(+), 11 deletions(-)
>>> 
> [snip]