On Mon, Oct 28, 2024 at 01:15:42PM +0100, Andrew Lunn wrote: > > +static const u8 lan9370_phy_addr[] = { > > + [0] = 2, /* Port 1, T1 AFE0 */ > > + [1] = 3, /* Port 2, T1 AFE1 */ > > + [2] = 5, /* Port 3, T1 AFE3 */ > > + [3] = 6, /* Port 4, T1 AFE4 */ > > + [4] = U8_MAX, /* Port 5, RGMII 2 */ > > +}; > > I think it would be good to add a #define for U8_MAX which gives a > hint at its meaning. ack > > + for (i = 0; i < dev->info->port_cnt; i++) { > > + if (phy_addr_map[i] == U8_MAX) > > + dev->phy_addr_map[i] = phy_addr_map[i]; > > + else > > + dev->phy_addr_map[i] = phy_addr_map[i] + offset; > > + } > > My first guess was that U8_MAX means the PHY is external, so could be > on any address depending on strapping. Looking at this code, i'm not > sure it does actually mean that. Yes, the U8_MAX for ports without integrated PHYs. This code calculates address for integrated PHYs, which can be different depending on strap configuration. I'll use some different define. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |