[linux-yocto] [PATCH 1/3] drivers/misc: Axxia MTC Driver Memory Initialization Check
Bruce Ashfield
bruce.ashfield at windriver.com
Mon Aug 29 05:50:41 PDT 2016
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.ashfield at windriver.com <mailto:bruce.ashfield at 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.Dragomir at windriver.com
> <mailto:Daniel.Dragomir at windriver.com>
> <mailto:Daniel.Dragomir at windriver.com
> <mailto:Daniel.Dragomir at windriver.com>>>
> wrote:
>
> Adding John Jacques, the proper person to respond on this.
>
> Regards,
> Daniel
> ________________________________________
> From: Winkler, Tomas [tomas.winkler at intel.com
> <mailto:tomas.winkler at intel.com>
> <mailto:tomas.winkler at intel.com
> <mailto:tomas.winkler at intel.com>>]
> Sent: Monday, August 22, 2016 12:52 PM
> To: Dragomir, Daniel; linux-yocto at yoctoproject.org
> <mailto:linux-yocto at yoctoproject.org>
> <mailto:linux-yocto at yoctoproject.org
> <mailto:linux-yocto at 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.joshi at intel.com
> <mailto:sreedevi.joshi at intel.com>
> <mailto:sreedevi.joshi at intel.com
> <mailto:sreedevi.joshi at 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 at yoctoproject.org
> <mailto:linux-yocto at yoctoproject.org>
> <mailto:linux-yocto at yoctoproject.org
> <mailto:linux-yocto at 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 at yoctoproject.org
> <mailto:linux-yocto at yoctoproject.org>
> <mailto:linux-yocto at yoctoproject.org
> <mailto:linux-yocto at 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>>
>
>
>
>
>
>
More information about the linux-yocto
mailing list