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

[Vikas MANOCHA]

Hi Marek,

> -----Original Message-----
> From: Vikas MANOCHA
> Sent: Thursday, August 27, 2015 9:03 AM
> To: Marek Vasut
> Cc: u-boot at lists.denx.de; sr at denx.de; grmoore at opensource.altera.com;
> jteki at openedev.com
> Subject: RE: [PATCH v5 1/5] spi: cadence_qspi: move trigger base
> configuration in init
> 
> Hi,
> 
> > -----Original Message-----
> > From: Marek Vasut [mailto:marex at denx.de]
> > Sent: Thursday, August 27, 2015 8:52 AM
> > To: Vikas MANOCHA
> > Cc: u-boot at lists.denx.de; sr at denx.de; grmoore at opensource.altera.com;
> > jteki at openedev.com
> > Subject: Re: [PATCH v5 1/5] spi: cadence_qspi: move trigger base
> > configuration in init
> >
> > On Thursday, August 27, 2015 at 05:46:12 PM, Vikas MANOCHA wrote:
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: Marek Vasut [mailto:marex at denx.de]
> > > > Sent: Thursday, August 27, 2015 1:36 AM
> > > > To: Vikas MANOCHA
> > > > Cc: u-boot at lists.denx.de; sr at denx.de;
> > grmoore at opensource.altera.com;
> > > > jteki at openedev.com
> > > > Subject: Re: [PATCH v5 1/5] spi: cadence_qspi: move trigger base
> > > > configuration in init
> > > >
> > > > On Thursday, August 27, 2015 at 12:44:26 AM, Vikas Manocha wrote:
> > > > > No need to configure indirect trigger address for every read/write.
> > > > >
> > > > > Signed-off-by: Vikas Manocha <vikas.manocha at st.com>
> > > > > ---
> > > > >
> > > > > Changes in v5: fixed type cast compilation warnings.
> > > > > Changes in v4: removed extra type casts.
> > > > > Changes in v3: added commit message & removed extra bracket.
> > > > > Changes in v2: Rebased to master
> > > > >
> > > > >  drivers/spi/cadence_qspi_apb.c |    9 ++-------
> > > > >  1 file changed, 2 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/spi/cadence_qspi_apb.c
> > > > > b/drivers/spi/cadence_qspi_apb.c index d053407..d377ad1 100644
> > > > > --- a/drivers/spi/cadence_qspi_apb.c
> > > > > +++ b/drivers/spi/cadence_qspi_apb.c
> > > > > @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
> > > > > cadence_spi_platdata *plat)
> > > > >
> > > > >  	/* Indirect mode configurations */
> > > > >  	writel((plat->sram_size/2), plat->regbase +
> > > > >
> > > > > CQSPI_REG_SRAMPARTITION);
> > > > > +	writel((u32)plat->ahbbase &
> > > >
> > > > CQSPI_INDIRECTTRIGGER_ADDR_MASK,
> > > >
> > > > Why can't you just drop this masking and the cast ?
> > >
> > > Masking is dropped in another patch in same series.
> >
> > So you're adding broken code first only to fix it later ? I don't like this.
> 
> Try to understand the code, if you drop masking here, that will break the
> code.
> 
> >
> > > Casting is required as ahbbase is a pointer.
> >
> > ahbbase is used as u32 value all around the place, so just fix it to
> > u32 and drop the cast.
> 
> Check ahbbase, it is declared as pointer. This patch is not to fix ahbbase type.

Let me know if you have any more comments for this v5.

Cheers,
Vikas

> 
> Cheers,
> Vikas
> 
> >
> > Best regards,
> > Marek Vasut