Re: [PATCH net v2] net: usb: lan78xx: fix use of improperly initialized dev->chipid in lan78xx_reset

[Khalid Aziz <khalid@kernel.org>]

On 10/15/25 10:51 AM, I Viswanath wrote:
> On Wed, 15 Oct 2025 at 21:25, Khalid Aziz <khalid@kernel.org> wrote:
> 
>> How did you determine this is the commit that introduced this bug?
>>
>>   From what I can see, commit a0db7d10b76e does not touch lan78xx_reset()
>> function. This bug was introduced when devid was replaced by chipid
>> (commit 87177ba6e47e "lan78xx: replace devid to chipid & chiprev") or
>> even earlier when the order of calls to lan78xx_init_mac_address() and
>> lan78xx_read_reg() was introduced in lan78xx_reset() depending upon if
>> lan78xx_init_mac_address() at that time used devid in its call sequence
>> at the time.
> 
> The commit a0db7d10b76e introduced the dependency on devid to
> lan78xx_read_raw_eeprom() and
> lan78xx_read_eeprom() and ultimately lan78xx_init_mac_address() and
> lan78xx_reset()
> 
> In lan78xx_init_mac_address()
> 
> Only lan78xx_read_eeprom() depends on devid as
> 
> lan78xx_read_reg() and lan78xx_write_reg() do not use devid
> 
> lan78xx_read_otp() depends on lan78xx_read_raw_otp() which depends
> only on lan78xx_write_reg() and lan78xx_read_reg()
> and hence doesn't use devid either
> 
> is_valid_ether_addr(), random_ether_addr() and ether_addr_copy() are
> net core functions and do not care about driver specific data
> 
> The devid read exists in this commit (was added in ce85e13ad6ef4)
> 
> a0db7d10b76e was supposed to move the devid read before the
> lan78xx_init_mac_address() because of the newly added
> dependency but it was a tricky detail that the author failed to see
> 
> Thanks,
> I Viswanath

Ah, I see. That makes sense.

--
Khalid