Bhindhiya, Why not follow the approach taken by other arch?
On Fri, Sep 18, 2020 at 9:34 PM Bhindhiya Raja <bhindhiy...@tataelxsi.co.in.invalid> wrote: > > Hello Team, > > We are trying to include Renesas RX65N architecture in GitHub pre-check which > requires code to have 0 warnings to be successful. > > Call to up_copystate() in the below functions produces warnings in NuttX > common files: > arch/renesas/src/common/up_blocktask.c: up_block_task() > arch/renesas/src/common/up_releasepending: up_release_pending() > arch/renesas/src/common/up_reprioritizertr: up_reprioritize_rtr() > arch/renesas/src/common/up_unblocktask: up_unblock_task() > > Changes in the above files will affect the other Renesas architectures (m16c > and sh1) as well. > > The function prototype of up_copystate() in file: > "arch/renesas/src/common/up_internal.h" is as follows: > void up_copystate(uint32_t *dest, uint32_t *src); > > g_current_regs is also defined in the same file as: > extern volatile uint32_t *g_current_regs; > > Calling up_copystate() with g_current_regs as 2nd argument gives the > following warning: > passing argument 2 of ‘up_copystate’ discards ‘volatile’ qualifier from > pointer target type [-Werror] > up_copystate(rtcb->xcp.regs, g_current_regs); > ^ > > The above call causes the 'volatile' nature of g_current_regs to be lost. > We understand that casting away the 'volatile' type qualifier of a variable > should be avoided if possible. > > We intend to do a PR by modifying the prototype of the above function to: > void up_copystate(uint32_t *dest, volatile uint32_t *src); > This modification produces the same warning in memcpy() function call. > Typecasting the argument when calling memcpy() alone will resolve this > warning. > > This retains the volatile nature of the variable as well as resolves the > warnings produced > due to the above function call with volatile g_current_regs argument. > > Are there any concerns with the above mentioned modifications? > > Regards, > Bhindhiya Raja. > ________________________________ > From: Bhindhiya Raja <bhindhiy...@tataelxsi.co.in> > Sent: Monday, September 14, 2020 5:09 PM > To: dev@nuttx.apache.org <dev@nuttx.apache.org> > Subject: Re: Warnings in Renesas common files > > Hello Greg and Nathan, > > Any input on the warnings generated due to the g_current_regs volatile type > qualifier? > > Best Regards, > > Bhindhiya Raja > > > SBU | Engineer > > TATA ELXSI LIMITED > > ITPB Road Whitefield > > Bengaluru 560 048 India > > www.tataelxsi.com<https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.tataelxsi.com%2F&data=02%7C01%7Cbhindhiya.r%40tataelxsi.co.in%7C27f3ef9461c64949d6e508d832f8ecbc%7Cad6a39dd96b6436882daf2ec4d92e26a%7C0%7C0%7C637315393618704041&sdata=FdDPWsO9WgR7HscuY%2BLQkX1oTtr%2BuNpCgEl35LtrZZ8%3D&reserved=0> > > ________________________________ > From: Nathan Hartman <hartman.nat...@gmail.com> > Sent: Saturday, September 5, 2020 3:51 AM > To: dev@nuttx.apache.org <dev@nuttx.apache.org> > Subject: Re: Warnings in Renesas common files > > ________________________________ > **This is an external email. Please check the sender’s full email address > (not just the sender name) and exercise caution before you respond or click > any embedded link/attachment.** > ________________________________ > > On Fri, Sep 4, 2020 at 9:40 AM Bhindhiya Raja > <bhindhiy...@tataelxsi.co.in.invalid> wrote: > > > > Hello Nathan, > > > > Thank you for your response. We have submitted 2 separate PRs as > > recommended by you to resolve warnings 6 (#1695) and 8 (#1702). > > > > About warnings 1 to 4, we understand that casting away the 'volatile' type > > qualifier of a variable should be avoided if possible. Can we consider > > modifying the prototype of the function up_copystate() in up_internal.h > > file, > > from: > > void up_copystate(uint32_t *dest, uint32_t *src); > > to: > > void up_copystate(uint32_t *dest, volatile uint32_t *src); > > in order to resolve this warning. > > > > We have tried the above fix in our local repository and checked build. > > We found that warnings 1 to 4 are resolved, however, it generates a new > > warning in file: > > arch/renesas/src/rx65n/rx65n_copystate.c: passing argument 2 of > > ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type > > memcpy(dest, src, XCPTCONTEXT_SIZE); > > > > expected ‘const void *’ but argument is of type ‘volatile uint32_t *’ > > FAR void *memcpy(FAR void *dest, FAR const void *src, size_t n); > > > > This can be resolved by typecasting the variables when calling memcpy() > > alone. > > > > As these files are common to other Renesas architectures as well, please > > confirm if the above modifications are okay. > > > Hello Bhindhiya, > > I am not sure what is the best choice here. I looked through NuttX > sources and I did not see any up_copystate() (for any arch) that had > volatile qualifiers. That said, looking at the call to up_copystate() > in up_unblock_task(), g_current_regs is indeed volatile (for good > reasons), so I wonder if we should propagate that as far as possible, > until the memcpy(), like you're suggesting, or, alternatively, if it > is safe to discard volatile like is being done already, because we are > switching tasks. > > I need to think about this some more. > > Maybe Greg will share some of his wisdom with us. :-) > > Nathan > ________________________________ > Disclaimer: This email and any files transmitted with it are confidential and > intended solely for the use of the individual or entity to whom they are > addressed. If you are not the intended recipient of this message , or if this > message has been addressed to you in error, please immediately alert the > sender by reply email and then delete this message and any attachments. If > you are not the intended recipient, you are hereby notified that any use, > dissemination, copying, or storage of this message or its attachments is > strictly prohibited. Email transmission cannot be guaranteed to be secure or > error-free, as information could be intercepted, corrupted, lost, destroyed, > arrive late or incomplete, or contain viruses. The sender, therefore, does > not accept liability for any errors, omissions or contaminations in the > contents of this message which might have occurred as a result of email > transmission. If verification is required, please request for a hard-copy > version. > ________________________________
