| Message ID | 1308770649-3802-3-git-send-email-premi@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Sanjeev, On 06/22/11 22:24, Sanjeev Premi wrote: > From: Sriramakrishnan <srk@ti.com> > > The GPIO pin used for resetting the external LAN chip has > changed for Rev.G board. > > Signed-off-by: Sriramakrishnan <srk@ti.com> > Signed-off-by: Sanjeev Premi <premi@ti.com> > --- > board/ti/evm/evm.c | 27 ++++++++++++++++++--------- > 1 files changed, 18 insertions(+), 9 deletions(-) > > diff --git a/board/ti/evm/evm.c b/board/ti/evm/evm.c > index 8f9f141..57e5fa5 100644 > --- a/board/ti/evm/evm.c > +++ b/board/ti/evm/evm.c > @@ -181,17 +181,26 @@ static void setup_net_chip(void) > */ > static void reset_net_chip(void) > { > - struct gpio *gpio3_base = (struct gpio *)OMAP34XX_GPIO3_BASE; > - > - /* Make GPIO 64 as output pin */ > - writel(readl(&gpio3_base->oe) & ~(GPIO0), &gpio3_base->oe); > - > - /* Now send a pulse on the GPIO pin */ > - writel(GPIO0, &gpio3_base->setdataout); > + struct gpio *gpio_base; > + u32 pin; > + > + if (get_omap3_evm_rev() == OMAP3EVM_BOARD_GEN_1) { > + gpio_base = (struct gpio *)OMAP34XX_GPIO3_BASE; > + pin = GPIO0; /* Output pin: GPIO Bank 3, pin 0 */ > + } else { > + gpio_base = (struct gpio *)OMAP34XX_GPIO1_BASE; > + pin = GPIO7; /* Output pin: GPIO Bank 0, pin 7 */ > + } > + > + /* Configure the pin as output */ > + writel(readl(&gpio_base->oe) & ~(pin), &gpio_base->oe); > + > + /* Send a pulse on the GPIO pin */ > + writel(pin, &gpio_base->setdataout); > udelay(1); > - writel(GPIO0, &gpio3_base->cleardataout); > + writel(pin, &gpio_base->cleardataout); > udelay(1); > - writel(GPIO0, &gpio3_base->setdataout); > + writel(pin, &gpio_base->setdataout); Why keep messing with the gpio registers? Why not use gpio framework? Though it is omap specific, but it will be much cleaner then the above.
> -----Original Message----- > From: Igor Grinberg [mailto:grinberg@compulab.co.il] > Sent: Thursday, June 23, 2011 2:38 PM > To: Premi, Sanjeev > Cc: u-boot@lists.denx.de; Govindarajan, Sriramakrishnan > Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > reset sequence for Rev.G board > > Hi Sanjeev, > > On 06/22/11 22:24, Sanjeev Premi wrote: > > From: Sriramakrishnan <srk@ti.com> > > > > The GPIO pin used for resetting the external LAN chip has > > changed for Rev.G board. > > > > Signed-off-by: Sriramakrishnan <srk@ti.com> > > Signed-off-by: Sanjeev Premi <premi@ti.com> > > --- > > board/ti/evm/evm.c | 27 ++++++++++++++++++--------- > > 1 files changed, 18 insertions(+), 9 deletions(-) > > > > diff --git a/board/ti/evm/evm.c b/board/ti/evm/evm.c > > index 8f9f141..57e5fa5 100644 > > --- a/board/ti/evm/evm.c > > +++ b/board/ti/evm/evm.c > > @@ -181,17 +181,26 @@ static void setup_net_chip(void) > > */ > > static void reset_net_chip(void) > > { > > - struct gpio *gpio3_base = (struct gpio *)OMAP34XX_GPIO3_BASE; > > - > > - /* Make GPIO 64 as output pin */ > > - writel(readl(&gpio3_base->oe) & ~(GPIO0), &gpio3_base->oe); > > - > > - /* Now send a pulse on the GPIO pin */ > > - writel(GPIO0, &gpio3_base->setdataout); > > + struct gpio *gpio_base; > > + u32 pin; > > + > > + if (get_omap3_evm_rev() == OMAP3EVM_BOARD_GEN_1) { > > + gpio_base = (struct gpio *)OMAP34XX_GPIO3_BASE; > > + pin = GPIO0; /* Output pin: GPIO Bank 3, pin 0 */ > > + } else { > > + gpio_base = (struct gpio *)OMAP34XX_GPIO1_BASE; > > + pin = GPIO7; /* Output pin: GPIO Bank 0, pin 7 */ > > + } > > + > > + /* Configure the pin as output */ > > + writel(readl(&gpio_base->oe) & ~(pin), &gpio_base->oe); > > + > > + /* Send a pulse on the GPIO pin */ > > + writel(pin, &gpio_base->setdataout); > > udelay(1); > > - writel(GPIO0, &gpio3_base->cleardataout); > > + writel(pin, &gpio_base->cleardataout); > > udelay(1); > > - writel(GPIO0, &gpio3_base->setdataout); > > + writel(pin, &gpio_base->setdataout); > > Why keep messing with the gpio registers? > Why not use gpio framework? > Though it is omap specific, but it will be much cleaner then > the above. [sp] I guess the intent was to keep code similar. But yes, gpio framework can be used. ~sanjeev > > > -- > Regards, > Igor. > >
> -----Original Message----- > From: u-boot-bounces@lists.denx.de > [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Premi, Sanjeev > Sent: Thursday, June 23, 2011 4:43 PM > To: Igor Grinberg > Cc: Govindarajan, Sriramakrishnan; u-boot@lists.denx.de > Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > reset sequence for Rev.G board > > > -----Original Message----- > > From: Igor Grinberg [mailto:grinberg@compulab.co.il] > > Sent: Thursday, June 23, 2011 2:38 PM > > To: Premi, Sanjeev > > Cc: u-boot@lists.denx.de; Govindarajan, Sriramakrishnan > > Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > > reset sequence for Rev.G board > > > > Hi Sanjeev, > > > > On 06/22/11 22:24, Sanjeev Premi wrote: > > > From: Sriramakrishnan <srk@ti.com> > > > > > > The GPIO pin used for resetting the external LAN chip has > > > changed for Rev.G board. > > > > > > Signed-off-by: Sriramakrishnan <srk@ti.com> > > > Signed-off-by: Sanjeev Premi <premi@ti.com> > > > --- > > > board/ti/evm/evm.c | 27 ++++++++++++++++++--------- > > > 1 files changed, 18 insertions(+), 9 deletions(-) > > > > > > diff --git a/board/ti/evm/evm.c b/board/ti/evm/evm.c > > > index 8f9f141..57e5fa5 100644 > > > --- a/board/ti/evm/evm.c > > > +++ b/board/ti/evm/evm.c > > > @@ -181,17 +181,26 @@ static void setup_net_chip(void) > > > */ > > > static void reset_net_chip(void) > > > { > > > - struct gpio *gpio3_base = (struct gpio *)OMAP34XX_GPIO3_BASE; > > > - > > > - /* Make GPIO 64 as output pin */ > > > - writel(readl(&gpio3_base->oe) & ~(GPIO0), &gpio3_base->oe); > > > - > > > - /* Now send a pulse on the GPIO pin */ > > > - writel(GPIO0, &gpio3_base->setdataout); > > > + struct gpio *gpio_base; > > > + u32 pin; > > > + > > > + if (get_omap3_evm_rev() == OMAP3EVM_BOARD_GEN_1) { > > > + gpio_base = (struct gpio *)OMAP34XX_GPIO3_BASE; > > > + pin = GPIO0; /* Output pin: GPIO Bank 3, pin 0 */ > > > + } else { > > > + gpio_base = (struct gpio *)OMAP34XX_GPIO1_BASE; > > > + pin = GPIO7; /* Output pin: GPIO Bank 0, pin 7 */ > > > + } > > > + > > > + /* Configure the pin as output */ > > > + writel(readl(&gpio_base->oe) & ~(pin), &gpio_base->oe); > > > + > > > + /* Send a pulse on the GPIO pin */ > > > + writel(pin, &gpio_base->setdataout); > > > udelay(1); > > > - writel(GPIO0, &gpio3_base->cleardataout); > > > + writel(pin, &gpio_base->cleardataout); > > > udelay(1); > > > - writel(GPIO0, &gpio3_base->setdataout); > > > + writel(pin, &gpio_base->setdataout); > > > > Why keep messing with the gpio registers? > > Why not use gpio framework? > > Though it is omap specific, but it will be much cleaner then > > the above. > > [sp] I guess the intent was to keep code similar. But yes, > gpio framework can be used. > [sp] Sorry, mail went earlier than I wanted :( The only issue is that I couln't see gpio framework for omap. Let me dig further... > ~sanjeev > > > > > > > -- > > Regards, > > Igor. > > > > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
> -----Original Message----- > From: Premi, Sanjeev > Sent: Thursday, June 23, 2011 4:48 PM > To: Premi, Sanjeev; Igor Grinberg > Cc: Govindarajan, Sriramakrishnan; u-boot@lists.denx.de > Subject: RE: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > reset sequence for Rev.G board > > > -----Original Message----- > > From: u-boot-bounces@lists.denx.de > > [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Premi, Sanjeev > > Sent: Thursday, June 23, 2011 4:43 PM > > To: Igor Grinberg > > Cc: Govindarajan, Sriramakrishnan; u-boot@lists.denx.de > > Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > > reset sequence for Rev.G board > > > > > -----Original Message----- > > > From: Igor Grinberg [mailto:grinberg@compulab.co.il] > > > Sent: Thursday, June 23, 2011 2:38 PM > > > To: Premi, Sanjeev > > > Cc: u-boot@lists.denx.de; Govindarajan, Sriramakrishnan > > > Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > > > reset sequence for Rev.G board > > > > > > Hi Sanjeev, > > > > > > On 06/22/11 22:24, Sanjeev Premi wrote: > > > > From: Sriramakrishnan <srk@ti.com> > > > > > > > > The GPIO pin used for resetting the external LAN chip has > > > > changed for Rev.G board. > > > > > > > > Signed-off-by: Sriramakrishnan <srk@ti.com> > > > > Signed-off-by: Sanjeev Premi <premi@ti.com> > > > > --- > > > > board/ti/evm/evm.c | 27 ++++++++++++++++++--------- > > > > 1 files changed, 18 insertions(+), 9 deletions(-) > > > > [snip]...[snip] > > > > + /* Send a pulse on the GPIO pin */ > > > > + writel(pin, &gpio_base->setdataout); > > > > udelay(1); > > > > - writel(GPIO0, &gpio3_base->cleardataout); > > > > + writel(pin, &gpio_base->cleardataout); > > > > udelay(1); > > > > - writel(GPIO0, &gpio3_base->setdataout); > > > > + writel(pin, &gpio_base->setdataout); > > > > > > Why keep messing with the gpio registers? > > > Why not use gpio framework? > > > Though it is omap specific, but it will be much cleaner then > > > the above. > > > > [sp] I guess the intent was to keep code similar. But yes, > > gpio framework can be used. > > > > [sp] Sorry, mail went earlier than I wanted :( > > The only issue is that I couln't see gpio framework for omap. > Let me dig further... > [sp] Implementing GPIO for OMAP would be a long task. It should be done for long term; but is it necessary pre-condition for the patch? ~sanjeev
On 06/27/11 08:06, Premi, Sanjeev wrote: >> -----Original Message----- >> From: Premi, Sanjeev >> Sent: Thursday, June 23, 2011 4:48 PM >> To: Premi, Sanjeev; Igor Grinberg >> Cc: Govindarajan, Sriramakrishnan; u-boot@lists.denx.de >> Subject: RE: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet >> reset sequence for Rev.G board >> >>> -----Original Message----- >>> From: u-boot-bounces@lists.denx.de >>> [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Premi, Sanjeev >>> Sent: Thursday, June 23, 2011 4:43 PM >>> To: Igor Grinberg >>> Cc: Govindarajan, Sriramakrishnan; u-boot@lists.denx.de >>> Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet >>> reset sequence for Rev.G board >>> >>>> -----Original Message----- >>>> From: Igor Grinberg [mailto:grinberg@compulab.co.il] >>>> Sent: Thursday, June 23, 2011 2:38 PM >>>> To: Premi, Sanjeev >>>> Cc: u-boot@lists.denx.de; Govindarajan, Sriramakrishnan >>>> Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet >>>> reset sequence for Rev.G board >>>> >>>> Hi Sanjeev, >>>> >>>> On 06/22/11 22:24, Sanjeev Premi wrote: >>>>> From: Sriramakrishnan <srk@ti.com> >>>>> >>>>> The GPIO pin used for resetting the external LAN chip has >>>>> changed for Rev.G board. >>>>> >>>>> Signed-off-by: Sriramakrishnan <srk@ti.com> >>>>> Signed-off-by: Sanjeev Premi <premi@ti.com> >>>>> --- >>>>> board/ti/evm/evm.c | 27 ++++++++++++++++++--------- >>>>> 1 files changed, 18 insertions(+), 9 deletions(-) >>>>> > [snip]...[snip] > >>>>> + /* Send a pulse on the GPIO pin */ >>>>> + writel(pin, &gpio_base->setdataout); >>>>> udelay(1); >>>>> - writel(GPIO0, &gpio3_base->cleardataout); >>>>> + writel(pin, &gpio_base->cleardataout); >>>>> udelay(1); >>>>> - writel(GPIO0, &gpio3_base->setdataout); >>>>> + writel(pin, &gpio_base->setdataout); >>>> Why keep messing with the gpio registers? >>>> Why not use gpio framework? >>>> Though it is omap specific, but it will be much cleaner then >>>> the above. >>> [sp] I guess the intent was to keep code similar. But yes, >>> gpio framework can be used. >>> >> [sp] Sorry, mail went earlier than I wanted :( >> >> The only issue is that I couln't see gpio framework for omap. >> Let me dig further... >> > [sp] Implementing GPIO for OMAP would be a long task. It should be > done for long term; but is it necessary pre-condition for the > patch? There is no need to implement GPIO for OMAP. It is already there, you just need to use it instead of writing directly to the GPIO registers. You can find all the implementation in: arch/arm/cpu/armv7/omap3/gpio.c and the header is: arch/arm/include/asm/arch-omap3/gpio.h All you need is to include the header, request the appropriate gpio, send the pulse and maybe (if you don't need it anymore) free that gpio. This is not hard or long at all.
> -----Original Message----- > From: Igor Grinberg [mailto:grinberg@compulab.co.il] > Sent: Monday, June 27, 2011 12:17 PM > To: Premi, Sanjeev > Cc: Govindarajan, Sriramakrishnan; u-boot@lists.denx.de > Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > reset sequence for Rev.G board > > On 06/27/11 08:06, Premi, Sanjeev wrote: > > >> -----Original Message----- > >> From: Premi, Sanjeev > >> Sent: Thursday, June 23, 2011 4:48 PM > >> To: Premi, Sanjeev; Igor Grinberg > >> Cc: Govindarajan, Sriramakrishnan; u-boot@lists.denx.de > >> Subject: RE: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > >> reset sequence for Rev.G board > >> > >>> -----Original Message----- > >>> From: u-boot-bounces@lists.denx.de > >>> [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Premi, Sanjeev > >>> Sent: Thursday, June 23, 2011 4:43 PM > >>> To: Igor Grinberg > >>> Cc: Govindarajan, Sriramakrishnan; u-boot@lists.denx.de > >>> Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > >>> reset sequence for Rev.G board > >>> > >>>> -----Original Message----- > >>>> From: Igor Grinberg [mailto:grinberg@compulab.co.il] > >>>> Sent: Thursday, June 23, 2011 2:38 PM > >>>> To: Premi, Sanjeev > >>>> Cc: u-boot@lists.denx.de; Govindarajan, Sriramakrishnan > >>>> Subject: Re: [U-Boot] [PATCH 2/3] omap3evm: Update ethernet > >>>> reset sequence for Rev.G board > >>>> > >>>> Hi Sanjeev, > >>>> > >>>> On 06/22/11 22:24, Sanjeev Premi wrote: > >>>>> From: Sriramakrishnan <srk@ti.com> > >>>>> > >>>>> The GPIO pin used for resetting the external LAN chip has > >>>>> changed for Rev.G board. > >>>>> > >>>>> Signed-off-by: Sriramakrishnan <srk@ti.com> > >>>>> Signed-off-by: Sanjeev Premi <premi@ti.com> > >>>>> --- > >>>>> board/ti/evm/evm.c | 27 ++++++++++++++++++--------- > >>>>> 1 files changed, 18 insertions(+), 9 deletions(-) > >>>>> > > [snip]...[snip] > > > >>>>> + /* Send a pulse on the GPIO pin */ > >>>>> + writel(pin, &gpio_base->setdataout); > >>>>> udelay(1); > >>>>> - writel(GPIO0, &gpio3_base->cleardataout); > >>>>> + writel(pin, &gpio_base->cleardataout); > >>>>> udelay(1); > >>>>> - writel(GPIO0, &gpio3_base->setdataout); > >>>>> + writel(pin, &gpio_base->setdataout); > >>>> Why keep messing with the gpio registers? > >>>> Why not use gpio framework? > >>>> Though it is omap specific, but it will be much cleaner then > >>>> the above. > >>> [sp] I guess the intent was to keep code similar. But yes, > >>> gpio framework can be used. > >>> > >> [sp] Sorry, mail went earlier than I wanted :( > >> > >> The only issue is that I couln't see gpio framework for omap. > >> Let me dig further... > >> > > [sp] Implementing GPIO for OMAP would be a long task. It should be > > done for long term; but is it necessary pre-condition for the > > patch? > > There is no need to implement GPIO for OMAP. It is already there, > you just need to use it instead of writing directly to the > GPIO registers. > You can find all the implementation in: > arch/arm/cpu/armv7/omap3/gpio.c > and the header is: arch/arm/include/asm/arch-omap3/gpio.h [sp] No wonder, I couldn't find it in drivers/gpio. (Didn't occur that it could be in ARCH specific dir) Will rebase and send an updated patch soon. ~sanjeev > > All you need is to include the header, request the appropriate gpio, > send the pulse and maybe (if you don't need it anymore) free > that gpio. > This is not hard or long at all. > > > > -- > Regards, > Igor. > >
Dear Sanjeev Premi, In message <1308770649-3802-3-git-send-email-premi@ti.com> you wrote: > > + if (get_omap3_evm_rev() == OMAP3EVM_BOARD_GEN_1) { > + gpio_base = (struct gpio *)OMAP34XX_GPIO3_BASE; > + pin = GPIO0; /* Output pin: GPIO Bank 3, pin 0 */ > + } else { > + gpio_base = (struct gpio *)OMAP34XX_GPIO1_BASE; > + pin = GPIO7; /* Output pin: GPIO Bank 0, pin 7 */ Is this bank 0 or bank 1? Best regards, Wolfgang Denk
diff --git a/board/ti/evm/evm.c b/board/ti/evm/evm.c index 8f9f141..57e5fa5 100644 --- a/board/ti/evm/evm.c +++ b/board/ti/evm/evm.c @@ -181,17 +181,26 @@ static void setup_net_chip(void) */ static void reset_net_chip(void) { - struct gpio *gpio3_base = (struct gpio *)OMAP34XX_GPIO3_BASE; - - /* Make GPIO 64 as output pin */ - writel(readl(&gpio3_base->oe) & ~(GPIO0), &gpio3_base->oe); - - /* Now send a pulse on the GPIO pin */ - writel(GPIO0, &gpio3_base->setdataout); + struct gpio *gpio_base; + u32 pin; + + if (get_omap3_evm_rev() == OMAP3EVM_BOARD_GEN_1) { + gpio_base = (struct gpio *)OMAP34XX_GPIO3_BASE; + pin = GPIO0; /* Output pin: GPIO Bank 3, pin 0 */ + } else { + gpio_base = (struct gpio *)OMAP34XX_GPIO1_BASE; + pin = GPIO7; /* Output pin: GPIO Bank 0, pin 7 */ + } + + /* Configure the pin as output */ + writel(readl(&gpio_base->oe) & ~(pin), &gpio_base->oe); + + /* Send a pulse on the GPIO pin */ + writel(pin, &gpio_base->setdataout); udelay(1); - writel(GPIO0, &gpio3_base->cleardataout); + writel(pin, &gpio_base->cleardataout); udelay(1); - writel(GPIO0, &gpio3_base->setdataout); + writel(pin, &gpio_base->setdataout); } int board_eth_init(bd_t *bis)