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

[Wolfgang Denk]

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.

Thanks.

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
It seems intuitively obvious to me, which  means  that  it  might  be
wrong.                                                 -- Chris Torek