From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
To: "RaghuramChary.Jallipalli@microchip.com"
<RaghuramChary.Jallipalli@microchip.com>
Cc: "andrew@lunn.ch" <andrew@lunn.ch>,
"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
"marc.zyngier@arm.com" <marc.zyngier@arm.com>,
"Woojung.Huh@microchip.com" <Woojung.Huh@microchip.com>,
"UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: net: lan78xx: fix "enabled interrupts" warninig
Date: Wed, 17 Apr 2019 08:22:07 +0000 [thread overview]
Message-ID: <20190417161357.4e29fedd@xhacker.debian> (raw)
On Wed, 17 Apr 2019 03:49:46 +0000 <RaghuramChary.Jallipalli@microchip.com> wrote:
>
> Hi Jisheng,
Hi,
>
> > > I want to understand if there is any functionality impact with this warning?
> > Because I'm afraid if the current changes are removed we might hit some
> > other issues (or older ones). We have to go through rigorous testing before
> > going ahead.
> >
> > Warning indicates there's something wrong in the code.
>
> Agree that the code is incorrect. Just wanted to understand if you had any functionality impact too.
>
> >
> > IMHO phy_mac_interrupt() and PHY_IGNORE_INTERRUPT is the correct
> > solution. If the phy_mac_interrupt() poll is fixed, I think maybe old issue
> > which commit cc89c323a30e want to fix won't exist.
> >
>
> I tried to reproduce the problem in PC environment but did not see the warnings.
> However, I tried your patch and did plug/unplug tests(rmmod/insmod continuously) and observed call traces from kernel. I don't see these traces without your patch.
> Attached log.
I believe this is another bug in lan78xx driver: after phy_disconnect() if
there's intr urb coming, we will call phy_mac_interrupt, so trigger
phy_state_machine(), but now the phydev->attached_dev is NULL, so NULL
pointer deference. Can you please try below patch:
--->8
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 246a0d1bbc6c..27d6fbdd58c1 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -3467,6 +3467,8 @@ static void lan78xx_disconnect(struct usb_interface *intf)
net = dev->net;
phydev = net->phydev;
+ usb_kill_urb(dev->urb_intr);
+
phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0);
phy_unregister_fixup_for_uid(PHY_LAN8835, 0xfffffff0);
@@ -3483,7 +3485,6 @@ static void lan78xx_disconnect(struct usb_interface *intf)
lan78xx_unbind(dev, intf);
- usb_kill_urb(dev->urb_intr);
usb_free_urb(dev->urb_intr);
free_netdev(net);
WARNING: multiple messages have this Message-ID (diff)
From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
To: "RaghuramChary.Jallipalli@microchip.com"
<RaghuramChary.Jallipalli@microchip.com>
Cc: "andrew@lunn.ch" <andrew@lunn.ch>,
"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
"marc.zyngier@arm.com" <marc.zyngier@arm.com>,
"Woojung.Huh@microchip.com" <Woojung.Huh@microchip.com>,
"UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net: lan78xx: fix "enabled interrupts" warninig
Date: Wed, 17 Apr 2019 08:22:07 +0000 [thread overview]
Message-ID: <20190417161357.4e29fedd@xhacker.debian> (raw)
Message-ID: <20190417082207.sgds2WCtqmhSZV6eM6VzbUnX8xAdERq0U4H68VUz3M4@z> (raw)
In-Reply-To: <BN6PR11MB39530B69C9BD75927F1F61B89F250@BN6PR11MB3953.namprd11.prod.outlook.com>
On Wed, 17 Apr 2019 03:49:46 +0000 <RaghuramChary.Jallipalli@microchip.com> wrote:
>
> Hi Jisheng,
Hi,
>
> > > I want to understand if there is any functionality impact with this warning?
> > Because I'm afraid if the current changes are removed we might hit some
> > other issues (or older ones). We have to go through rigorous testing before
> > going ahead.
> >
> > Warning indicates there's something wrong in the code.
>
> Agree that the code is incorrect. Just wanted to understand if you had any functionality impact too.
>
> >
> > IMHO phy_mac_interrupt() and PHY_IGNORE_INTERRUPT is the correct
> > solution. If the phy_mac_interrupt() poll is fixed, I think maybe old issue
> > which commit cc89c323a30e want to fix won't exist.
> >
>
> I tried to reproduce the problem in PC environment but did not see the warnings.
> However, I tried your patch and did plug/unplug tests(rmmod/insmod continuously) and observed call traces from kernel. I don't see these traces without your patch.
> Attached log.
I believe this is another bug in lan78xx driver: after phy_disconnect() if
there's intr urb coming, we will call phy_mac_interrupt, so trigger
phy_state_machine(), but now the phydev->attached_dev is NULL, so NULL
pointer deference. Can you please try below patch:
--->8
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 246a0d1bbc6c..27d6fbdd58c1 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -3467,6 +3467,8 @@ static void lan78xx_disconnect(struct usb_interface *intf)
net = dev->net;
phydev = net->phydev;
+ usb_kill_urb(dev->urb_intr);
+
phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0);
phy_unregister_fixup_for_uid(PHY_LAN8835, 0xfffffff0);
@@ -3483,7 +3485,6 @@ static void lan78xx_disconnect(struct usb_interface *intf)
lan78xx_unbind(dev, intf);
- usb_kill_urb(dev->urb_intr);
usb_free_urb(dev->urb_intr);
free_netdev(net);
next reply other threads:[~2019-04-17 8:22 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-17 8:22 Jisheng Zhang [this message]
2019-04-17 8:22 ` [PATCH] net: lan78xx: fix "enabled interrupts" warninig Jisheng Zhang
-- strict thread matches above, loose matches on Subject: below --
2019-04-22 5:32 Raghuram Chary J
2019-04-22 5:32 ` [PATCH] " RaghuramChary.Jallipalli
2019-04-17 3:49 Raghuram Chary J
2019-04-17 3:49 ` [PATCH] " RaghuramChary.Jallipalli
2019-04-10 10:27 Marc Zyngier
2019-04-10 10:27 ` [PATCH] " Marc Zyngier
2019-04-10 9:53 Jisheng Zhang
2019-04-10 9:53 ` [PATCH] " Jisheng Zhang
2019-04-10 9:20 Raghuram Chary J
2019-04-10 9:20 ` [PATCH] " RaghuramChary.Jallipalli
2019-04-09 5:26 Raghuram Chary J
2019-04-09 5:26 ` [PATCH] " RaghuramChary.Jallipalli
2019-04-09 1:36 Jisheng Zhang
2019-04-09 1:36 ` [PATCH] " Jisheng Zhang
2019-04-08 10:46 Raghuram Chary J
2019-04-08 10:46 ` [PATCH] " RaghuramChary.Jallipalli
2019-04-08 8:07 Jisheng Zhang
2019-04-08 8:07 ` [PATCH] " Jisheng Zhang
2019-04-08 7:46 Raghuram Chary J
2019-04-08 7:46 ` [PATCH] " RaghuramChary.Jallipalli
2019-04-08 6:10 Jisheng Zhang
2019-04-08 6:10 ` [PATCH] " Jisheng Zhang
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=20190417161357.4e29fedd@xhacker.debian \
--to=jisheng.zhang@synaptics.com \
--cc=RaghuramChary.Jallipalli@microchip.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=Woojung.Huh@microchip.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=netdev@vger.kernel.org \
/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.