Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933506AbbHLQIy (ORCPT ); Wed, 12 Aug 2015 12:08:54 -0400 Received: from smtp14.mail.ru ([94.100.181.95]:35730 "EHLO smtp14.mail.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932137AbbHLQIw (ORCPT ); Wed, 12 Aug 2015 12:08:52 -0400 Subject: Re: [PATCH 0/2] of: fsl/fman: reuse the fixed node parsing code To: Madalin-Cristian Bucur , Florian Fainelli , "netdev@vger.kernel.org" , "grant.likely@linaro.org" , "robh+dt@kernel.org" References: <1438785745-15517-1-git-send-email-madalin.bucur@freescale.com> <55C63D3B.5020005@gmail.com> <55CA1C14.3000202@list.ru> <55CA29C8.5000707@list.ru> <55CB50F1.7050308@list.ru> <55CB61BC.1050209@list.ru> Cc: "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Liberman Igal , Stas Sergeev , "joakim.tjernlund@transmode.se" , Shaohui Xie From: Stas Sergeev Message-ID: <55CB6E35.4020808@list.ru> Date: Wed, 12 Aug 2015 19:03:01 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3215 Lines: 64 12.08.2015 18:27, Madalin-Cristian Bucur пишет: >>> Then I could call only of_* functions but the end result would be the same >>> and of_phy_register_fixed_phy() would not justify its existence that much... >> You didn't say you wanted to obsolete the of_phy_register_fixed_phy(). >> Since it is there (and even changed by me in a way your >> patch will likely clash), IMHO it would be better if it is used, >> rather than copy/pasted into the driver. > Please note I was referring to a fictional new function that would embed the call to > fixed_phy_register(). I was not talking about some existing API, just about a new > of_call named of_phy_register_fixed_phy() that would in the end be called by > of_phy_register_fixed_link() and by some drivers that want to get in the middle > of things and get a hold on status. Hmm, and for exactly unknown reason in your pseudocode of_phy_register_fixed_phy() doesn't take status as an argument. :) So I didn't see its point. If you fix your pseudo-code, you'll add the status argument, because it is needed for fixed_phy_register() anyway. After that, the drivers that want to provide the status, will just use it rather than to call fixed_phy_register() directly. And with my changes it really have even more merits to exist. > Maybe the fact we're reviewing two patches in one thread is what makes the > discussion less than optimal. I guess the bugs in the pseudo-code made me to miss its point. >>> The getter for status you suggest would be fine, but not sure how one >>> would retrieve >>> it from the mac_node unless we change of_phy_register_fixed_link() to >>> return the >>> pointer to phy (and all the drivers that use it...). >> If you look for instance to mvneta.c, you'll find the following: >> --- >> err = of_phy_register_fixed_link(dn); >> /* In the case of a fixed PHY, the DT node associated >> * to the PHY is the Ethernet MAC DT node. >> */ >> phy_node = of_node_get(dn); >> ... >> phy = of_phy_find_device(dn); >> --- >> >> So the answer is: just use the same mac_node for both. > I understand, I'll use this approach although is suboptimal imho to Exactly! But at least this way is used in many currently existing drivers, while getting the fixed-link parameters directly from DT - is a new way of circumventing the existing API. So I'd vote for the currently existing hacks, and in fact I already tried to start a discussion about getting rid of the need for of_phy_find_device(), but it didn't go. > scan the device tree again to get a phy pointer that you need just > to get some of info that was parsed in a call you just made. Maybe (just maybe) of_phy_register_fixed_link() could return the phy_device pointer. At least it will solve your problem very cheaply. But I am sure such API additions require a separate discussion, can't be done in a context of discussing a small fix. If you have some free time, feel free to raise such a discussion with API extension proposals. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/