dracutdevs / dracut

dracut the event driven initramfs infrastructure
https://github.com/dracutdevs/dracut/wiki
GNU General Public License v2.0
597 stars 396 forks source link

feat(kernel-network-modules): Add Qualcomm IPC router to enable USB #2531

Open jlinton opened 11 months ago

jlinton commented 11 months ago

The x13s, and possibly other Qualcomm based devices, need the QC IPC router driver to enable USB. Without it, it's not possible to boot from USB-C attached disks.

Checklist

Fixes #

LaszloGombos commented 10 months ago

Would https://github.com/dracutdevs/dracut/blob/master/modules.d/90kernel-network-modules/module-setup.sh#L40 be a better place to add this driver ?

jlinton commented 10 months ago

I don't think so, but maybe?

Despite the "networking" it is largely for the USB controller functionality. Which I guess could be a USB nic as well as USB storage. So, maybe my understanding of the difference between this file and that one surrounds whether one wants actual networking rather than the needed driver happens to be in the kernel networking directory, because well, it is a proprietary hardware/firmware IPC protocol.

jlinton commented 10 months ago

Alright, I will switch it to the networking modules.

ausil commented 10 months ago

I strongly disagree about using the networking modules to pull in qrtr. The Fedora-Minimal disk image for instance does not install dracut-network. with the patch as is the disk image installed on a USB stick will not be bootable on the x13s as qrtr will be missing. As it is needed for working USB and block storage it needs to always be included.

aafeijoo-suse commented 10 months ago

I strongly disagree about using the networking modules to pull in qrtr. The Fedora-Minimal disk image for instance does not install dracut-network. with the patch as is the disk image installed on a USB stick will not be bootable on the x13s as qrtr will be missing. As it is needed for working USB and block storage it needs to always be included.

Then you can add it via conf file shipped by the distro.

ausil commented 10 months ago

I strongly disagree about using the networking modules to pull in qrtr. The Fedora-Minimal disk image for instance does not install dracut-network. with the patch as is the disk image installed on a USB stick will not be bootable on the x13s as qrtr will be missing. As it is needed for working USB and block storage it needs to always be included.

Then you can add it via conf file shipped by the distro.

https://android.googlesource.com/kernel/msm/+/android-7.1.0_r0.2/Documentation/arm/msm/msm_ipc_router.txt outlines the purpose of qrtr. It does networking between Processors inside of a single system. It honestly probably needs to be moved out of net/ in the kernel tree, as I understand things, it was added there because its initial use was to configure Qualcomm modems.

LaszloGombos commented 10 months ago

It honestly probably needs to be moved out of net/ in the kernel tree

Would it make sense to file a kernel bug upstream ? Ideally we should get feedback first from upstream..

As it stands, I see this as a workaround for a bug that's not in dracut. That said I am open for adding it back to the kernel-modules with some kind comment or reference or resolution to the bigger upstream issue.

jlinton commented 10 months ago

Well (AFAIK) the stuff in net/qrtr is intended to be the userspace access point to the bus, so in that regard it makes sense to be in the upstream net directory.

But looking closer at how dracut-network is used in fedora, I think my original assumption that it belongs in the general kernel modules is correct. This fix only works because kdump (maybe incorrectly) is pulling it in for the minority of users who might want to kdump to a network target.

So, I agree with @ausil here that it doesn't really belong in kernel-network-modules.

aafeijoo-suse commented 10 months ago

As it stands, I see this as a workaround for a bug that's not in dracut. That said I am open for adding it back to the kernel-modules with some kind comment or reference or resolution to the bigger upstream issue.

I agree. Taking into consideration everything discussed, maybe it's better to add this in kernel-modules, but please commenting why.

jlinton commented 10 months ago

Ok, moved back with a larger commit message about what is going on.

Conan-Kudo commented 10 months ago

Please downcase "add" to make the bot happy:

Error: commit_message:1:23: error: don't use title case in the description
feat(kernel-modules): Add Qualcomm IPC router to enable USB
                      ^~~~~~~~~~~~
jlinton commented 10 months ago

Ah, right, that got lost when I reverted to the original commit. It should be fixed now.

stale[bot] commented 4 months ago

This issue is being marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. If this is still an issue in the latest release of Dracut and you would like to keep it open please comment on this issue within the next 7 days. Thank you for your contributions.