Re: [PATCH net-next 3/3] r8169: release firmware on application failure

quoted

Right now, firmware application failures are ignored, so the device
can keep going even if applying the firmware fails. The TODO also
specifically says to release the firmware on failure, which seems to
imply that continuing without it was the intended behavior, and that
releasing it was mainly meant to prevent retrying the same failed
firmware application later.

Making firmware application failures fatal also creates a state problem.

I don't know this driver in detail, but fatal errors generally don't
cause state problem. The probe method fails, so the device is
destroyed. Is firmware download not performed in probe?

This driver does support a number of different devices. How many do
you have for testing? If you artificially cause firmware download to
fail, is the device usable?

	   Andrew

In the r8169 case, the firmware is not applied during probe, but rather
during rtl_open().

I got my hands on a machine with an RTL8105E. While it runs without
firmware, partial firmware application causes the NIC to stop working
unless the failure happens near the beginning or end of the application.

At this point I think the TODO I was trying to solve is stale. The proper
handling here would be, as you suggested, to fail rtl_open() on a firmware
application failure, which would release the firmware through the existing
rtl_open() error cleanup. That would instead turn this into broader
initialization error handling, which I am not knowledgeable enough about
the netdev and r8169 code yet to do confidently.

If desired, I could send the iopoll patch separately, but I do not think
I am going to continue with this series. Thanks for your help and patience.
I am still new to Linux kernel development and trying to get the hang of it.