From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dirk Behme Date: Fri, 3 May 2013 07:58:45 +0200 Subject: [U-Boot] [PATCH] spi: mxc_spi: Fix pre and post divider calculation In-Reply-To: <5182B2AA.9040606@boundarydevices.com> References: <1367492386-20464-1-git-send-email-dirk.behme@de.bosch.com> <5182B2AA.9040606@boundarydevices.com> Message-ID: <51835215.90602@de.bosch.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 02.05.2013 20:38, Troy Kisky wrote: > On 5/2/2013 3:59 AM, Dirk Behme wrote: >> From: Dirk Behme >> >> Fix two issues with the calculation of pre_div and post_div: >> >> 1. pre_div: While the calculation of pre_div looks correct, to set the >> CONREG[15-12] bits pre_div needs to be decremented by 1: >> >> The i.MX 6Dual/6Quad Applications Processor Reference Manual (IMX6DQRM >> Rev. 0, 11/2012) states: >> >> CONREG[15-12]: PRE_DIVIDER >> 0000 Divide by 1 >> 0001 Divide by 2 >> 0010 Divide by 3 >> ... >> 1101 Divide by 14 >> 1110 Divide by 15 >> 1111 Divide by 16 >> >> I.e. if we want to divide by 2, we have to write 1 to CONREG[15-12]. >> >> 2. In case the post divider becomes necessary, pre_div will be set to 15. To >> calculate the post divider, divide by 15, too. And not 16. >> >> Both issues above are tested using the following examples: >> >> clk_src = 60000000 (60MHz, default i.MX6 ECSPI clock) >> >> a) max_hz == 23000000 (23MHz, max i.MX6 ECSPI read clock) >> >> -> pre_div = 3 (divide by 3 => CONREG[15-12] == 2) >> -> post_div = 0 (divide by 1 => CONREG[11- 8] == 0) >> => 60MHz / 3 = 20MHz SPI clock >> >> b) max_hz == 2000000 (2MHz) >> >> -> pre_div = 15 (divide by 15 => CONREG[15-12] == 14) >> -> post_div = 1 (divide by 2 => CONREG[11- 8] == 1) >> => 60MHz / 30 = 2MHz SPI clock >> >> c) max_hz == 1000000 (1MHz) >> >> -> pre_div = 15 (divide by 15 => CONREG[15-12] == 14) >> -> post_div = 2 (divide by 4 => CONREG[11- 8] == 2) >> => 60MHz / 60 = 1MHz SPI clock >> >> d) max_hz == 500000 (500kHz) >> >> -> pre_div = 15 (divide by 15 => CONREG[15-12] == 14) >> -> post_div = 3 (divide by 8 => CONREG[11- 8] == 3) >> => 60MHz / 120 = 500kHz SPI clock >> >> Signed-off-by: Dirk Behme >> --- >> drivers/spi/mxc_spi.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c >> index 5bed858..8630bbd 100644 >> --- a/drivers/spi/mxc_spi.c >> +++ b/drivers/spi/mxc_spi.c >> @@ -128,7 +128,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, >> unsigned int max_hz, unsigned int mode) >> { >> u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); >> - s32 pre_div = 0, post_div = 0, i, reg_ctrl, reg_config; >> + s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; >> u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; >> struct cspi_regs *regs = (struct cspi_regs *)mxcs->base; >> >> @@ -153,7 +153,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, >> if (clk_src > max_hz) { >> pre_div = DIV_ROUND_UP(clk_src, max_hz); >> if (pre_div > 16) { >> - post_div = pre_div / 16; >> + post_div = pre_div / 15; > I think this should not change Hmm, why? You get wrong post_div dividers if you run with 'pre_div = 15' and '/16'. Hmm, reading your below comment do you want to say "I agree that both lines post_div = pre_div / 16; pre_div = 15; have to use the same value, but instead of using 15, use 16 " ? >> pre_div = 15; > and this should be = 16, because you now subtract 1 below Do you want to say you propose post_div = pre_div / 16; pre_div = 16; ? If so: First, I agree that we have to use the same dividers in both lines. But, second, this would mean that you use /16 as max pre_div. For the i.MX6 case where clk_src is 60MHz this would result in a pre-divided clock of 3.75Mhz (instead of 4MHz with /15). So using /15 or /16 is just a decision of which end clocks most probably are needed. If you want to be able to configure 4MHz, 2MHz, 1MHz, 500kHz etc then /15 is the better choice. If you want to be able to configure 3.75Mhz, 1.875MHz, 937.5kHz, 468.75kHz etc then /16 is the better choice. I vote for /15 as done by my patch. Best regards Dirk >> } >> if (post_div != 0) { >> @@ -174,7 +174,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, >> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_SELCHAN(3)) | >> MXC_CSPICTRL_SELCHAN(cs); >> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_PREDIV(0x0F)) | >> - MXC_CSPICTRL_PREDIV(pre_div); >> + MXC_CSPICTRL_PREDIV(pre_div - 1); >> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | >> MXC_CSPICTRL_POSTDIV(post_div); >> > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > -- ====================================================================== Dirk Behme Robert Bosch Car Multimedia GmbH CM-AI/PJ-CF32 Phone: +49 5121 49-3274 Dirk Behme Fax: +49 711 811 5053274 PO Box 77 77 77 mailto:dirk.behme at de.bosch.com D-31132 Hildesheim - Germany Bosch Group, Car Multimedia (CM) Automotive Navigation and Infotainment Systems (AI) ProJect - CoreFunctions (PJ-CF) Robert Bosch Car Multimedia GmbH - Ein Unternehmen der Bosch Gruppe Sitz: Hildesheim Registergericht: Amtsgericht Hildesheim HRB 201334 Aufsichtsratsvorsitzender: Volkmar Denner Gesch?ftsf?hrung: Uwe Thomas, Michael Bolle, Robby Drave, Egbert Hellwig ======================================================================