* [U-Boot] Checkpatch warnings for "volatile" @ 2011-10-12 10:35 Prabhakar Lad 2011-10-14 9:16 ` Prabhakar Lad 2011-10-14 13:12 ` Jason 0 siblings, 2 replies; 10+ messages in thread From: Prabhakar Lad @ 2011-10-12 10:35 UTC (permalink / raw) To: u-boot Hi Wolfgang, The checkpatch complains for volatile keyword, if "volatile" is necessary and cannot be removed, Is it necessary to inform in the cover letter or the patch itself stating that these warnings should be neglected ? Regards --Prabhakar Lad ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] Checkpatch warnings for "volatile" 2011-10-12 10:35 [U-Boot] Checkpatch warnings for "volatile" Prabhakar Lad @ 2011-10-14 9:16 ` Prabhakar Lad 2011-10-14 13:12 ` Jason 1 sibling, 0 replies; 10+ messages in thread From: Prabhakar Lad @ 2011-10-14 9:16 UTC (permalink / raw) To: u-boot any one? Regards --Prabhakar Lad On Wed, Oct 12, 2011 at 4:05 PM, Prabhakar Lad <prabhakar.csengg@gmail.com>wrote: > Hi Wolfgang, > > The checkpatch complains for volatile keyword, if "volatile" is necessary > and cannot be removed, > Is it necessary to inform in the cover letter or the patch itself stating > that these warnings should be neglected ? > > Regards > --Prabhakar Lad ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] Checkpatch warnings for "volatile" 2011-10-12 10:35 [U-Boot] Checkpatch warnings for "volatile" Prabhakar Lad 2011-10-14 9:16 ` Prabhakar Lad @ 2011-10-14 13:12 ` Jason 2011-10-14 19:58 ` Wolfgang Denk 1 sibling, 1 reply; 10+ messages in thread From: Jason @ 2011-10-14 13:12 UTC (permalink / raw) To: u-boot Prabhakar, On Wed, Oct 12, 2011 at 04:05:57PM +0530, Prabhakar Lad wrote: > The checkpatch complains for volatile keyword, Just to make sure [1], since there is a lot of misunderstanding regarding volatile. > if "volatile" is necessary and cannot be removed, Please see [2], people are more inclined to help if you provide the code and the warning. Stating what research you've already done is helpful as well. > Is it necessary to inform in the cover letter or the patch itself > stating that these warnings should be neglected ? Assuming you've read [1] and [2] and it is indeed a valid use of volatile, I would put it in the comment area of the patch email, eg: --- Notes: 1.) checkpatch.pl complained about volatile in source.c:312. This is a register we bitbang in a loop and we don't want the compiler optimizing it out. diff... hth, Jason. [1] http://www.mjmwired.net/kernel/Documentation/volatile-considered-harmful.txt [2] http://catb.org/~esr/faqs/smart-questions.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] Checkpatch warnings for "volatile" 2011-10-14 13:12 ` Jason @ 2011-10-14 19:58 ` Wolfgang Denk 2011-10-14 20:22 ` Jason 0 siblings, 1 reply; 10+ messages in thread From: Wolfgang Denk @ 2011-10-14 19:58 UTC (permalink / raw) To: u-boot Dear Jason, In message <20111014131245.GF7102@titan.lakedaemon.net> you wrote: > > 1.) checkpatch.pl complained about volatile in source.c:312. This is > a register we bitbang in a loop and we don't want the compiler > optimizing it out. This would, in almost all cases, trigger a NAK due to the fact that device register accesses should be done through I/O accesors, and never through volatile pointers. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de A quarrel is quickly settled when deserted by one party; there is no battle unless there be two. - Seneca ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] Checkpatch warnings for "volatile" 2011-10-14 19:58 ` Wolfgang Denk @ 2011-10-14 20:22 ` Jason 2011-10-14 21:24 ` Wolfgang Denk 0 siblings, 1 reply; 10+ messages in thread From: Jason @ 2011-10-14 20:22 UTC (permalink / raw) To: u-boot Wolfgang, On Fri, Oct 14, 2011 at 09:58:44PM +0200, Wolfgang Denk wrote: > Dear Jason, > > In message <20111014131245.GF7102@titan.lakedaemon.net> you wrote: > > > > 1.) checkpatch.pl complained about volatile in source.c:312. This is > > a register we bitbang in a loop and we don't want the compiler > > optimizing it out. > > This would, in almost all cases, trigger a NAK due to the fact that > device register accesses should be done through I/O accesors, and > never through volatile pointers. very true, I was attempting to give a generic example. Sorry if that led to any confusion. thx, Jason. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] Checkpatch warnings for "volatile" 2011-10-14 20:22 ` Jason @ 2011-10-14 21:24 ` Wolfgang Denk 2011-10-15 6:01 ` Prabhakar Lad 0 siblings, 1 reply; 10+ messages in thread From: Wolfgang Denk @ 2011-10-14 21:24 UTC (permalink / raw) To: u-boot Dear Jason, In message <20111014202224.GI7102@titan.lakedaemon.net> you wrote: > > > > 1.) checkpatch.pl complained about volatile in source.c:312. This is > > > a register we bitbang in a loop and we don't want the compiler > > > optimizing it out. > > > > This would, in almost all cases, trigger a NAK due to the fact that > > device register accesses should be done through I/O accesors, and > > never through volatile pointers. > > very true, I was attempting to give a generic example. Sorry if that > led to any confusion. I've explained this a number of times recently - there are actually very, very few occasions where "volatile" actually makes sense. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Conceptual integrity in turn dictates that the design must proceed from one mind, or from a very small number of agreeing resonant minds. - Frederick Brooks Jr., "The Mythical Man Month" ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] Checkpatch warnings for "volatile" 2011-10-14 21:24 ` Wolfgang Denk @ 2011-10-15 6:01 ` Prabhakar Lad 2011-10-15 8:56 ` Wolfgang Denk 0 siblings, 1 reply; 10+ messages in thread From: Prabhakar Lad @ 2011-10-15 6:01 UTC (permalink / raw) To: u-boot Hi Wolfgang, On Sat, Oct 15, 2011 at 2:54 AM, Wolfgang Denk <wd@denx.de> wrote: > Dear Jason, > > In message <20111014202224.GI7102@titan.lakedaemon.net> you wrote: > > > > > > 1.) checkpatch.pl complained about volatile in source.c:312. This > is > > > > a register we bitbang in a loop and we don't want the compiler > > > > optimizing it out. > > > > > > This would, in almost all cases, trigger a NAK due to the fact that > > > device register accesses should be done through I/O accesors, and > > > never through volatile pointers. > > > > very true, I was attempting to give a generic example. Sorry if that > > led to any confusion. > > I've explained this a number of times recently - there are actually > very, very few occasions where "volatile" actually makes sense. > > Agreed, but I see a piece of code where virtual address are compared. For example in arch/arm/cpu/arm926ejs/davinci/cpu.c In this function static inline unsigned pll_prediv(unsigned pllbase) and also in this static inline unsigned pll_postdiv(unsigned pllbase) Any suggestion on this on how to tackle or let it remain stagnant? Regards -- Prabhakar Lad > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de > Conceptual integrity in turn dictates that the design must proceed > from one mind, or from a very small number of agreeing resonant > minds. - Frederick Brooks Jr., "The Mythical Man Month" > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] Checkpatch warnings for "volatile" 2011-10-15 6:01 ` Prabhakar Lad @ 2011-10-15 8:56 ` Wolfgang Denk 2011-10-15 14:57 ` Prabhakar Lad 2011-10-15 18:11 ` Marek Vasut 0 siblings, 2 replies; 10+ messages in thread From: Wolfgang Denk @ 2011-10-15 8:56 UTC (permalink / raw) To: u-boot Dear Prabhakar Lad, In message <CA+V-a8sYRZJDZojEpQ55ZGRZ6--Niq0ThKVV8e_RtQrRuShE8A@mail.gmail.com> you wrote: > > > I've explained this a number of times recently - there are actually > > very, very few occasions where "volatile" actually makes sense. > > > > Agreed, but I see a piece of code where virtual address are compared. > For example in arch/arm/cpu/arm926ejs/davinci/cpu.c > In this function static inline unsigned pll_prediv(unsigned pllbase) > and > also in this static inline unsigned pll_postdiv(unsigned pllbase) > > Any suggestion on this on how to tackle or let it remain stagnant? I cannot see a justification for any of the ""volatile" in this file. Of course, all these ugly REG() calls should be converted to proper use of I/O accessors. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Hindsight is an exact science. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] Checkpatch warnings for "volatile" 2011-10-15 8:56 ` Wolfgang Denk @ 2011-10-15 14:57 ` Prabhakar Lad 2011-10-15 18:11 ` Marek Vasut 1 sibling, 0 replies; 10+ messages in thread From: Prabhakar Lad @ 2011-10-15 14:57 UTC (permalink / raw) To: u-boot Hi Wolfgang On 10/15/11, Wolfgang Denk <wd@denx.de> wrote: > Dear Prabhakar Lad, > > In message > <CA+V-a8sYRZJDZojEpQ55ZGRZ6--Niq0ThKVV8e_RtQrRuShE8A@mail.gmail.com> you > wrote: >> >> > I've explained this a number of times recently - there are actually >> > very, very few occasions where "volatile" actually makes sense. >> > >> > Agreed, but I see a piece of code where virtual address are >> > compared. >> For example in arch/arm/cpu/arm926ejs/davinci/cpu.c >> In this function static inline unsigned pll_prediv(unsigned pllbase) >> and >> also in this static inline unsigned pll_postdiv(unsigned pllbase) >> >> Any suggestion on this on how to tackle or let it remain stagnant? > > I cannot see a justification for any of the ""volatile" in this file. > > Of course, all these ugly REG() calls should be converted to proper > use of I/O accessors. > Thanks for the reply. Regards --Prabhakar Lad > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de > Hindsight is an exact science. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] Checkpatch warnings for "volatile" 2011-10-15 8:56 ` Wolfgang Denk 2011-10-15 14:57 ` Prabhakar Lad @ 2011-10-15 18:11 ` Marek Vasut 1 sibling, 0 replies; 10+ messages in thread From: Marek Vasut @ 2011-10-15 18:11 UTC (permalink / raw) To: u-boot On Saturday, October 15, 2011 10:56:54 AM Wolfgang Denk wrote: > Dear Prabhakar Lad, > > In message <CA+V-a8sYRZJDZojEpQ55ZGRZ6-- Niq0ThKVV8e_RtQrRuShE8A@mail.gmail.com> you wrote: > > > I've explained this a number of times recently - there are actually > > > very, very few occasions where "volatile" actually makes sense. > > > > > > Agreed, but I see a piece of code where virtual address are > > > compared. > > > > For example in arch/arm/cpu/arm926ejs/davinci/cpu.c > > In this function static inline unsigned pll_prediv(unsigned pllbase) > > > > and > > > > also in this static inline unsigned pll_postdiv(unsigned pllbase) > > > > Any suggestion on this on how to tackle or let it remain stagnant? > > I cannot see a justification for any of the ""volatile" in this file. > > Of course, all these ugly REG() calls should be converted to proper > use of I/O accessors. Definitelly ... but I'm not swiping this one, I have enough mess on my hands already ;-) Cheers ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-10-15 18:11 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-10-12 10:35 [U-Boot] Checkpatch warnings for "volatile" Prabhakar Lad 2011-10-14 9:16 ` Prabhakar Lad 2011-10-14 13:12 ` Jason 2011-10-14 19:58 ` Wolfgang Denk 2011-10-14 20:22 ` Jason 2011-10-14 21:24 ` Wolfgang Denk 2011-10-15 6:01 ` Prabhakar Lad 2011-10-15 8:56 ` Wolfgang Denk 2011-10-15 14:57 ` Prabhakar Lad 2011-10-15 18:11 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox