* [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen
@ 2025-08-28 21:44 Colin Foster
2025-09-01 20:57 ` Jakub Kicinski
2026-08-10 8:20 ` kernel test robot
0 siblings, 2 replies; 5+ messages in thread
From: Colin Foster @ 2025-08-28 21:44 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Steve Glendinning, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Colin Foster
When the EEPROM MAC is read by way of ADDRH, it can return all 0s the
first time. Subsequent reads succeed.
Re-read the ADDRH when this behaviour is observed, in an attempt to
correctly apply the EEPROM MAC address.
Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
drivers/net/ethernet/smsc/smsc911x.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index a2e511912e6a9..63ed221edc00a 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2162,8 +2162,20 @@ static const struct net_device_ops smsc911x_netdev_ops = {
static void smsc911x_read_mac_address(struct net_device *dev)
{
struct smsc911x_data *pdata = netdev_priv(dev);
- u32 mac_high16 = smsc911x_mac_read(pdata, ADDRH);
- u32 mac_low32 = smsc911x_mac_read(pdata, ADDRL);
+ u32 mac_high16, mac_low32;
+
+ mac_high16 = smsc911x_mac_read(pdata, ADDRH);
+ mac_low32 = smsc911x_mac_read(pdata, ADDRL);
+
+ /*
+ * The first mac_read always returns 0. Re-read it to get the
+ * full MAC
+ */
+ if (mac_high16 == 0) {
+ SMSC_TRACE(pdata, probe, "Re-read MAC ADDRH\n");
+ mac_high16 = smsc911x_mac_read(pdata, ADDRH);
+ }
+
u8 addr[ETH_ALEN];
addr[0] = (u8)(mac_low32);
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen
2025-08-28 21:44 [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen Colin Foster
@ 2025-09-01 20:57 ` Jakub Kicinski
2025-09-02 12:31 ` Colin Foster
2026-08-10 8:20 ` kernel test robot
1 sibling, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2025-09-01 20:57 UTC (permalink / raw)
To: Colin Foster
Cc: linux-kernel, netdev, Steve Glendinning, David S. Miller,
Eric Dumazet, Paolo Abeni
On Thu, 28 Aug 2025 16:44:52 -0500 Colin Foster wrote:
> When the EEPROM MAC is read by way of ADDRH, it can return all 0s the
> first time. Subsequent reads succeed.
>
> Re-read the ADDRH when this behaviour is observed, in an attempt to
> correctly apply the EEPROM MAC address.
Please name the device, and FW version if applicable, on which you
observe the issue.
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> ---
> drivers/net/ethernet/smsc/smsc911x.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index a2e511912e6a9..63ed221edc00a 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -2162,8 +2162,20 @@ static const struct net_device_ops smsc911x_netdev_ops = {
> static void smsc911x_read_mac_address(struct net_device *dev)
> {
> struct smsc911x_data *pdata = netdev_priv(dev);
> - u32 mac_high16 = smsc911x_mac_read(pdata, ADDRH);
> - u32 mac_low32 = smsc911x_mac_read(pdata, ADDRL);
> + u32 mac_high16, mac_low32;
> +
> + mac_high16 = smsc911x_mac_read(pdata, ADDRH);
> + mac_low32 = smsc911x_mac_read(pdata, ADDRL);
> +
> + /*
nit: netdev multi-line comment style doesn't place /* on a separate
line:
> + * The first mac_read always returns 0. Re-read it to get the
> + * full MAC
Always? Strange, why did nobody notice until now?
> + */
> + if (mac_high16 == 0) {
> + SMSC_TRACE(pdata, probe, "Re-read MAC ADDRH\n");
> + mac_high16 = smsc911x_mac_read(pdata, ADDRH);
> + }
> u8 addr[ETH_ALEN];
Please don't add code in the middle of variable declarations
--
pw-bot: cr
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen
2025-09-01 20:57 ` Jakub Kicinski
@ 2025-09-02 12:31 ` Colin Foster
2025-09-02 19:05 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Colin Foster @ 2025-09-02 12:31 UTC (permalink / raw)
To: Jakub Kicinski
Cc: linux-kernel, netdev, Steve Glendinning, David S. Miller,
Eric Dumazet, Paolo Abeni
Hi Jakub,
On Mon, Sep 01, 2025 at 01:57:12PM -0700, Jakub Kicinski wrote:
> On Thu, 28 Aug 2025 16:44:52 -0500 Colin Foster wrote:
> > When the EEPROM MAC is read by way of ADDRH, it can return all 0s the
> > first time. Subsequent reads succeed.
> >
> > Re-read the ADDRH when this behaviour is observed, in an attempt to
> > correctly apply the EEPROM MAC address.
>
> Please name the device, and FW version if applicable, on which you
> observe the issue.
I'll add that to the commit message. FWIW it is the Phytec PCM049 SOM.
>
> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> > ---
> > drivers/net/ethernet/smsc/smsc911x.c | 16 ++++++++++++++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> > index a2e511912e6a9..63ed221edc00a 100644
> > --- a/drivers/net/ethernet/smsc/smsc911x.c
> > +++ b/drivers/net/ethernet/smsc/smsc911x.c
> > @@ -2162,8 +2162,20 @@ static const struct net_device_ops smsc911x_netdev_ops = {
> > static void smsc911x_read_mac_address(struct net_device *dev)
> > {
> > struct smsc911x_data *pdata = netdev_priv(dev);
> > - u32 mac_high16 = smsc911x_mac_read(pdata, ADDRH);
> > - u32 mac_low32 = smsc911x_mac_read(pdata, ADDRL);
> > + u32 mac_high16, mac_low32;
> > +
> > + mac_high16 = smsc911x_mac_read(pdata, ADDRH);
> > + mac_low32 = smsc911x_mac_read(pdata, ADDRL);
> > +
> > + /*
>
> nit: netdev multi-line comment style doesn't place /* on a separate
> line:
Apologies - that shouldn't have slipped through.
>
>
> > + * The first mac_read always returns 0. Re-read it to get the
> > + * full MAC
>
> Always? Strange, why did nobody notice until now?
For me it is 100% reproduceable. The first read is always 0. I've added
delays in case timing was the issue. I've swapped ADDRH and ADDRL and
the opposite effect happened (where the first four MAC octets were
zero). Re-reads always succeed.
Without the patch, the last two MAC octets are always zero.
We didn't notice it until we started hooking multiple devices on the
same network.
If there is anyone else running this hardware, I'd love verification.
Its an SMSC9221.
That's a long way of saying "I don't know" unfortunately.
>
> > + */
> > + if (mac_high16 == 0) {
> > + SMSC_TRACE(pdata, probe, "Re-read MAC ADDRH\n");
> > + mac_high16 = smsc911x_mac_read(pdata, ADDRH);
> > + }
>
> > u8 addr[ETH_ALEN];
>
> Please don't add code in the middle of variable declarations
Ack.
> --
> pw-bot: cr
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen
2025-09-02 12:31 ` Colin Foster
@ 2025-09-02 19:05 ` Jakub Kicinski
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2025-09-02 19:05 UTC (permalink / raw)
To: Colin Foster
Cc: linux-kernel, netdev, Steve Glendinning, David S. Miller,
Eric Dumazet, Paolo Abeni
On Tue, 2 Sep 2025 07:31:13 -0500 Colin Foster wrote:
> > > + * The first mac_read always returns 0. Re-read it to get the
> > > + * full MAC
> >
> > Always? Strange, why did nobody notice until now?
>
> For me it is 100% reproduceable. The first read is always 0. I've added
> delays in case timing was the issue. I've swapped ADDRH and ADDRL and
> the opposite effect happened (where the first four MAC octets were
> zero). Re-reads always succeed.
>
> Without the patch, the last two MAC octets are always zero.
>
> We didn't notice it until we started hooking multiple devices on the
> same network.
>
> If there is anyone else running this hardware, I'd love verification.
> Its an SMSC9221.
>
> That's a long way of saying "I don't know" unfortunately.
Right, I think we should avoid saying "always" in the comment then.
Let's weasel word it a little bit given the uncertainty..
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen
2025-08-28 21:44 [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen Colin Foster
2025-09-01 20:57 ` Jakub Kicinski
@ 2026-08-10 8:20 ` kernel test robot
1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2026-08-10 8:20 UTC (permalink / raw)
To: Colin Foster, linux-kernel, netdev
Cc: oe-kbuild-all, Steve Glendinning, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Colin Foster
Hi Colin,
kernel test robot noticed the following build warnings:
[auto build test WARNING on horms-ipvs/master]
[cannot apply to linus/master v7.2-rc6 next-20260807]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Colin-Foster/smsc911x-add-second-read-of-EEPROM-mac-when-possible-corruption-seen/20260810-123819
base: https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
patch link: https://lore.kernel.org/r/20250828214452.11683-1-colin.foster%40in-advantage.com
patch subject: [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20260810/202608101549.Sbhie1Im-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 16.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260810/202608101549.Sbhie1Im-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202608101549.Sbhie1Im-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/net/ethernet/smsc/smsc911x.c: In function 'smsc911x_read_mac_address':
>> drivers/net/ethernet/smsc/smsc911x.c:2179:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
2179 | u8 addr[ETH_ALEN];
| ^~
vim +2179 drivers/net/ethernet/smsc/smsc911x.c
631b7568c7d8f0 drivers/net/smsc911x.c Steve Glendinning 2008-12-25 2160
31f4574774e98a drivers/net/smsc911x.c Steve Glendinning 2009-01-27 2161 /* copies the current mac address from hardware to dev->dev_addr */
8489ec1f970064 drivers/net/ethernet/smsc/smsc911x.c Bill Pemberton 2012-12-03 2162 static void smsc911x_read_mac_address(struct net_device *dev)
31f4574774e98a drivers/net/smsc911x.c Steve Glendinning 2009-01-27 2163 {
31f4574774e98a drivers/net/smsc911x.c Steve Glendinning 2009-01-27 2164 struct smsc911x_data *pdata = netdev_priv(dev);
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2165 u32 mac_high16, mac_low32;
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2166
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2167 mac_high16 = smsc911x_mac_read(pdata, ADDRH);
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2168 mac_low32 = smsc911x_mac_read(pdata, ADDRL);
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2169
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2170 /*
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2171 * The first mac_read always returns 0. Re-read it to get the
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2172 * full MAC
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2173 */
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2174 if (mac_high16 == 0) {
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2175 SMSC_TRACE(pdata, probe, "Re-read MAC ADDRH\n");
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2176 mac_high16 = smsc911x_mac_read(pdata, ADDRH);
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2177 }
8b1e595e2e7800 drivers/net/ethernet/smsc/smsc911x.c Colin Foster 2025-08-28 2178
f15fef4c0675d4 drivers/net/ethernet/smsc/smsc911x.c Jakub Kicinski 2021-10-18 @2179 u8 addr[ETH_ALEN];
f15fef4c0675d4 drivers/net/ethernet/smsc/smsc911x.c Jakub Kicinski 2021-10-18 2180
f15fef4c0675d4 drivers/net/ethernet/smsc/smsc911x.c Jakub Kicinski 2021-10-18 2181 addr[0] = (u8)(mac_low32);
f15fef4c0675d4 drivers/net/ethernet/smsc/smsc911x.c Jakub Kicinski 2021-10-18 2182 addr[1] = (u8)(mac_low32 >> 8);
f15fef4c0675d4 drivers/net/ethernet/smsc/smsc911x.c Jakub Kicinski 2021-10-18 2183 addr[2] = (u8)(mac_low32 >> 16);
f15fef4c0675d4 drivers/net/ethernet/smsc/smsc911x.c Jakub Kicinski 2021-10-18 2184 addr[3] = (u8)(mac_low32 >> 24);
f15fef4c0675d4 drivers/net/ethernet/smsc/smsc911x.c Jakub Kicinski 2021-10-18 2185 addr[4] = (u8)(mac_high16);
f15fef4c0675d4 drivers/net/ethernet/smsc/smsc911x.c Jakub Kicinski 2021-10-18 2186 addr[5] = (u8)(mac_high16 >> 8);
f15fef4c0675d4 drivers/net/ethernet/smsc/smsc911x.c Jakub Kicinski 2021-10-18 2187 eth_hw_addr_set(dev, addr);
31f4574774e98a drivers/net/smsc911x.c Steve Glendinning 2009-01-27 2188 }
31f4574774e98a drivers/net/smsc911x.c Steve Glendinning 2009-01-27 2189
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-10 8:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-28 21:44 [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen Colin Foster
2025-09-01 20:57 ` Jakub Kicinski
2025-09-02 12:31 ` Colin Foster
2025-09-02 19:05 ` Jakub Kicinski
2026-08-10 8:20 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox