Re: [PATCHv4 net-next] net: modernize ioremap in probe
From: Andrew Lunn
Date: Tue Dec 03 2024 - 18:43:39 EST
> This is not equivalent. This means if ioremap() fails inside
> devm_platform_ioremap_resource(), we end up printing a message that
> blames the firmware, which is wrong.
>
> It also changes a "resource missing, proceed anyway" situation into
> a failure situation.
>
> Please drop this change, "cleaning" this up is introducing bugs.
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
says:
1.6.5. Using device-managed and cleanup.h constructs
Netdev remains skeptical about promises of all “auto-cleanup” APIs,
including even devm_ helpers, historically. They are not the
preferred style of implementation, merely an acceptable one.
1.6.6. Clean-up patches
Netdev discourages patches which perform simple clean-ups, which are
not in the context of other work. For example:
Addressing checkpatch.pl warnings
Addressing Local variable ordering issues
Conversions to device-managed APIs (devm_ helpers)
This is because it is felt that the churn that such changes produce
comes at a greater cost than the value of such clean-ups.
As Russell points out, you are breaking things which probably worked
before. "If its not broken, don't fix it". These changes cost reviewer
time trying to find what you have broken, when it would be better
spent other places.
If you have the hardware, and wont to work on the driver to add new
features, then yes, you can do this sort of conversion, because you
should find your own bugs via testing. If you don't have the hardware,
please just leave it alone.
Andrew