Re: [PATCH v3 2/7] net: pch_gbe: Pull PHY GPIO handling out of Minnow code

[Andrew Lunn <andrew@lunn.ch>]

On Mon, Jun 05, 2017 at 10:21:50AM -0700, Paul Burton wrote:
> Hi Andrew,
> 
> On Saturday, 3 June 2017 10:52:00 PDT Andrew Lunn wrote:
> > > diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> > > b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c index
> > > d38198718005..cb9b904786e4 100644
> > > --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> > > +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> > > @@ -360,6 +360,16 @@ static void pch_gbe_mac_mar_set(struct pch_gbe_hw
> > > *hw, u8 * addr, u32 index)> 
> > >  	pch_gbe_wait_clr_bit(&hw->reg->ADDR_MASK, PCH_GBE_BUSY);
> > >  
> > >  }
> > > 
> > > +static void pch_gbe_phy_set_reset(struct pch_gbe_hw *hw, int value)
> > > +{
> > > +	struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
> > > +
> > > +	if (!adapter->pdata || !adapter->pdata->phy_reset_gpio)
> > > +		return;
> > > +
> > > +	gpiod_set_value(adapter->pdata->phy_reset_gpio, value);
> > 
> > Hi Paul
> > 
> > Since you are using the gpiod_ API, the core will take notice of the
> > active low/active high flag when performing this set.
> 
> Correct, and as desired.
> 
> > >  	ret = devm_gpio_request_one(&pdev->dev, gpio, flags,
> > >  	
> > >  				    "minnow_phy_reset");
> > > 
> > > -	if (ret) {
> > > +	if (!ret)
> > > +		pdata->phy_reset_gpio = gpio_to_desc(gpio);
> > 
> > Here however, you are using the gpio_ API, which ignores the active
> > high/low flag in device tree. And in your binding patch, you give the
> > example:
> > 
> > +               phy-reset-gpios = <&eg20t_gpio 6
> > +                                  GPIO_ACTIVE_LOW>;
> > 
> > This active low is totally ignored.
> 
> First of all, this path is for the existing Minnow platform, which doesn't use 
> the device tree. That is, this code is the non-DT path so looking at what 
> happens to flags in the device tree here makes no sense.

Thanks for the explanation. Now it makes sense.

       Andrew