From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755506AbcDKWqx (ORCPT ); Mon, 11 Apr 2016 18:46:53 -0400 Received: from mail.kernel.org ([198.145.29.136]:51974 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755129AbcDKWqu (ORCPT ); Mon, 11 Apr 2016 18:46:50 -0400 MIME-Version: 1.0 In-Reply-To: <570BFACF.30507@cogentembedded.com> References: <81129033.NXiOLTg1so@wasted.cogentembedded.com> <3381543.SDQLqND8Pp@wasted.cogentembedded.com> <20160411192521.GA6896@rob-hp-laptop> <570BFACF.30507@cogentembedded.com> From: Rob Herring Date: Mon, 11 Apr 2016 17:46:27 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH RFT 1/2] phylib: add device reset GPIO support To: Sergei Shtylyov Cc: Grant Likely , "devicetree@vger.kernel.org" , Florian Fainelli , netdev , Frank Rowand , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 11, 2016 at 2:28 PM, Sergei Shtylyov wrote: > On 04/11/2016 10:25 PM, Rob Herring wrote: > >>> The PHY devices sometimes do have their reset signal (maybe even power >>> supply?) tied to some GPIO and sometimes it also does happen that a boot >>> loader does not leave it deasserted. So far this issue has been attacked >>> from (as I believe) a wrong angle: by teaching the MAC driver to >>> manipulate >>> the GPIO in question; that solution, when applied to the device trees, >>> led to adding the PHY reset GPIO properties to the MAC device node, with >>> one exception: Cadence MACB driver which could handle the "reset-gpios" >>> prop in a PHY device subnode. I believe that the correct approach is >>> to >>> teach the 'phylib' to get the MDIO device reset GPIO from the device tree >>> node corresponding to this device -- which this patch is doing... >>> >>> Note that I had to modify the AT803x PHY driver as it would stop working >>> otherwise as it made use of the reset GPIO for its own purposes... >> >> >> Lots of double spaces in here. Please fix. > > > Oh, it's you again! :-D Yep, one of those picky kernel maintainers that like a bad rash just won't go away. :) >>> Signed-off-by: Sergei Shtylyov >>> >>> --- >>> Documentation/devicetree/bindings/net/phy.txt | 2 + >>> drivers/net/phy/at803x.c | 19 ++------------ >>> drivers/net/phy/mdio_bus.c | 4 +++ >>> drivers/net/phy/mdio_device.c | 27 >>> +++++++++++++++++++-- >>> drivers/net/phy/phy_device.c | 33 >>> ++++++++++++++++++++++++-- >>> drivers/of/of_mdio.c | 16 ++++++++++++ >>> include/linux/mdio.h | 3 ++ >>> include/linux/phy.h | 5 +++ >>> 8 files changed, 89 insertions(+), 20 deletions(-) >> >> >> [...] >> >>> Index: net-next/drivers/of/of_mdio.c >>> =================================================================== >>> --- net-next.orig/drivers/of/of_mdio.c >>> +++ net-next/drivers/of/of_mdio.c >>> @@ -44,6 +44,7 @@ static int of_get_phy_id(struct device_n >>> static int of_mdiobus_register_phy(struct mii_bus *mdio, struct >>> device_node *child, >>> u32 addr) >>> { >>> + struct gpio_desc *gpiod; >>> struct phy_device *phy; >>> bool is_c45; >>> int rc; >>> @@ -52,10 +53,17 @@ static int of_mdiobus_register_phy(struc >>> is_c45 = of_device_is_compatible(child, >>> "ethernet-phy-ieee802.3-c45"); >>> >>> + gpiod = fwnode_get_named_gpiod(&child->fwnode, "reset-gpios"); >> >> >> Calling fwnode_* functions in a DT specific file/function? That doesn't >> make sense. > > > Really?! 8-) > Where is a DT-only analog I wonder... Ah, you're right. NM. Rob