From: Leon Romanovsky <leon@kernel.org>
To: Lauri Jakku <ljakku77@gmail.com>
Cc: netdev@vger.kernel.org, Heiner Kallweit <hkallweit1@gmail.com>,
nic_swsd@realtek.com
Subject: Re: NET: r8168/r8169 identifying fix
Date: Mon, 13 Apr 2020 13:58:38 +0300 [thread overview]
Message-ID: <20200413105838.GK334007@unreal> (raw)
In-Reply-To: <4bc0fc0c-1437-fc41-1c50-38298214ec75@gmail.com>
On Mon, Apr 13, 2020 at 01:30:13PM +0300, Lauri Jakku wrote:
> From 2d41edd4e6455187094f3a13d58c46eeee35aa31 Mon Sep 17 00:00:00 2001
> From: Lauri Jakku <lja@iki.fi>
> Date: Mon, 13 Apr 2020 13:18:35 +0300
> Subject: [PATCH] NET: r8168/r8169 identifying fix
>
> The driver installation determination made properly by
> checking PHY vs DRIVER id's.
> ---
> drivers/net/ethernet/realtek/r8169_main.c | 70 ++++++++++++++++++++---
> drivers/net/phy/mdio_bus.c | 11 +++-
> 2 files changed, 72 insertions(+), 9 deletions(-)
I would say that most of the code is debug prints.
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index bf5bf05970a2..1ea6f121b561 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5149,6 +5149,9 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
> {
> struct pci_dev *pdev = tp->pci_dev;
> struct mii_bus *new_bus;
> + u32 phydev_id = 0;
> + u32 phydrv_id = 0;
> + u32 phydrv_id_mask = 0;
> int ret;
>
> new_bus = devm_mdiobus_alloc(&pdev->dev);
> @@ -5165,20 +5168,62 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
> new_bus->write = r8169_mdio_write_reg;
>
> ret = mdiobus_register(new_bus);
> + dev_info(&pdev->dev,
> + "mdiobus_register: %s, %d\n",
> + new_bus->id, ret);
> if (ret)
> return ret;
>
> tp->phydev = mdiobus_get_phy(new_bus, 0);
> +
> if (!tp->phydev) {
> mdiobus_unregister(new_bus);
> return -ENODEV;
> - } else if (!tp->phydev->drv) {
> - /* Most chip versions fail with the genphy driver.
> - * Therefore ensure that the dedicated PHY driver is loaded.
> - */
> - dev_err(&pdev->dev, "realtek.ko not loaded, maybe it needs to be added to initramfs?\n");
> - mdiobus_unregister(new_bus);
> - return -EUNATCH;
> + } else {
> + /* tp -> phydev ok */
> + int everything_OK = 0;
> +
> + /* Check driver id versus phy */
> +
> + if (tp->phydev->drv) {
> + u32 phydev_masked = 0xBEEFDEAD;
> + u32 drv_masked = ~0;
> + u32 phydev_match = ~0;
> + u32 drv_match = 0xDEADBEEF;
> +
> + phydev_id = tp->phydev->phy_id;
> + phydrv_id = tp->phydev->drv->phy_id;
> + phydrv_id_mask = tp->phydev->drv->phy_id_mask;
> +
> + drv_masked = phydrv_id & phydrv_id_mask;
> + phydev_masked = phydev_id & phydrv_id_mask;
Please don't do vertical space alignment, every change later near such
lines will be whitespace disaster.
Thanks
> +
> + dev_debug(&pdev->dev,
> + "%s: ID Check: (%x -> %x), drv (%x -> %x)\n",
> + new_bus->id, phydev_id, phydev_masked,
> + phydrv_id, drv_masked);
> +
> + phydev_match = phydev_masked & drv_masked;
> + phydev_match = phydev_match == phydev_masked;
> +
> + drv_match = phydev_masked & drv_masked;
> + drv_match = drv_match == drv_masked;
> +
> + dev_debug(&pdev->dev, "%s: ID Check: %x == %x\n",
> + new_bus->id, phydev_match, drv_match);
> +
> + everything_OK = (phydev_match == drv_match);
> + }
> +
> + if (!everything_OK) {
> + /* Most chip versions fail with the genphy driver.
> + * Therefore ensure that the dedicated PHY driver
> + * is loaded.
> + */
> + dev_err(&pdev->dev, "realtek.ko not loaded, maybe it needs to be added to initramfs?\n");
> + mdiobus_unregister(new_bus);
> + return -EUNATCH;
> + }
> }
>
> /* PHY will be woken up in rtl_open() */
> @@ -5435,6 +5480,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> u64_stats_init(&tp->rx_stats.syncp);
> u64_stats_init(&tp->tx_stats.syncp);
>
> + dev_dbg(&pdev->dev, "init: MAC\n");
> rtl_init_mac_address(tp);
>
> dev->ethtool_ops = &rtl8169_ethtool_ops;
> @@ -5483,12 +5529,15 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> dev->hw_features |= NETIF_F_RXFCS;
>
> jumbo_max = rtl_jumbo_max(tp);
> + dev_dbg(&pdev->dev, "init: jumbo max: %d\n", jumbo_max);
> if (jumbo_max)
> dev->max_mtu = jumbo_max;
>
> + dev_dbg(&pdev->dev, "init: irq mask\n");
> rtl_set_irq_mask(tp);
>
> tp->fw_name = rtl_chip_infos[chipset].fw_name;
> + dev_dbg(&pdev->dev, "init: FW name: %s\n", tp->fw_name);
>
> tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
> &tp->counters_phys_addr,
> @@ -5496,16 +5545,21 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (!tp->counters)
> return -ENOMEM;
>
> + dev_dbg(&pdev->dev, "init: set driver data\n");
> pci_set_drvdata(pdev, dev);
>
> + dev_dbg(&pdev->dev, "init: register mdio\n");
> rc = r8169_mdio_register(tp);
> + dev_dbg(&pdev->dev, "init: mdio register: %d\n", rc);
> if (rc)
> return rc;
>
> /* chip gets powered up in rtl_open() */
> + dev_dbg(&pdev->dev, "init: pll pwr down\n");
> rtl_pll_power_down(tp);
>
> rc = register_netdev(dev);
> + dev_dbg(&pdev->dev, "init: netdev register: %d\n", rc);
> if (rc)
> goto err_mdio_unregister;
>
> @@ -5525,6 +5579,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (pci_dev_run_wake(pdev))
> pm_runtime_put_sync(&pdev->dev);
>
> + dev_dbg(&pdev->dev, "init: ALL DONE!\n");
> +
> return 0;
>
> err_mdio_unregister:
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 522760c8bca6..719ea48164f6 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -112,6 +112,9 @@ EXPORT_SYMBOL(mdiobus_unregister_device);
> struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr)
> {
> struct mdio_device *mdiodev = bus->mdio_map[addr];
> + struct phy_device *rv = NULL;
> +
> + pr_debug("mii_bus %s addr %d, %p\n", bus->id, addr, mdiodev);
>
> if (!mdiodev)
> return NULL;
> @@ -119,7 +122,10 @@ struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr)
> if (!(mdiodev->flags & MDIO_DEVICE_FLAG_PHY))
> return NULL;
>
> - return container_of(mdiodev, struct phy_device, mdio);
> + rv = container_of(mdiodev, struct phy_device, mdio);
> + pr_debug("mii_bus OK? %s addr %d, %p -> %p\n",
> + bus->id, addr, mdiodev, rv);
> + return rv;
> }
> EXPORT_SYMBOL(mdiobus_get_phy);
>
> @@ -645,10 +651,11 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
> mdiobus_setup_mdiodev_from_board_info(bus, mdiobus_create_device);
>
> bus->state = MDIOBUS_REGISTERED;
> - pr_info("%s: probed\n", bus->name);
> + pr_info("%s: probed (mdiobus_register)\n", bus->name);
> return 0;
>
> error:
> + pr_err("%s: Error while in mdiobus_register: %d\n", bus->name, err);
> while (--i >= 0) {
> mdiodev = bus->mdio_map[i];
> if (!mdiodev)
> --
> 2.26.0
>
>
> From 2d41edd4e6455187094f3a13d58c46eeee35aa31 Mon Sep 17 00:00:00 2001
> From: Lauri Jakku <lja@iki.fi>
> Date: Mon, 13 Apr 2020 13:18:35 +0300
> Subject: [PATCH] NET: r8168/r8169 identifying fix
>
> The driver installation determination made properly by
> checking PHY vs DRIVER id's.
> ---
> drivers/net/ethernet/realtek/r8169_main.c | 70 ++++++++++++++++++++---
> drivers/net/phy/mdio_bus.c | 11 +++-
> 2 files changed, 72 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index bf5bf05970a2..1ea6f121b561 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5149,6 +5149,9 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
> {
> struct pci_dev *pdev = tp->pci_dev;
> struct mii_bus *new_bus;
> + u32 phydev_id = 0;
> + u32 phydrv_id = 0;
> + u32 phydrv_id_mask = 0;
> int ret;
>
> new_bus = devm_mdiobus_alloc(&pdev->dev);
> @@ -5165,20 +5168,62 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
> new_bus->write = r8169_mdio_write_reg;
>
> ret = mdiobus_register(new_bus);
> + dev_info(&pdev->dev,
> + "mdiobus_register: %s, %d\n",
> + new_bus->id, ret);
> if (ret)
> return ret;
>
> tp->phydev = mdiobus_get_phy(new_bus, 0);
> +
> if (!tp->phydev) {
> mdiobus_unregister(new_bus);
> return -ENODEV;
> - } else if (!tp->phydev->drv) {
> - /* Most chip versions fail with the genphy driver.
> - * Therefore ensure that the dedicated PHY driver is loaded.
> - */
> - dev_err(&pdev->dev, "realtek.ko not loaded, maybe it needs to be added to initramfs?\n");
> - mdiobus_unregister(new_bus);
> - return -EUNATCH;
> + } else {
> + /* tp -> phydev ok */
> + int everything_OK = 0;
> +
> + /* Check driver id versus phy */
> +
> + if (tp->phydev->drv) {
> + u32 phydev_masked = 0xBEEFDEAD;
> + u32 drv_masked = ~0;
> + u32 phydev_match = ~0;
> + u32 drv_match = 0xDEADBEEF;
> +
> + phydev_id = tp->phydev->phy_id;
> + phydrv_id = tp->phydev->drv->phy_id;
> + phydrv_id_mask = tp->phydev->drv->phy_id_mask;
> +
> + drv_masked = phydrv_id & phydrv_id_mask;
> + phydev_masked = phydev_id & phydrv_id_mask;
> +
> + dev_debug(&pdev->dev,
> + "%s: ID Check: (%x -> %x), drv (%x -> %x)\n",
> + new_bus->id, phydev_id, phydev_masked,
> + phydrv_id, drv_masked);
> +
> + phydev_match = phydev_masked & drv_masked;
> + phydev_match = phydev_match == phydev_masked;
> +
> + drv_match = phydev_masked & drv_masked;
> + drv_match = drv_match == drv_masked;
> +
> + dev_debug(&pdev->dev, "%s: ID Check: %x == %x\n",
> + new_bus->id, phydev_match, drv_match);
> +
> + everything_OK = (phydev_match == drv_match);
> + }
> +
> + if (!everything_OK) {
> + /* Most chip versions fail with the genphy driver.
> + * Therefore ensure that the dedicated PHY driver
> + * is loaded.
> + */
> + dev_err(&pdev->dev, "realtek.ko not loaded, maybe it needs to be added to initramfs?\n");
> + mdiobus_unregister(new_bus);
> + return -EUNATCH;
> + }
> }
>
> /* PHY will be woken up in rtl_open() */
> @@ -5435,6 +5480,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> u64_stats_init(&tp->rx_stats.syncp);
> u64_stats_init(&tp->tx_stats.syncp);
>
> + dev_dbg(&pdev->dev, "init: MAC\n");
> rtl_init_mac_address(tp);
>
> dev->ethtool_ops = &rtl8169_ethtool_ops;
> @@ -5483,12 +5529,15 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> dev->hw_features |= NETIF_F_RXFCS;
>
> jumbo_max = rtl_jumbo_max(tp);
> + dev_dbg(&pdev->dev, "init: jumbo max: %d\n", jumbo_max);
> if (jumbo_max)
> dev->max_mtu = jumbo_max;
>
> + dev_dbg(&pdev->dev, "init: irq mask\n");
> rtl_set_irq_mask(tp);
>
> tp->fw_name = rtl_chip_infos[chipset].fw_name;
> + dev_dbg(&pdev->dev, "init: FW name: %s\n", tp->fw_name);
>
> tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
> &tp->counters_phys_addr,
> @@ -5496,16 +5545,21 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (!tp->counters)
> return -ENOMEM;
>
> + dev_dbg(&pdev->dev, "init: set driver data\n");
> pci_set_drvdata(pdev, dev);
>
> + dev_dbg(&pdev->dev, "init: register mdio\n");
> rc = r8169_mdio_register(tp);
> + dev_dbg(&pdev->dev, "init: mdio register: %d\n", rc);
> if (rc)
> return rc;
>
> /* chip gets powered up in rtl_open() */
> + dev_dbg(&pdev->dev, "init: pll pwr down\n");
> rtl_pll_power_down(tp);
>
> rc = register_netdev(dev);
> + dev_dbg(&pdev->dev, "init: netdev register: %d\n", rc);
> if (rc)
> goto err_mdio_unregister;
>
> @@ -5525,6 +5579,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (pci_dev_run_wake(pdev))
> pm_runtime_put_sync(&pdev->dev);
>
> + dev_dbg(&pdev->dev, "init: ALL DONE!\n");
> +
> return 0;
>
> err_mdio_unregister:
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 522760c8bca6..719ea48164f6 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -112,6 +112,9 @@ EXPORT_SYMBOL(mdiobus_unregister_device);
> struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr)
> {
> struct mdio_device *mdiodev = bus->mdio_map[addr];
> + struct phy_device *rv = NULL;
> +
> + pr_debug("mii_bus %s addr %d, %p\n", bus->id, addr, mdiodev);
>
> if (!mdiodev)
> return NULL;
> @@ -119,7 +122,10 @@ struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr)
> if (!(mdiodev->flags & MDIO_DEVICE_FLAG_PHY))
> return NULL;
>
> - return container_of(mdiodev, struct phy_device, mdio);
> + rv = container_of(mdiodev, struct phy_device, mdio);
> + pr_debug("mii_bus OK? %s addr %d, %p -> %p\n",
> + bus->id, addr, mdiodev, rv);
> + return rv;
> }
> EXPORT_SYMBOL(mdiobus_get_phy);
>
> @@ -645,10 +651,11 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
> mdiobus_setup_mdiodev_from_board_info(bus, mdiobus_create_device);
>
> bus->state = MDIOBUS_REGISTERED;
> - pr_info("%s: probed\n", bus->name);
> + pr_info("%s: probed (mdiobus_register)\n", bus->name);
> return 0;
>
> error:
> + pr_err("%s: Error while in mdiobus_register: %d\n", bus->name, err);
> while (--i >= 0) {
> mdiodev = bus->mdio_map[i];
> if (!mdiodev)
> --
> 2.26.0
>
next prev parent reply other threads:[~2020-04-13 10:58 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-13 10:30 NET: r8168/r8169 identifying fix Lauri Jakku
2020-04-13 10:58 ` Leon Romanovsky [this message]
2020-04-13 11:02 ` Lauri Jakku
2020-04-13 11:34 ` Leon Romanovsky
2020-04-13 11:46 ` Lauri Jakku
[not found] ` <d3adc7f2-06bb-45bc-ab02-3d443999cefd@gmail.com>
2020-04-15 16:18 ` Heiner Kallweit
[not found] ` <4860e57e-93e4-24f5-6103-fa80acbdfa0d@pp.inet.fi>
2020-04-16 18:26 ` Heiner Kallweit
2020-04-16 18:37 ` Lauri Jakku
2020-04-16 19:58 ` Lauri Jakku
2020-04-16 20:02 ` Heiner Kallweit
2020-04-16 20:10 ` Lauri Jakku
2020-04-16 20:38 ` Lauri Jakku
2020-04-16 20:50 ` Heiner Kallweit
2020-04-17 6:23 ` Lauri Jakku
2020-04-17 7:30 ` Lauri Jakku
2020-04-17 8:57 ` Heiner Kallweit
2020-04-18 11:06 ` Lauri Jakku
2020-04-18 18:46 ` Lauri Jakku
2020-04-19 15:09 ` Lauri Jakku
2020-04-19 16:49 ` Lauri Jakku
2020-04-19 16:00 ` Heiner Kallweit
2020-04-19 21:00 ` Lauri Jakku
2020-04-20 19:56 ` Lauri Jakku
2020-05-01 19:12 ` Lauri Jakku
2020-05-02 17:56 ` Lauri Jakku
2020-05-02 22:48 ` Lauri Jakku
[not found] ` <5cdc7f73-b109-2a37-8473-12889506b6a9@pp.inet.fi>
2020-05-02 23:15 ` Heiner Kallweit
2020-05-03 0:11 ` Lauri Jakku
2020-05-03 1:34 ` Lauri Jakku
2020-05-03 2:28 ` Lauri Jakku
2020-05-03 8:33 ` Heiner Kallweit
2020-05-03 13:54 ` Lauri Jakku
2020-05-19 5:15 ` Lauri Jakku
2020-05-11 13:09 ` Lauri Jakku
2021-03-11 16:00 ` gmail
2021-03-11 16:23 ` Heiner Kallweit
2021-03-11 16:43 ` gmail
2021-08-10 21:50 ` Late @ Gmail
2021-08-11 6:09 ` Heiner Kallweit
2021-08-11 13:17 ` Late @ Gmail
2021-08-11 19:47 ` Heiner Kallweit
2021-08-12 14:58 ` Late @ Gmail
2020-04-13 12:01 ` Lauri Jakku
2020-04-13 12:18 ` Leon Romanovsky
2020-04-13 14:44 ` Lauri Jakku
2020-04-13 15:33 ` Heiner Kallweit
2020-04-13 15:50 ` Lauri Jakku
2020-04-13 15:54 ` Heiner Kallweit
2020-04-13 16:10 ` Lauri Jakku
2020-04-13 16:17 ` Heiner Kallweit
2020-04-13 16:37 ` Lauri Jakku
2020-04-13 11:06 ` Lauri Jakku
2020-04-13 11:28 ` Heiner Kallweit
2020-04-13 11:40 ` Lauri Jakku
2020-04-13 11:57 ` Heiner Kallweit
2020-04-13 12:04 ` Lauri Jakku
-- strict thread matches above, loose matches on Subject: below --
2020-04-13 10:31 Lauri Jakku
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200413105838.GK334007@unreal \
--to=leon@kernel.org \
--cc=hkallweit1@gmail.com \
--cc=ljakku77@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=nic_swsd@realtek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.