All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiner Kallweit <hkallweit1@gmail.com>
To: Lauri Jakku <lauri.jakku@pp.inet.fi>
Cc: Leon Romanovsky <leon@kernel.org>,
	netdev@vger.kernel.org, nic_swsd@realtek.com
Subject: Re: NET: r8168/r8169 identifying fix
Date: Mon, 13 Apr 2020 18:17:27 +0200	[thread overview]
Message-ID: <6e965a73-a119-7af2-a0e4-0775602276fe@gmail.com> (raw)
In-Reply-To: <a5f15ebb-10fc-4bb6-79a0-8352e036d88f@pp.inet.fi>

On 13.04.2020 18:10, Lauri Jakku wrote:
> 
> 
> On 13.4.2020 18.54, Heiner Kallweit wrote:
>> On 13.04.2020 17:50, Lauri Jakku wrote:
>>> Hi,
>>>
>>> On 13.4.2020 18.33, Heiner Kallweit wrote:
>>>> On 13.04.2020 16:44, Lauri Jakku wrote:
>>>>> Hi,
>>>>>
>>>>> On 13.4.2020 15.18, Leon Romanovsky wrote:
>>>>>> On Mon, Apr 13, 2020 at 03:01:23PM +0300, Lauri Jakku wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I've tought that the debug's are worth to save behind an definition/commented out, so they can be enabled if needed.
>>>>>>>
>>>>>>
>>>>>> Please stop to do top-posting.
>>>>>
>>>>>  I did not realise, sorry. Trying to do better.
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Latest version:
>>>>>>>
>>>>>>> From 1a75f6f9065a58180de1fa3c48fd80418af6c347 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 | 114 ++++++++++++++++++++--
>>>>>>>  drivers/net/phy/mdio_bus.c                |  15 ++-
>>>>>>>  2 files changed, 119 insertions(+), 10 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>>>>>>> index bf5bf05970a2..5e992f285527 100644
>>>>>>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>>>>>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>>>>>>> @@ -61,6 +61,11 @@
>>>>>>>  #define R8169_MSG_DEFAULT \
>>>>>>>  	(NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_IFUP | NETIF_MSG_IFDOWN)
>>>>>>>
>>>>>>> +
>>>>>>> +/*
>>>>>>> +#define R8169_PROBE_DEBUG
>>>>>>> +*/
>>>>>>
>>>>>> Of course not, it is even worse than before.
>>>>>> If user recompiles module, he will add prints.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>
>>>>> New patch at the end and in attachments.
>>>>>
>>>> Please do as suggested and read through the kernel developer beginners
>>>> guides first. The patch itself, and sending patches as attachments
>>>> is not acceptable.
>>>>
>>>> Please provide the requested logs and information first so that we can
>>>> understand your issue.
>>>>
>>>
>>> I'm compiling newest patch + 5.6.2-2 and I'll then provide logs from 
>>> 5.3, 5.4 and 5.6 (without and with the patch).
>>>
>>> steps i take:
>>> 1. power off computer properly
>>> 2. take output of dmesg 
>>> 3. take output of ip link
>>>
>>> Initramfs does have libphy, realtek and r8169 modules added.
>>>
>> Typically you need the network driver in initramfs only when
>> loading rootfs from network, e.g. via NFS.
>> How is it if you remove r8169 from initramfs?
>>
> 
> Hmm, I'm using Manjaro's stock packages for kernels. I removed
> modules from mkinitcpio.conf and reinstall the kernel packages
> now. i check do they have them installed. 
> 
> One thing that I've noticed is that when doing reboot, the NIC
> does not work ok after bootup. When proper power cycle is done
> the NIC works ok, so something in settings etc. does not reset.
> 
> I check if adding reset to driver's probe helps, i make diffrent
> patch for that.
> 
No need for such a patch, probe is resetting the chip already.
Best provide a dmesg log of such a reboot.
The issue can also be caused by a BIOS bug, as I don't see
this behavior on my systems.

> 
>>> This wille take some time, i try to provide the logs today.
>>>
>>> Do you like if they are provided in tar per configuration, or 
>>> how ?
>>>
>> The logs you can attach to the mail (name them properly).
>>
>>> I make the commit message have log of problem and log of solution
>>> case.
>>>
>>>
>>>>>
>>>>>>> +
>>>>>>>  /* Maximum number of multicast addresses to filter (vs. Rx-all-multicast).
>>>>>>>     The RTL chips use a 64 element hash table based on the Ethernet CRC. */
>>>>>>>  #define	MC_FILTER_LIMIT	32
>>>>>>> @@ -5149,6 +5154,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 +5173,69 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>>>>>>>  	new_bus->write = r8169_mdio_write_reg;
>>>>>>>
>>>>>>>  	ret = mdiobus_register(new_bus);
>>>>>>> +
>>>>>>> +#ifdef R8169_PROBE_DEBUG
>>>>>>> +	dev_info(&pdev->dev,
>>>>>>> +		 "mdiobus_register: %s, %d\n",
>>>>>>> +		 new_bus->id, ret);
>>>>>>> +#endif
>>>>>>>  	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;
>>>>>>> +
>>>>>>> +#ifdef R8169_PROBE_DEBUG
>>>>>>> +			dev_debug(&pdev->dev,
>>>>>>> +				  "%s: ID Check: (%x -> %x), drv (%x -> %x)\n",
>>>>>>> +				new_bus->id, phydev_id, phydev_masked,
>>>>>>> +				phydrv_id, drv_masked);
>>>>>>> +#endif
>>>>>>> +
>>>>>>> +			phydev_match = phydev_masked & drv_masked;
>>>>>>> +			phydev_match = phydev_match == phydev_masked;
>>>>>>> +
>>>>>>> +			drv_match = phydev_masked & drv_masked;
>>>>>>> +			drv_match = drv_match == drv_masked;
>>>>>>> +
>>>>>>> +#ifdef R8169_PROBE_DEBUG
>>>>>>> +			dev_debug(&pdev->dev, "%s: ID Check: %x == %x\n",
>>>>>>> +				  new_bus->id, phydev_match, drv_match);
>>>>>>> +#endif
>>>>>>> +
>>>>>>> +			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 +5492,10 @@ 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);
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: MAC\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	rtl_init_mac_address(tp);
>>>>>>>
>>>>>>>  	dev->ethtool_ops = &rtl8169_ethtool_ops;
>>>>>>> @@ -5483,29 +5544,64 @@ 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);
>>>>>>> +
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: jumbo max: %d\n", jumbo_max);
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	if (jumbo_max)
>>>>>>>  		dev->max_mtu = jumbo_max;
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: irq mask\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	rtl_set_irq_mask(tp);
>>>>>>>
>>>>>>>  	tp->fw_name = rtl_chip_infos[chipset].fw_name;
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: FW name: %s\n", tp->fw_name);
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
>>>>>>>  					    &tp->counters_phys_addr,
>>>>>>>  					    GFP_KERNEL);
>>>>>>>  	if (!tp->counters)
>>>>>>>  		return -ENOMEM;
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: set driver data\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	pci_set_drvdata(pdev, dev);
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: register mdio\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	rc = r8169_mdio_register(tp);
>>>>>>> +
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: mdio register: %d\n", rc);
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	if (rc)
>>>>>>>  		return rc;
>>>>>>>
>>>>>>>  	/* chip gets powered up in rtl_open() */
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: pll pwr down\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	rtl_pll_power_down(tp);
>>>>>>>
>>>>>>>  	rc = register_netdev(dev);
>>>>>>> +
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: netdev register: %d\n", rc);
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	if (rc)
>>>>>>>  		goto err_mdio_unregister;
>>>>>>>
>>>>>>> @@ -5525,6 +5621,10 @@ 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);
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: ALL DONE!\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	return 0;
>>>>>>>
>>>>>>>  err_mdio_unregister:
>>>>>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>>>>>>> index 522760c8bca6..41777f379a57 100644
>>>>>>> --- a/drivers/net/phy/mdio_bus.c
>>>>>>> +++ b/drivers/net/phy/mdio_bus.c
>>>>>>> @@ -112,14 +112,22 @@ 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;
>>>>>>>
>>>>>>>  	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 +653,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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 2020-04-13 14:34, Leon Romanovsky wrote:
>>>>>>>> On Mon, Apr 13, 2020 at 02:02:01PM +0300, Lauri Jakku wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Comments inline.
>>>>>>>>>
>>>>>>>>> On 2020-04-13 13:58, Leon Romanovsky wrote:
>>>>>>>>>> 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.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I tought that they are helpful to keep, they are using the debug calls, so
>>>>>>>>> they are not visible if user does not like those.
>>>>>>>>
>>>>>>>> You are missing the point of who are your users.
>>>>>>>>
>>>>>>>> Users want to have working device and the code. They don't need or like
>>>>>>>> to debug their kernel.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Br,
>>>>>>> Lauri J.
>>>>>>
>>>>>>> From 1a75f6f9065a58180de1fa3c48fd80418af6c347 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 | 114 ++++++++++++++++++++--
>>>>>>>  drivers/net/phy/mdio_bus.c                |  15 ++-
>>>>>>>  2 files changed, 119 insertions(+), 10 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>>>>>>> index bf5bf05970a2..5e992f285527 100644
>>>>>>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>>>>>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>>>>>>> @@ -61,6 +61,11 @@
>>>>>>>  #define R8169_MSG_DEFAULT \
>>>>>>>  	(NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_IFUP | NETIF_MSG_IFDOWN)
>>>>>>>
>>>>>>> +
>>>>>>> +/*
>>>>>>> +#define R8169_PROBE_DEBUG
>>>>>>> +*/
>>>>>>> +
>>>>>>>  /* Maximum number of multicast addresses to filter (vs. Rx-all-multicast).
>>>>>>>     The RTL chips use a 64 element hash table based on the Ethernet CRC. */
>>>>>>>  #define	MC_FILTER_LIMIT	32
>>>>>>> @@ -5149,6 +5154,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 +5173,69 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>>>>>>>  	new_bus->write = r8169_mdio_write_reg;
>>>>>>>
>>>>>>>  	ret = mdiobus_register(new_bus);
>>>>>>> +
>>>>>>> +#ifdef R8169_PROBE_DEBUG
>>>>>>> +	dev_info(&pdev->dev,
>>>>>>> +		 "mdiobus_register: %s, %d\n",
>>>>>>> +		 new_bus->id, ret);
>>>>>>> +#endif
>>>>>>>  	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;
>>>>>>> +
>>>>>>> +#ifdef R8169_PROBE_DEBUG
>>>>>>> +			dev_debug(&pdev->dev,
>>>>>>> +				  "%s: ID Check: (%x -> %x), drv (%x -> %x)\n",
>>>>>>> +				new_bus->id, phydev_id, phydev_masked,
>>>>>>> +				phydrv_id, drv_masked);
>>>>>>> +#endif
>>>>>>> +
>>>>>>> +			phydev_match = phydev_masked & drv_masked;
>>>>>>> +			phydev_match = phydev_match == phydev_masked;
>>>>>>> +
>>>>>>> +			drv_match = phydev_masked & drv_masked;
>>>>>>> +			drv_match = drv_match == drv_masked;
>>>>>>> +
>>>>>>> +#ifdef R8169_PROBE_DEBUG
>>>>>>> +			dev_debug(&pdev->dev, "%s: ID Check: %x == %x\n",
>>>>>>> +				  new_bus->id, phydev_match, drv_match);
>>>>>>> +#endif
>>>>>>> +
>>>>>>> +			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 +5492,10 @@ 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);
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: MAC\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	rtl_init_mac_address(tp);
>>>>>>>
>>>>>>>  	dev->ethtool_ops = &rtl8169_ethtool_ops;
>>>>>>> @@ -5483,29 +5544,64 @@ 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);
>>>>>>> +
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: jumbo max: %d\n", jumbo_max);
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	if (jumbo_max)
>>>>>>>  		dev->max_mtu = jumbo_max;
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: irq mask\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	rtl_set_irq_mask(tp);
>>>>>>>
>>>>>>>  	tp->fw_name = rtl_chip_infos[chipset].fw_name;
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: FW name: %s\n", tp->fw_name);
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
>>>>>>>  					    &tp->counters_phys_addr,
>>>>>>>  					    GFP_KERNEL);
>>>>>>>  	if (!tp->counters)
>>>>>>>  		return -ENOMEM;
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: set driver data\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	pci_set_drvdata(pdev, dev);
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: register mdio\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	rc = r8169_mdio_register(tp);
>>>>>>> +
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: mdio register: %d\n", rc);
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	if (rc)
>>>>>>>  		return rc;
>>>>>>>
>>>>>>>  	/* chip gets powered up in rtl_open() */
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: pll pwr down\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	rtl_pll_power_down(tp);
>>>>>>>
>>>>>>>  	rc = register_netdev(dev);
>>>>>>> +
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: netdev register: %d\n", rc);
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	if (rc)
>>>>>>>  		goto err_mdio_unregister;
>>>>>>>
>>>>>>> @@ -5525,6 +5621,10 @@ 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);
>>>>>>>
>>>>>>> +#ifdef R816X_PROBE_DEBUG
>>>>>>> +	dev_dbg(&pdev->dev, "init: ALL DONE!\n");
>>>>>>> +#endif
>>>>>>> +
>>>>>>>  	return 0;
>>>>>>>
>>>>>>>  err_mdio_unregister:
>>>>>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>>>>>>> index 522760c8bca6..41777f379a57 100644
>>>>>>> --- a/drivers/net/phy/mdio_bus.c
>>>>>>> +++ b/drivers/net/phy/mdio_bus.c
>>>>>>> @@ -112,14 +112,22 @@ 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;
>>>>>>>
>>>>>>>  	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 +653,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 85766c0ab77750faa4488ca7018784e3e8e9eba0 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 | 47 +++++++++++++++++++----
>>>>>  1 file changed, 40 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>>>>> index bf5bf05970a2..da08b1b1047c 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);
>>>>> @@ -5172,13 +5175,43 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>>>>>  	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;
>>>>> +
>>>>> +			phydev_match = phydev_masked & drv_masked;
>>>>> +			phydev_match = phydev_match == phydev_masked;
>>>>> +
>>>>> +			drv_match = phydev_masked & drv_masked;
>>>>> +			drv_match = drv_match == drv_masked;
>>>>> +
>>>>> +			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() */
>>>>>
>>>>
>>>>
>>>
>>
> 


  reply	other threads:[~2020-04-13 16:17 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
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 [this message]
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
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=6e965a73-a119-7af2-a0e4-0775602276fe@gmail.com \
    --to=hkallweit1@gmail.com \
    --cc=lauri.jakku@pp.inet.fi \
    --cc=leon@kernel.org \
    --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.