On 2016-08-26 07:12 PM, John Jacques wrote:
I have a proposed patch, but will get the original developer to run her
unit test before I submit. As for the infinite loop, that's handled, I
think, by the call to time_before(). Is that not the case? The updated
Yep, that is the case. From your description of the plan, I thought
you were doing to replace time_before() with only the udelay in the
main loop.
section will be as follows. The loop will only run for, at most, 1000
msecs. I'll see if we can reduce that, as it seems MUCH too long if the
expectation from the hardware group is that it should take a few usecs.
Sounds good.
Bruce
...
tmo = jiffies + MTC_POLL_TIMEOUT;
do {
udelay(1);
init_reg = readl(&(dev->regs->mem_init));
if ((init_reg & 0x101) == 0x101)
break;
} while (time_before(jiffies, tmo));
if ((init_reg & 0x101) != 0x101) {
pr_debug("warning: mem_init failed value=0x%x
(expected:0x101)\n\
",
init_reg);
}
...
On Thu, Aug 25, 2016 at 9:02 AM, Bruce Ashfield
<bruce.ashfi...@windriver.com <mailto:bruce.ashfi...@windriver.com>> wrote:
On 2016-08-24 06:31 PM, John Jacques wrote:
Bruce,
I agree with you on both. I spoke to Sreedevi and there's isn't a
reason she didn't use readl(). As for the delay, we expect
about 2 us.
So, I assume udelay(1) would be appropriate? If both are okay
(readl()
instead of direct access and udelay(1)), I'll update the commit.
Don't thank me for this one, it was one of our reviewers on the list
.. but the sentiment is the same :) I'm glad that we can scan patches
and some some useful feedback.
But to the technical part of the question, I'm assuming that this
does in fact need to be a busy loop, so udelay will be a more
accurate way to delay and wait for the value.
But without seeing a patch, it isn't clear to me how the switch to
udelay won't cause a potential infinite loop if the code stays the
same. Are you thinking that a single delay of 2usecs and then a
register read will get the value that you need ?
To keep my merge queue clean, I'll wait for a new revision of the
patch, and a re-send of the entire series before queuing it.
Bruce
Thanks for looking at this!
On Tue, Aug 23, 2016 at 10:08 AM, Dragomir, Daniel
<daniel.drago...@windriver.com
<mailto:daniel.drago...@windriver.com>
<mailto:daniel.drago...@windriver.com
<mailto:daniel.drago...@windriver.com>>>
wrote:
Adding John Jacques, the proper person to respond on this.
Regards,
Daniel
________________________________________
From: Winkler, Tomas [tomas.wink...@intel.com
<mailto:tomas.wink...@intel.com>
<mailto:tomas.wink...@intel.com
<mailto:tomas.wink...@intel.com>>]
Sent: Monday, August 22, 2016 12:52 PM
To: Dragomir, Daniel; linux-yocto@yoctoproject.org
<mailto:linux-yocto@yoctoproject.org>
<mailto:linux-yocto@yoctoproject.org
<mailto:linux-yocto@yoctoproject.org>>
Subject: RE: [linux-yocto] [PATCH 1/3] drivers/misc: Axxia MTC
Driver Memory Initialization Check
>
> Axxia MTC driver changes:
> - Memory initialization completion check added
> - ECC error status clearing added
>
> Signed-off-by: Sreedevi Joshi <sreedevi.jo...@intel.com
<mailto:sreedevi.jo...@intel.com>
<mailto:sreedevi.jo...@intel.com
<mailto:sreedevi.jo...@intel.com>>>
> ---
> drivers/misc/lsi-mtc.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/misc/lsi-mtc.c
b/drivers/misc/lsi-mtc.c index
> 55c3403..f4fbe6f 100644
> --- a/drivers/misc/lsi-mtc.c
> +++ b/drivers/misc/lsi-mtc.c
> @@ -31,6 +31,7 @@
> #include <linux/string.h>
> #include "linux/lsi_mtc_ioctl.h"
>
> +#define MTC_POLL_TIMEOUT (msecs_to_jiffies(1000))
>
> /*
> device tree node:
> @@ -4114,6 +4115,8 @@ static long _mtc_config(struct
mtc_device *dev,
> struct lsi_mtc_cfg_t *pMTCCfg)
> struct ncp_axis_mtc_MTC_CONFIG0_REG_ADDR_r_t cfg0 =
{ 0 };
> struct ncp_axis_mtc_MTC_CONFIG1_REG_ADDR_r_t cfg1 =
{ 0 };
> struct ncp_axis_mtc_MTC_EXECUTE1_REG_ADDR_r_t exec1
= { 0 };
> + u32 init_reg = { 0 };
> + unsigned long tmo = 0;
>
> if ((!pMTCCfg) || (!dev))
> return -EINVAL;
> @@ -4129,6 +4132,21 @@ static long _mtc_config(struct
mtc_device *dev,
> struct lsi_mtc_cfg_t *pMTCCfg)
> exec1.sw_reset = 1;
> dev->regs->execute = *((u32 *) &exec1);
> dev->regs->mem_init = 0x202;
> + /* wait for the init to complete */
> + tmo = jiffies + MTC_POLL_TIMEOUT;
> + do {
> + init_reg = *(&(dev->regs->mem_init));+
I'm not familiar with this code but I don't think this is
the way to
read a register , why not using readl(), you are at least
missing
memory barrier here.
> + if ((init_reg & 0x101) == 0x101)
> + break;
> + } while (time_before(jiffies, tmo));
This is busy loop, how fast is that going to settle ?
> +
> + if ((init_reg & 0x101) != 0x101) {
> + pr_debug("warning: mem_init failed value=0x%x
> (expected:0x101)\n",
> + init_reg);
> + }
> +
> + /* clear ECC interrupt status */
> + dev->regs->ecc_int_status = 0xF;
>
> /* 3. config MTC */
> cfg0 =
> --
> 2.7.4
>
> --
> _______________________________________________
> linux-yocto mailing list
> linux-yocto@yoctoproject.org
<mailto:linux-yocto@yoctoproject.org>
<mailto:linux-yocto@yoctoproject.org
<mailto:linux-yocto@yoctoproject.org>>
> https://lists.yoctoproject.org/listinfo/linux-yocto
<https://lists.yoctoproject.org/listinfo/linux-yocto>
<https://lists.yoctoproject.org/listinfo/linux-yocto
<https://lists.yoctoproject.org/listinfo/linux-yocto>>
--
_______________________________________________
linux-yocto mailing list
linux-yocto@yoctoproject.org
<mailto:linux-yocto@yoctoproject.org>
<mailto:linux-yocto@yoctoproject.org
<mailto:linux-yocto@yoctoproject.org>>
https://lists.yoctoproject.org/listinfo/linux-yocto
<https://lists.yoctoproject.org/listinfo/linux-yocto>
<https://lists.yoctoproject.org/listinfo/linux-yocto
<https://lists.yoctoproject.org/listinfo/linux-yocto>>
--
_______________________________________________
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto
