configs: Enable ATA and AHCI for SATA drive support.

[geerlingguy]

As I've been testing more configurations for Raspberry Pi-based storage, including NASes and storage servers, it seems like the common thread with any SATA-based storage is enabling ATA and AHCI support. This allows almost any of the Marvell-based controllers which seem to be most common on commodity SATA PCIe cards to work out of the box on the Compute Module 4.

I have added the config parameters I set in my cross-compiled kernel to both the 32-bit and 64-bit Pi OS builds for the BCM2711 chip. I don't know if there's any value adding them for other platforms though, since at this time only the BCM2711 has hardware that can easily interface with PCIe cards (e.g. the Compute Module 4 + IO Board).

[6by9]

I'd support adding the modules for these SATA boards, but you seem to have gained more options than I was expecting.
My NAS kernel only has CONFIG_ATA=m and CONFIG_SATA_AHCI=m to work with my Marvell 9215 card.

Do you really believe the other options are necessary? Were they generated by make savedefconfig?
HAVE_PATA_PLATFORM for example is only ever selected by other modules, it doesn't appear to be a menu option.
SATA_HOST likewise will be automatically selected by

Selected by [m]:                                                                                                                                                                         │  
  │   - SATA_AHCI [=m] && ATA [=m] && HAS_DMA [=y] && PCI [=y]       

so whilst it will be =y, that is automagic to the build system. Several of the others seem to fall into the same category of being the defaults selected by the system.

For reference, the modules CONFIG_ATA and CONFIG_SATA_AHCI added on 5.9.1 were
ahci.ko - 80kB
libahci.ko - 69kB
libata.ko - 441kB

There's absolutely no point in adding them to the other variants, and a call needs to be made as to whether we start to allow PCIe device drivers to be merged for all BCM2711 kernel builds, even though it is only the CM4 that can make use of them.

[geerlingguy]

Do you really believe the other options are necessary? Were they generated by make savedefconfig?

Not necessarily; they were generated by make and I just never tested with only the two you mention, I've done all my testing with all the other defaults selected.

I'd be happy to whittle it down to just two though... I think I tested at least one of my cards by just adding those two and not using make menuconfig to select them.

A call needs to be made as to whether we start to allow PCIe device drivers to be merged for all BCM2711 kernel builds, even though it is only the CM4 that can make use of them.

It's my secret hope that some future model B variant exposes an M.2 slot and has at least 2 lanes (ideally 4 or more) of PCIe bandwidth available... might add too much expense, but I can dream!

In any case, yes, I think the key would be to be extremely selective about what to add, since only CM4 users would really benefit from added support.

My main point is that it would greatly expand the pool of users who would be comfortable using CM4 in specific use cases, like for NAS builds with SATA controllers. I know of the Wiretrustee SATA board coming, and a couple other hobby boards (no commercial run intended), but it would be neat to see more options open up where someone buys a board, pops on a CM4, and plugs in 2-4 drives and has a perfectly adequate Pi-based NAS.

For a lot of the other cards I'm testing, I think the use case is more exotic and/or the current Pi generation is not a great fit (e.g. for networking there are too many different controllers that would need to be added, and the Pi needs extra tuning to handle more than 1 Gbps of network throughput).

[6by9]

The defconfigs only take things that aren't the default or are automatically selected by other things. make savedefconfig strips out all the extra or default things and saves a file called defconfig. That's the file that you want to copy back to arch/arm[64]/configs and commit.

As above, my NAS is still running 5.9.1 as I set it up a fair while ago possibly when CM4 wasn't actually released(!), and I haven't updated it yet.

I won't make any comment about future boards.

[geerlingguy]

I won't make any comment about future boards.

Heh, can't stop me from trying to extract information ;)

I'll update the config then later, thanks!

[mi-hol]

a call needs to be made as to whether we start to allow PCIe device drivers to be merged for all BCM2711 kernel builds,

Maybe the best option would be a separate image just for the CM4, because only this device has the flexibility & need for the large list of PCIe devices to be supported
or
build a significantly enhanced version of raspi-config to achieve this goal

[mi-hol]

@6by9 as outsiders we are unaware of the Foundation's decision making process for strategic decisions.
Is there a process (i.e. review board) that we can support (i.e. by presenting pro&con for selected options)?

[6by9]

It'll be a conversation with the 3-4 main developers involved at PiTowers, with @pelwell being the main decision-maker (although I believe he's meant to be on holiday this week).
As with all the other module requests made through this issue tracker, where the options are reasonable they get added. Being a module is the key thing as otherwise it has a memory impact to all users, not just a storage impact. This is why I quoted the module sizes earlier in this thread, as that is the penalty to all users.

Building and maintaining a separate list of modules for CM4 is a pain. For a few sensible PCIe cards it is probably reasonable to add them, but for example adding all the PCIe based DVB tuners would be excessive.
Even for CM4 users there is a consideration for storage space, particularly on the smaller EMMC modules. However there is an expectation that industrial users would be building their own kernels for the reduced set of modules that they require in their specific application, so the standard kernel defconfigs are less relevant.

[pelwell]

@mi-hol The process for making a config change into an rpi kernel is to open an Issue or PR here. Being pro-active with gathering and presenting information about the benefits and costs of doing so will speed up the process, but ultimately it comes down to weighing up the benefits to some users against the costs to all in terms of storage space or (worse) free memory.

Adding a dedicated CM4 build would make the situation worse. The core kernel is likely to be near-identical to the regular Pi 4 build, just with a larger set of modules. As the kernel apt package and rpi-update releases contain all kernels it would greatly increase the size, especially with the 64-bit variant as well, unless you are suggesting an independent, parallel installation channel just for CM4 - that's not going to happen.

As @6by9 says, the expectation is that industrial users will do their own builds, likely using buildroot and with custom DTBs, and change them infrequently. Kernel building is so easy (https://www.raspberrypi.org/documentation/linux/kernel/building.md) that it shouldn't be a barrier to hobbyists moving on to CMs.

[mi-hol]

Kernel building is so easy

Unfortunately I can't confirm this view :(
This view stems from the experience gained from asking users to execute these instructions to install OpenMediaVault (OMV) on RPis and looking at the resulting failure rate manifested as forum posts.

Back to the open PR, was a decision already taken?

[geerlingguy]

I still haven't taken the time to update the PR, but hope to do so next week (busy week wrapping up a few projects). My hope is that this particular change is deemed as low-impact/high-reward, since there are more and more people gaining interest in projects where SATA is involved (for example, mebs_t just finished off his nice custom NAS: https://github.com/mebs/CM4-NAS/

And it's such a widely-used and low-cost solution for storage that pairs nicely with Pi (since the SATA bus is not bottlenecked quite so much as NVMe on the current-gen Pis).

At least that's my argument for inclusion. I will try to update this PR soon though, and it should only have the essential modules added at that point.

(The main reason it takes me a little while is I have my linux checkout on a separate drive that's not with me, since my main drive is case-insensitive, which leads to issues working with the Linux kernel, which has some identically-named files :().

[6by9]

Changes have been requested, or at least require justification.

I've just checked my (5.9) NAS and it has all the changes listed bar one as defaults from having enabled just CONFIG_ATA and CONFIG_SATA_AHCI. They therefore shouldn't be present in the defconfig.

CONFIG_SATA_MV=m is the exception. That is listed as

 This option enables support for the Marvell Serial ATA family.                                                                                                                            
 Currently supports 88SX[56]0[48][01] PCI(-X) chips,                                                                                                                                        
 as well as the newer [67]042 PCI-X/PCIe and SOC devices.

Do we actually have devices working using one of those chips? If so then it would be reasonable to enable it.

On 5.10.25 the module sizes are

-rw-rw-r-- 1 dave dave  53252 Apr  8 18:09 ahci.ko
-rw-rw-r-- 1 dave dave  47884 Apr  8 18:09 libahci.ko
-rw-rw-r-- 1 dave dave 323880 Apr  8 18:09 libata.ko
-rw-rw-r-- 1 dave dave  49344 Apr  8 18:09 sata_mv.ko

so again a not unreasonable hit for all users IMHO.

[mi-hol]

@geerlingguy as a friendly follow up.
How are chances to address the requested changes?

[geerlingguy]

@6by9 / @mi-hol - Just rebased and updated using make defconfig and only pulling the required changes in. Sorry about the noise.

The change only requires adding:

CONFIG_ATA=m
CONFIG_SATA_AHCI=m
CONFIG_SATA_MV=m

I didn't try rebuilding without the Marvel support... that may work—I just realized all the chipsets I'm using are in the 9xxx range... so just judging by the listing it doesn't seem like that module would be needed?

[6by9]

TBH I'm OK with this as is - the overall hit of ~500kB.
Dropping CONFIG_SATA_MV to save 50kB is sort of in the noise, but @pelwell take a different view.

SATA_AHCI, same as the USB3 XHCI, adds support for the majority of basic controllers through the standardised interface, so I'd say it's worth adding. That means anyone using CM4 for a NAS can continue using standard kernels instead of custom builds.

[pelwell]

I'm OK with the new version.

[hideout]

finally thanks for sata support!