From: Dinh Nguyen <dinguyen@opensource•altera.com>
To: Marek Vasut <marex@denx•de>, <netdev@vger•kernel.org>
Cc: <peppe.cavallaro@st•com>, <alexandre.torgue@st•com>,
Matthew Gerlach <mgerlach@opensource•altera.com>,
"David S . Miller" <davem@davemloft•net>
Subject: Re: [PATCH V3] net: stmmac: socfpga: Remove re-registration of reset controller
Date: Thu, 21 Apr 2016 09:39:56 -0500 [thread overview]
Message-ID: <5718E63C.6050605@opensource.altera.com> (raw)
In-Reply-To: <1461240710-5381-1-git-send-email-marex@denx.de>
On 04/21/2016 07:11 AM, Marek Vasut wrote:
> Both socfpga_dwmac_parse_data() in dwmac-socfpga.c and stmmac_dvr_probe()
> in stmmac_main.c functions call devm_reset_control_get() to register an
> reset controller for the stmmac. This results in an attempt to register
> two reset controllers for the same non-shared reset line.
>
> The first attempt to register the reset controller works fine. The second
> attempt fails with warning from the reset controller core, see below.
> The warning is produced because the reset line is non-shared and thus
> it is allowed to have only up-to one reset controller associated with
> that reset line, not two or more.
>
> The solution has multiple parts. First, the original socfpga_dwmac_init()
> is tweaked to use reset controller pointer from the stmmac_priv (private
> data of the stmmac core) instead of the local instance, which was used
> before. The local re-registration of the reset controller is removed.
>
> Next, the socfpga_dwmac_init() is moved after stmmac_dvr_probe() in the
> probe function. This order is legal according to Altera and it makes the
> code much easier, since there is no need to temporarily register and
> unregister the reset controller ; the reset controller is already registered
> by the stmmac_dvr_probe().
>
> Finally, plat_dat->exit and socfpga_dwmac_exit() is no longer necessary,
> since the functionality is already performed by the stmmac core.
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at drivers/reset/core.c:187 __of_reset_control_get+0x218/0x270
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc4-next-20160419-00015-gabb2477-dirty #4
> Hardware name: Altera SOCFPGA
> [<c010f290>] (unwind_backtrace) from [<c010b82c>] (show_stack+0x10/0x14)
> [<c010b82c>] (show_stack) from [<c0373da4>] (dump_stack+0x94/0xa8)
> [<c0373da4>] (dump_stack) from [<c011bcc0>] (__warn+0xec/0x104)
> [<c011bcc0>] (__warn) from [<c011bd88>] (warn_slowpath_null+0x20/0x28)
> [<c011bd88>] (warn_slowpath_null) from [<c03a6eb4>] (__of_reset_control_get+0x218/0x270)
> [<c03a6eb4>] (__of_reset_control_get) from [<c03a701c>] (__devm_reset_control_get+0x54/0x90)
> [<c03a701c>] (__devm_reset_control_get) from [<c041fa30>] (stmmac_dvr_probe+0x1b4/0x8e8)
> [<c041fa30>] (stmmac_dvr_probe) from [<c04298c8>] (socfpga_dwmac_probe+0x1b8/0x28c)
> [<c04298c8>] (socfpga_dwmac_probe) from [<c03d6ffc>] (platform_drv_probe+0x4c/0xb0)
> [<c03d6ffc>] (platform_drv_probe) from [<c03d54ec>] (driver_probe_device+0x224/0x2bc)
> [<c03d54ec>] (driver_probe_device) from [<c03d5630>] (__driver_attach+0xac/0xb0)
> [<c03d5630>] (__driver_attach) from [<c03d382c>] (bus_for_each_dev+0x6c/0xa0)
> [<c03d382c>] (bus_for_each_dev) from [<c03d4ad4>] (bus_add_driver+0x1a4/0x21c)
> [<c03d4ad4>] (bus_add_driver) from [<c03d60ac>] (driver_register+0x78/0xf8)
> [<c03d60ac>] (driver_register) from [<c0101760>] (do_one_initcall+0x40/0x170)
> [<c0101760>] (do_one_initcall) from [<c0800e38>] (kernel_init_freeable+0x1dc/0x27c)
> [<c0800e38>] (kernel_init_freeable) from [<c05d1bd4>] (kernel_init+0x8/0x114)
> [<c05d1bd4>] (kernel_init) from [<c01076f8>] (ret_from_fork+0x14/0x3c)
> ---[ end trace 059d2fbe87608fa9 ]---
>
> Signed-off-by: Marek Vasut <marex@denx•de>
> Cc: Matthew Gerlach <mgerlach@opensource•altera.com>
> Cc: Dinh Nguyen <dinguyen@opensource•altera.com>
> Cc: David S. Miller <davem@davemloft•net>
> ---
> V2: Add missing stmmac_rst = NULL; into socfpga_dwmac_init_probe()
> V3: Greatly simplify the code by moving socfpga_dwmac_init() after
> stmmac_dvr_probe(), which is legal. This removes the need for
> temporary registration of the reset controller, which is super.
>
Tested-by: Dinh Nguyen <dinguyen@opensource•altera.com>
Thanks,
Dinh
next prev parent reply other threads:[~2016-04-21 14:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-21 12:11 [PATCH V3] net: stmmac: socfpga: Remove re-registration of reset controller Marek Vasut
2016-04-21 14:39 ` Dinh Nguyen [this message]
2016-04-24 18:40 ` David Miller
2016-04-25 18:11 ` Joachim Eastwood
2016-04-25 22:55 ` Marek Vasut
2016-04-26 12:26 ` Joachim Eastwood
2016-04-26 12:47 ` Marek Vasut
2016-04-26 21:22 ` Joachim Eastwood
2016-04-26 22:33 ` Marek Vasut
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=5718E63C.6050605@opensource.altera.com \
--to=dinguyen@opensource$(echo .)altera.com \
--cc=alexandre.torgue@st$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=marex@denx$(echo .)de \
--cc=mgerlach@opensource$(echo .)altera.com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=peppe.cavallaro@st$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox