Message ID | 1314226061-6933-1-git-send-email-agnel.joel@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | Tom Rini |
Headers | show |
On Thursday, August 25, 2011 12:47:41 AM Joel A Fernandes wrote: > From: Christian Spielberger <c.spielberger@bct-electronic.com> > > Signed-off-by: Joel A Fernandes <joelagnel@ti.com> > Cc: "Eric Bأ�nard" <eric@eukrea.com> > Cc: "Christian Spielberger" <c.spielberger@bct-electronic.com> > --- > Pushing this patch myself as no one else did. > > Previous discussions: > http://article.gmane.org/gmane.comp.boot-loaders.u-boot/105663/ > http://article.gmane.org/gmane.comp.boot-loaders.u-boot/101540/ > > board/ti/beagle/beagle.c | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c > index 13fe39b..9d65e9e 100644 > --- a/board/ti/beagle/beagle.c > +++ b/board/ti/beagle/beagle.c > @@ -332,7 +332,15 @@ int misc_init_r(void) > setenv(expansion_config.env_var, expansion_config.env_setting); > > twl4030_power_init(); > - twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); > + switch (get_board_revision()) { > + case REVISION_XM_C: > + case REVISION_C4: > + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); > + break; > + default: > + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); > + break; > + } Am I just too sleepy or the Subject doesn't correlate with what the patch does ... My understanding of Subject is that it enables power on HUB ... USB HUB ? But the patch enables some LEDs ? I might be wrong. CHeers > > /* Set GPIO states before they are made outputs */ > writel(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1,
On Wed, Aug 24, 2011 at 11:22 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > On Thursday, August 25, 2011 12:47:41 AM Joel A Fernandes wrote: >> From: Christian Spielberger <c.spielberger@bct-electronic.com> >> >> Signed-off-by: Joel A Fernandes <joelagnel@ti.com> >> Cc: "Eric Bأ�nard" <eric@eukrea.com> >> Cc: "Christian Spielberger" <c.spielberger@bct-electronic.com> >> --- >> Pushing this patch myself as no one else did. >> >> Previous discussions: >> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/105663/ >> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/101540/ >> >> board/ti/beagle/beagle.c | 10 +++++++++- >> 1 files changed, 9 insertions(+), 1 deletions(-) >> >> diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c >> index 13fe39b..9d65e9e 100644 >> --- a/board/ti/beagle/beagle.c >> +++ b/board/ti/beagle/beagle.c >> @@ -332,7 +332,15 @@ int misc_init_r(void) >> setenv(expansion_config.env_var, expansion_config.env_setting); >> >> twl4030_power_init(); >> - twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); >> + switch (get_board_revision()) { >> + case REVISION_XM_C: >> + case REVISION_C4: >> + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); >> + break; >> + default: >> + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); >> + break; >> + } Please excuse if this is a duplicate, but I'm configuring a new mail tool and I don't think my other message got sent. It is XM_A and XM_B that are odd-balls, so the correct code should be something like: + switch (get_board_revision()) { + case REVISION_XM_A: + case REVISION_XM_B: + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); /* LEDA signal set low */ + break; + default: + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); + break; >> + } > > Am I just too sleepy or the Subject doesn't correlate with what the patch does > ... > > My understanding of Subject is that it enables power on HUB ... USB HUB ? But > the patch enables some LEDs ? The LED drivers on the power management device (TWL4030-compatible) are used to enable the hub power. > > I might be wrong. > > CHeers >> >> /* Set GPIO states before they are made outputs */ >> writel(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1, > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
On Thursday, August 25, 2011 05:30:59 AM Jason Kridner wrote: > On Wed, Aug 24, 2011 at 11:22 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > > On Thursday, August 25, 2011 12:47:41 AM Joel A Fernandes wrote: > >> From: Christian Spielberger <c.spielberger@bct-electronic.com> > >> > >> Signed-off-by: Joel A Fernandes <joelagnel@ti.com> > >> Cc: "Eric Bأ�nard" <eric@eukrea.com> > >> Cc: "Christian Spielberger" <c.spielberger@bct-electronic.com> > >> --- > >> Pushing this patch myself as no one else did. > >> > >> Previous discussions: > >> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/105663/ > >> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/101540/ > >> > >> board/ti/beagle/beagle.c | 10 +++++++++- > >> 1 files changed, 9 insertions(+), 1 deletions(-) > >> > >> diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c > >> index 13fe39b..9d65e9e 100644 > >> --- a/board/ti/beagle/beagle.c > >> +++ b/board/ti/beagle/beagle.c > >> @@ -332,7 +332,15 @@ int misc_init_r(void) > >> setenv(expansion_config.env_var, > >> expansion_config.env_setting); > >> > >> twl4030_power_init(); > >> - twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | > >> TWL4030_LED_LEDEN_LEDBON); + switch (get_board_revision()) { > >> + case REVISION_XM_C: > >> + case REVISION_C4: > >> + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | > >> TWL4030_LED_LEDEN_LEDBON); + break; > >> + default: > >> + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); > >> + break; > >> + } > > Please excuse if this is a duplicate, but I'm configuring a new mail > tool and I don't think my other message got sent. > > It is XM_A and XM_B that are odd-balls, so the correct code should be > something like: > > + switch (get_board_revision()) { > + case REVISION_XM_A: > + case REVISION_XM_B: > + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); /* LEDA > signal set low */ > + break; > + default: > + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | > TWL4030_LED_LEDEN_LEDBON); > + break; > > >> + } > > > > Am I just too sleepy or the Subject doesn't correlate with what the patch > > does ... > > > > My understanding of Subject is that it enables power on HUB ... USB HUB ? > > But the patch enables some LEDs ? > > The LED drivers on the power management device (TWL4030-compatible) > are used to enable the hub power. Oh uh ... ok, now I understand. Please add a comment explaining it. Also, change the subject se it's clear you're operating with USB HUB here. Thanks! > > > I might be wrong. > > > > CHeers > > > >> /* Set GPIO states before they are made outputs */ > >> writel(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1, > > > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > http://lists.denx.de/mailman/listinfo/u-boot
On Wed, Aug 24, 2011 at 10:30 PM, Jason Kridner <jkridner@beagleboard.org> wrote: > On Wed, Aug 24, 2011 at 11:22 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >> On Thursday, August 25, 2011 12:47:41 AM Joel A Fernandes wrote: >>> From: Christian Spielberger <c.spielberger@bct-electronic.com> >>> >>> Signed-off-by: Joel A Fernandes <joelagnel@ti.com> >>> Cc: "Eric Bأ�nard" <eric@eukrea.com> >>> Cc: "Christian Spielberger" <c.spielberger@bct-electronic.com> >>> --- >>> Pushing this patch myself as no one else did. >>> >>> Previous discussions: >>> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/105663/ >>> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/101540/ >>> >>> board/ti/beagle/beagle.c | 10 +++++++++- >>> 1 files changed, 9 insertions(+), 1 deletions(-) >>> >>> diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c >>> index 13fe39b..9d65e9e 100644 >>> --- a/board/ti/beagle/beagle.c >>> +++ b/board/ti/beagle/beagle.c >>> @@ -332,7 +332,15 @@ int misc_init_r(void) >>> setenv(expansion_config.env_var, expansion_config.env_setting); >>> >>> twl4030_power_init(); >>> - twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); >>> + switch (get_board_revision()) { >>> + case REVISION_XM_C: >>> + case REVISION_C4: >>> + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); >>> + break; >>> + default: >>> + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); >>> + break; >>> + } > > Please excuse if this is a duplicate, but I'm configuring a new mail > tool and I don't think my other message got sent. > > It is XM_A and XM_B that are odd-balls, so the correct code should be > something like: > > + switch (get_board_revision()) { > + case REVISION_XM_A: > + case REVISION_XM_B: > + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); /* LEDA > signal set low */ > + break; > + default: > + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | > TWL4030_LED_LEDEN_LEDBON); > + break; Hi Jason, I think it should be: + switch (get_board_revision()) { + case REVISION_XM_A: + case REVISION_XM_B: + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); /* LEDA signal set low */ + break; + default: + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); + break; Here's the equivalent in the linux kernel board file (default is GPIOF_OUT_INIT_LOW) printk(KERN_INFO "OMAP3 Beagle Rev: xM Ax/Bx\n"); omap3_beagle_version = OMAP3BEAGLE_BOARD_XM; beagle_config.usb_pwr_level = GPIOF_OUT_INIT_HIGH; Let me know your comments. thanks, Joel
+U-boot list ---------- Forwarded message ---------- From: Joel A Fernandes <agnel.joel@gmail.com> Date: Fri, Aug 26, 2011 at 11:45 PM Subject: Re: [PATCH] beagleboard: enable HUB power on all variants of the BeagleBoard To: Jason Kridner <jkridner@beagleboard.org> > > The logic is wrong here. I don't remember the polarity, but I do remember that it is > xM-A and xM-B that are the oddballs. I believe this is the correct change: Hi Jason, LEDAON should be high for HUB power up on xMA and xMB, so in your code snip: > > + switch (get_board_revision()) { > + case REVISION_XM_A: > + case REVISION_XM_B: > + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); So this should be TWL4030_LED_LEDEN_LEDBON | TWL4030_LED_LEDEN_LEDAON > + break; > + default: > + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); and, this should be TWL4030_LED_LEDEN_LEDBON Am I correct? Thanks Joel
Hi Joel, Le 27/08/2011 06:46, Joel A Fernandes a écrit : >> The logic is wrong here. I don't remember the polarity, but I do remember that it is >> xM-A and xM-B that are the oddballs. I believe this is the correct change: > > Hi Jason, > > LEDAON should be high for HUB power up on xMA and xMB, so in your code snip: > >> >> + switch (get_board_revision()) { >> + case REVISION_XM_A: >> + case REVISION_XM_B: >> + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); > > So this should be TWL4030_LED_LEDEN_LEDBON | TWL4030_LED_LEDEN_LEDAON > >> + break; >> + default: >> + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); > > and, this should be TWL4030_LED_LEDEN_LEDBON > > Am I correct? > LEDA & LEDB are active low Open Drain outputs so enabling LEDAON bit in LEDEN means LEDA output is at low level. Eric
On Sat, Aug 27, 2011 at 9:51 AM, Eric Bénard <eric@eukrea.com> wrote: > Hi Joel, > > Le 27/08/2011 06:46, Joel A Fernandes a écrit : >>> The logic is wrong here. I don't remember the polarity, but I do remember that it is >>> xM-A and xM-B that are the oddballs. I believe this is the correct change: >> >> Hi Jason, >> >> LEDAON should be high for HUB power up on xMA and xMB, so in your code snip: >> >>> >>> + switch (get_board_revision()) { >>> + case REVISION_XM_A: >>> + case REVISION_XM_B: >>> + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); >> >> So this should be TWL4030_LED_LEDEN_LEDBON | TWL4030_LED_LEDEN_LEDAON >> >>> + break; >>> + default: >>> + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); >> >> and, this should be TWL4030_LED_LEDEN_LEDBON >> >> Am I correct? >> > LEDA & LEDB are active low Open Drain outputs so enabling LEDAON bit in LEDEN > means LEDA output is at low level. Thanks for resolving that mystery. I just figured out what I thought it should be by looking at the previous default, which should be the new default as well with only XM_A and XM_B are different. > > Eric > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
All, I'm post a v2 now, please give ACKs, thanks :) On Sat, Aug 27, 2011 at 11:19 AM, Jason Kridner <jkridner@beagleboard.org> wrote: > On Sat, Aug 27, 2011 at 9:51 AM, Eric Bénard <eric@eukrea.com> wrote: >> Hi Joel, >> >> Le 27/08/2011 06:46, Joel A Fernandes a écrit : >>>> The logic is wrong here. I don't remember the polarity, but I do remember that it is >>>> xM-A and xM-B that are the oddballs. I believe this is the correct change: >>> >>> Hi Jason, >>> >>> LEDAON should be high for HUB power up on xMA and xMB, so in your code snip: >>> >>>> >>>> + switch (get_board_revision()) { >>>> + case REVISION_XM_A: >>>> + case REVISION_XM_B: >>>> + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); >>> >>> So this should be TWL4030_LED_LEDEN_LEDBON | TWL4030_LED_LEDEN_LEDAON >>> >>>> + break; >>>> + default: >>>> + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); >>> >>> and, this should be TWL4030_LED_LEDEN_LEDBON >>> >>> Am I correct? >>> >> LEDA & LEDB are active low Open Drain outputs so enabling LEDAON bit in LEDEN >> means LEDA output is at low level. > > Thanks for resolving that mystery. I just figured out what I thought > it should be by looking at the previous default, which should be the > new default as well with only XM_A and XM_B are different. > > >> >> Eric >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot >> >
diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c index 13fe39b..9d65e9e 100644 --- a/board/ti/beagle/beagle.c +++ b/board/ti/beagle/beagle.c @@ -332,7 +332,15 @@ int misc_init_r(void) setenv(expansion_config.env_var, expansion_config.env_setting); twl4030_power_init(); - twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); + switch (get_board_revision()) { + case REVISION_XM_C: + case REVISION_C4: + twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON); + break; + default: + twl4030_led_init(TWL4030_LED_LEDEN_LEDBON); + break; + } /* Set GPIO states before they are made outputs */ writel(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1,