From: schwab@suse.de (Andreas Schwab)
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 15:57:17 +0100 [thread overview]
Message-ID: <mvm8t1pazea.fsf@suse.de> (raw)
In-Reply-To: <20181008100728.24959-2-quentin.schulz@bootlin.com> (Quentin Schulz's message of "Mon, 8 Oct 2018 12:07:23 +0200")
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.
[<0>] __schedule+0x25e/0x74c
[<0>] schedule+0x1a/0x58
[<0>] schedule_preempt_disabled+0xc/0x14
[<0>] __mutex_lock.isra.0+0x10e/0x22e
[<0>] __mutex_lock_slowpath+0xe/0x16
[<0>] mutex_lock+0x22/0x2a
[<0>] mdiobus_read+0x36/0x60
[<0>] vsc85xx_config_init+0x4c/0x1e2
[<0>] phy_init_hw+0x3c/0x52
[<0>] phy_attach_direct+0xec/0x1dc
[<0>] phy_connect_direct+0x1a/0x56
[<0>] macb_probe+0x800/0xb5e [macb]
[<0>] platform_drv_probe+0x3e/0x7e
[<0>] really_probe+0xba/0x310
[<0>] driver_probe_device+0x54/0xf2
[<0>] __driver_attach+0xde/0x106
[<0>] bus_for_each_dev+0x4a/0x72
[<0>] driver_attach+0x1a/0x22
[<0>] bus_add_driver+0x1ce/0x212
[<0>] driver_register+0x3a/0xd0
[<0>] __platform_driver_register+0x3a/0x42
[<0>] macb_driver_init+0x20/0x28 [macb]
[<0>] do_one_initcall+0x48/0x128
[<0>] do_init_module+0x4a/0x186
[<0>] load_module+0xd6a/0xe6a
[<0>] sys_finit_module+0xc6/0xfc
[<0>] check_syscall_nr+0x22/0x22
Andreas.
--
Andreas Schwab, SUSE Labs, schwab at suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
WARNING: multiple messages have this Message-ID (diff)
From: Andreas Schwab <schwab@suse.de>
To: Quentin Schulz <quentin.schulz@bootlin.com>
Cc: andrew@lunn.ch, alexandre.belloni@bootlin.com,
f.fainelli@gmail.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 15:57:17 +0100 [thread overview]
Message-ID: <mvm8t1pazea.fsf@suse.de> (raw)
Message-ID: <20181119145717.O7CVyvrtGGZ0ueePpuJ7g8Wo1bZJy3Uw4v0dORpuOT4@z> (raw)
In-Reply-To: <20181008100728.24959-2-quentin.schulz@bootlin.com> (Quentin Schulz's message of "Mon, 8 Oct 2018 12:07:23 +0200")
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.
[<0>] __schedule+0x25e/0x74c
[<0>] schedule+0x1a/0x58
[<0>] schedule_preempt_disabled+0xc/0x14
[<0>] __mutex_lock.isra.0+0x10e/0x22e
[<0>] __mutex_lock_slowpath+0xe/0x16
[<0>] mutex_lock+0x22/0x2a
[<0>] mdiobus_read+0x36/0x60
[<0>] vsc85xx_config_init+0x4c/0x1e2
[<0>] phy_init_hw+0x3c/0x52
[<0>] phy_attach_direct+0xec/0x1dc
[<0>] phy_connect_direct+0x1a/0x56
[<0>] macb_probe+0x800/0xb5e [macb]
[<0>] platform_drv_probe+0x3e/0x7e
[<0>] really_probe+0xba/0x310
[<0>] driver_probe_device+0x54/0xf2
[<0>] __driver_attach+0xde/0x106
[<0>] bus_for_each_dev+0x4a/0x72
[<0>] driver_attach+0x1a/0x22
[<0>] bus_add_driver+0x1ce/0x212
[<0>] driver_register+0x3a/0xd0
[<0>] __platform_driver_register+0x3a/0x42
[<0>] macb_driver_init+0x20/0x28 [macb]
[<0>] do_one_initcall+0x48/0x128
[<0>] do_init_module+0x4a/0x186
[<0>] load_module+0xd6a/0xe6a
[<0>] sys_finit_module+0xc6/0xfc
[<0>] check_syscall_nr+0x22/0x22
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
_______________________________________________
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: Andreas Schwab <schwab@suse.de>
To: Quentin Schulz <quentin.schulz@bootlin.com>
Cc: davem@davemloft.net, andrew@lunn.ch, 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 15:57:17 +0100 [thread overview]
Message-ID: <mvm8t1pazea.fsf@suse.de> (raw)
In-Reply-To: <20181008100728.24959-2-quentin.schulz@bootlin.com> (Quentin Schulz's message of "Mon, 8 Oct 2018 12:07:23 +0200")
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.
[<0>] __schedule+0x25e/0x74c
[<0>] schedule+0x1a/0x58
[<0>] schedule_preempt_disabled+0xc/0x14
[<0>] __mutex_lock.isra.0+0x10e/0x22e
[<0>] __mutex_lock_slowpath+0xe/0x16
[<0>] mutex_lock+0x22/0x2a
[<0>] mdiobus_read+0x36/0x60
[<0>] vsc85xx_config_init+0x4c/0x1e2
[<0>] phy_init_hw+0x3c/0x52
[<0>] phy_attach_direct+0xec/0x1dc
[<0>] phy_connect_direct+0x1a/0x56
[<0>] macb_probe+0x800/0xb5e [macb]
[<0>] platform_drv_probe+0x3e/0x7e
[<0>] really_probe+0xba/0x310
[<0>] driver_probe_device+0x54/0xf2
[<0>] __driver_attach+0xde/0x106
[<0>] bus_for_each_dev+0x4a/0x72
[<0>] driver_attach+0x1a/0x22
[<0>] bus_add_driver+0x1ce/0x212
[<0>] driver_register+0x3a/0xd0
[<0>] __platform_driver_register+0x3a/0x42
[<0>] macb_driver_init+0x20/0x28 [macb]
[<0>] do_one_initcall+0x48/0x128
[<0>] do_init_module+0x4a/0x186
[<0>] load_module+0xd6a/0xe6a
[<0>] sys_finit_module+0xc6/0xfc
[<0>] check_syscall_nr+0x22/0x22
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
next prev parent reply other threads:[~2018-11-19 14:57 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 [this message]
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
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=mvm8t1pazea.fsf@suse.de \
--to=schwab@suse.de \
--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.