From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <504FF362.4070609@freescale.com> Date: Wed, 12 Sep 2012 10:28:50 +0800 From: Huang Shijie MIME-Version: 1.0 To: Vikram Narayanan Subject: Re: [PATCH 6/9] mtd: gpmi: simplify the setting DLL code References: <1347344231-10295-1-git-send-email-b32955@freescale.com> <1347344231-10295-7-git-send-email-b32955@freescale.com> <504F7E10.8030406@gmail.com> In-Reply-To: <504F7E10.8030406@gmail.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable Cc: dirk.behme@de.bosch.com, dedekind1@gmail.com, linux-mtd@lists.infradead.org, ffainelli@freebox.fr, shawn.guo@linaro.org, dwmw2@infradead.org, linux-arm-kernel@lists.infradead.org List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , =E4=BA=8E 2012=E5=B9=B409=E6=9C=8812=E6=97=A5 02:08, Vikram Narayanan =E5= =86=99=E9=81=93: > Hello Huang Shijie, > > On 9/11/2012 11:47 AM, Huang Shijie wrote: >> The setting DLL code is a little mess. >> Just simplify the code and the comments. >> >> Signed-off-by: Huang Shijie >> --- >> drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 22 +++++++++------------- >> 1 files changed, 9 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c=20 >> b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c >> index 037438a..83c5573 100644 >> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c >> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c >> @@ -779,30 +779,26 @@ void gpmi_begin(struct gpmi_nand_data *this) >> writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR); >> >> /* Clear out the DLL control fields. */ >> - writel(BM_GPMI_CTRL1_RDN_DELAY, gpmi_regs + HW_GPMI_CTRL1_CLR); >> - writel(BM_GPMI_CTRL1_HALF_PERIOD, gpmi_regs + HW_GPMI_CTRL1_CLR); >> + reg =3D BM_GPMI_CTRL1_RDN_DELAY | BM_GPMI_CTRL1_HALF_PERIOD; >> + writel(reg, gpmi_regs + HW_GPMI_CTRL1_CLR); >> >> /* If no sample delay is called for, return immediately. */ >> if (!hw.sample_delay_factor) >> return; >> >> - /* Configure the HALF_PERIOD flag. */ >> - if (hw.use_half_periods) >> - writel(BM_GPMI_CTRL1_HALF_PERIOD, >> - gpmi_regs + HW_GPMI_CTRL1_SET); >> + /* Set RDN_DELAY or HALF_PERIOD. */ >> + reg =3D ((hw.use_half_periods) ? BM_GPMI_CTRL1_HALF_PERIOD : 0) >> + | BF_GPMI_CTRL1_RDN_DELAY(hw.sample_delay_factor); >> >> - /* Set the delay factor. */ >> - writel(BF_GPMI_CTRL1_RDN_DELAY(hw.sample_delay_factor), >> - gpmi_regs + HW_GPMI_CTRL1_SET); >> + writel(reg, gpmi_regs + HW_GPMI_CTRL1_SET); >> >> - /* Enable the DLL. */ >> + /* At last, we enable the DLL. */ >> writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_SET); >> >> /* >> * After we enable the GPMI DLL, we have to wait 64 clock cycles before >> - * we can use the GPMI. >> - * >> - * Calculate the amount of time we need to wait, in microseconds. >> + * we can use the GPMI. Calculate the amount of time we need to wait, >> + * in microseconds. >> */ >> clock_period_in_ns =3D 1000000000 / clk_get_rate(r->clock[0]); > > NSEC_PER_SEC macro in the above statement? thanks. I like this macro. > > Don't curse me for commenting about the code that you've not written.=20 > As this patch does some cleanups, I'm suggesting this. > sorry, I do not understand it very well. could you tell me in detail what's the "commenting about the code that=20 you've not written" meaning? Which comments do you think is not proper? Best Regards Huang Shijie > >> dll_wait_time_in_us =3D (clock_period_in_ns * 64) / 1000; > > Regards, > Vikram >