From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grygorii Strashko Subject: Re: [PATCH] net: ethernet: cpsw-phy-sel: prefer phandle for phy sel and update binding Date: Thu, 9 Aug 2018 17:47:59 -0500 Message-ID: <3db0048e-21e9-60ea-9e6f-a72bd961f33f@ti.com> References: <20180808080354.100146-1-tony@atomide.com> <20180808115838.GA18314@lunn.ch> <20180808134836.GS99251@atomide.com> <20180809104653.GV99251@atomide.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: David Miller , , , , Ivan Khoronzhuk , Mark Rutland , Murali Karicheri , Rob Herring To: Tony Lindgren , Andrew Lunn Return-path: Received: from lelv0143.ext.ti.com ([198.47.23.248]:46082 "EHLO lelv0143.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727390AbeHJBPR (ORCPT ); Thu, 9 Aug 2018 21:15:17 -0400 In-Reply-To: <20180809104653.GV99251@atomide.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 08/09/2018 05:46 AM, Tony Lindgren wrote: > * Tony Lindgren [180808 13:52]: >> * Andrew Lunn [180808 12:02]: >>> >>> Do you need to handle EPROBE_DEFER here? The phandle points to a >>> device which has not yet been loaded? I'm not sure exactly where it >>> will be returned, maybe it is bus_find_device(), but i expect to see >>> some handling of it somewhere in this function. > > If no device is found the driver just produces a warning currently. > And in that case cpsw attempts to continue with bootloader settings. > > And looking at the caller function cpsw_slave_open() it also just > produces warnings for phy_connect() too.. > > I agree that in general this this whole pile of cpsw related drivers sure > could use some better error handling. Starting with making cpsw_slave_open() > and cpsw_phy_sel() return errors instead of just ignoring them might be a > good start. > > Grygorii, care to add that note of things to do into your cpsw maintainer > hat? Right. EPROBE_DEFER not supported for this module as of now. > >> With the proper interconnect hierarchy in the device tree there should be >> no EPROBE_DEFER happening here as the interconnects are probed in the >> right order with the always on interrupt with system control module first :) >> >> But then again, adding support for EPROBE_DEFER here won't hurt either, >> will take a look. > > I'll just add some notes about that to the patch description considering > the above. thanks Tony. -- regards, -grygorii