Hi Wolfgang, On 25 September 2015 at 00:26, Wolfgang Denk <w...@denx.de> wrote: > Dear Vikas, > > In message <9026814fbf99304f9fa3ac3fb72f3e2f04bff85...@safex1mail4.st.com> > you wrote: >> >> > CQSPI_INDIRECTTRIGGER_ADDR_MASK, >> > > + plat->regbase + CQSPI_REG_INDIRECTTRIGGER); >> > >> > I did not mention this explicitly, so I do it here: >> > >> > Please fix this type cast issue globally, in all your patches. >> >> I agree it should be done but this patchset is not introducing the >> typecasting, it only moves the statement to another logical location. >> e.g. the above code is not new, it was just moved from other location >> to init function. > > I am fully aware of that. But I feel if we touch a piece of code, and > notice it has issues, we should fix these while we are at it. > Otherwise there is always the risk that we add more and more such bad > code, and/or forget about cleaning up later. > >> This fix to remove typecasting from all variables (triggerbase, >> flashbase,regbase) is a significant change in many routines in terms >> of parameters passing/handling & deserve separate patch/set. > > Hm... this makes me wander about the overall code quality. I have to > admit that I don't have any in-depth understanding of this specific > driver, but it looks.... well, let's state it looks pretty much > dfferent from the corresponding Linux kernel driver code. Which does > not have such issues. So if you say it would take _significant_ > efforts to clean up this implementation I'm asking myuself how much > more (or less?) effort would it take to adapt the Linux driver for > U-Boot instead? Having a common driver code base has been very > beneficial in a number of other areas, too... > >> I am ready to send a separate patch/set for the same later. Please >> let me know if you agree. > > If we follow strict rules, such a cleanup patch should go in first.
Looks like driver code itself has some issues and Vikas made changes wrt to existing code, cleaning up existing driver and then make Vikas changes might be reasonable and time consuming task. What if we move Vikas changes now for this release as he stated in previous mail "about sending patches later". My idea is someone is trying to clean it up existing code then give him a chance to move his code as well because he sent couple times. thanks! -- Jagan. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
