* [PATCH 0/3] mmc: sunxi: Changes in the host driver
@ 2016-07-21 6:28 Jean-Francois Moine
[not found] ` <cover.1469082481.git.moinejf-GANU6spQydw@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Jean-Francois Moine @ 2016-07-21 6:28 UTC (permalink / raw)
To: Ulf Hansson, Maxime Ripard, Chen-Yu Tsai
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
While testing the eMMC access in a Banana Pi M3, I found and fixed
some problems:
- the driver was trying to set a strange rate when clk_round_rate()
was returning an error
- the 'new timing mode' could not work because a register was not set
- with a parent clock at 1.2GHz, the output and sample phases were
not correct for the A83T
Jean-Francois Moine (3):
mmc: sunxi: Check the value returned by clk_round_rate
mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers
mmc: sunxi: Add support to the Allwinner A83T
drivers/mmc/host/sunxi-mmc.c | 45 +++++++++++++++++++++++++++++++++++---------
1 file changed, 38 insertions(+), 7 deletions(-)
--
2.9.2
^ permalink raw reply [flat|nested] 17+ messages in thread[parent not found: <cover.1469082481.git.moinejf-GANU6spQydw@public.gmane.org>]
* [PATCH 1/3] mmc: sunxi: Check the value returned by clk_round_rate [not found] ` <cover.1469082481.git.moinejf-GANU6spQydw@public.gmane.org> @ 2016-07-20 18:01 ` Jean-Francois Moine [not found] ` <c2231f3df66cf77e4fb9051f7b0c27ea69b8e63d.1469082481.git.moinejf-GANU6spQydw@public.gmane.org> 2016-07-20 18:16 ` [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers Jean-Francois Moine 2016-07-20 18:28 ` [PATCH 3/3] mmc: sunxi: Add support to the Allwinner A83T Jean-Francois Moine 2 siblings, 1 reply; 17+ messages in thread From: Jean-Francois Moine @ 2016-07-20 18:01 UTC (permalink / raw) To: Ulf Hansson, Maxime Ripard, Chen-Yu Tsai Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw clk_round_rate() may return an error. Check it. Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> --- drivers/mmc/host/sunxi-mmc.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index 2ee4c21..ba647b7 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -656,7 +656,8 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en) static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, struct mmc_ios *ios) { - u32 rate, oclk_dly, rval, sclk_dly; + long rate; + u32 oclk_dly, rval, sclk_dly; u32 clock = ios->clock; int ret; @@ -666,13 +667,18 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, clock <<= 1; rate = clk_round_rate(host->clk_mmc, clock); - dev_dbg(mmc_dev(host->mmc), "setting clk to %d, rounded %d\n", + if (rate < 0) { + dev_err(mmc_dev(host->mmc), "error rounding clk to %d: %ld\n", + clock, rate); + return rate; + } + dev_dbg(mmc_dev(host->mmc), "setting clk to %d, rounded %ld\n", clock, rate); /* setting clock rate */ ret = clk_set_rate(host->clk_mmc, rate); if (ret) { - dev_err(mmc_dev(host->mmc), "error setting clk to %d: %d\n", + dev_err(mmc_dev(host->mmc), "error setting clk to %ld: %d\n", rate, ret); return ret; } -- 2.9.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <c2231f3df66cf77e4fb9051f7b0c27ea69b8e63d.1469082481.git.moinejf-GANU6spQydw@public.gmane.org>]
* Re: [PATCH 1/3] mmc: sunxi: Check the value returned by clk_round_rate [not found] ` <c2231f3df66cf77e4fb9051f7b0c27ea69b8e63d.1469082481.git.moinejf-GANU6spQydw@public.gmane.org> @ 2016-07-21 8:49 ` Maxime Ripard 0 siblings, 0 replies; 17+ messages in thread From: Maxime Ripard @ 2016-07-21 8:49 UTC (permalink / raw) To: Jean-Francois Moine Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw [-- Attachment #1: Type: text/plain, Size: 403 bytes --] On Wed, Jul 20, 2016 at 08:01:47PM +0200, Jean-Francois Moine wrote: > clk_round_rate() may return an error. Check it. > > Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers [not found] ` <cover.1469082481.git.moinejf-GANU6spQydw@public.gmane.org> 2016-07-20 18:01 ` [PATCH 1/3] mmc: sunxi: Check the value returned by clk_round_rate Jean-Francois Moine @ 2016-07-20 18:16 ` Jean-Francois Moine [not found] ` <bc7b767cad6db159a44c4b98d5b1f3eb53c26bd3.1469082481.git.moinejf-GANU6spQydw@public.gmane.org> 2016-07-20 18:28 ` [PATCH 3/3] mmc: sunxi: Add support to the Allwinner A83T Jean-Francois Moine 2 siblings, 1 reply; 17+ messages in thread From: Jean-Francois Moine @ 2016-07-20 18:16 UTC (permalink / raw) To: Ulf Hansson, Maxime Ripard, Chen-Yu Tsai Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw The 'new timing mode' with 8 bits DDR works correctly when the NewTiming register is set. Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> --- Note about the 'new timing mode'. This patch assumes that, when the new mode is used, the clock driver sets the mode select in the MMC clock and multiplies the clock rate by 2: - MMC side: - with a timing 8 bits DDR at 50MHz, the MMC driver calls clk_set_rate() with a rate 50*2 = 100MHz, - clock side: - the clock driver sets the hardware MMC clock to 100*2 = 200MHz, - setting the 'mode select' of the hardware MMC clock divides the rate by 2, - MMC side: - setting the MMC clock divider register to 1 divides the rate by 2. So, the final rate is 50MHz. For this to work, the clock driver must know when the 'mode select' is used. Instead of using a new special clock function, in my driver, the 'mode select' capable clocks are declared as such with an associated rate (actually, mmc2 in A83T, and mmc{1,2} in H3). When this rate is asked for (100MHz), the mode select is set. This works without change in the MMC driver. If a 100MHz rate would be used without 'mode select' for such clocks, an other mechanism should be found (information in the low bits of the rate?). --- drivers/mmc/host/sunxi-mmc.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index ba647b7..7f9c31a 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -64,6 +64,7 @@ #define SDXC_REG_CBCR (0x48) /* SMC CIU Byte Count Register */ #define SDXC_REG_BBCR (0x4C) /* SMC BIU Byte Count Register */ #define SDXC_REG_DBGC (0x50) /* SMC Debug Enable Register */ +#define SDXC_REG_NTSR (0x5c) /* SMC NewTiming Set Register */ #define SDXC_REG_HWRST (0x78) /* SMC Card Hardware Reset for Register */ #define SDXC_REG_DMAC (0x80) /* SMC IDMAC Control Register */ #define SDXC_REG_DLBA (0x84) /* SMC IDMAC Descriptor List Base Addre */ @@ -171,6 +172,9 @@ #define SDXC_SEND_AUTO_STOPCCSD BIT(9) #define SDXC_CEATA_DEV_IRQ_ENABLE BIT(10) +/* NewTiming Set Register */ +#define SDXC_NEWMODE_ENABLE BIT(31) + /* IDMA controller bus mod bit field */ #define SDXC_IDMAC_SOFT_RESET BIT(0) #define SDXC_IDMAC_FIX_BURST BIT(1) @@ -661,10 +665,20 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, u32 clock = ios->clock; int ret; - /* 8 bit DDR requires a higher module clock */ - if (ios->timing == MMC_TIMING_MMC_DDR52 && - ios->bus_width == MMC_BUS_WIDTH_8) - clock <<= 1; + /* + * 8 bit DDR requires a higher module clock + * and the 'new mode' + */ + if (ios->bus_width == MMC_BUS_WIDTH_8) { /* eMMC */ + rval = mmc_readl(host, REG_NTSR); + if (ios->timing == MMC_TIMING_MMC_DDR52) { + clock <<= 1; + rval |= SDXC_NEWMODE_ENABLE; + } else { + rval &= ~SDXC_NEWMODE_ENABLE; + } + mmc_writel(host, REG_NTSR, rval); + } rate = clk_round_rate(host->clk_mmc, clock); if (rate < 0) { -- 2.9.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <bc7b767cad6db159a44c4b98d5b1f3eb53c26bd3.1469082481.git.moinejf-GANU6spQydw@public.gmane.org>]
* Re: [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers [not found] ` <bc7b767cad6db159a44c4b98d5b1f3eb53c26bd3.1469082481.git.moinejf-GANU6spQydw@public.gmane.org> @ 2016-07-21 8:56 ` Maxime Ripard 2016-07-21 9:26 ` Jean-Francois Moine 0 siblings, 1 reply; 17+ messages in thread From: Maxime Ripard @ 2016-07-21 8:56 UTC (permalink / raw) To: Jean-Francois Moine Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw [-- Attachment #1: Type: text/plain, Size: 1061 bytes --] Hi, On Wed, Jul 20, 2016 at 08:16:28PM +0200, Jean-Francois Moine wrote: > The 'new timing mode' with 8 bits DDR works correctly when the NewTiming > register is set. What does that mode brings to the table? > > Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> > --- > Note about the 'new timing mode'. > > This patch assumes that, when the new mode is used, the clock driver > sets the mode select in the MMC clock and multiplies the clock rate > by 2: > - MMC side: > - with a timing 8 bits DDR at 50MHz, the MMC driver calls > clk_set_rate() with a rate 50*2 = 100MHz, > - clock side: > - the clock driver sets the hardware MMC clock to 100*2 = 200MHz, > - setting the 'mode select' of the hardware MMC clock divides the > rate by 2, > - MMC side: > - setting the MMC clock divider register to 1 divides the rate by 2. > So, the final rate is 50MHz. What happens if you actually want to set it to 100MHz? Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers 2016-07-21 8:56 ` Maxime Ripard @ 2016-07-21 9:26 ` Jean-Francois Moine [not found] ` <20160721112655.941b1dad04f7a5b94d4172c1-GANU6spQydw@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Jean-Francois Moine @ 2016-07-21 9:26 UTC (permalink / raw) To: Maxime Ripard Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw On Thu, 21 Jul 2016 10:56:15 +0200 Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > On Wed, Jul 20, 2016 at 08:16:28PM +0200, Jean-Francois Moine wrote: > > The 'new timing mode' with 8 bits DDR works correctly when the NewTiming > > register is set. > > What does that mode brings to the table? >From my tests, the eMMC of the Banana Pi M3 (A83T) cannot work when the new mode is not used. > > > > Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> > > --- > > Note about the 'new timing mode'. > > > > This patch assumes that, when the new mode is used, the clock driver > > sets the mode select in the MMC clock and multiplies the clock rate > > by 2: > > - MMC side: > > - with a timing 8 bits DDR at 50MHz, the MMC driver calls > > clk_set_rate() with a rate 50*2 = 100MHz, > > - clock side: > > - the clock driver sets the hardware MMC clock to 100*2 = 200MHz, > > - setting the 'mode select' of the hardware MMC clock divides the > > rate by 2, > > - MMC side: > > - setting the MMC clock divider register to 1 divides the rate by 2. > > So, the final rate is 50MHz. > > What happens if you actually want to set it to 100MHz? There is no SDXC_CLK_100M in the mainline driver, and 100MHz is asked only for 8 bits DDR at 50MHz. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20160721112655.941b1dad04f7a5b94d4172c1-GANU6spQydw@public.gmane.org>]
* Re: [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers [not found] ` <20160721112655.941b1dad04f7a5b94d4172c1-GANU6spQydw@public.gmane.org> @ 2016-07-29 19:17 ` Maxime Ripard 2016-07-30 5:18 ` Jean-Francois Moine 2016-07-30 10:19 ` [linux-sunxi] " Hans de Goede 2016-07-29 19:36 ` Maxime Ripard 1 sibling, 2 replies; 17+ messages in thread From: Maxime Ripard @ 2016-07-29 19:17 UTC (permalink / raw) To: Jean-Francois Moine Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw [-- Attachment #1: Type: text/plain, Size: 1864 bytes --] On Thu, Jul 21, 2016 at 11:26:55AM +0200, Jean-Francois Moine wrote: > On Thu, 21 Jul 2016 10:56:15 +0200 > Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > > > On Wed, Jul 20, 2016 at 08:16:28PM +0200, Jean-Francois Moine wrote: > > > The 'new timing mode' with 8 bits DDR works correctly when the NewTiming > > > register is set. > > > > What does that mode brings to the table? > > From my tests, the eMMC of the Banana Pi M3 (A83T) cannot work when the > new mode is not used. > > > > > > > Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> > > > --- > > > Note about the 'new timing mode'. > > > > > > This patch assumes that, when the new mode is used, the clock driver > > > sets the mode select in the MMC clock and multiplies the clock rate > > > by 2: > > > - MMC side: > > > - with a timing 8 bits DDR at 50MHz, the MMC driver calls > > > clk_set_rate() with a rate 50*2 = 100MHz, > > > - clock side: > > > - the clock driver sets the hardware MMC clock to 100*2 = 200MHz, > > > - setting the 'mode select' of the hardware MMC clock divides the > > > rate by 2, > > > - MMC side: > > > - setting the MMC clock divider register to 1 divides the rate by 2. > > > So, the final rate is 50MHz. > > > > What happens if you actually want to set it to 100MHz? > > There is no SDXC_CLK_100M in the mainline driver, and 100MHz is asked > only for 8 bits DDR at 50MHz. You're missing the point. clk_set_rate is supposed to apply a rate as close as possible as requested, there's no reason why you would request a rate twice as high as need. You want to switch the clock from one mode to another, fine, create a new function for that. But don't hack an existing one. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers 2016-07-29 19:17 ` Maxime Ripard @ 2016-07-30 5:18 ` Jean-Francois Moine 2016-07-30 10:19 ` [linux-sunxi] " Hans de Goede 1 sibling, 0 replies; 17+ messages in thread From: Jean-Francois Moine @ 2016-07-30 5:18 UTC (permalink / raw) To: Maxime Ripard Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw On Fri, 29 Jul 2016 21:17:30 +0200 Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > > > What happens if you actually want to set it to 100MHz? > > > > There is no SDXC_CLK_100M in the mainline driver, and 100MHz is asked > > only for 8 bits DDR at 50MHz. > > You're missing the point. > > clk_set_rate is supposed to apply a rate as close as possible as > requested, there's no reason why you would request a rate twice as > high as need. > > You want to switch the clock from one mode to another, fine, create a > new function for that. But don't hack an existing one. I will not change the core clock stuff for this marginal case. Flagging the clock as 'new mode capable' is enough. Anyway, setting the 'new mode' bit to the clock divides the rate by two, so, there is no reason to not use it. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [linux-sunxi] Re: [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers 2016-07-29 19:17 ` Maxime Ripard 2016-07-30 5:18 ` Jean-Francois Moine @ 2016-07-30 10:19 ` Hans de Goede [not found] ` <9414aab0-ac2b-01c2-e16b-8f7394ea7b68-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 1 sibling, 1 reply; 17+ messages in thread From: Hans de Goede @ 2016-07-30 10:19 UTC (permalink / raw) To: maxime.ripard, Jean-Francois Moine Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel, linux-mmc, linux-sunxi Hi, On 29-07-16 21:17, Maxime Ripard wrote: > On Thu, Jul 21, 2016 at 11:26:55AM +0200, Jean-Francois Moine wrote: >> On Thu, 21 Jul 2016 10:56:15 +0200 >> Maxime Ripard <maxime.ripard@free-electrons.com> wrote: >> >>> On Wed, Jul 20, 2016 at 08:16:28PM +0200, Jean-Francois Moine wrote: >>>> The 'new timing mode' with 8 bits DDR works correctly when the NewTiming >>>> register is set. >>> >>> What does that mode brings to the table? >> >> From my tests, the eMMC of the Banana Pi M3 (A83T) cannot work when the >> new mode is not used. >> >>>> >>>> Signed-off-by: Jean-Francois Moine <moinejf@free.fr> >>>> --- >>>> Note about the 'new timing mode'. >>>> >>>> This patch assumes that, when the new mode is used, the clock driver >>>> sets the mode select in the MMC clock and multiplies the clock rate >>>> by 2: >>>> - MMC side: >>>> - with a timing 8 bits DDR at 50MHz, the MMC driver calls >>>> clk_set_rate() with a rate 50*2 = 100MHz, >>>> - clock side: >>>> - the clock driver sets the hardware MMC clock to 100*2 = 200MHz, >>>> - setting the 'mode select' of the hardware MMC clock divides the >>>> rate by 2, >>>> - MMC side: >>>> - setting the MMC clock divider register to 1 divides the rate by 2. >>>> So, the final rate is 50MHz. >>> >>> What happens if you actually want to set it to 100MHz? >> >> There is no SDXC_CLK_100M in the mainline driver, and 100MHz is asked >> only for 8 bits DDR at 50MHz. > > You're missing the point. > > clk_set_rate is supposed to apply a rate as close as possible as > requested, there's no reason why you would request a rate twice as > high as need. On all SoCs which support it, requesting DDR-8bit needs a twice as high clock as the mmc base clock (due to the DDR). If you look at the current code you will see that what actually happens is that we pass twice as high clock as the mmc-subsys asks for to clk_set_rate and set the controllers *internal* divider to 2 (instead of the usual 1), this is not new and not introduced by Jean-Francois' patch. All Jean-Francois' patch adds is the setting of the new SDXC_REG_NTSR reg msb bit when we do this. Note no changes to the clock rate calculations are made. If this fixes the eMMC on A83T boards this seems like a good patch to me, but IMHO we should only write the SDXC_REG_NTSR on the A83t and not everywhere as Jean-Francois' patch does. Jean-Francois, can you submit a v2 of your patch and make the writing of SDXC_REG_NTSR depend on a new sun8i-a83t-mmc compatible ? Also you should probably drop the bits about the clock stuff from the commit message as that just seems to confuse people. Regards, Hans ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <9414aab0-ac2b-01c2-e16b-8f7394ea7b68-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: Re: [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers [not found] ` <9414aab0-ac2b-01c2-e16b-8f7394ea7b68-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-08-01 13:52 ` Jean-Francois Moine [not found] ` <20160801155246.b7b3c7f0f582394b4c25f6dc-GANU6spQydw@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Jean-Francois Moine @ 2016-08-01 13:52 UTC (permalink / raw) To: Hans de Goede Cc: maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw On Sat, 30 Jul 2016 12:19:03 +0200 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > Jean-Francois, can you submit a v2 of your patch and make the writing of > SDXC_REG_NTSR depend on a new sun8i-a83t-mmc compatible ? > > Also you should probably drop the bits about the clock stuff from the > commit message as that just seems to confuse people. Hi Hans, I submitted a new patch (sorry, I forgot the history), but it asks for some explanation. - in the old timings, the phase delays are set in the clock. That's why there is a function clk_set_phase() which is called from the MMC side. - in the new timings, the delays are in the MMC register SDXC_REG_NTSR only. In this case, the function clk_set_phase() is of no use (also, by test, it seems that the phase delays set by hardware reset do work, so, there is no need to set them), but, - there is a bit in the clock telling that the new timings are used, i.e. that the phase delays of the clock must be ignored. Not setting this bit prevents the device to work (at least in the A83T, the H3 seems more helpful). -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20160801155246.b7b3c7f0f582394b4c25f6dc-GANU6spQydw@public.gmane.org>]
* Re: Re: [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers [not found] ` <20160801155246.b7b3c7f0f582394b4c25f6dc-GANU6spQydw@public.gmane.org> @ 2016-08-02 8:37 ` Hans de Goede 0 siblings, 0 replies; 17+ messages in thread From: Hans de Goede @ 2016-08-02 8:37 UTC (permalink / raw) To: moinejf-GANU6spQydw Cc: maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw Hi, On 01-08-16 15:52, Jean-Francois Moine wrote: > On Sat, 30 Jul 2016 12:19:03 +0200 > Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > >> Jean-Francois, can you submit a v2 of your patch and make the writing of >> SDXC_REG_NTSR depend on a new sun8i-a83t-mmc compatible ? >> >> Also you should probably drop the bits about the clock stuff from the >> commit message as that just seems to confuse people. > > Hi Hans, > > I submitted a new patch (sorry, I forgot the history), but it asks for > some explanation. > > - in the old timings, the phase delays are set in the clock. > That's why there is a function clk_set_phase() which is called from > the MMC side. > > - in the new timings, the delays are in the MMC register SDXC_REG_NTSR > only. > In this case, the function clk_set_phase() is of no use > (also, by test, it seems that the phase delays set by hardware > reset do work, so, there is no need to set them), but, > > - there is a bit in the clock telling that the new timings are used, > i.e. that the phase delays of the clock must be ignored. > > Not setting this bit prevents the device to work (at least in the > A83T, the H3 seems more helpful). Thanks for the explanation, it would be good to put this in the commit msg of v3 of the patch. Regards, Hans ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers [not found] ` <20160721112655.941b1dad04f7a5b94d4172c1-GANU6spQydw@public.gmane.org> 2016-07-29 19:17 ` Maxime Ripard @ 2016-07-29 19:36 ` Maxime Ripard 2016-07-30 5:20 ` Jean-Francois Moine 1 sibling, 1 reply; 17+ messages in thread From: Maxime Ripard @ 2016-07-29 19:36 UTC (permalink / raw) To: Jean-Francois Moine Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw [-- Attachment #1: Type: text/plain, Size: 736 bytes --] On Thu, Jul 21, 2016 at 11:26:55AM +0200, Jean-Francois Moine wrote: > On Thu, 21 Jul 2016 10:56:15 +0200 > Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > > > On Wed, Jul 20, 2016 at 08:16:28PM +0200, Jean-Francois Moine wrote: > > > The 'new timing mode' with 8 bits DDR works correctly when the NewTiming > > > register is set. > > > > What does that mode brings to the table? > > From my tests, the eMMC of the Banana Pi M3 (A83T) cannot work when the > new mode is not used. That's odd. The one in the Pine64 seems to work just fine, and yet there's only the new mode on the A64. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers 2016-07-29 19:36 ` Maxime Ripard @ 2016-07-30 5:20 ` Jean-Francois Moine 0 siblings, 0 replies; 17+ messages in thread From: Jean-Francois Moine @ 2016-07-30 5:20 UTC (permalink / raw) To: Maxime Ripard Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw On Fri, 29 Jul 2016 21:36:34 +0200 Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > On Thu, Jul 21, 2016 at 11:26:55AM +0200, Jean-Francois Moine wrote: > > On Thu, 21 Jul 2016 10:56:15 +0200 > > Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > > > > > On Wed, Jul 20, 2016 at 08:16:28PM +0200, Jean-Francois Moine wrote: > > > > The 'new timing mode' with 8 bits DDR works correctly when the NewTiming > > > > register is set. > > > > > > What does that mode brings to the table? > > > > From my tests, the eMMC of the Banana Pi M3 (A83T) cannot work when the > > new mode is not used. > > That's odd. The one in the Pine64 seems to work just fine, and yet > there's only the new mode on the A64. I spent 2 weeks on this problem. You may try it by yourself. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] mmc: sunxi: Add support to the Allwinner A83T [not found] ` <cover.1469082481.git.moinejf-GANU6spQydw@public.gmane.org> 2016-07-20 18:01 ` [PATCH 1/3] mmc: sunxi: Check the value returned by clk_round_rate Jean-Francois Moine 2016-07-20 18:16 ` [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers Jean-Francois Moine @ 2016-07-20 18:28 ` Jean-Francois Moine 2016-07-21 8:58 ` Maxime Ripard 2 siblings, 1 reply; 17+ messages in thread From: Jean-Francois Moine @ 2016-07-20 18:28 UTC (permalink / raw) To: Ulf Hansson, Maxime Ripard, Chen-Yu Tsai Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-mmc-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw The rate of the PLL-PERIPH clock is usually set to 1.2GHz in the A83T. This patch sets the phase delays of the output and sample clocks accordingly. Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> --- Note: The impacted phase delays are only for 50MHz. The phase delays are not used in 50MHz 8 bits DDR (new timing mode). --- drivers/mmc/host/sunxi-mmc.c | 11 +++++++++++ 1 file changed, 11 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index 7f9c31a..e161a64 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -961,6 +961,7 @@ static int sunxi_mmc_card_busy(struct mmc_host *mmc) static const struct of_device_id sunxi_mmc_of_match[] = { { .compatible = "allwinner,sun4i-a10-mmc", }, { .compatible = "allwinner,sun5i-a13-mmc", }, + { .compatible = "allwinner,sun8i-a83t-mmc", }, { .compatible = "allwinner,sun9i-a80-mmc", }, { /* sentinel */ } }; @@ -986,6 +987,14 @@ static const struct sunxi_mmc_clk_delay sunxi_mmc_clk_delays[] = { [SDXC_CLK_50M_DDR_8BIT] = { .output = 90, .sample = 180 }, }; +static const struct sunxi_mmc_clk_delay sun8i_a83t_mmc_clk_delays[] = { + [SDXC_CLK_400K] = { .output = 180, .sample = 180 }, + [SDXC_CLK_25M] = { .output = 180, .sample = 75 }, + [SDXC_CLK_50M] = { .output = 90, .sample = 105 }, + [SDXC_CLK_50M_DDR] = { .output = 60, .sample = 120 }, + [SDXC_CLK_50M_DDR_8BIT] = { .output = 180, .sample = 180 }, +}; + static const struct sunxi_mmc_clk_delay sun9i_mmc_clk_delays[] = { [SDXC_CLK_400K] = { .output = 180, .sample = 180 }, [SDXC_CLK_25M] = { .output = 180, .sample = 75 }, @@ -1007,6 +1016,8 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host, if (of_device_is_compatible(np, "allwinner,sun9i-a80-mmc")) host->clk_delays = sun9i_mmc_clk_delays; + else if (of_device_is_compatible(np, "allwinner,sun8i-a83t-mmc")) + host->clk_delays = sun8i_a83t_mmc_clk_delays; else host->clk_delays = sunxi_mmc_clk_delays; -- 2.9.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] mmc: sunxi: Add support to the Allwinner A83T 2016-07-20 18:28 ` [PATCH 3/3] mmc: sunxi: Add support to the Allwinner A83T Jean-Francois Moine @ 2016-07-21 8:58 ` Maxime Ripard 2016-07-21 9:18 ` Jean-Francois Moine 0 siblings, 1 reply; 17+ messages in thread From: Maxime Ripard @ 2016-07-21 8:58 UTC (permalink / raw) To: Jean-Francois Moine Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel, linux-mmc, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 658 bytes --] Maxime On Wed, Jul 20, 2016 at 08:28:47PM +0200, Jean-Francois Moine wrote: > The rate of the PLL-PERIPH clock is usually set to 1.2GHz in the A83T. Uh? The datasheet says to set it to 600MHz. > This patch sets the phase delays of the output and sample clocks > accordingly. > > Signed-off-by: Jean-Francois Moine <moinejf@free.fr> > --- > Note: The impacted phase delays are only for 50MHz. > The phase delays are not used in 50MHz 8 bits DDR (new timing mode). Actually, they seem to be, in the new timing mode register. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] mmc: sunxi: Add support to the Allwinner A83T 2016-07-21 8:58 ` Maxime Ripard @ 2016-07-21 9:18 ` Jean-Francois Moine [not found] ` <20160721111851.133a3f16d6e9de1ee45b3654-GANU6spQydw@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Jean-Francois Moine @ 2016-07-21 9:18 UTC (permalink / raw) To: Maxime Ripard Cc: Ulf Hansson, Chen-Yu Tsai, linux-arm-kernel, linux-mmc, linux-sunxi On Thu, 21 Jul 2016 10:58:38 +0200 Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Wed, Jul 20, 2016 at 08:28:47PM +0200, Jean-Francois Moine wrote: > > The rate of the PLL-PERIPH clock is usually set to 1.2GHz in the A83T. > > Uh? The datasheet says to set it to 600MHz. Right. But the driver of the SDK for the Banana Pi M3 (A83T) sets it to 1.2GHz. > > This patch sets the phase delays of the output and sample clocks > > accordingly. > > > > Signed-off-by: Jean-Francois Moine <moinejf@free.fr> > > --- > > Note: The impacted phase delays are only for 50MHz. > > The phase delays are not used in 50MHz 8 bits DDR (new timing mode). > > Actually, they seem to be, in the new timing mode register. In the SDK driver, nothing is set in the new timing mode register. Anyway, the eMMC works fine in both the Banana Pis M2+ and M3 with no delay. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20160721111851.133a3f16d6e9de1ee45b3654-GANU6spQydw@public.gmane.org>]
* Re: [PATCH 3/3] mmc: sunxi: Add support to the Allwinner A83T [not found] ` <20160721111851.133a3f16d6e9de1ee45b3654-GANU6spQydw@public.gmane.org> @ 2016-07-22 19:00 ` Jean-Francois Moine 0 siblings, 0 replies; 17+ messages in thread From: Jean-Francois Moine @ 2016-07-22 19:00 UTC (permalink / raw) To: Jean-Francois Moine Cc: Maxime Ripard, Ulf Hansson, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-mmc-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Thu, 21 Jul 2016 11:18:51 +0200 Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> wrote: > > On Wed, Jul 20, 2016 at 08:28:47PM +0200, Jean-Francois Moine wrote: > > > The rate of the PLL-PERIPH clock is usually set to 1.2GHz in the A83T. > > > > Uh? The datasheet says to set it to 600MHz. > > Right. But the driver of the SDK for the Banana Pi M3 (A83T) sets it > to 1.2GHz. I tested again, and, with the pll-periph at 600MHz, both the SDcard and the eMMC are working fine. So, you may forget about this patch, sorry for the noise. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-08-02 8:37 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-21 6:28 [PATCH 0/3] mmc: sunxi: Changes in the host driver Jean-Francois Moine
[not found] ` <cover.1469082481.git.moinejf-GANU6spQydw@public.gmane.org>
2016-07-20 18:01 ` [PATCH 1/3] mmc: sunxi: Check the value returned by clk_round_rate Jean-Francois Moine
[not found] ` <c2231f3df66cf77e4fb9051f7b0c27ea69b8e63d.1469082481.git.moinejf-GANU6spQydw@public.gmane.org>
2016-07-21 8:49 ` Maxime Ripard
2016-07-20 18:16 ` [PATCH 2/3] mmc: sunxi: Set the 'New Timing' register for 8 bits DDR transfers Jean-Francois Moine
[not found] ` <bc7b767cad6db159a44c4b98d5b1f3eb53c26bd3.1469082481.git.moinejf-GANU6spQydw@public.gmane.org>
2016-07-21 8:56 ` Maxime Ripard
2016-07-21 9:26 ` Jean-Francois Moine
[not found] ` <20160721112655.941b1dad04f7a5b94d4172c1-GANU6spQydw@public.gmane.org>
2016-07-29 19:17 ` Maxime Ripard
2016-07-30 5:18 ` Jean-Francois Moine
2016-07-30 10:19 ` [linux-sunxi] " Hans de Goede
[not found] ` <9414aab0-ac2b-01c2-e16b-8f7394ea7b68-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-08-01 13:52 ` Jean-Francois Moine
[not found] ` <20160801155246.b7b3c7f0f582394b4c25f6dc-GANU6spQydw@public.gmane.org>
2016-08-02 8:37 ` Hans de Goede
2016-07-29 19:36 ` Maxime Ripard
2016-07-30 5:20 ` Jean-Francois Moine
2016-07-20 18:28 ` [PATCH 3/3] mmc: sunxi: Add support to the Allwinner A83T Jean-Francois Moine
2016-07-21 8:58 ` Maxime Ripard
2016-07-21 9:18 ` Jean-Francois Moine
[not found] ` <20160721111851.133a3f16d6e9de1ee45b3654-GANU6spQydw@public.gmane.org>
2016-07-22 19:00 ` Jean-Francois Moine
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).