[U-Boot] [PATCH v7 1/6] spi: cadence_qspi: move trigger base configuration in init

[Jagan Teki]

Hi Wolfgang,

On 25 September 2015 at 00:26, Wolfgang Denk <wd at denx.de> wrote:
> Dear Vikas,
>
> In message <9026814FBF99304F9FA3AC3FB72F3E2F04BFF85EFE at 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.