| Message ID | 1529241759-5641-1-git-send-email-michael@amarulasolutions.com |
|---|---|
| State | Accepted |
| Commit | efd0b791069af93e9d439a70d1fe2ae8994dbbfa |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [U-Boot,V2] eth: dm: fec: Add gpio phy reset binding | expand |
On Sun, Jun 17, 2018 at 8:22 AM, Michael Trimarchi <michael@amarulasolutions.com> wrote: > Add the missing gpio phy reset binding to the gpio and > reset time configuration > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
On Sun, Jun 17, 2018 at 6:52 PM, Michael Trimarchi <michael@amarulasolutions.com> wrote: > Add the missing gpio phy reset binding to the gpio and > reset time configuration > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> > --- > Changes v1 -> v2: > - fix commit message > - fix timeout property read > --- > drivers/net/fec_mxc.c | 43 +++++++++++++++++++++++++++++++++++++------ > drivers/net/fec_mxc.h | 5 ++++- > 2 files changed, 41 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c > index 694a0b2..dac07b6 100644 > --- a/drivers/net/fec_mxc.c > +++ b/drivers/net/fec_mxc.c > @@ -15,7 +15,6 @@ > #include <miiphy.h> > #include <net.h> > #include <netdev.h> > -#include "fec_mxc.h" > > #include <asm/io.h> > #include <linux/errno.h> > @@ -24,6 +23,9 @@ > #include <asm/arch/clock.h> > #include <asm/arch/imx-regs.h> > #include <asm/mach-imx/sys_proto.h> > +#include <asm-generic/gpio.h> > + > +#include "fec_mxc.h" > > DECLARE_GLOBAL_DATA_PTR; > > @@ -1245,6 +1247,19 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) > return 0; > } > > +#ifdef CONFIG_DM_GPIO > +/* FEC GPIO reset */ > +static void fec_gpio_reset(struct fec_priv *priv) > +{ > + debug("fec_gpio_reset: fec_gpio_reset(dev)\n"); > + if (dm_gpio_is_valid(&priv->phy_reset_gpio)) { > + dm_gpio_set_value(&priv->phy_reset_gpio, 1); > + udelay(priv->reset_delay); > + dm_gpio_set_value(&priv->phy_reset_gpio, 0); > + } > +} > +#endif > + > static int fecmxc_probe(struct udevice *dev) > { > struct eth_pdata *pdata = dev_get_platdata(dev); > @@ -1257,6 +1272,9 @@ static int fecmxc_probe(struct udevice *dev) > if (ret) > return ret; > > +#ifdef CONFIG_DM_GPIO > + fec_gpio_reset(priv); > +#endif > /* Reset chip. */ > writel(readl(&priv->eth->ecntrl) | FEC_ECNTRL_RESET, > &priv->eth->ecntrl); > @@ -1314,6 +1332,7 @@ static int fecmxc_remove(struct udevice *dev) > > static int fecmxc_ofdata_to_platdata(struct udevice *dev) > { > + int ret = 0; > struct eth_pdata *pdata = dev_get_platdata(dev); > struct fec_priv *priv = dev_get_priv(dev); > const char *phy_mode; > @@ -1331,12 +1350,24 @@ static int fecmxc_ofdata_to_platdata(struct udevice *dev) > return -EINVAL; > } > > - /* TODO > - * Need to get the reset-gpio and related properties from DT > - * and implemet the enet reset code on .probe call > - */ > +#ifdef CONFIG_DM_GPIO > + ret = gpio_request_by_name(dev, "phy-reset-gpios", 0, > + &priv->phy_reset_gpio, GPIOD_IS_OUT); > + if (ret == 0) { > + ret = dev_read_u32_array(dev, "phy-reset-duration", > + &priv->reset_delay, 1); This is return -1 if none have phy-reset-duration and function return -1 at the end.
Hi Jagan, On 07/07/2018 13:36, Jagan Teki wrote: > On Sun, Jun 17, 2018 at 6:52 PM, Michael Trimarchi > <michael@amarulasolutions.com> wrote: >> Add the missing gpio phy reset binding to the gpio and >> reset time configuration >> >> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> >> --- >> Changes v1 -> v2: >> - fix commit message >> - fix timeout property read >> --- >> drivers/net/fec_mxc.c | 43 +++++++++++++++++++++++++++++++++++++------ >> drivers/net/fec_mxc.h | 5 ++++- >> 2 files changed, 41 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c >> index 694a0b2..dac07b6 100644 >> --- a/drivers/net/fec_mxc.c >> +++ b/drivers/net/fec_mxc.c >> @@ -15,7 +15,6 @@ >> #include <miiphy.h> >> #include <net.h> >> #include <netdev.h> >> -#include "fec_mxc.h" >> >> #include <asm/io.h> >> #include <linux/errno.h> >> @@ -24,6 +23,9 @@ >> #include <asm/arch/clock.h> >> #include <asm/arch/imx-regs.h> >> #include <asm/mach-imx/sys_proto.h> >> +#include <asm-generic/gpio.h> >> + >> +#include "fec_mxc.h" >> >> DECLARE_GLOBAL_DATA_PTR; >> >> @@ -1245,6 +1247,19 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) >> return 0; >> } >> >> +#ifdef CONFIG_DM_GPIO >> +/* FEC GPIO reset */ >> +static void fec_gpio_reset(struct fec_priv *priv) >> +{ >> + debug("fec_gpio_reset: fec_gpio_reset(dev)\n"); >> + if (dm_gpio_is_valid(&priv->phy_reset_gpio)) { >> + dm_gpio_set_value(&priv->phy_reset_gpio, 1); >> + udelay(priv->reset_delay); >> + dm_gpio_set_value(&priv->phy_reset_gpio, 0); >> + } >> +} >> +#endif >> + >> static int fecmxc_probe(struct udevice *dev) >> { >> struct eth_pdata *pdata = dev_get_platdata(dev); >> @@ -1257,6 +1272,9 @@ static int fecmxc_probe(struct udevice *dev) >> if (ret) >> return ret; >> >> +#ifdef CONFIG_DM_GPIO >> + fec_gpio_reset(priv); >> +#endif >> /* Reset chip. */ >> writel(readl(&priv->eth->ecntrl) | FEC_ECNTRL_RESET, >> &priv->eth->ecntrl); >> @@ -1314,6 +1332,7 @@ static int fecmxc_remove(struct udevice *dev) >> >> static int fecmxc_ofdata_to_platdata(struct udevice *dev) >> { >> + int ret = 0; >> struct eth_pdata *pdata = dev_get_platdata(dev); >> struct fec_priv *priv = dev_get_priv(dev); >> const char *phy_mode; >> @@ -1331,12 +1350,24 @@ static int fecmxc_ofdata_to_platdata(struct udevice *dev) >> return -EINVAL; >> } >> >> - /* TODO >> - * Need to get the reset-gpio and related properties from DT >> - * and implemet the enet reset code on .probe call >> - */ >> +#ifdef CONFIG_DM_GPIO >> + ret = gpio_request_by_name(dev, "phy-reset-gpios", 0, >> + &priv->phy_reset_gpio, GPIOD_IS_OUT); >> + if (ret == 0) { >> + ret = dev_read_u32_array(dev, "phy-reset-duration", >> + &priv->reset_delay, 1); > > This is return -1 if none have phy-reset-duration and function return > -1 at the end. Patch is landed on my desk... I am not sure what you mind. It is also thinkable that some products have no GPIO reset at all, and function simply ignores them. And setting phy-reset-duration to a default value seems quite logical. Let me know which are the issues here, I had thought I should apply this. Best regards, Stefano
On Mon, Jul 23, 2018 at 1:57 PM, Stefano Babic <sbabic@denx.de> wrote: > Hi Jagan, > > On 07/07/2018 13:36, Jagan Teki wrote: >> On Sun, Jun 17, 2018 at 6:52 PM, Michael Trimarchi >> <michael@amarulasolutions.com> wrote: >>> Add the missing gpio phy reset binding to the gpio and >>> reset time configuration >>> >>> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> >>> --- >>> Changes v1 -> v2: >>> - fix commit message >>> - fix timeout property read >>> --- >>> drivers/net/fec_mxc.c | 43 +++++++++++++++++++++++++++++++++++++------ >>> drivers/net/fec_mxc.h | 5 ++++- >>> 2 files changed, 41 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c >>> index 694a0b2..dac07b6 100644 >>> --- a/drivers/net/fec_mxc.c >>> +++ b/drivers/net/fec_mxc.c >>> @@ -15,7 +15,6 @@ >>> #include <miiphy.h> >>> #include <net.h> >>> #include <netdev.h> >>> -#include "fec_mxc.h" >>> >>> #include <asm/io.h> >>> #include <linux/errno.h> >>> @@ -24,6 +23,9 @@ >>> #include <asm/arch/clock.h> >>> #include <asm/arch/imx-regs.h> >>> #include <asm/mach-imx/sys_proto.h> >>> +#include <asm-generic/gpio.h> >>> + >>> +#include "fec_mxc.h" >>> >>> DECLARE_GLOBAL_DATA_PTR; >>> >>> @@ -1245,6 +1247,19 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) >>> return 0; >>> } >>> >>> +#ifdef CONFIG_DM_GPIO >>> +/* FEC GPIO reset */ >>> +static void fec_gpio_reset(struct fec_priv *priv) >>> +{ >>> + debug("fec_gpio_reset: fec_gpio_reset(dev)\n"); >>> + if (dm_gpio_is_valid(&priv->phy_reset_gpio)) { >>> + dm_gpio_set_value(&priv->phy_reset_gpio, 1); >>> + udelay(priv->reset_delay); >>> + dm_gpio_set_value(&priv->phy_reset_gpio, 0); >>> + } >>> +} >>> +#endif >>> + >>> static int fecmxc_probe(struct udevice *dev) >>> { >>> struct eth_pdata *pdata = dev_get_platdata(dev); >>> @@ -1257,6 +1272,9 @@ static int fecmxc_probe(struct udevice *dev) >>> if (ret) >>> return ret; >>> >>> +#ifdef CONFIG_DM_GPIO >>> + fec_gpio_reset(priv); >>> +#endif >>> /* Reset chip. */ >>> writel(readl(&priv->eth->ecntrl) | FEC_ECNTRL_RESET, >>> &priv->eth->ecntrl); >>> @@ -1314,6 +1332,7 @@ static int fecmxc_remove(struct udevice *dev) >>> >>> static int fecmxc_ofdata_to_platdata(struct udevice *dev) >>> { >>> + int ret = 0; >>> struct eth_pdata *pdata = dev_get_platdata(dev); >>> struct fec_priv *priv = dev_get_priv(dev); >>> const char *phy_mode; >>> @@ -1331,12 +1350,24 @@ static int fecmxc_ofdata_to_platdata(struct udevice *dev) >>> return -EINVAL; >>> } >>> >>> - /* TODO >>> - * Need to get the reset-gpio and related properties from DT >>> - * and implemet the enet reset code on .probe call >>> - */ >>> +#ifdef CONFIG_DM_GPIO >>> + ret = gpio_request_by_name(dev, "phy-reset-gpios", 0, >>> + &priv->phy_reset_gpio, GPIOD_IS_OUT); >>> + if (ret == 0) { >>> + ret = dev_read_u32_array(dev, "phy-reset-duration", >>> + &priv->reset_delay, 1); >> >> This is return -1 if none have phy-reset-duration and function return >> -1 at the end. > > Patch is landed on my desk... > > I am not sure what you mind. It is also thinkable that some products > have no GPIO reset at all, and function simply ignores them. And setting > phy-reset-duration to a default value seems quite logical. > > Let me know which are the issues here, I had thought I should apply this. We are re-working this, will send the next version.
Hi Jagan On Mon, Jul 23, 2018 at 10:40 AM, Jagan Teki <jagan@amarulasolutions.com> wrote: > On Mon, Jul 23, 2018 at 1:57 PM, Stefano Babic <sbabic@denx.de> wrote: >> Hi Jagan, >> >> On 07/07/2018 13:36, Jagan Teki wrote: >>> On Sun, Jun 17, 2018 at 6:52 PM, Michael Trimarchi >>> <michael@amarulasolutions.com> wrote: >>>> Add the missing gpio phy reset binding to the gpio and >>>> reset time configuration >>>> >>>> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> >>>> --- >>>> Changes v1 -> v2: >>>> - fix commit message >>>> - fix timeout property read >>>> --- >>>> drivers/net/fec_mxc.c | 43 +++++++++++++++++++++++++++++++++++++------ >>>> drivers/net/fec_mxc.h | 5 ++++- >>>> 2 files changed, 41 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c >>>> index 694a0b2..dac07b6 100644 >>>> --- a/drivers/net/fec_mxc.c >>>> +++ b/drivers/net/fec_mxc.c >>>> @@ -15,7 +15,6 @@ >>>> #include <miiphy.h> >>>> #include <net.h> >>>> #include <netdev.h> >>>> -#include "fec_mxc.h" >>>> >>>> #include <asm/io.h> >>>> #include <linux/errno.h> >>>> @@ -24,6 +23,9 @@ >>>> #include <asm/arch/clock.h> >>>> #include <asm/arch/imx-regs.h> >>>> #include <asm/mach-imx/sys_proto.h> >>>> +#include <asm-generic/gpio.h> >>>> + >>>> +#include "fec_mxc.h" >>>> >>>> DECLARE_GLOBAL_DATA_PTR; >>>> >>>> @@ -1245,6 +1247,19 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) >>>> return 0; >>>> } >>>> >>>> +#ifdef CONFIG_DM_GPIO >>>> +/* FEC GPIO reset */ >>>> +static void fec_gpio_reset(struct fec_priv *priv) >>>> +{ >>>> + debug("fec_gpio_reset: fec_gpio_reset(dev)\n"); >>>> + if (dm_gpio_is_valid(&priv->phy_reset_gpio)) { >>>> + dm_gpio_set_value(&priv->phy_reset_gpio, 1); >>>> + udelay(priv->reset_delay); >>>> + dm_gpio_set_value(&priv->phy_reset_gpio, 0); >>>> + } >>>> +} >>>> +#endif >>>> + >>>> static int fecmxc_probe(struct udevice *dev) >>>> { >>>> struct eth_pdata *pdata = dev_get_platdata(dev); >>>> @@ -1257,6 +1272,9 @@ static int fecmxc_probe(struct udevice *dev) >>>> if (ret) >>>> return ret; >>>> >>>> +#ifdef CONFIG_DM_GPIO >>>> + fec_gpio_reset(priv); >>>> +#endif >>>> /* Reset chip. */ >>>> writel(readl(&priv->eth->ecntrl) | FEC_ECNTRL_RESET, >>>> &priv->eth->ecntrl); >>>> @@ -1314,6 +1332,7 @@ static int fecmxc_remove(struct udevice *dev) >>>> >>>> static int fecmxc_ofdata_to_platdata(struct udevice *dev) >>>> { >>>> + int ret = 0; >>>> struct eth_pdata *pdata = dev_get_platdata(dev); >>>> struct fec_priv *priv = dev_get_priv(dev); >>>> const char *phy_mode; >>>> @@ -1331,12 +1350,24 @@ static int fecmxc_ofdata_to_platdata(struct udevice *dev) >>>> return -EINVAL; >>>> } >>>> >>>> - /* TODO >>>> - * Need to get the reset-gpio and related properties from DT >>>> - * and implemet the enet reset code on .probe call >>>> - */ >>>> +#ifdef CONFIG_DM_GPIO >>>> + ret = gpio_request_by_name(dev, "phy-reset-gpios", 0, >>>> + &priv->phy_reset_gpio, GPIOD_IS_OUT); >>>> + if (ret == 0) { >>>> + ret = dev_read_u32_array(dev, "phy-reset-duration", >>>> + &priv->reset_delay, 1); >>> >>> This is return -1 if none have phy-reset-duration and function return >>> -1 at the end. >> >> Patch is landed on my desk... >> >> I am not sure what you mind. It is also thinkable that some products >> have no GPIO reset at all, and function simply ignores them. And setting >> phy-reset-duration to a default value seems quite logical. >> >> Let me know which are the issues here, I had thought I should apply this. > > We are re-working this, will send the next version. Please explain what kind of rework needs a 10 lines ;) patch. Anyway, when is suppose to be upload it again? Michael
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 694a0b2..dac07b6 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -15,7 +15,6 @@ #include <miiphy.h> #include <net.h> #include <netdev.h> -#include "fec_mxc.h" #include <asm/io.h> #include <linux/errno.h> @@ -24,6 +23,9 @@ #include <asm/arch/clock.h> #include <asm/arch/imx-regs.h> #include <asm/mach-imx/sys_proto.h> +#include <asm-generic/gpio.h> + +#include "fec_mxc.h" DECLARE_GLOBAL_DATA_PTR; @@ -1245,6 +1247,19 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) return 0; } +#ifdef CONFIG_DM_GPIO +/* FEC GPIO reset */ +static void fec_gpio_reset(struct fec_priv *priv) +{ + debug("fec_gpio_reset: fec_gpio_reset(dev)\n"); + if (dm_gpio_is_valid(&priv->phy_reset_gpio)) { + dm_gpio_set_value(&priv->phy_reset_gpio, 1); + udelay(priv->reset_delay); + dm_gpio_set_value(&priv->phy_reset_gpio, 0); + } +} +#endif + static int fecmxc_probe(struct udevice *dev) { struct eth_pdata *pdata = dev_get_platdata(dev); @@ -1257,6 +1272,9 @@ static int fecmxc_probe(struct udevice *dev) if (ret) return ret; +#ifdef CONFIG_DM_GPIO + fec_gpio_reset(priv); +#endif /* Reset chip. */ writel(readl(&priv->eth->ecntrl) | FEC_ECNTRL_RESET, &priv->eth->ecntrl); @@ -1314,6 +1332,7 @@ static int fecmxc_remove(struct udevice *dev) static int fecmxc_ofdata_to_platdata(struct udevice *dev) { + int ret = 0; struct eth_pdata *pdata = dev_get_platdata(dev); struct fec_priv *priv = dev_get_priv(dev); const char *phy_mode; @@ -1331,12 +1350,24 @@ static int fecmxc_ofdata_to_platdata(struct udevice *dev) return -EINVAL; } - /* TODO - * Need to get the reset-gpio and related properties from DT - * and implemet the enet reset code on .probe call - */ +#ifdef CONFIG_DM_GPIO + ret = gpio_request_by_name(dev, "phy-reset-gpios", 0, + &priv->phy_reset_gpio, GPIOD_IS_OUT); + if (ret == 0) { + ret = dev_read_u32_array(dev, "phy-reset-duration", + &priv->reset_delay, 1); + } else if (ret == -ENOENT) { + priv->reset_delay = 1000; + ret = 0; + } - return 0; + if (priv->reset_delay > 1000) { + printf("FEX MXC: gpio reset timeout should be less the 1000\n"); + priv->reset_delay = 1000; + } +#endif + + return ret; } static const struct udevice_id fecmxc_ids[] = { diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h index 3b935af..fd89443 100644 --- a/drivers/net/fec_mxc.h +++ b/drivers/net/fec_mxc.h @@ -250,7 +250,10 @@ struct fec_priv { int phy_id; int (*mii_postcall)(int); #endif - +#ifdef CONFIG_DM_GPIO + struct gpio_desc phy_reset_gpio; + uint32_t reset_delay; +#endif #ifdef CONFIG_DM_ETH u32 interface; #endif
Add the missing gpio phy reset binding to the gpio and reset time configuration Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> --- Changes v1 -> v2: - fix commit message - fix timeout property read --- drivers/net/fec_mxc.c | 43 +++++++++++++++++++++++++++++++++++++------ drivers/net/fec_mxc.h | 5 ++++- 2 files changed, 41 insertions(+), 7 deletions(-)