From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dinh Nguyen Subject: Re: [PATCH V3] net: stmmac: socfpga: Remove re-registration of reset controller Date: Thu, 21 Apr 2016 09:39:56 -0500 Message-ID: <5718E63C.6050605@opensource.altera.com> References: <1461240710-5381-1-git-send-email-marex@denx.de> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Cc: , , Matthew Gerlach , "David S . Miller" To: Marek Vasut , Return-path: Received: from mail-bn1on0069.outbound.protection.outlook.com ([157.56.110.69]:9856 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751658AbcDUOkN (ORCPT ); Thu, 21 Apr 2016 10:40:13 -0400 In-Reply-To: <1461240710-5381-1-git-send-email-marex@denx.de> Sender: netdev-owner@vger.kernel.org List-ID: 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 > [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [] (show_stack) from [] (dump_stack+0x94/0xa8) > [] (dump_stack) from [] (__warn+0xec/0x104) > [] (__warn) from [] (warn_slowpath_null+0x20/0x28) > [] (warn_slowpath_null) from [] (__of_reset_control_get+0x218/0x270) > [] (__of_reset_control_get) from [] (__devm_reset_control_get+0x54/0x90) > [] (__devm_reset_control_get) from [] (stmmac_dvr_probe+0x1b4/0x8e8) > [] (stmmac_dvr_probe) from [] (socfpga_dwmac_probe+0x1b8/0x28c) > [] (socfpga_dwmac_probe) from [] (platform_drv_probe+0x4c/0xb0) > [] (platform_drv_probe) from [] (driver_probe_device+0x224/0x2bc) > [] (driver_probe_device) from [] (__driver_attach+0xac/0xb0) > [] (__driver_attach) from [] (bus_for_each_dev+0x6c/0xa0) > [] (bus_for_each_dev) from [] (bus_add_driver+0x1a4/0x21c) > [] (bus_add_driver) from [] (driver_register+0x78/0xf8) > [] (driver_register) from [] (do_one_initcall+0x40/0x170) > [] (do_one_initcall) from [] (kernel_init_freeable+0x1dc/0x27c) > [] (kernel_init_freeable) from [] (kernel_init+0x8/0x114) > [] (kernel_init) from [] (ret_from_fork+0x14/0x3c) > ---[ end trace 059d2fbe87608fa9 ]--- > > Signed-off-by: Marek Vasut > Cc: Matthew Gerlach > Cc: Dinh Nguyen > Cc: David S. Miller > --- > 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 Thanks, Dinh