| 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
On Tue, 7 May 2024 at 12:16, Bjørn Mork <bjorn@mork.no> wrote: > > 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. We can find out if we look at the function signature: int phy_read_paged(struct phy_device *phydev, int page, u32 regnum); so that's a page, not a mask. Upstream usage is mostly hex, with the icplus phy driver being the sole exception. And unsurprisingly, realtek drivers always use magic numbers, never defines. Can't have other people understand how the hardware works. Best Regards, Jonas
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(-)