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

[Vikas MANOCHA]

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.

Cheers,
Vikas

> 
> Best regards,
> Marek Vasut