* [U-Boot] [PATCH] sunxi: mmc: Fix clk-delay settings @ 2015-09-23 19:59 Hans de Goede 2015-09-24 9:40 ` Ian Campbell 0 siblings, 1 reply; 6+ messages in thread From: Hans de Goede @ 2015-09-23 19:59 UTC (permalink / raw) To: u-boot In recent allwinner kernel sources the mmc clk-delay settings have been slightly tweaked, and for sun9i they are completely different then what we are using. This commit brings us in sync with what allwinner does, fixing problems accessing sdcards on some A33 devices (and likely others). For pre sun9i hardware this makes the following changes: -At 400Khz change the sample delay from 7 to 0 (first introduced in A31 sdk) -At 50 Mhz change the sample delay from 5 to 4 (first introduced in A23 sdk) Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/mmc/sunxi_mmc.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c index 25f18ad..e717c44 100644 --- a/drivers/mmc/sunxi_mmc.c +++ b/drivers/mmc/sunxi_mmc.c @@ -120,17 +120,27 @@ static int mmc_set_mod_clk(struct sunxi_mmc_host *mmchost, unsigned int hz) /* determine delays */ if (hz <= 400000) { oclk_dly = 0; - sclk_dly = 7; + sclk_dly = 0; } else if (hz <= 25000000) { oclk_dly = 0; sclk_dly = 5; +#ifdef CONFIG_MACH_SUN9I } else if (hz <= 50000000) { - oclk_dly = 3; - sclk_dly = 5; + oclk_dly = 5; + sclk_dly = 4; } else { /* hz > 50000000 */ oclk_dly = 2; sclk_dly = 4; +#else + } else if (hz <= 50000000) { + oclk_dly = 3; + sclk_dly = 4; + } else { + /* hz > 50000000 */ + oclk_dly = 1; + sclk_dly = 4; +#endif } writel(CCM_MMC_CTRL_ENABLE | pll | CCM_MMC_CTRL_SCLK_DLY(sclk_dly) | -- 2.5.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: mmc: Fix clk-delay settings 2015-09-23 19:59 [U-Boot] [PATCH] sunxi: mmc: Fix clk-delay settings Hans de Goede @ 2015-09-24 9:40 ` Ian Campbell 2015-09-24 10:01 ` Hans de Goede 0 siblings, 1 reply; 6+ messages in thread From: Ian Campbell @ 2015-09-24 9:40 UTC (permalink / raw) To: u-boot On Wed, 2015-09-23 at 21:59 +0200, Hans de Goede wrote: > In recent allwinner kernel sources the mmc clk-delay settings have been > slightly tweaked, and for sun9i they are completely different then what > we are using. > > This commit brings us in sync with what allwinner does, fixing problems > accessing sdcards on some A33 devices (and likely others). > > For pre sun9i hardware this makes the following changes: > -At 400Khz change the sample delay from 7 to 0 (first introduced in A31 sdk) This one applied to sun9i as well as pre I think? > -At 50 Mhz change the sample delay from 5 to 4 (first introduced in A23 sdk) By my reading it also changes oclk from 2 to 1 on <sun9i @ >50MHz. I don't really follow the a/w SDK stuff, is it the case that a given Ann SDK is tested and supported by Allwinner and used on real devices for processors Amm < Ann (chronologically, ? IOW if a change was introduced in the A31 SDK do we then have confidence that it doesn't break A20 due to Allwinners (and their customer's) use of the A31 SDK on A20 or are we relying on our own + community testing? > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/mmc/sunxi_mmc.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c > index 25f18ad..e717c44 100644 > --- a/drivers/mmc/sunxi_mmc.c > +++ b/drivers/mmc/sunxi_mmc.c > @@ -120,17 +120,27 @@ static int mmc_set_mod_clk(struct sunxi_mmc_host > *mmchost, unsigned int hz) > /* determine delays */ > if (hz <= 400000) { > oclk_dly = 0; > - sclk_dly = 7; > + sclk_dly = 0; > } else if (hz <= 25000000) { > oclk_dly = 0; > sclk_dly = 5; > +#ifdef CONFIG_MACH_SUN9I > } else if (hz <= 50000000) { > - oclk_dly = 3; > - sclk_dly = 5; > + oclk_dly = 5; > + sclk_dly = 4; > } else { > /* hz > 50000000 */ > oclk_dly = 2; > sclk_dly = 4; > +#else > + } else if (hz <= 50000000) { > + oclk_dly = 3; > + sclk_dly = 4; > + } else { > + /* hz > 50000000 */ > + oclk_dly = 1; > + sclk_dly = 4; > +#endif > } > > writel(CCM_MMC_CTRL_ENABLE | pll | > CCM_MMC_CTRL_SCLK_DLY(sclk_dly) | ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: mmc: Fix clk-delay settings 2015-09-24 9:40 ` Ian Campbell @ 2015-09-24 10:01 ` Hans de Goede 2015-09-24 10:29 ` Ian Campbell 0 siblings, 1 reply; 6+ messages in thread From: Hans de Goede @ 2015-09-24 10:01 UTC (permalink / raw) To: u-boot Hi, On 24-09-15 11:40, Ian Campbell wrote: > On Wed, 2015-09-23 at 21:59 +0200, Hans de Goede wrote: >> In recent allwinner kernel sources the mmc clk-delay settings have been >> slightly tweaked, and for sun9i they are completely different then what >> we are using. >> >> This commit brings us in sync with what allwinner does, fixing problems >> accessing sdcards on some A33 devices (and likely others). >> >> For pre sun9i hardware this makes the following changes: >> -At 400Khz change the sample delay from 7 to 0 (first introduced in A31 sdk) > > This one applied to sun9i as well as pre I think? > >> -At 50 Mhz change the sample delay from 5 to 4 (first introduced in A23 sdk) > > By my reading it also changes oclk from 2 to 1 on <sun9i @ >50MHz. Right, I did not list that as we never do > 50MHz, later on in the code we've: cfg->f_max = 52000000; And that 52 is somewhat symbolically there, it is meant for 52MHz sdio where as mmc / sdcards run at 50MHz max. > I don't really follow the a/w SDK stuff, is it the case that a given Ann > SDK is tested and supported by Allwinner and used on real devices for > processors Amm < Ann (chronologically, ? Yes and no, Allwinner used to have separate kernel sources for each die, so one for sun4i, sun5i, sun6i and sun7i. Recently they have changed to one unified kernel tree though, so all of sun6i, sun8i and sun9i build from the same kernel tree. Note that sun7i is missing from the list. > IOW if a change was introduced in the A31 SDK do we then have confidence > that it doesn't break A20 due to Allwinners (and their customer's) use of > the A31 SDK on A20 or are we relying on our own + community testing? It is safe to assume that the A23 changes do not break on the A31 since that is using the same kernel sources. The changes may cause issues on A20 though (A10 / A13 do not use the sample delay bits of the register AFAICT). So I've run several tests on A20 and things seem to work fine with the new settings there. I believe that the mmc controller and the mmc clock bits really are the same over all of A20 / A23 / A31 / A33, and that these new tweaked values are better to use everywhere. Regards, Hans ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: mmc: Fix clk-delay settings 2015-09-24 10:01 ` Hans de Goede @ 2015-09-24 10:29 ` Ian Campbell 2015-09-24 11:59 ` Hans de Goede 0 siblings, 1 reply; 6+ messages in thread From: Ian Campbell @ 2015-09-24 10:29 UTC (permalink / raw) To: u-boot On Thu, 2015-09-24 at 12:01 +0200, Hans de Goede wrote: > Hi, > > On 24-09-15 11:40, Ian Campbell wrote: > > On Wed, 2015-09-23 at 21:59 +0200, Hans de Goede wrote: > > > In recent allwinner kernel sources the mmc clk-delay settings have > > > been > > > slightly tweaked, and for sun9i they are completely different then > > > what > > > we are using. > > > > > > This commit brings us in sync with what allwinner does, fixing > > > problems > > > accessing sdcards on some A33 devices (and likely others). > > > > > > For pre sun9i hardware this makes the following changes: > > > -At 400Khz change the sample delay from 7 to 0 (first introduced in > > > A31 sdk) > > > > This one applied to sun9i as well as pre I think? > > > > > -At 50 Mhz change the sample delay from 5 to 4 (first introduced in > > > A23 sdk) > > > > By my reading it also changes oclk from 2 to 1 on <sun9i @ >50MHz. > > Right, I did not list that as we never do > 50MHz, But the being changed here does handle that (the else hz > 50000000 case) and that is changing even if we somehow coincidentally arrange that this code is never used today I think it is worth mentioning so that if it does become used we have the history of when/why it was changed. > later on in the code > we've: > > cfg->f_max = 52000000; > > And that 52 is somewhat symbolically there, it is meant for 52MHz sdio > where as mmc / sdcards run at 50MHz max. Are these timings used for sdio as well then? That seems worth noting. > > > I don't really follow the a/w SDK stuff, is it the case that a given > > Ann > > SDK is tested and supported by Allwinner and used on real devices for > > processors Amm < Ann (chronologically, ? > > Yes and no, Allwinner used to have separate kernel sources for each > die, so one for sun4i, sun5i, sun6i and sun7i. > > Recently they have changed to one unified kernel tree though, so all > of sun6i, sun8i and sun9i build from the same kernel tree. Note that > sun7i is missing from the list. > > > IOW if a change was introduced in the A31 SDK do we then have > > confidence > > that it doesn't break A20 due to Allwinners (and their customer's) use > > of > > the A31 SDK on A20 or are we relying on our own + community testing? > > It is safe to assume that the A23 changes do not break on the A31 since > that is using the same kernel sources. The changes may cause issues > on A20 though (A10 / A13 do not use the sample delay bits of the register > AFAICT). So I've run several tests on A20 and things seem to work fine > with the new settings there. Great. > I believe that the mmc controller and the mmc clock bits really are the > same over all of A20 / A23 / A31 / A33, and that these new tweaked values > are better to use everywhere. > > Regards, > > Hans > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: mmc: Fix clk-delay settings 2015-09-24 10:29 ` Ian Campbell @ 2015-09-24 11:59 ` Hans de Goede 2015-09-24 13:56 ` Ian Campbell 0 siblings, 1 reply; 6+ messages in thread From: Hans de Goede @ 2015-09-24 11:59 UTC (permalink / raw) To: u-boot Hi, On 24-09-15 12:29, Ian Campbell wrote: > On Thu, 2015-09-24 at 12:01 +0200, Hans de Goede wrote: >> Hi, >> >> On 24-09-15 11:40, Ian Campbell wrote: >>> On Wed, 2015-09-23 at 21:59 +0200, Hans de Goede wrote: >>>> In recent allwinner kernel sources the mmc clk-delay settings have >>>> been >>>> slightly tweaked, and for sun9i they are completely different then >>>> what >>>> we are using. >>>> >>>> This commit brings us in sync with what allwinner does, fixing >>>> problems >>>> accessing sdcards on some A33 devices (and likely others). >>>> >>>> For pre sun9i hardware this makes the following changes: >>>> -At 400Khz change the sample delay from 7 to 0 (first introduced in >>>> A31 sdk) >>> >>> This one applied to sun9i as well as pre I think? >>> >>>> -At 50 Mhz change the sample delay from 5 to 4 (first introduced in >>>> A23 sdk) >>> >>> By my reading it also changes oclk from 2 to 1 on <sun9i @ >50MHz. >> >> Right, I did not list that as we never do > 50MHz, > > But the being changed here does handle that (the else hz > 50000000 case) > and that is changing even if we somehow coincidentally arrange that this > code is never used today I think it is worth mentioning so that if it does > become used we have the history of when/why it was changed. Ok, I'll amend the commit message. >> later on in the code >> we've: >> >> cfg->f_max = 52000000; >> >> And that 52 is somewhat symbolically there, it is meant for 52MHz sdio >> where as mmc / sdcards run at 50MHz max. > > Are these timings used for sdio as well then? That seems worth noting. Yes, but we never use sdio in u-boot, more in general mmc/sdio is usually used referred to as just mmc (see e.g. the subsystem name, etc.). >>> I don't really follow the a/w SDK stuff, is it the case that a given >>> Ann >>> SDK is tested and supported by Allwinner and used on real devices for >>> processors Amm < Ann (chronologically, ? >> >> Yes and no, Allwinner used to have separate kernel sources for each >> die, so one for sun4i, sun5i, sun6i and sun7i. >> >> Recently they have changed to one unified kernel tree though, so all >> of sun6i, sun8i and sun9i build from the same kernel tree. Note that >> sun7i is missing from the list. >> >>> IOW if a change was introduced in the A31 SDK do we then have >>> confidence >>> that it doesn't break A20 due to Allwinners (and their customer's) use >>> of >>> the A31 SDK on A20 or are we relying on our own + community testing? >> >> It is safe to assume that the A23 changes do not break on the A31 since >> that is using the same kernel sources. The changes may cause issues >> on A20 though (A10 / A13 do not use the sample delay bits of the register >> AFAICT). So I've run several tests on A20 and things seem to work fine >> with the new settings there. > > Great. > >> I believe that the mmc controller and the mmc clock bits really are the >> same over all of A20 / A23 / A31 / A33, and that these new tweaked values >> are better to use everywhere. >> >> Regards, >> >> Hans >> Regards, Hans ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: mmc: Fix clk-delay settings 2015-09-24 11:59 ` Hans de Goede @ 2015-09-24 13:56 ` Ian Campbell 0 siblings, 0 replies; 6+ messages in thread From: Ian Campbell @ 2015-09-24 13:56 UTC (permalink / raw) To: u-boot On Thu, 2015-09-24 at 13:59 +0200, Hans de Goede wrote: > Hi, > > On 24-09-15 12:29, Ian Campbell wrote: > > On Thu, 2015-09-24 at 12:01 +0200, Hans de Goede wrote: > > > Hi, > > > > > > On 24-09-15 11:40, Ian Campbell wrote: > > > > On Wed, 2015-09-23 at 21:59 +0200, Hans de Goede wrote: > > > > > In recent allwinner kernel sources the mmc clk-delay settings > > > > > have > > > > > been > > > > > slightly tweaked, and for sun9i they are completely different > > > > > then > > > > > what > > > > > we are using. > > > > > > > > > > This commit brings us in sync with what allwinner does, fixing > > > > > problems > > > > > accessing sdcards on some A33 devices (and likely others). > > > > > > > > > > For pre sun9i hardware this makes the following changes: > > > > > -At 400Khz change the sample delay from 7 to 0 (first introduced > > > > > in > > > > > A31 sdk) > > > > > > > > This one applied to sun9i as well as pre I think? > > > > > > > > > -At 50 Mhz change the sample delay from 5 to 4 (first introduced > > > > > in > > > > > A23 sdk) > > > > > > > > By my reading it also changes oclk from 2 to 1 on <sun9i @ >50MHz. > > > > > > Right, I did not list that as we never do > 50MHz, > > > > But the being changed here does handle that (the else hz > 50000000 > > case) > > and that is changing even if we somehow coincidentally arrange that > > this > > code is never used today I think it is worth mentioning so that if it > > does > > become used we have the history of when/why it was changed. > > Ok, I'll amend the commit message. Thanks. > > > later on in the code > > > we've: > > > > > > cfg->f_max = 52000000; > > > > > > And that 52 is somewhat symbolically there, it is meant for 52MHz > > > sdio > > > where as mmc / sdcards run at 50MHz max. > > > > Are these timings used for sdio as well then? That seems worth noting. > > Yes, but we never use sdio in u-boot, more in general mmc/sdio is > usually used referred to as just mmc (see e.g. the subsystem name, etc.). Right, which makes it all the more likely that one day this code might be used for sdio too I think. Ian. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-24 13:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-23 19:59 [U-Boot] [PATCH] sunxi: mmc: Fix clk-delay settings Hans de Goede 2015-09-24 9:40 ` Ian Campbell 2015-09-24 10:01 ` Hans de Goede 2015-09-24 10:29 ` Ian Campbell 2015-09-24 11:59 ` Hans de Goede 2015-09-24 13:56 ` Ian Campbell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox