Re: [PATCHv4 net-next] net: modernize ioremap in probe

[Andrew Lunn <andrew@lunn.ch>]

> 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