netdev - Re: [PATCH v3 2/7] net: pch_gbe: Pull PHY GPIO handling out of Minnow 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]

Date:   Mon, 5 Jun 2017 20:43:55 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Paul Burton <paul.burton@...tec.com>
Cc:     netdev@...r.kernel.org, Tobias Klauser <tklauser@...tanz.ch>,
        "David S . Miller" <davem@...emloft.net>,
        Jarod Wilson <jarod@...hat.com>, linux-mips@...ux-mips.org,
        Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH v3 2/7] net: pch_gbe: Pull PHY GPIO handling out of
 Minnow code

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