lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 12 Oct 2022 16:28:47 +0000 From: Hau <hau@...ltek.com> To: Heiner Kallweit <hkallweit1@...il.com> CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, nic_swsd <nic_swsd@...ltek.com>, Andrew Lunn <andrew@...n.ch> Subject: RE: [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application > > On 15.09.2022 16:48, Chunhao Lin wrote: > > rtl8168h(revid 0x2a) + rtl8211fs is for fiber related application. > > rtl8168h is connected to rtl8211fs mdio bus via its eeprom or gpio pins. > > > > In this patch, use bitbanged MDIO framework to access rtl8211fs via > > rtl8168h's eeprom or gpio pins. > > > > And set mdiobb_ops owner to NULL to avoid increase module's refcount > > to prevent rmmod cannot be done. > > https://patchwork.kernel.org/project/linux-renesas-soc/patch/202007301 > > 00151.7490-1-ashiduka@...itsu.com/ > > > > Signed-off-by: Chunhao Lin <hau@...ltek.com> > > --- > > drivers/net/ethernet/realtek/Kconfig | 1 + > > drivers/net/ethernet/realtek/r8169_main.c | 286 > > +++++++++++++++++++++- > > 2 files changed, 286 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/realtek/Kconfig > > b/drivers/net/ethernet/realtek/Kconfig > > index 93d9df55b361..20367114ac72 100644 > > --- a/drivers/net/ethernet/realtek/Kconfig > > +++ b/drivers/net/ethernet/realtek/Kconfig > > @@ -100,6 +100,7 @@ config R8169 > > depends on PCI > > select FW_LOADER > > select CRC32 > > + select MDIO_BITBANG > > select PHYLIB > > select REALTEK_PHY > > help > > diff --git a/drivers/net/ethernet/realtek/r8169_main.c > > b/drivers/net/ethernet/realtek/r8169_main.c > > index f6f63ba6593a..395eae62050a 100644 > > --- a/drivers/net/ethernet/realtek/r8169_main.c > > +++ b/drivers/net/ethernet/realtek/r8169_main.c > > @@ -28,6 +28,7 @@ > > #include <linux/bitfield.h> > > #include <linux/prefetch.h> > > #include <linux/ipv6.h> > > +#include <linux/mdio-bitbang.h> > > #include <asm/unaligned.h> > > #include <net/ip6_checksum.h> > > > > @@ -333,6 +334,15 @@ enum rtl8125_registers { > > EEE_TXIDLE_TIMER_8125 = 0x6048, > > }; > > > > +enum rtl8168_sfp_registers { > > + MDIO_IN = 0xdc04, > > + PINOE = 0xdc06, > > + PIN_I_SEL_1 = 0xdc08, > > + PIN_I_SEL_2 = 0xdc0A, > > + PINPU = 0xdc18, > > + GPOUTPIN_SEL = 0xdc20, > > +}; > > + > > #define RX_VLAN_INNER_8125 BIT(22) > > #define RX_VLAN_OUTER_8125 BIT(23) > > #define RX_VLAN_8125 (RX_VLAN_INNER_8125 | > RX_VLAN_OUTER_8125) > > @@ -573,6 +583,24 @@ struct rtl8169_tc_offsets { > > __le16 rx_missed; > > }; > > > > +struct rtl_sfp_if_info { > > + u16 mdio_oe_i; > > + u16 mdio_oe_o; > > + u16 mdio_pu; > > + u16 mdio_pd; > > + u16 mdc_pu; > > + u16 mdc_pd; > > +}; > > + > > +struct rtl_sfp_if_mask { > > + const u16 pin; > > + const u16 mdio_oe; > > + const u16 mdio; > > + const u16 mdc; > > + const u16 phy_addr; > > + const u16 rb_pos; > > +}; > > + > > enum rtl_flag { > > RTL_FLAG_TASK_ENABLED = 0, > > RTL_FLAG_TASK_RESET_PENDING, > > @@ -585,6 +613,12 @@ enum rtl_dash_type { > > RTL_DASH_EP, > > }; > > > > +enum rtl_sfp_if_type { > > + RTL_SFP_IF_NONE, > > + RTL_SFP_IF_EEPROM, > > + RTL_SFP_IF_GPIO, > > +}; > > + > > struct rtl8169_private { > > void __iomem *mmio_addr; /* memory map physical address */ > > struct pci_dev *pci_dev; > > @@ -624,6 +658,10 @@ struct rtl8169_private { > > struct rtl_fw *rtl_fw; > > > > u32 ocp_base; > > + > > + enum rtl_sfp_if_type sfp_if_type; > > + > > + struct mii_bus *mii_bus; /* MDIO bus control */ > > This is at least misleading because mii_bus is unused for the integrated > copper PHY's. Maybe you can store the mii_bus in struct bb_info and store > the bb_info instance in struct rtl8169_private. > I will do this. > > }; > > > > typedef void (*rtl_generic_fct)(struct rtl8169_private *tp); @@ > > -1199,6 +1237,242 @@ static enum rtl_dash_type rtl_check_dash(struct > rtl8169_private *tp) > > } > > } > > > > +struct bb_info { > > + struct rtl8169_private *tp; > > + struct mdiobb_ctrl ctrl; > > + struct rtl_sfp_if_mask mask; > > + u16 pinoe; > > + u16 pin_i_sel_1; > > + u16 pin_i_sel_2; > > +}; > > + > > +/* Data I/O pin control */ > > +static void rtl_bb_mdio_dir(struct mdiobb_ctrl *ctrl, int output) { > > + struct bb_info *bitbang = container_of(ctrl, struct bb_info, ctrl); > > + struct rtl8169_private *tp = bitbang->tp; > > + const u16 mask = bitbang->mask.mdio_oe; > > + const u16 reg = PINOE; > > + u16 value; > > + > > + value = bitbang->pinoe; > > + if (output) > > + value |= mask; > > + else > > + value &= ~mask; > > + r8168_mac_ocp_write(tp, reg, value); } > > + > > +/* Set bit data*/ > > +static void rtl_bb_set_mdio(struct mdiobb_ctrl *ctrl, int set) { > > + struct bb_info *bitbang = container_of(ctrl, struct bb_info, ctrl); > > + struct rtl8169_private *tp = bitbang->tp; > > + const u16 mask = bitbang->mask.mdio; > > + const u16 reg = PIN_I_SEL_2; > > + u16 value; > > + > > + value = bitbang->pin_i_sel_2; > > + if (set) > > + value |= mask; > > + else > > + value &= ~mask; > > + r8168_mac_ocp_write(tp, reg, value); } > > + > > +/* Get bit data*/ > > +static int rtl_bb_get_mdio(struct mdiobb_ctrl *ctrl) { > > + struct bb_info *bitbang = container_of(ctrl, struct bb_info, ctrl); > > + struct rtl8169_private *tp = bitbang->tp; > > + const u16 reg = MDIO_IN; > > + > > + return (r8168_mac_ocp_read(tp, reg) & BIT(bitbang- > >mask.rb_pos)) != > > +0; } > > + > > +/* MDC pin control */ > > +static void rtl_bb_mdc_ctrl(struct mdiobb_ctrl *ctrl, int set) { > > + struct bb_info *bitbang = container_of(ctrl, struct bb_info, ctrl); > > + struct rtl8169_private *tp = bitbang->tp; > > + const u16 mask = bitbang->mask.mdc; > > + const u16 mdc_reg = PIN_I_SEL_1; > > + u16 value; > > + > > + value = bitbang->pin_i_sel_1; > > + if (set) > > + value |= mask; > > + else > > + value &= ~mask; > > + r8168_mac_ocp_write(tp, mdc_reg, value); } > > + > > +/* mdio bus control struct */ > > +static const struct mdiobb_ops bb_ops = { > > + .owner = NULL, /* set to NULL for not increase refcount */ > > + .set_mdc = rtl_bb_mdc_ctrl, > > + .set_mdio_dir = rtl_bb_mdio_dir, > > + .set_mdio_data = rtl_bb_set_mdio, > > + .get_mdio_data = rtl_bb_get_mdio, > > +}; > > + > > +#define MDIO_READ 2 > > +#define MDIO_WRITE 1 > > This duplicates the defines in drivers/net/mdio/mdio-bitbang.c > I will check this. > > +/* MDIO bus init function */ > > +static int rtl_mdio_bitbang_init(struct rtl8169_private *tp) { > > + struct device *d = tp_to_dev(tp); > > + struct bb_info *bitbang; > > + struct mii_bus *new_bus; > > + > > + /* create bit control struct for PHY */ > > + bitbang = devm_kzalloc(d, sizeof(struct bb_info), GFP_KERNEL); > > + if (!bitbang) > > + return -ENOMEM; > > + > > + /* bitbang init */ > > + bitbang->tp = tp; > > + bitbang->ctrl.ops = &bb_ops; > > + bitbang->ctrl.op_c22_read = MDIO_READ; > > + bitbang->ctrl.op_c22_write = MDIO_WRITE; > > MDIO_READ/WRITE are assigned by alloc_mdio_bitbang(), you don't need > this here. > I will check this. > > + > > + /* MII controller setting */ > > + new_bus = alloc_mdio_bitbang(&bitbang->ctrl); > > It seems the allocated MDIO bus is never registered. > This should be added. > > > + if (!new_bus) > > + return -ENOMEM; > > + > > + tp->mii_bus = new_bus; > > + > > + return 0; > > +} > > + > > +static void rtl_sfp_bitbang_init(struct rtl8169_private *tp, > > + struct rtl_sfp_if_mask *mask) > > +{ > > + struct bb_info *bitbang = > > + container_of(tp->mii_bus->priv, struct bb_info, ctrl); > > + > > + r8168_mac_ocp_modify(tp, PINPU, mask->pin, 0); > > + r8168_mac_ocp_modify(tp, PINOE, 0, mask->pin); > > + bitbang->pinoe = r8168_mac_ocp_read(tp, PINOE); > > + bitbang->pin_i_sel_1 = r8168_mac_ocp_read(tp, PIN_I_SEL_1); > > + bitbang->pin_i_sel_2 = r8168_mac_ocp_read(tp, PIN_I_SEL_2); > > + memcpy(&bitbang->mask, mask, sizeof(struct rtl_sfp_if_mask)); } > > + > > +static void rtl_sfp_mdio_write(struct rtl8169_private *tp, > > + u8 reg, > > + u16 val) > > +{ > > + struct mii_bus *bus = tp->mii_bus; > > + struct bb_info *bitbang; > > + > > + if (!bus) > > + return; > > + > > + bitbang = container_of(bus->priv, struct bb_info, ctrl); > > + bus->write(bus, bitbang->mask.phy_addr, reg, val); } > > + > > +static u16 rtl_sfp_mdio_read(struct rtl8169_private *tp, > > + u8 reg) > > +{ > > + struct mii_bus *bus = tp->mii_bus; > > + struct bb_info *bitbang; > > + > > + if (!bus) > > + return ~0; > > + > > + bitbang = container_of(bus->priv, struct bb_info, ctrl); > > + > > + return bus->read(bus, bitbang->mask.phy_addr, reg); } > > + > > +static void rtl_sfp_mdio_modify(struct rtl8169_private *tp, u32 reg, u16 > mask, > > + u16 set) > > +{ > > + u16 data = rtl_sfp_mdio_read(tp, reg); > > + > > + rtl_sfp_mdio_write(tp, reg, (data & ~mask) | set); } > > + > > +#define RTL8211FS_PHY_ID_1 0x001c > > +#define RTL8211FS_PHY_ID_2 0xc916 > > + > > As stated earlier, there should be no dependency on a specific external PHY > in a MAC driver. It's not clear why the bitbanged MDIO bus can't be used with > other PHY's. > I will try to remove this restriction. > > +static enum rtl_sfp_if_type rtl8168h_check_sfp(struct rtl8169_private > > +*tp) { > > + static struct rtl_sfp_if_mask rtl_sfp_if_eeprom_mask = { > > + 0x0050, 0x0040, 0x000f, 0x0f00, 0, 6}; > > + static struct rtl_sfp_if_mask rtl_sfp_if_gpo_mask = { > > + 0x0210, 0x0200, 0xf000, 0x0f00, 1, 9}; > > You hard-code PHY addresses 0/1 here. Why not auto-probing? > And can't there be designs using other PHY addresses? > Not this application only for rtl8211fs. > > + int const checkcnt = 4; > > + int i; > > + > > + if (rtl_mdio_bitbang_init(tp)) > > + return RTL_SFP_IF_NONE; > > + > > + rtl_sfp_bitbang_init(tp, &rtl_sfp_if_eeprom_mask); > > + rtl_sfp_mdio_write(tp, 0x1f, 0x0000); > > + for (i = 0; i < checkcnt; i++) { > > + if (rtl_sfp_mdio_read(tp, MII_PHYSID1) != > RTL8211FS_PHY_ID_1 || > > + rtl_sfp_mdio_read(tp, MII_PHYSID2) != > RTL8211FS_PHY_ID_2) > > + break; > > See earlier comment on dependency on a specific PHY model. > > > + } > > + > > + if (i == checkcnt) > > + return RTL_SFP_IF_EEPROM; > > + > > + rtl_sfp_bitbang_init(tp, &rtl_sfp_if_gpo_mask); > > + rtl_sfp_mdio_write(tp, 0x1f, 0x0000); > > + for (i = 0; i < checkcnt; i++) { > > + if (rtl_sfp_mdio_read(tp, MII_PHYSID1) != > RTL8211FS_PHY_ID_1 || > > + rtl_sfp_mdio_read(tp, MII_PHYSID2) != > RTL8211FS_PHY_ID_2) > > + break; > > + } > > + > > + if (i == checkcnt) > > + return RTL_SFP_IF_GPIO; > > + > > + return RTL_SFP_IF_NONE; > > +} > > + > > +static enum rtl_sfp_if_type rtl_check_sfp(struct rtl8169_private *tp) > > +{ > > + switch (tp->mac_version) { > > + case RTL_GIGA_MAC_VER_46: > > + if (tp->pci_dev->revision == 0x2a) > > + return rtl8168h_check_sfp(tp); > > + else > > + return RTL_SFP_IF_NONE; > > + default: > > + return RTL_SFP_IF_NONE; > > + } > > +} > > + > > +static void rtl_hw_sfp_phy_config(struct rtl8169_private *tp) { > > This PHY model specific initialization belongs to the PHY driver. > Presumably RTL8211FS support should be added to drivers/net/phy/realtek.c > I am not familiar with phy driver. May I know how to do this? > > + /* disable ctap */ > > + rtl_sfp_mdio_write(tp, 0x1f, 0x0a43); > > + rtl_sfp_mdio_modify(tp, 0x19, BIT(6), 0); > > + > > + /* change Rx threshold */ > > + rtl_sfp_mdio_write(tp, 0x1f, 0x0dcc); > > + rtl_sfp_mdio_modify(tp, 0x14, 0, BIT(2) | BIT(3) | BIT(4)); > > + > > + /* switch pin34 to PMEB pin */ > > + rtl_sfp_mdio_write(tp, 0x1f, 0x0d40); > > + rtl_sfp_mdio_modify(tp, 0x16, 0, BIT(5)); > > + > > + /* enable pass_linkctl_en */ > > + rtl_sfp_mdio_write(tp, 0x1f, 0x0a4b); > > + rtl_sfp_mdio_modify(tp, 0x11, 0, BIT(4)); > > + > > + rtl_sfp_mdio_write(tp, 0x1f, 0x0000); > > + > > + /* disable ctap */ > > + phy_modify_paged(tp->phydev, 0x0a43, 0x11, BIT(6), 0); } > > + > > static void rtl_set_d3_pll_down(struct rtl8169_private *tp, bool > > enable) { > > switch (tp->mac_version) { > > @@ -2168,6 +2442,9 @@ static void rtl8169_init_phy(struct rtl8169_private > *tp) > > tp->pci_dev->subsystem_device == 0xe000) > > phy_write_paged(tp->phydev, 0x0001, 0x10, 0xf01b); > > > > + if (tp->sfp_if_type != RTL_SFP_IF_NONE) > > + rtl_hw_sfp_phy_config(tp); > > + > > /* We may have called phy_speed_down before */ > > phy_speed_up(tp->phydev); > > > > @@ -2224,7 +2501,8 @@ static void rtl_prepare_power_down(struct > rtl8169_private *tp) > > rtl_ephy_write(tp, 0x19, 0xff64); > > > > if (device_may_wakeup(tp_to_dev(tp))) { > > - phy_speed_down(tp->phydev, false); > > + if (tp->sfp_if_type == RTL_SFP_IF_NONE) > > + phy_speed_down(tp->phydev, false); > > rtl_wol_enable_rx(tp); > > } > > } > > @@ -4866,6 +5144,10 @@ static void rtl_remove_one(struct pci_dev > *pdev) > > if (tp->dash_type != RTL_DASH_NONE) > > rtl8168_driver_stop(tp); > > > > + /* free bitbang info */ > > + if (tp->mii_bus) > > + free_mdio_bitbang(tp->mii_bus); > > + > > rtl_release_firmware(tp); > > > > /* restore original MAC address */ > > @@ -5210,6 +5492,8 @@ static int rtl_init_one(struct pci_dev *pdev, > > const struct pci_device_id *ent) > > > > tp->dash_type = rtl_check_dash(tp); > > > > + tp->sfp_if_type = rtl_check_sfp(tp); > > + > > tp->cp_cmd = RTL_R16(tp, CPlusCmd) & CPCMD_MASK; > > > > if (sizeof(dma_addr_t) > 4 && tp->mac_version >= > RTL_GIGA_MAC_VER_18 > > && > > > ------Please consider the environment before printing this e-mail.
Powered by blists - more mailing lists