Thanks for the review. The sdram_was_busy is a holdover from some timing
analysis to see if the SDRAM was actually taking any significant length of
time. In practice on the DISCO board it never does (sdram_was_busy stays 0
throughout initialization) I'll remove it and re-submit. I'll also swap in
hard 8 space tabs. Which, if its ok with folks will necessitate a vim
modeline to insure that those characteristics are set.


On Fri, Jan 24, 2014 at 10:44 AM, Karl Palsson <notifications@github.com>wrote:

> In lib/stm32/f4/fmc.c:
>
> > +        case SDRAM_PALL:
> > +            tmp_reg |= FMC_SDCMR_MODE_PALL;
> > +            break;
> > +        case SDRAM_SELF_REFRESH:
> > +            tmp_reg |= FMC_SDCMR_MODE_SELF_REFRESH;
> > +            break;
> > +        case SDRAM_POWER_DOWN:
> > +            tmp_reg |= FMC_SDCMR_MODE_POWER_DOWN;
> > +            break;
> > +        case SDRAM_NORMAL:
> > +        default:
> > +            break;
> > +    }
> > +
> > +    /* Wait for the next chance to talk to the controller */
> > +    while (FMC_SDSR & (uint32_t) FMC_SDSR_BUSY) sdram_was_busy++;
>
> why does this need a cast, and is the sdram_was_busy really needed and the
> volatile global state variable?
> This should get taken care of by FMC_SDSR being a MMIO and doign the right
> volatile access.
>
> Otherwise looks good to merge for me :)
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/libopencm3/libopencm3/pull/229/files#r9158274>
> .
>