This looks ok, with perhaps the exception of the missing systemd handling of rmtfs.service

https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_systemd

(In reply to Yanko Kaneti from comment #1)
> This looks ok, with perhaps the exception of the missing systemd handling of
> rmtfs.service
> 
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/
> #_systemd

Thanks a lot for your feedback. I've added these now.

The Spec and SRPM files are in the same URL that was mentioned in Comment #0.

I don't see the service checking if it's running on any relevant device ?
Can you make the appropriate condition so that the service only enable itself on the appropriate device ?

Also despite the package built on others arches, I see little point to have it on something else than aarch64. or is there any ?

(In reply to Nicolas Chauvet (kwizart) from comment #3)
> I don't see the service checking if it's running on any relevant device ?
> Can you make the appropriate condition so that the service only enable
> itself on the appropriate device ?
>

Can you give me an example of a service that does this check ? I've never
seen it.

> Also despite the package built on others arches, I see little point to have
> it on something else than aarch64. or is there any ?

The guidelines say that packagers should "Fedora packagers should make every
effort to support all primary architectures":

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_support

Yes, arch specific code like this may be restricted to aarch64, but as packager
I still find it useful to try local builds on my laptop, etc.

I don't have a strong opinion though.

I guess stuff like this could still be useful in some testing/emulation capacity on something pretending to be a quallcom device?
In any case its small and I don't think it hurts a lot having it at least build on all..this can be changed at any point.

License is OK
Name is OK.
Builds in koji.

APPROVED

if debugging or emulating, it still good to have a local build, but the packager guideline also stipulate to have a package generally useful
Having an aarch64 only build would prevent the package to end in any non-aarch64 produced Fedora deliverable media.

For the unit file condition, you can have a look at mcelog on x86_64:
systemctl cat mcelog

Namely: ConditionPathExists=!/sys/module/edac_mce_amd/initstate

Of course, you need to find a path that can discriminate your particular device.
This is particularly useful to avoid any error on non-qcom aarch64 devices (or even qcom that do not need this driver).

An even better alternative is to use service activation with udev. (we use that for the nvidia driver at rpmfusion), like
cat /usr/lib/udev/rules.d/10-nvidia.rules
SUBSYSTEM=="pci", ATTRS{vendor}=="0x10de", ATTRS{class}=="0x030[02]00", TAG+="systemd", ENV{SYSTEMD_WANTS}="nvidia-fallback.service" 


To be adapted as appropriate...

(In reply to Nicolas Chauvet (kwizart) from comment #6)
> if debugging or emulating, it still good to have a local build, but the
> packager guideline also stipulate to have a package generally useful
> Having an aarch64 only build would prevent the package to end in any
> non-aarch64 produced Fedora deliverable media.
> 
> For the unit file condition, you can have a look at mcelog on x86_64:
> systemctl cat mcelog
> 
> Namely: ConditionPathExists=!/sys/module/edac_mce_amd/initstate
> 
> Of course, you need to find a path that can discriminate your particular
> device.

This is for most Windows-on-ARM QC based laptops and QC based Chromebooks.
It would be hard to maintain the allow list of devices using a combination
of DMI information and Device Tree compatible strings (I can't think of any
other way to discriminate).

Not worth it in my opinion.

> This is particularly useful to avoid any error on non-qcom aarch64 devices
> (or even qcom that do not need this driver).
> 
> An even better alternative is to use service activation with udev. (we use
> that for the nvidia driver at rpmfusion), like
> cat /usr/lib/udev/rules.d/10-nvidia.rules
> SUBSYSTEM=="pci", ATTRS{vendor}=="0x10de", ATTRS{class}=="0x030[02]00",
> TAG+="systemd", ENV{SYSTEMD_WANTS}="nvidia-fallback.service" 
>

This is a good idea and can be done as a follow-up. For now the service files
are not enabled by default anyways and have to be manually enabled / started.

(In reply to Yanko Kaneti from comment #5)
> I guess stuff like this could still be useful in some testing/emulation
> capacity on something pretending to be a quallcom device?
> In any case its small and I don't think it hurts a lot having it at least
> build on all..this can be changed at any point.
> 
> License is OK
> Name is OK.
> Builds in koji.
> 
> APPROVED

Thanks!

The Pagure repository was created at https://src.fedoraproject.org/rpms/rmtfs

> This is a good idea and can be done as a follow-up. For now the service files
> are not enabled by default anyways and have to be manually enabled / started.

Right, but enabling it by default is the action right after. Then, get it into the default media.
So I don't get why it's not discussed as part of the review. This will save back and forth implementation tryouts.

My idea since it's tight to a wireless module is to activate the service only if the qcom wifi module is loaded.
That way, blacklisting the wifi module (for any reason) will also prevent this service to load the firmware.