netdev - Re: [PATCH RFT 2/2] macb: kill PHY reset code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160412134001.GB29895@lunn.ch>
Date:	Tue, 12 Apr 2016 15:40:01 +0200
From:	Andrew Lunn <andrew@...n.ch>
To:	Nicolas Ferre <nicolas.ferre@...el.com>
Cc:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFT 2/2] macb: kill PHY reset code

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 <sergei.shtylyov@...entembedded.com>
> >>
> >> ---
> >>  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