[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9e970294-912a-4bc4-8841-3515f789d582@gmx.de>
Date: Tue, 1 Oct 2024 15:31:13 +0200
From: Ingo van Lil <inguin@....de>
To: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Cc: "dmurphy@...com" <dmurphy@...com>
Subject: Re: [PATCH] net: phy: dp83869: fix memory corruption when enabling
fiber
On 10/1/24 12:40, Sverdlin, Alexander wrote:
>> diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
>> index d7aaefb5226b..9c5ac5d6a9fd 100644
>> --- a/drivers/net/phy/dp83869.c
>> +++ b/drivers/net/phy/dp83869.c
>> @@ -645,7 +645,7 @@ static int dp83869_configure_fiber(struct phy_device *phydev,
>> phydev->supported);
>>
>> linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported);
>> - linkmode_set_bit(ADVERTISED_FIBRE, phydev->advertising);
>> + linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->advertising);
>
> Are you sure this linkmode_set_bit() is required at all?
You're right, it's probably not required. I just tracked a weird bug
down to this clear mistake and wanted to change as little as possible.
The logic of the function seems a bit odd to me: At the beginning,
advertising is ANDed with supported, and at the end it's ORed again.
Inside the function they are mostly manipulated together.
Couldn't that be replaced with a simple "phydev->advertising =
phydev->supported;" at the end?
Regards,
Ingo
Powered by blists - more mailing lists