Patchwork [U-Boot,4/5] omap3_beagle: support findfdt and loadfdt for devicetree support

login
register
mail settings
Submitter Nishanth Menon
Date July 11, 2013, 10:17 p.m.
Message ID <20130711221736.GA14444@kahuna>
Download mbox | patch
Permalink /patch/258630/
State Superseded
Delegated to: Tom Rini
Headers show

Comments

Nishanth Menon - July 11, 2013, 10:17 p.m.
On 17:05-20130711, Robert Nelson wrote:
> On Thu, Jul 11, 2013 at 5:03 PM, Nishanth Menon <nm@ti.com> wrote:
> > On 17:02-20130711, Robert Nelson wrote:
> >> On Thu, Jul 11, 2013 at 4:52 PM, Nishanth Menon <nm@ti.com> wrote:
> >> > For folks not using concatenated device tree with uImage, having
> >> > an handy function to find and load device tree is very handy.
> >> >
> >> > So introduce findfdt and loadfdt and run findfdt by default to make
> >> > it easier on user scripts.
> >> >
> >> > Signed-off-by: Nishanth Menon <nm@ti.com>
> >> > ---
> >> >  include/configs/omap3_beagle.h |   17 +++++++++++++++++
> >> >  1 file changed, 17 insertions(+)
> >> >
> >> > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> >> > index bdeee17..7833208 100644
> >> > --- a/include/configs/omap3_beagle.h
> >> > +++ b/include/configs/omap3_beagle.h
> >> > @@ -210,6 +210,8 @@
> >> >  #define CONFIG_EXTRA_ENV_SETTINGS \
> >> >         "loadaddr=0x80200000\0" \
> >> >         "rdaddr=0x81000000\0" \
> >> > +       "fdt_high=0xffffffff\0" \
> >> > +       "fdtaddr=0x80f80000\0" \
> >> >         "usbtty=cdc_acm\0" \
> >> >         "bootfile=uImage\0" \
> >> >         "ramdisk=ramdisk.gz\0" \
> >> > @@ -250,6 +252,19 @@
> >> >                 "omapdss.def_disp=${defaultdisplay} " \
> >> >                 "root=${nandroot} " \
> >> >                 "rootfstype=${nandrootfstype}\0" \
> >> > +       "findfdt=" \
> >> > +               "if test $beaglerev = AxBx; then " \
> >> > +                       "setenv fdtfile omap3-beagle.dtb; fi; " \
> >> > +               "if test $beaglerev = Cx; then " \
> >> > +                       "setenv fdtfile omap3-beagle.dtb; fi; " \
> >> > +               "if test $beaglerev = xMA; then " \
> >> > +                       "setenv fdtfile omap3-beagle-xm.dtb; fi; " \
> >> > +               "if test $beaglerev = xMB; then " \
> >> > +                       "setenv fdtfile omap3-beagle-xm.dtb; fi; " \
> >>
> >> There is no need for the "xMB" variant, as the gpio pins used for
> >> identification where never changed from the xMA when the newer silcon
> >> was used for the xMB (i guess if we look at the omap's es revision, we
> >> could figure out which are xMB's, but right now that isn't done in
> >> u-boot....)
> > I suppose then the following is ok to do as well?
> > diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
> > index c686f40..6094b0f 100644
> > --- a/board/ti/beagle/beagle.c
> > +++ b/board/ti/beagle/beagle.c
> > @@ -369,16 +369,6 @@ int misc_init_r(void)
> >                                         TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
> >                                         TWL4030_PM_RECEIVER_DEV_GRP_P1);
> >                 break;
> > -       case REVISION_XM_B:
> > -               printf("Beagle xM Rev B\n");
> > -               setenv("beaglerev", "xMB");
> > -               MUX_BEAGLE_XM();
> > -               /* Set VAUX2 to 1.8V for EHCI PHY */
> > -               twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
> > -                                       TWL4030_PM_RECEIVER_VAUX2_VSEL_18,
> > -                                       TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
> > -                                       TWL4030_PM_RECEIVER_DEV_GRP_P1);
> > -               break;
> >         case REVISION_XM_C:
> >                 printf("Beagle xM Rev C\n");
> >                 setenv("beaglerev", "xMC");
> 
> Correct, as a cleanup we can nuke that xMB section..
> 
> For some bike shed paining we could rename:
> 
> Beagle xM Rev A -> Beagle xM Rev A/B
> 
> Above that too. ;)
OK - I suspect and am not sure how dvi_pup will get handled in the
resultant diff, What do folks think of the following?
robertcnelson@gmail.com - July 11, 2013, 11:17 p.m.
On Thu, Jul 11, 2013 at 5:17 PM, Nishanth Menon <nm@ti.com> wrote:
> On 17:05-20130711, Robert Nelson wrote:
>> On Thu, Jul 11, 2013 at 5:03 PM, Nishanth Menon <nm@ti.com> wrote:
>> > On 17:02-20130711, Robert Nelson wrote:
>> >> On Thu, Jul 11, 2013 at 4:52 PM, Nishanth Menon <nm@ti.com> wrote:
>> >> > For folks not using concatenated device tree with uImage, having
>> >> > an handy function to find and load device tree is very handy.
>> >> >
>> >> > So introduce findfdt and loadfdt and run findfdt by default to make
>> >> > it easier on user scripts.
>> >> >
>> >> > Signed-off-by: Nishanth Menon <nm@ti.com>
>> >> > ---
>> >> >  include/configs/omap3_beagle.h |   17 +++++++++++++++++
>> >> >  1 file changed, 17 insertions(+)
>> >> >
>> >> > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
>> >> > index bdeee17..7833208 100644
>> >> > --- a/include/configs/omap3_beagle.h
>> >> > +++ b/include/configs/omap3_beagle.h
>> >> > @@ -210,6 +210,8 @@
>> >> >  #define CONFIG_EXTRA_ENV_SETTINGS \
>> >> >         "loadaddr=0x80200000\0" \
>> >> >         "rdaddr=0x81000000\0" \
>> >> > +       "fdt_high=0xffffffff\0" \
>> >> > +       "fdtaddr=0x80f80000\0" \
>> >> >         "usbtty=cdc_acm\0" \
>> >> >         "bootfile=uImage\0" \
>> >> >         "ramdisk=ramdisk.gz\0" \
>> >> > @@ -250,6 +252,19 @@
>> >> >                 "omapdss.def_disp=${defaultdisplay} " \
>> >> >                 "root=${nandroot} " \
>> >> >                 "rootfstype=${nandrootfstype}\0" \
>> >> > +       "findfdt=" \
>> >> > +               "if test $beaglerev = AxBx; then " \
>> >> > +                       "setenv fdtfile omap3-beagle.dtb; fi; " \
>> >> > +               "if test $beaglerev = Cx; then " \
>> >> > +                       "setenv fdtfile omap3-beagle.dtb; fi; " \
>> >> > +               "if test $beaglerev = xMA; then " \
>> >> > +                       "setenv fdtfile omap3-beagle-xm.dtb; fi; " \
>> >> > +               "if test $beaglerev = xMB; then " \
>> >> > +                       "setenv fdtfile omap3-beagle-xm.dtb; fi; " \
>> >>
>> >> There is no need for the "xMB" variant, as the gpio pins used for
>> >> identification where never changed from the xMA when the newer silcon
>> >> was used for the xMB (i guess if we look at the omap's es revision, we
>> >> could figure out which are xMB's, but right now that isn't done in
>> >> u-boot....)
>> > I suppose then the following is ok to do as well?
>> > diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
>> > index c686f40..6094b0f 100644
>> > --- a/board/ti/beagle/beagle.c
>> > +++ b/board/ti/beagle/beagle.c
>> > @@ -369,16 +369,6 @@ int misc_init_r(void)
>> >                                         TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
>> >                                         TWL4030_PM_RECEIVER_DEV_GRP_P1);
>> >                 break;
>> > -       case REVISION_XM_B:
>> > -               printf("Beagle xM Rev B\n");
>> > -               setenv("beaglerev", "xMB");
>> > -               MUX_BEAGLE_XM();
>> > -               /* Set VAUX2 to 1.8V for EHCI PHY */
>> > -               twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
>> > -                                       TWL4030_PM_RECEIVER_VAUX2_VSEL_18,
>> > -                                       TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
>> > -                                       TWL4030_PM_RECEIVER_DEV_GRP_P1);
>> > -               break;
>> >         case REVISION_XM_C:
>> >                 printf("Beagle xM Rev C\n");
>> >                 setenv("beaglerev", "xMC");
>>
>> Correct, as a cleanup we can nuke that xMB section..
>>
>> For some bike shed paining we could rename:
>>
>> Beagle xM Rev A -> Beagle xM Rev A/B
>>
>> Above that too. ;)
> OK - I suspect and am not sure how dvi_pup will get handled in the
> resultant diff, What do folks think of the following?

A quick review of all my old xM schematics i have access to and the
ones on https://github.com/CircuitCo/

For the "dvi_pup" function

GPIO_170 was only used on the pre-production xM's P8/P9, i doubt any
of them still operate (mine died)..

It was then switched to GPIO_129 for the xMA, however there where MMC
issues, so it was changed for xMA1 (and thus xMA2)
notes, top of this:
https://github.com/CircuitCo/BeagleBoard-xM-RevC/blob/master/BeagleBoard-xM_revC_SCH.pdf

TPS_GPIO_2: was then used on xMA3 -> and all later generations...

So also looking at:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-omap2/board-omap3beagle.c#n161

Looks like the original beagle needs GPIO_170..

So I'd just set GPIO_170 for:

case REVISION_AXBX:
case REVISION_CX:
case REVISION_C4:

and ignore all the pre-production xM P8/P8's/xMA..

Side note, I think 'first' customers just got the xMA2 and newer.. As
i was one of the first, and it's still running downstairs..

Regards,
Nishanth Menon - July 12, 2013, 4:06 a.m.
On 18:17-20130711, Robert Nelson wrote:
> On Thu, Jul 11, 2013 at 5:17 PM, Nishanth Menon <nm@ti.com> wrote:
> > On 17:05-20130711, Robert Nelson wrote:
> >> On Thu, Jul 11, 2013 at 5:03 PM, Nishanth Menon <nm@ti.com> wrote:
> >> > On 17:02-20130711, Robert Nelson wrote:
> >> >> On Thu, Jul 11, 2013 at 4:52 PM, Nishanth Menon <nm@ti.com> wrote:
> >> >> > For folks not using concatenated device tree with uImage, having
> >> >> > an handy function to find and load device tree is very handy.
> >> >> >
> >> >> > So introduce findfdt and loadfdt and run findfdt by default to make
> >> >> > it easier on user scripts.
> >> >> >
> >> >> > Signed-off-by: Nishanth Menon <nm@ti.com>
> >> >> > ---
> >> >> >  include/configs/omap3_beagle.h |   17 +++++++++++++++++
> >> >> >  1 file changed, 17 insertions(+)
> >> >> >
> >> >> > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> >> >> > index bdeee17..7833208 100644
> >> >> > --- a/include/configs/omap3_beagle.h
> >> >> > +++ b/include/configs/omap3_beagle.h
> >> >> > @@ -210,6 +210,8 @@
> >> >> >  #define CONFIG_EXTRA_ENV_SETTINGS \
> >> >> >         "loadaddr=0x80200000\0" \
> >> >> >         "rdaddr=0x81000000\0" \
> >> >> > +       "fdt_high=0xffffffff\0" \
> >> >> > +       "fdtaddr=0x80f80000\0" \
> >> >> >         "usbtty=cdc_acm\0" \
> >> >> >         "bootfile=uImage\0" \
> >> >> >         "ramdisk=ramdisk.gz\0" \
> >> >> > @@ -250,6 +252,19 @@
> >> >> >                 "omapdss.def_disp=${defaultdisplay} " \
> >> >> >                 "root=${nandroot} " \
> >> >> >                 "rootfstype=${nandrootfstype}\0" \
> >> >> > +       "findfdt=" \
> >> >> > +               "if test $beaglerev = AxBx; then " \
> >> >> > +                       "setenv fdtfile omap3-beagle.dtb; fi; " \
> >> >> > +               "if test $beaglerev = Cx; then " \
> >> >> > +                       "setenv fdtfile omap3-beagle.dtb; fi; " \
> >> >> > +               "if test $beaglerev = xMA; then " \
> >> >> > +                       "setenv fdtfile omap3-beagle-xm.dtb; fi; " \
> >> >> > +               "if test $beaglerev = xMB; then " \
> >> >> > +                       "setenv fdtfile omap3-beagle-xm.dtb; fi; " \
> >> >>
> >> >> There is no need for the "xMB" variant, as the gpio pins used for
> >> >> identification where never changed from the xMA when the newer silcon
> >> >> was used for the xMB (i guess if we look at the omap's es revision, we
> >> >> could figure out which are xMB's, but right now that isn't done in
> >> >> u-boot....)
> >> > I suppose then the following is ok to do as well?
> >> > diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
> >> > index c686f40..6094b0f 100644
> >> > --- a/board/ti/beagle/beagle.c
> >> > +++ b/board/ti/beagle/beagle.c
> >> > @@ -369,16 +369,6 @@ int misc_init_r(void)
> >> >                                         TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
> >> >                                         TWL4030_PM_RECEIVER_DEV_GRP_P1);
> >> >                 break;
> >> > -       case REVISION_XM_B:
> >> > -               printf("Beagle xM Rev B\n");
> >> > -               setenv("beaglerev", "xMB");
> >> > -               MUX_BEAGLE_XM();
> >> > -               /* Set VAUX2 to 1.8V for EHCI PHY */
> >> > -               twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
> >> > -                                       TWL4030_PM_RECEIVER_VAUX2_VSEL_18,
> >> > -                                       TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
> >> > -                                       TWL4030_PM_RECEIVER_DEV_GRP_P1);
> >> > -               break;
> >> >         case REVISION_XM_C:
> >> >                 printf("Beagle xM Rev C\n");
> >> >                 setenv("beaglerev", "xMC");
> >>
> >> Correct, as a cleanup we can nuke that xMB section..
> >>
> >> For some bike shed paining we could rename:
> >>
> >> Beagle xM Rev A -> Beagle xM Rev A/B
> >>
> >> Above that too. ;)
> > OK - I suspect and am not sure how dvi_pup will get handled in the
> > resultant diff, What do folks think of the following?
> 
> A quick review of all my old xM schematics i have access to and the
> ones on https://github.com/CircuitCo/
> 
> For the "dvi_pup" function
> 
> GPIO_170 was only used on the pre-production xM's P8/P9, i doubt any
> of them still operate (mine died)..
> 
> It was then switched to GPIO_129 for the xMA, however there where MMC
> issues, so it was changed for xMA1 (and thus xMA2)
> notes, top of this:
> https://github.com/CircuitCo/BeagleBoard-xM-RevC/blob/master/BeagleBoard-xM_revC_SCH.pdf
> 
> TPS_GPIO_2: was then used on xMA3 -> and all later generations...
> 
> So also looking at:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-omap2/board-omap3beagle.c#n161
> 
> Looks like the original beagle needs GPIO_170..
> 
> So I'd just set GPIO_170 for:
> 
> case REVISION_AXBX:
> case REVISION_CX:
> case REVISION_C4:
> 
> and ignore all the pre-production xM P8/P8's/xMA..
> 
> Side note, I think 'first' customers just got the xMA2 and newer.. As
> i was one of the first, and it's still running downstairs..

ok, here is how the code looks like with the V2 of my series:

Do we just default to none for XM_AB for dvi_pup?

static void beagle_dvi_pup(void)
{
	uchar val;

	switch (get_board_revision()) {
	case REVISION_AXBX:
	case REVISION_CX:
	case REVISION_C4:
	case REVISION_XM_AB:
		gpio_request(170, "");
		gpio_direction_output(170, 0);
		gpio_set_value(170, 1);
		break;
	case REVISION_XM_C:
	default:
		#define GPIODATADIR1 (TWL4030_BASEADD_GPIO+3)
		#define GPIODATAOUT1 (TWL4030_BASEADD_GPIO+6)

		i2c_read(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);
		val |= 4;
		i2c_write(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);

		i2c_read(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
		val |= 4;
		i2c_write(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
		break;
	}
}
robertcnelson@gmail.com - July 12, 2013, 1:32 p.m.
On Thu, Jul 11, 2013 at 11:06 PM, Nishanth Menon <nm@ti.com> wrote:
> On 18:17-20130711, Robert Nelson wrote:
>> On Thu, Jul 11, 2013 at 5:17 PM, Nishanth Menon <nm@ti.com> wrote:
>> > On 17:05-20130711, Robert Nelson wrote:
>> >> On Thu, Jul 11, 2013 at 5:03 PM, Nishanth Menon <nm@ti.com> wrote:
>> >> > On 17:02-20130711, Robert Nelson wrote:
>> >> >> On Thu, Jul 11, 2013 at 4:52 PM, Nishanth Menon <nm@ti.com> wrote:
>> >> >> > For folks not using concatenated device tree with uImage, having
>> >> >> > an handy function to find and load device tree is very handy.
>> >> >> >
>> >> >> > So introduce findfdt and loadfdt and run findfdt by default to make
>> >> >> > it easier on user scripts.
>> >> >> >
>> >> >> > Signed-off-by: Nishanth Menon <nm@ti.com>
>> >> >> > ---
>> >> >> >  include/configs/omap3_beagle.h |   17 +++++++++++++++++
>> >> >> >  1 file changed, 17 insertions(+)
>> >> >> >
>> >> >> > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
>> >> >> > index bdeee17..7833208 100644
>> >> >> > --- a/include/configs/omap3_beagle.h
>> >> >> > +++ b/include/configs/omap3_beagle.h
>> >> >> > @@ -210,6 +210,8 @@
>> >> >> >  #define CONFIG_EXTRA_ENV_SETTINGS \
>> >> >> >         "loadaddr=0x80200000\0" \
>> >> >> >         "rdaddr=0x81000000\0" \
>> >> >> > +       "fdt_high=0xffffffff\0" \
>> >> >> > +       "fdtaddr=0x80f80000\0" \
>> >> >> >         "usbtty=cdc_acm\0" \
>> >> >> >         "bootfile=uImage\0" \
>> >> >> >         "ramdisk=ramdisk.gz\0" \
>> >> >> > @@ -250,6 +252,19 @@
>> >> >> >                 "omapdss.def_disp=${defaultdisplay} " \
>> >> >> >                 "root=${nandroot} " \
>> >> >> >                 "rootfstype=${nandrootfstype}\0" \
>> >> >> > +       "findfdt=" \
>> >> >> > +               "if test $beaglerev = AxBx; then " \
>> >> >> > +                       "setenv fdtfile omap3-beagle.dtb; fi; " \
>> >> >> > +               "if test $beaglerev = Cx; then " \
>> >> >> > +                       "setenv fdtfile omap3-beagle.dtb; fi; " \
>> >> >> > +               "if test $beaglerev = xMA; then " \
>> >> >> > +                       "setenv fdtfile omap3-beagle-xm.dtb; fi; " \
>> >> >> > +               "if test $beaglerev = xMB; then " \
>> >> >> > +                       "setenv fdtfile omap3-beagle-xm.dtb; fi; " \
>> >> >>
>> >> >> There is no need for the "xMB" variant, as the gpio pins used for
>> >> >> identification where never changed from the xMA when the newer silcon
>> >> >> was used for the xMB (i guess if we look at the omap's es revision, we
>> >> >> could figure out which are xMB's, but right now that isn't done in
>> >> >> u-boot....)
>> >> > I suppose then the following is ok to do as well?
>> >> > diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
>> >> > index c686f40..6094b0f 100644
>> >> > --- a/board/ti/beagle/beagle.c
>> >> > +++ b/board/ti/beagle/beagle.c
>> >> > @@ -369,16 +369,6 @@ int misc_init_r(void)
>> >> >                                         TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
>> >> >                                         TWL4030_PM_RECEIVER_DEV_GRP_P1);
>> >> >                 break;
>> >> > -       case REVISION_XM_B:
>> >> > -               printf("Beagle xM Rev B\n");
>> >> > -               setenv("beaglerev", "xMB");
>> >> > -               MUX_BEAGLE_XM();
>> >> > -               /* Set VAUX2 to 1.8V for EHCI PHY */
>> >> > -               twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
>> >> > -                                       TWL4030_PM_RECEIVER_VAUX2_VSEL_18,
>> >> > -                                       TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
>> >> > -                                       TWL4030_PM_RECEIVER_DEV_GRP_P1);
>> >> > -               break;
>> >> >         case REVISION_XM_C:
>> >> >                 printf("Beagle xM Rev C\n");
>> >> >                 setenv("beaglerev", "xMC");
>> >>
>> >> Correct, as a cleanup we can nuke that xMB section..
>> >>
>> >> For some bike shed paining we could rename:
>> >>
>> >> Beagle xM Rev A -> Beagle xM Rev A/B
>> >>
>> >> Above that too. ;)
>> > OK - I suspect and am not sure how dvi_pup will get handled in the
>> > resultant diff, What do folks think of the following?
>>
>> A quick review of all my old xM schematics i have access to and the
>> ones on https://github.com/CircuitCo/
>>
>> For the "dvi_pup" function
>>
>> GPIO_170 was only used on the pre-production xM's P8/P9, i doubt any
>> of them still operate (mine died)..
>>
>> It was then switched to GPIO_129 for the xMA, however there where MMC
>> issues, so it was changed for xMA1 (and thus xMA2)
>> notes, top of this:
>> https://github.com/CircuitCo/BeagleBoard-xM-RevC/blob/master/BeagleBoard-xM_revC_SCH.pdf
>>
>> TPS_GPIO_2: was then used on xMA3 -> and all later generations...
>>
>> So also looking at:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-omap2/board-omap3beagle.c#n161
>>
>> Looks like the original beagle needs GPIO_170..
>>
>> So I'd just set GPIO_170 for:
>>
>> case REVISION_AXBX:
>> case REVISION_CX:
>> case REVISION_C4:
>>
>> and ignore all the pre-production xM P8/P8's/xMA..
>>
>> Side note, I think 'first' customers just got the xMA2 and newer.. As
>> i was one of the first, and it's still running downstairs..
>
> ok, here is how the code looks like with the V2 of my series:
>
> Do we just default to none for XM_AB for dvi_pup?
>
> static void beagle_dvi_pup(void)
> {
>         uchar val;
>
>         switch (get_board_revision()) {
>         case REVISION_AXBX:
>         case REVISION_CX:
>         case REVISION_C4:
>         case REVISION_XM_AB:
>                 gpio_request(170, "");
>                 gpio_direction_output(170, 0);
>                 gpio_set_value(170, 1);
>                 break;
>         case REVISION_XM_C:
>         default:
>                 #define GPIODATADIR1 (TWL4030_BASEADD_GPIO+3)
>                 #define GPIODATAOUT1 (TWL4030_BASEADD_GPIO+6)
>
>                 i2c_read(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);
>                 val |= 4;
>                 i2c_write(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);
>
>                 i2c_read(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
>                 val |= 4;
>                 i2c_write(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
>                 break;
>         }
> }

I'd do it like this, as...

On the xMA, xMA1, & xMA2: TPS_GPIO2 is not connected to anything...
xMA3->xMC: TPS_GPIO2 is connected to dvi_pup

static void beagle_dvi_pup(void)
{
        uchar val;

        switch (get_board_revision()) {
        case REVISION_AXBX:
        case REVISION_CX:
        case REVISION_C4:
                gpio_request(170, "");
                gpio_direction_output(170, 0);
                gpio_set_value(170, 1);
                break;
        case REVISION_XM_AB:
        case REVISION_XM_C:
        default:
                #define GPIODATADIR1 (TWL4030_BASEADD_GPIO+3)
                #define GPIODATAOUT1 (TWL4030_BASEADD_GPIO+6)

                i2c_read(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);
                val |= 4;
                i2c_write(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);

                i2c_read(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
                val |= 4;
                i2c_write(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
                break;
        }
}


Regards,
Nishanth Menon - July 12, 2013, 2:28 p.m.
On 08:32-20130712, Robert Nelson wrote:
> On Thu, Jul 11, 2013 at 11:06 PM, Nishanth Menon <nm@ti.com> wrote:
> > ok, here is how the code looks like with the V2 of my series:
> >
> > Do we just default to none for XM_AB for dvi_pup?
> >
> > static void beagle_dvi_pup(void)
> > {
> >         uchar val;
> >
> >         switch (get_board_revision()) {
> >         case REVISION_AXBX:
> >         case REVISION_CX:
> >         case REVISION_C4:
> >         case REVISION_XM_AB:
> >                 gpio_request(170, "");
> >                 gpio_direction_output(170, 0);
> >                 gpio_set_value(170, 1);
> >                 break;
> >         case REVISION_XM_C:
> >         default:
> >                 #define GPIODATADIR1 (TWL4030_BASEADD_GPIO+3)
> >                 #define GPIODATAOUT1 (TWL4030_BASEADD_GPIO+6)
> >
> >                 i2c_read(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);
> >                 val |= 4;
> >                 i2c_write(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);
> >
> >                 i2c_read(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
> >                 val |= 4;
> >                 i2c_write(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
> >                 break;
> >         }
> > }
> 
> I'd do it like this, as...
> 
> On the xMA, xMA1, & xMA2: TPS_GPIO2 is not connected to anything...
> xMA3->xMC: TPS_GPIO2 is connected to dvi_pup
> 
> static void beagle_dvi_pup(void)
> {
>         uchar val;
> 
>         switch (get_board_revision()) {
>         case REVISION_AXBX:
>         case REVISION_CX:
>         case REVISION_C4:
>                 gpio_request(170, "");
>                 gpio_direction_output(170, 0);
>                 gpio_set_value(170, 1);
>                 break;
>         case REVISION_XM_AB:
>         case REVISION_XM_C:
>         default:
>                 #define GPIODATADIR1 (TWL4030_BASEADD_GPIO+3)
>                 #define GPIODATAOUT1 (TWL4030_BASEADD_GPIO+6)
> 
>                 i2c_read(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);
>                 val |= 4;
>                 i2c_write(TWL4030_CHIP_GPIO, GPIODATADIR1, 1, &val, 1);
> 
>                 i2c_read(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
>                 val |= 4;
>                 i2c_write(TWL4030_CHIP_GPIO, GPIODATAOUT1, 1, &val, 1);
>                 break;
>         }
> }
Thanks for the review. sounds good. I will update 
http://patchwork.ozlabs.org/patch/258634/ accordingly for a rev3.

Will wait till Monday before posting a new series to accommodate any
further comments.

Patch

diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index c686f40..01d76a5 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -182,8 +182,7 @@  void get_board_mem_timings(struct board_sdrc_timings *timings)
 			timings->rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_200MHz;
 			break;
 		}
-	case REVISION_XM_A:
-	case REVISION_XM_B:
+	case REVISION_XM_AB:
 	case REVISION_XM_C:
 		if (pop_mfr == 0) {
 			/* 256MB DDR */
@@ -256,8 +255,7 @@  static void beagle_display_init(void)
 	case REVISION_C4:
 		omap3_dss_panel_config(&dvid_cfg);
 		break;
-	case REVISION_XM_A:
-	case REVISION_XM_B:
+	case REVISION_XM_AB:
 	case REVISION_XM_C:
 	default:
 		omap3_dss_panel_config(&dvid_cfg_xm);
@@ -276,12 +274,11 @@  static void beagle_dvi_pup(void)
 	case REVISION_AXBX:
 	case REVISION_CX:
 	case REVISION_C4:
-	case REVISION_XM_A:
+	case REVISION_XM_AB:
 		gpio_request(170, "");
 		gpio_direction_output(170, 0);
 		gpio_set_value(170, 1);
 		break;
-	case REVISION_XM_B:
 	case REVISION_XM_C:
 	default:
 		#define GPIODATADIR1 (TWL4030_BASEADD_GPIO+3)
@@ -359,19 +356,9 @@  int misc_init_r(void)
 					TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
 					TWL4030_PM_RECEIVER_DEV_GRP_P1);
 		break;
-	case REVISION_XM_A:
-		printf("Beagle xM Rev A\n");
-		setenv("beaglerev", "xMA");
-		MUX_BEAGLE_XM();
-		/* Set VAUX2 to 1.8V for EHCI PHY */
-		twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
-					TWL4030_PM_RECEIVER_VAUX2_VSEL_18,
-					TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
-					TWL4030_PM_RECEIVER_DEV_GRP_P1);
-		break;
-	case REVISION_XM_B:
-		printf("Beagle xM Rev B\n");
-		setenv("beaglerev", "xMB");
+	case REVISION_XM_AB:
+		printf("Beagle xM Rev A/B\n");
+		setenv("beaglerev", "xMA/xMB");
 		MUX_BEAGLE_XM();
 		/* Set VAUX2 to 1.8V for EHCI PHY */
 		twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
@@ -484,8 +471,7 @@  int misc_init_r(void)
 
 	twl4030_power_init();
 	switch (get_board_revision()) {
-	case REVISION_XM_A:
-	case REVISION_XM_B:
+	case REVISION_XM_AB:
 		twl4030_led_init(TWL4030_LED_LEDEN_LEDBON);
 		break;
 	default:
diff --git a/board/ti/beagle/beagle.h b/board/ti/beagle/beagle.h
index 6d71bbc..27f8b12 100644
--- a/board/ti/beagle/beagle.h
+++ b/board/ti/beagle/beagle.h
@@ -39,8 +39,7 @@  const omap3_sysinfo sysinfo = {
 #define REVISION_AXBX	0x7
 #define REVISION_CX	0x6
 #define REVISION_C4	0x5
-#define REVISION_XM_A	0x0
-#define REVISION_XM_B	0x1
+#define REVISION_XM_AB	0x0
 #define REVISION_XM_C	0x2
 
 /*