On Mon, May 3, 2010 at 9:24 PM, Tony Lindgren <[email protected]> wrote: > * Mike Rapoport <[email protected]> [100429 01:44]: >> Signed-off-by: Mike Rapoport <[email protected]> > > Please add a proper description to all the patches. > >> --- a/arch/arm/mach-omap2/gpmc-nand.c >> +++ b/arch/arm/mach-omap2/gpmc-nand.c >> @@ -116,6 +124,11 @@ int __init gpmc_nand_init(struct >> omap_nand_platform_data *_nand_data) >> return err; >> } >> >> + if (gpmc_nand_data->keep_timings) { >> + gpmc_cs_get_timings(gpmc_nand_data->cs, &gpmc_default_timings); >> + gpmc_nand_data->gpmc_t = &gpmc_default_timings; >> + } >> + >> err = gpmc_nand_setup(); >> if (err < 0) { >> dev_err(dev, "NAND platform setup failed: %d\n", err); > > Hmm, so you're setting the timings based on the bootloader values? > > I' think the problem with that is that chances are that it still won't > work for other L3 frequencies because of rounding errors. > > With gpmc_cs_get_timings() you're already using tick rounded timings, > so you won't get the required accuracy out of those for the other > L3 frequencies.
Agree. But even if the timings are in nanoseconds there are rounding errors, and there are still chances that L3 frequency change will break NAND So it comes down to what provides better tolerance, the explicit NAND timings in nanosecs or (rounded) timings in ticks derived from bootloader settings... > So maybe just not do anything, and print a warning on gpmc L3 changes > if the timings are not set? I don't quite understand where exactly this warning should go. I haven't found any treatment of L3 frequency changes in gpmc related code neither in linux-omap nor in linux-omap-pm... > Regards, > > Tony > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Sincerely Yours, Mike. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
