| Message ID | 20240426224008.3400172-2-stijn@linux-ipv6.be |
|---|---|
| State | Accepted, archived |
| Delegated to: | Stijn Tintel |
| Headers | show |
| Series | [1/2] realtek/rtl839x: respect phy-is-integrated property | expand |
stijn@linux-ipv6.be writes: > phy_write_paged(phydev, 31, 27, 0x0002); > val = phy_read_paged(phydev, 31, 28); .. > phy_write_paged(phydev, 0x1f, 0x1b, 0x0002); > val = phy_read_paged(phydev, 0x1f, 0x1c); While you're doing spring cleaning.... That piece of cut-n-paste code looks very funny. Bjørn
On 27/04/2024 11:16, Bjørn Mork wrote: > stijn@linux-ipv6.be writes: > >> phy_write_paged(phydev, 31, 27, 0x0002); >> val = phy_read_paged(phydev, 31, 28); > .. >> phy_write_paged(phydev, 0x1f, 0x1b, 0x0002); >> val = phy_read_paged(phydev, 0x1f, 0x1c); > > While you're doing spring cleaning.... That piece of cut-n-paste code > looks very funny. > I'd gladly do some more spring cleaning, but what's your actual suggestion here? Use hex everywhere instead of mixed hex/base 10? Thanks, Stijn
Stijn Tintel <stijn@linux-ipv6.be> writes: > On 27/04/2024 11:16, Bjørn Mork wrote: >> stijn@linux-ipv6.be writes: >> >>> phy_write_paged(phydev, 31, 27, 0x0002); >>> val = phy_read_paged(phydev, 31, 28); >> .. >>> phy_write_paged(phydev, 0x1f, 0x1b, 0x0002); >>> val = phy_read_paged(phydev, 0x1f, 0x1c); >> >> While you're doing spring cleaning.... That piece of cut-n-paste code >> looks very funny. >> > I'd gladly do some more spring cleaning, but what's your actual > suggestion here? Use hex everywhere instead of mixed hex/base 10? I would have used hex since that 0x1f looks like a mask. But it should at least be consistent so that it is obvious that this code is a target for de-duplication. Bjørn
diff --git a/target/linux/realtek/files-5.15/drivers/net/phy/rtl83xx-phy.c b/target/linux/realtek/files-5.15/drivers/net/phy/rtl83xx-phy.c index 56e8a7f49d..490020989f 100644 --- a/target/linux/realtek/files-5.15/drivers/net/phy/rtl83xx-phy.c +++ b/target/linux/realtek/files-5.15/drivers/net/phy/rtl83xx-phy.c @@ -46,6 +46,8 @@ extern struct mutex smi_lock; /* external RTL821X PHY uses register 0x1e to select media page */ #define RTL821XEXT_MEDIA_PAGE_SELECT 0x1e +#define RTL821X_CHIP_ID 0x6276 + #define RTL821X_MEDIA_PAGE_AUTO 0 #define RTL821X_MEDIA_PAGE_COPPER 1 #define RTL821X_MEDIA_PAGE_FIBRE 3 @@ -834,7 +836,7 @@ static int rtl8380_configure_ext_rtl8218b(struct phy_device *phydev) /* Read internal PHY ID */ phy_write_paged(phydev, 31, 27, 0x0002); val = phy_read_paged(phydev, 31, 28); - if (val != 0x6276) { + if (val != RTL821X_CHIP_ID) { phydev_err(phydev, "Expected external RTL8218B, found PHY-ID %x\n", val); return -1; } @@ -1331,7 +1333,7 @@ static int rtl8380_configure_rtl8214fc(struct phy_device *phydev) phy_write_paged(phydev, 0, RTL821XEXT_MEDIA_PAGE_SELECT, RTL821X_MEDIA_PAGE_COPPER); phy_write_paged(phydev, 0x1f, 0x1b, 0x0002); val = phy_read_paged(phydev, 0x1f, 0x1c); - if (val != 0x6276) { + if (val != RTL821X_CHIP_ID) { phydev_err(phydev, "Expected external RTL8214FC, found PHY-ID %x\n", val); return -1; }
According to the Realtek SDK code, the RTL8214FC, RTL8218B and RTL8218FB all have the same chip ID 0x6276. Let's add a constant for it, as we're using it in more than one location. Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be> --- .../linux/realtek/files-5.15/drivers/net/phy/rtl83xx-phy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)