Re: patches for fsample/850

Brian Swetland
2006-04-09T21:05:15Z

[tony@atomide.com]
> Hi,
>
> Few comments below.
>
> * Brian Swetland <swetland@google.com> [060408 08:45]:
> > --- linux-omap-2.6/arch/arm/mach-omap1/clock.h 2006-04-05 10:33:20.000000000 -0700
> > +++ linux-omap-fsample/arch/arm/mach-omap1/clock.h 2006-04-06 13:15:34.000000000 -0700
> > @@ -101,44 +101,50 @@
> > /*-------------------------------------------------------------------------
> > * Omap1 MPU rate table
> > *-------------------------------------------------------------------------*/
> > +#if defined(CONFIG_ARCH_OMAP730)
> > + #define CKCTL_RESERVED_1_MASK (0x2000)
> > +#else
> > + #define CKCTL_RESERVED_1_MASK (0x0000)
> > +#endif
>
> This addds yet another extra hurdle compiling 730 in with 15xx and 16xx.
> Although we cannot do it because of the interrupt handler, maybe we'll
> fix that someday. So let's try not to add more hurdles if we can avoid
> it.
>
> How about doing the masking when the CKCTL value is written based on the
> cpu_is_omap730()?

We can do that. I'll change it.

> > --- linux-omap-2.6/arch/arm/mach-omap1/pm.c 2006-04-05 10:33:20.000000000 -0700
> > +++ linux-omap-fsample/arch/arm/mach-omap1/pm.c 2006-04-06 13:15:34.000000000 -0700
> > @@ -326,8 +326,10 @@
> > /* stop DSP */
> > omap_writew(omap_readw(ARM_RSTCT1) & ~(1 << DSP_EN), ARM_RSTCT1);
> >
> > +#if !defined(CONFIG_ARCH_OMAP730)
> > /* shut down dsp_ck */
> > omap_writew(omap_readw(ARM_CKCTL) & ~(1 << EN_DSPCK), ARM_CKCTL);
> > +#endif
>
> This too would be better with if (!cpu_is_omap730()).

Okay.

> > @@ -189,8 +197,6 @@
> > omap_writel(value, OMAP730_IO_CONF_4);
> >
> > omap_uwire_configure_mode(0,16);
> > -
> > - omap_uwire_data_transfer(LCD_UWIRE_CS, LCD_DISOFF, 9, 0,NULL,1);
> > omap_uwire_data_transfer(LCD_UWIRE_CS, LCD_SLPIN, 9, 0,NULL,1);
> > omap_uwire_data_transfer(LCD_UWIRE_CS, LCD_DISNOR, 9, 0,NULL,1);
> > omap_uwire_data_transfer(LCD_UWIRE_CS, LCD_GSSET, 9, 0,NULL,1);
>
> Is the removal of one of the omap_uwire_data_transfer() calls intended?

Oops. That was unintentional.

> Then does it compile and boot for p2 and fsample both selected?

Probably not -- is there something like cpu_is_*() that I could
use to make this conditional at runtime?

> Also, could you please do one patch for the core support, and then
> another patch for files that don't touch the omap core files?
>
> That way we can send you patch directly upstream when we merge next
> time. Otherwise I have to redo the patch to strip out the drivers when
> we merge as not many omap drivers are yet merged with the mainline tree.

Sure. I'll clean up the above and break it into two pieces.

Brian