Re: [PATCH net-next 3/3] r8169: release firmware on application failure
From: Matheus Alves de Almeida
Date: Wed Sep 16 2026 - 18:22:32 EST
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