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

[Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>]

On 2026-09-16 19:02, Andrew Lunn wrote:
> On Wed, Sep 16, 2026 at 06:15:00PM -0300, Matheus Alves de Almeida wrote:
>> > If the firmware cannot be written, is the device dead? Should this
>> > return an error, so the caller can abort the probe?
>> 
>> A timeout while applying firmware could indeed indicate a PHY access
>> problem. However, firmware loading failures are already non-fatal,
>> and the callers of r8169_apply_firmware() do not propagate errors
>> either. Making firmware application failures fatal would require
>> broader changes to several r8169 PHY initialization paths.
> 
> But you are making such changes, returning errors up the call chain.
> Why are you making these changes? We either assume nothing can fail,
> so we throw away the return code, or we should assume everything can
> fail, and propagate the errors.
> 
> If we assume error can happen, if there is an error in firmware
> download, isn't that fatal? Should we even care about care about PHY
> read/write errors if firmware download has failed? And since firmware
> download is probably the first thing to happen, if anything is likely
> to fair, i would expect firmware download is what is going to fail.
> 
>    Andrew

You're right. It is one of the first things that happen, so checking it
makes sense. I will make firmware application errors propagate up to
rtl_open() in v2.

Matheus