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

[Andrew Lunn]

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@xxxxxxxxxxxxxxxxxx>
> >>
> >> ---
> >>  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