* [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application
@ 2022-09-15 14:48 Chunhao Lin
[not found] ` <CAFSsGVvKYRKtxXTSRuqhf9nec0hQv3PQS5rjZBjZDGyjU4+Bbw@mail.gmail.com>
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Chunhao Lin @ 2022-09-15 14:48 UTC (permalink / raw)
To: hkallweit1; +Cc: netdev, nic_swsd, Chunhao Lin
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/20200730100151.7490-1-ashiduka@fujitsu.com/
Signed-off-by: Chunhao Lin <hau@realtek.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 */
};
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
+/* 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;
+
+ /* MII controller setting */
+ new_bus = alloc_mdio_bitbang(&bitbang->ctrl);
+ 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
+
+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};
+ 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;
+ }
+
+ 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)
+{
+ /* 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 &&
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application
[not found] ` <5cca2b9476fa42a1b0397e1465f0ef73@realtek.com>
@ 2022-09-15 18:13 ` Hau
0 siblings, 0 replies; 8+ messages in thread
From: Hau @ 2022-09-15 18:13 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: <netdev@vger.kernel.org>, nic_swsd
> Chunhao Lin <hau@realtek.com> schrieb am Do., 15. Sept. 2022, 16:48:
> 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/20200730100151.7490-1-ashiduka@fujitsu.com/
>
> The commit message misses a change log.
>
v4:
- Update commit message.
- Update variable and function name.
- Remove rtl8168h + rtl8211fs not power down phy patch.
- Enable pass_linkctl_en. This is the patch for reboot ping fail issue.
- Set bb_ops owner to NULL to prevent rmmod cannot be done.
- Use alloc_mdio_bitbang() to allocate mdio bus.
v3:
- Update commit message.
v2:
- Use bitbanged MDIO framework to access rtl8211fs phy.
>
> Signed-off-by: Chunhao Lin <hau@realtek.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 */
> };
>
> 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
> +/* 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;
> +
> + /* MII controller setting */
> + new_bus = alloc_mdio_bitbang(&bitbang->ctrl);
> + 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
> +
>
> This hard dependency on a specific PHY isn't acceptable. At least you should
> explain in the commit message why only this and no other PHY can be used.
>
> +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};
> + 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;
> + }
> +
> + 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 seems to configure the RTL8211FS. If that's the case it should be done in
> the PHY driver.
>
>
> +{
> + /* 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 &&
> --
> 2.25.1
------Please consider the environment before printing this e-mail.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application
2022-09-15 14:48 [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application Chunhao Lin
[not found] ` <CAFSsGVvKYRKtxXTSRuqhf9nec0hQv3PQS5rjZBjZDGyjU4+Bbw@mail.gmail.com>
@ 2022-09-20 21:59 ` Jakub Kicinski
2022-09-22 0:27 ` Jakub Kicinski
2022-10-08 21:36 ` Heiner Kallweit
2 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2022-09-20 21:59 UTC (permalink / raw)
To: hkallweit1, Andrew Lunn; +Cc: Chunhao Lin, netdev, nic_swsd
On Thu, 15 Sep 2022 22:48:07 +0800 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/20200730100151.7490-1-ashiduka@fujitsu.com/
Heiner, Andrew, good?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application
2022-09-20 21:59 ` Jakub Kicinski
@ 2022-09-22 0:27 ` Jakub Kicinski
2022-10-03 12:28 ` Hau
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2022-09-22 0:27 UTC (permalink / raw)
To: hkallweit1, Andrew Lunn; +Cc: Chunhao Lin, netdev, nic_swsd
On Tue, 20 Sep 2022 14:59:44 -0700 Jakub Kicinski wrote:
> On Thu, 15 Sep 2022 22:48:07 +0800 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/20200730100151.7490-1-ashiduka@fujitsu.com/
>
> Heiner, Andrew, good?
Well, we need Hainer or Andrew to ack, I'm marking this patch
as Deferred until such endorsement is obtained.
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application
2022-09-22 0:27 ` Jakub Kicinski
@ 2022-10-03 12:28 ` Hau
0 siblings, 0 replies; 8+ messages in thread
From: Hau @ 2022-10-03 12:28 UTC (permalink / raw)
To: Jakub Kicinski, hkallweit1, Andrew Lunn; +Cc: netdev, nic_swsd
> From: Jakub Kicinski [mailto:kuba@kernel.org]
> Sent: Thursday, September 22, 2022 8:27 AM
> To: hkallweit1@gmail.com; Andrew Lunn <andrew@lunn.ch>
> Cc: Hau <hau@realtek.com>; netdev@vger.kernel.org; nic_swsd
> <nic_swsd@realtek.com>
> Subject: Re: [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a)
> + rtl8211fs fiber application
>
> On Tue, 20 Sep 2022 14:59:44 -0700 Jakub Kicinski wrote:
> > On Thu, 15 Sep 2022 22:48:07 +0800 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/2020073
> > > 0100151.7490-1-ashiduka@fujitsu.com/
> >
> > Heiner, Andrew, good?
>
> Well, we need Hainer or Andrew to ack, I'm marking this patch as Deferred
> until such endorsement is obtained.
>
Hi Heiner, could you help to check this patch?
Thanks.
------Please consider the environment before printing this e-mail.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application
2022-09-15 14:48 [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application Chunhao Lin
[not found] ` <CAFSsGVvKYRKtxXTSRuqhf9nec0hQv3PQS5rjZBjZDGyjU4+Bbw@mail.gmail.com>
2022-09-20 21:59 ` Jakub Kicinski
@ 2022-10-08 21:36 ` Heiner Kallweit
2022-10-12 16:28 ` Hau
2 siblings, 1 reply; 8+ messages in thread
From: Heiner Kallweit @ 2022-10-08 21:36 UTC (permalink / raw)
To: Chunhao Lin; +Cc: netdev, nic_swsd, Andrew Lunn
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/20200730100151.7490-1-ashiduka@fujitsu.com/
>
> Signed-off-by: Chunhao Lin <hau@realtek.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.
> };
>
> 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
> +/* 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.
> +
> + /* 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.
> +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?
> + 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
> + /* 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 &&
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application
2022-10-08 21:36 ` Heiner Kallweit
@ 2022-10-12 16:28 ` Hau
2022-10-15 8:29 ` Heiner Kallweit
0 siblings, 1 reply; 8+ messages in thread
From: Hau @ 2022-10-12 16:28 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: netdev, nic_swsd, Andrew Lunn
>
> 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@fujitsu.com/
> >
> > Signed-off-by: Chunhao Lin <hau@realtek.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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application
2022-10-12 16:28 ` Hau
@ 2022-10-15 8:29 ` Heiner Kallweit
0 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2022-10-15 8:29 UTC (permalink / raw)
To: Hau; +Cc: netdev, nic_swsd, Andrew Lunn
On 12.10.2022 18:28, Hau wrote:
>>
>> 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@fujitsu.com/
>>>
>>> Signed-off-by: Chunhao Lin <hau@realtek.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.
>
This would need a more detailed description in the commit message.
You establish a standard bitbanged MDIO bus. So at least on that
level the MAC should be able to commmunicate with any PHY.
What creates the constraint that only RTL8211FS can be used here?
>>> + 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?
> The Realtek PHY driver would recognize RTL8211FS as RTL8211F currently,
because both have the same PHY ID. So you would have to find a way to
distinguish between them and implement match_phy_device() callback.
You can find an example of match_phy_device() usage in the driver.
Then a RTL8211FS-specific config_init() implementation would have the
following code.
>>> + /* 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-10-15 8:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-15 14:48 [PATCH net-next v4] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application Chunhao Lin
[not found] ` <CAFSsGVvKYRKtxXTSRuqhf9nec0hQv3PQS5rjZBjZDGyjU4+Bbw@mail.gmail.com>
[not found] ` <5cca2b9476fa42a1b0397e1465f0ef73@realtek.com>
2022-09-15 18:13 ` Hau
2022-09-20 21:59 ` Jakub Kicinski
2022-09-22 0:27 ` Jakub Kicinski
2022-10-03 12:28 ` Hau
2022-10-08 21:36 ` Heiner Kallweit
2022-10-12 16:28 ` Hau
2022-10-15 8:29 ` Heiner Kallweit
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.