From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933758AbcDLNkH (ORCPT ); Tue, 12 Apr 2016 09:40:07 -0400 Received: from vps0.lunn.ch ([178.209.37.122]:53952 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933205AbcDLNkF (ORCPT ); Tue, 12 Apr 2016 09:40:05 -0400 Date: Tue, 12 Apr 2016 15:40:01 +0200 From: Andrew Lunn To: Nicolas Ferre Cc: Sergei Shtylyov , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH RFT 2/2] macb: kill PHY reset code Message-ID: <20160412134001.GB29895@lunn.ch> References: <81129033.NXiOLTg1so@wasted.cogentembedded.com> <2811962.eGX2i5RJbZ@wasted.cogentembedded.com> <20160411022802.GB4307@lunn.ch> <570CBE42.50309@atmel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <570CBE42.50309@atmel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 12, 2016 at 11:22:10AM +0200, Nicolas Ferre wrote: > Le 11/04/2016 04:28, Andrew Lunn a écrit : > > On Sat, Apr 09, 2016 at 01:25:03AM +0300, Sergei Shtylyov wrote: > >> With the 'phylib' now being aware of the "reset-gpios" PHY node property, > >> there should be no need to frob the PHY reset in this driver anymore... > >> > >> Signed-off-by: Sergei Shtylyov > >> > >> --- > >> drivers/net/ethernet/cadence/macb.c | 17 ----------------- > >> drivers/net/ethernet/cadence/macb.h | 1 - > >> 2 files changed, 18 deletions(-) > >> > >> Index: net-next/drivers/net/ethernet/cadence/macb.c > >> =================================================================== > >> --- net-next.orig/drivers/net/ethernet/cadence/macb.c > >> +++ net-next/drivers/net/ethernet/cadence/macb.c > >> @@ -2884,7 +2884,6 @@ static int macb_probe(struct platform_de > >> = macb_clk_init; > >> int (*init)(struct platform_device *) = macb_init; > >> struct device_node *np = pdev->dev.of_node; > >> - struct device_node *phy_node; > >> const struct macb_config *macb_config = NULL; > >> struct clk *pclk, *hclk = NULL, *tx_clk = NULL; > >> unsigned int queue_mask, num_queues; > >> @@ -2977,18 +2976,6 @@ static int macb_probe(struct platform_de > >> else > >> macb_get_hwaddr(bp); > >> > >> - /* Power up the PHY if there is a GPIO reset */ > >> - phy_node = of_get_next_available_child(np, NULL); > >> - if (phy_node) { > >> - int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0); > >> - > >> - if (gpio_is_valid(gpio)) { > >> - bp->reset_gpio = gpio_to_desc(gpio); > >> - gpiod_direction_output(bp->reset_gpio, 1); > > > > Hi Sergei > > > > The code you are deleting would of ignored the flags in the gpio > I don't parse this. > > The code deleted does take the flag into account. And the DT property > associated to it seems correct to me (I mean, with proper flag > specification). Hi Nicolas of_get_named_gpio() does not do anything with the flags. So for example, gpios = <&gpio0 12 GPIO_ACTIVE_LOW>; the GPIO_ACTIVE_LOW would be ignored. If you want the flags to be respected, you need to use the gpiod API for all calls, in particular, you need to use something which calls gpiod_get_index(), since that is the only function to call gpiod_parse_flags() to translate GPIO_ACTIVE_LOW into a flag within the gpio descriptor. Andrew