From: andrew@lunn.ch (Andrew Lunn)
To: linux-riscv@lists.infradead.org
Subject: [PATCH net-next v3 1/6] net: phy: mscc: migrate to phy_select/restore_page functions
Date: Mon, 19 Nov 2018 16:28:30 +0100 [thread overview]
Message-ID: <20181119152830.GE26852@lunn.ch> (raw)
In-Reply-To: <mvm4lcdaynt.fsf@suse.de>
On Mon, Nov 19, 2018 at 04:13:10PM +0100, Andreas Schwab wrote:
> On Nov 19 2018, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > On Mon, Nov 19, 2018 at 03:57:17PM +0100, Andreas Schwab wrote:
> >> On Okt 08 2018, Quentin Schulz <quentin.schulz@bootlin.com> wrote:
> >>
> >> > The Microsemi PHYs have multiple banks of registers (called pages).
> >> > Registers can only be accessed from one page, if we need a register from
> >> > another page, we need to switch the page and the registers of all other
> >> > pages are not accessible anymore.
> >> >
> >> > Basically, to read register 5 from page 0, 1, 2, etc., you do the same
> >> > phy_read(phydev, 5); but you need to set the desired page beforehand.
> >> >
> >> > In order to guarantee that two concurrent functions do not change the
> >> > page, we need to do some locking per page. This can be achieved with the
> >> > use of phy_select_page and phy_restore_page functions but phy_write/read
> >> > calls in-between those two functions shall be replaced by their
> >> > lock-free alternative __phy_write/read.
> >> >
> >> > Let's migrate this driver to those functions.
> >>
> >> This has some serious locking problem.
> >
> > Hi Andreas
> >
> > Could you be more specific. Are you getting a deadlock? A WARN_ON?
>
> See the stack trace. That's where it hangs.
So you never said it hangs. The stacktrace helps, but a description of
what actually happens also helps. And i expect Quentin has booted this
code lots of times and not had a hang. So some hits how to reproduce
it would also help. Maybe your kernel config?
I'm interested because he is using the core mdio locking
primitives. If those are broken, i want to know.
Thanks
Andrew
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Lunn <andrew@lunn.ch>
To: Andreas Schwab <schwab@suse.de>
Cc: alexandre.belloni@bootlin.com, f.fainelli@gmail.com,
Quentin Schulz <quentin.schulz@bootlin.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
allan.nielsen@microchip.com, thomas.petazzoni@bootlin.com,
linux-riscv@lists.infradead.org, davem@davemloft.net
Subject: Re: [PATCH net-next v3 1/6] net: phy: mscc: migrate to phy_select/restore_page functions
Date: Mon, 19 Nov 2018 16:28:30 +0100 [thread overview]
Message-ID: <20181119152830.GE26852@lunn.ch> (raw)
Message-ID: <20181119152830.G9QTZpQ4MaHa7tdYKJlYfshhJntra1bTt0TxmKuHYzw@z> (raw)
In-Reply-To: <mvm4lcdaynt.fsf@suse.de>
On Mon, Nov 19, 2018 at 04:13:10PM +0100, Andreas Schwab wrote:
> On Nov 19 2018, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > On Mon, Nov 19, 2018 at 03:57:17PM +0100, Andreas Schwab wrote:
> >> On Okt 08 2018, Quentin Schulz <quentin.schulz@bootlin.com> wrote:
> >>
> >> > The Microsemi PHYs have multiple banks of registers (called pages).
> >> > Registers can only be accessed from one page, if we need a register from
> >> > another page, we need to switch the page and the registers of all other
> >> > pages are not accessible anymore.
> >> >
> >> > Basically, to read register 5 from page 0, 1, 2, etc., you do the same
> >> > phy_read(phydev, 5); but you need to set the desired page beforehand.
> >> >
> >> > In order to guarantee that two concurrent functions do not change the
> >> > page, we need to do some locking per page. This can be achieved with the
> >> > use of phy_select_page and phy_restore_page functions but phy_write/read
> >> > calls in-between those two functions shall be replaced by their
> >> > lock-free alternative __phy_write/read.
> >> >
> >> > Let's migrate this driver to those functions.
> >>
> >> This has some serious locking problem.
> >
> > Hi Andreas
> >
> > Could you be more specific. Are you getting a deadlock? A WARN_ON?
>
> See the stack trace. That's where it hangs.
So you never said it hangs. The stacktrace helps, but a description of
what actually happens also helps. And i expect Quentin has booted this
code lots of times and not had a hang. So some hits how to reproduce
it would also help. Maybe your kernel config?
I'm interested because he is using the core mdio locking
primitives. If those are broken, i want to know.
Thanks
Andrew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Lunn <andrew@lunn.ch>
To: Andreas Schwab <schwab@suse.de>
Cc: Quentin Schulz <quentin.schulz@bootlin.com>,
davem@davemloft.net, f.fainelli@gmail.com,
allan.nielsen@microchip.com, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, thomas.petazzoni@bootlin.com,
alexandre.belloni@bootlin.com, linux-riscv@lists.infradead.org
Subject: Re: [PATCH net-next v3 1/6] net: phy: mscc: migrate to phy_select/restore_page functions
Date: Mon, 19 Nov 2018 16:28:30 +0100 [thread overview]
Message-ID: <20181119152830.GE26852@lunn.ch> (raw)
In-Reply-To: <mvm4lcdaynt.fsf@suse.de>
On Mon, Nov 19, 2018 at 04:13:10PM +0100, Andreas Schwab wrote:
> On Nov 19 2018, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > On Mon, Nov 19, 2018 at 03:57:17PM +0100, Andreas Schwab wrote:
> >> On Okt 08 2018, Quentin Schulz <quentin.schulz@bootlin.com> wrote:
> >>
> >> > The Microsemi PHYs have multiple banks of registers (called pages).
> >> > Registers can only be accessed from one page, if we need a register from
> >> > another page, we need to switch the page and the registers of all other
> >> > pages are not accessible anymore.
> >> >
> >> > Basically, to read register 5 from page 0, 1, 2, etc., you do the same
> >> > phy_read(phydev, 5); but you need to set the desired page beforehand.
> >> >
> >> > In order to guarantee that two concurrent functions do not change the
> >> > page, we need to do some locking per page. This can be achieved with the
> >> > use of phy_select_page and phy_restore_page functions but phy_write/read
> >> > calls in-between those two functions shall be replaced by their
> >> > lock-free alternative __phy_write/read.
> >> >
> >> > Let's migrate this driver to those functions.
> >>
> >> This has some serious locking problem.
> >
> > Hi Andreas
> >
> > Could you be more specific. Are you getting a deadlock? A WARN_ON?
>
> See the stack trace. That's where it hangs.
So you never said it hangs. The stacktrace helps, but a description of
what actually happens also helps. And i expect Quentin has booted this
code lots of times and not had a hang. So some hits how to reproduce
it would also help. Maybe your kernel config?
I'm interested because he is using the core mdio locking
primitives. If those are broken, i want to know.
Thanks
Andrew
next prev parent reply other threads:[~2018-11-19 15:28 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-08 10:07 [PATCH net-next v3 0/6] net: phy: mscc: various improvements to Microsemi PHY driver Quentin Schulz
2018-10-08 10:07 ` [PATCH net-next v3 1/6] net: phy: mscc: migrate to phy_select/restore_page functions Quentin Schulz
2018-11-19 14:57 ` Andreas Schwab
2018-11-19 14:57 ` Andreas Schwab
2018-11-19 14:57 ` Andreas Schwab
2018-11-19 15:10 ` Andrew Lunn
2018-11-19 15:10 ` Andrew Lunn
2018-11-19 15:10 ` Andrew Lunn
2018-11-19 15:13 ` Andreas Schwab
2018-11-19 15:13 ` Andreas Schwab
2018-11-19 15:13 ` Andreas Schwab
2018-11-19 15:28 ` Andrew Lunn [this message]
2018-11-19 15:28 ` Andrew Lunn
2018-11-19 15:28 ` Andrew Lunn
2018-11-19 15:40 ` Alexandre Belloni
2018-11-19 15:40 ` Alexandre Belloni
2018-11-19 15:40 ` Alexandre Belloni
2018-11-19 15:50 ` Andreas Schwab
2018-11-19 15:50 ` Andreas Schwab
2018-11-19 15:50 ` Andreas Schwab
2018-11-19 16:12 ` Andrew Lunn
2018-11-19 16:12 ` Andrew Lunn
2018-11-19 16:12 ` Andrew Lunn
2018-11-19 16:14 ` Andreas Schwab
2018-11-19 16:14 ` Andreas Schwab
2018-11-19 16:14 ` Andreas Schwab
2018-11-19 16:25 ` Andrew Lunn
2018-11-19 16:25 ` Andrew Lunn
2018-11-19 16:25 ` Andrew Lunn
2018-11-19 16:32 ` Andreas Schwab
2018-11-19 16:32 ` Andreas Schwab
2018-11-19 16:32 ` Andreas Schwab
2018-11-19 16:44 ` Andrew Lunn
2018-11-19 16:44 ` Andrew Lunn
2018-11-19 16:44 ` Andrew Lunn
2018-11-20 11:39 ` Andreas Schwab
2018-11-20 11:39 ` Andreas Schwab
2018-11-20 11:39 ` Andreas Schwab
2018-11-20 13:20 ` Quentin Schulz
2018-11-20 13:20 ` Quentin Schulz
2018-11-20 13:20 ` Quentin Schulz
2018-11-20 13:48 ` [PATCH] net: phy: mscc: fix locking in vsc85xx_default_config Andreas Schwab
2018-11-20 13:48 ` Andreas Schwab
2018-11-20 13:48 ` Andreas Schwab
2018-11-20 13:55 ` Quentin Schulz
2018-11-20 13:55 ` Quentin Schulz
2018-11-20 13:55 ` Quentin Schulz
2018-11-20 14:01 ` Andreas Schwab
2018-11-20 14:01 ` Andreas Schwab
2018-11-20 14:01 ` Andreas Schwab
2018-11-20 14:17 ` Quentin Schulz
2018-11-20 14:17 ` Quentin Schulz
2018-11-20 14:17 ` Quentin Schulz
2018-10-08 10:07 ` [PATCH net-next v3 2/6] net: phy: mscc: add ethtool statistics counters Quentin Schulz
2018-10-08 10:07 ` [PATCH net-next v3 3/6] net: phy: mscc: Add EEE init sequence Quentin Schulz
2018-10-08 10:07 ` [PATCH net-next v3 4/6] net: phy: mscc: remove unneeded parenthesis Quentin Schulz
2018-10-08 10:07 ` [PATCH net-next v3 5/6] net: phy: mscc: shorten `x != 0` condition to `x` Quentin Schulz
2018-10-08 10:07 ` [PATCH net-next v3 6/6] net: phy: mscc: remove unneeded temporary variable Quentin Schulz
2018-10-08 17:29 ` [PATCH net-next v3 0/6] net: phy: mscc: various improvements to Microsemi PHY driver David Miller
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=20181119152830.GE26852@lunn.ch \
--to=andrew@lunn.ch \
--cc=linux-riscv@lists.infradead.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.