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
