Re: [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
On 8 September 2015 at 06:47, Chin Liang Seewrote: > Ensuring spi_calibration is run when there is a change of sclk > frequency. This will ensure the qspi flash access works for high > sclk frequency > > Signed-off-by: Chin Liang See > Cc: Dinh Nguyen > Cc: Dinh Nguyen > Cc: Marek Vasut > Cc: Stefan Roese > Cc: Vikas Manocha > Cc: Jagannadh Teki > Cc: Pavel Machek > --- > Changes for v2 > - remove frequency set before calibration > - introducing previous_hz to store requested frequency > - prevent calibration run when request same frequency > --- Reviewed-by: Jagan Teki thanks! -- Jagan | openedev. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
On Tuesday, September 08, 2015 at 02:25:54 PM, Jagan Teki wrote: > On 8 September 2015 at 16:38, Marek Vasutwrote: > > On Tuesday, September 08, 2015 at 03:17:49 AM, Chin Liang See wrote: > >> Ensuring spi_calibration is run when there is a change of sclk > >> frequency. This will ensure the qspi flash access works for high > >> sclk frequency > >> > >> Signed-off-by: Chin Liang See > >> Cc: Dinh Nguyen > >> Cc: Dinh Nguyen > >> Cc: Marek Vasut > >> Cc: Stefan Roese > >> Cc: Vikas Manocha > >> Cc: Jagannadh Teki > >> Cc: Pavel Machek > > > > Acked-by: Marek Vasut > > > > Jagan, please apply for -next or I'll pick it up. Which do you prefer ? > > You can pick, no issues. once 2/4, 3/4 ready. OK, will do. Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
On Tuesday, September 08, 2015 at 03:17:49 AM, Chin Liang See wrote: > Ensuring spi_calibration is run when there is a change of sclk > frequency. This will ensure the qspi flash access works for high > sclk frequency > > Signed-off-by: Chin Liang See> Cc: Dinh Nguyen > Cc: Dinh Nguyen > Cc: Marek Vasut > Cc: Stefan Roese > Cc: Vikas Manocha > Cc: Jagannadh Teki > Cc: Pavel Machek Acked-by: Marek Vasut Jagan, please apply for -next or I'll pick it up. Which do you prefer ? [...] > diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h > index 98e57aa..2912e36 100644 > --- a/drivers/spi/cadence_qspi.h > +++ b/drivers/spi/cadence_qspi.h > @@ -38,6 +38,7 @@ struct cadence_spi_priv { > int qspi_is_init; > unsigned intqspi_calibrated_hz; > unsigned intqspi_calibrated_cs; Idea for future patch: we should get rid of these qspi_ prefixes, they are really useless. > + unsigned intprevious_hz; > }; > > /* Functions call declaration */ Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
On 8 September 2015 at 16:38, Marek Vasutwrote: > On Tuesday, September 08, 2015 at 03:17:49 AM, Chin Liang See wrote: >> Ensuring spi_calibration is run when there is a change of sclk >> frequency. This will ensure the qspi flash access works for high >> sclk frequency >> >> Signed-off-by: Chin Liang See >> Cc: Dinh Nguyen >> Cc: Dinh Nguyen >> Cc: Marek Vasut >> Cc: Stefan Roese >> Cc: Vikas Manocha >> Cc: Jagannadh Teki >> Cc: Pavel Machek > > Acked-by: Marek Vasut > > Jagan, please apply for -next or I'll pick it up. Which do you prefer ? You can pick, no issues. once 2/4, 3/4 ready. > > [...] > >> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h >> index 98e57aa..2912e36 100644 >> --- a/drivers/spi/cadence_qspi.h >> +++ b/drivers/spi/cadence_qspi.h >> @@ -38,6 +38,7 @@ struct cadence_spi_priv { >> int qspi_is_init; >> unsigned intqspi_calibrated_hz; >> unsigned intqspi_calibrated_cs; > > Idea for future patch: we should get rid of these qspi_ prefixes, they are > really useless. > >> + unsigned intprevious_hz; >> }; >> >> /* Functions call declaration */ > thanks! -- Jagan | openedev. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
Hi, On Thu, 2015-09-03 at 16:17 +0200, ma...@denx.de wrote: > On Thursday, September 03, 2015 at 03:41:59 PM, Chin Liang See wrote: > > Ensuring spi_calibration is run when there is a change of sclk > > frequency. This will ensure the qspi flash access works for high > > sclk frequency > > > > Signed-off-by: Chin Liang See> > Cc: Dinh Nguyen > > Cc: Dinh Nguyen > > Cc: Marek Vasut > > Cc: Stefan Roese > > Cc: Vikas Manocha > > Cc: Jagannadh Teki > > Cc: Pavel Machek > > --- > > drivers/spi/cadence_qspi.c |8 > > 1 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > > index 34a0f46..300934e 100644 > > --- a/drivers/spi/cadence_qspi.c > > +++ b/drivers/spi/cadence_qspi.c > > @@ -37,7 +37,7 @@ static int cadence_spi_write_speed(struct udevice *bus, > > uint hz) } > > > > /* Calibration sequence to determine the read data capture delay register > > */ -static int spi_calibration(struct udevice *bus) > > +static int spi_calibration(struct udevice *bus, uint hz) > > { > > struct cadence_spi_platdata *plat = bus->platdata; > > struct cadence_spi_priv *priv = dev_get_priv(bus); > > @@ -64,7 +64,7 @@ static int spi_calibration(struct udevice *bus) > > } > > > > /* use back the intended clock and find low range */ > > - cadence_spi_write_speed(bus, plat->max_hz); > > + cadence_spi_write_speed(bus, hz); > > for (i = 0; i < CQSPI_READ_CAPTURE_MAX_DELAY; i++) { > > /* Disable QSPI */ > > cadence_qspi_apb_controller_disable(base); > > @@ -111,7 +111,7 @@ static int spi_calibration(struct udevice *bus) > > (range_hi + range_lo) / 2, range_lo, range_hi); > > > > /* just to ensure we do once only when speed or chip select change */ > > - priv->qspi_calibrated_hz = plat->max_hz; > > + priv->qspi_calibrated_hz = hz; > > priv->qspi_calibrated_cs = spi_chip_select(bus); > > > > return 0; > > @@ -131,7 +131,7 @@ static int cadence_spi_set_speed(struct udevice *bus, > > Hi, > > My impression is that the logic here should be like this: > > if (hz > plat->max_hz) { > printf("error, freq. too high"); > return -EINVAL; > } > if (priv->previous_hz != hz) /* Bus frequency changed, re-calibrate */ > spi_calibrate(bus, hz) > cadence_spi_write_speed(bus, priv->qspi_calibrated_hz); > priv->previous_hz = hz; > > Note that you need a new private variable, previous_hz, to hold the previous > value of "hz". This is needed since the calibrated frequency might not be > equal to requested frequency. > I know where you coming from. Yah, calibration can be skipped when requested frequency same as previous or calibrated one. Let me add that. > > uint hz) /* Calibration required for different SCLK speed or chip select > > */ if (priv->qspi_calibrated_hz != plat->max_hz || > > priv->qspi_calibrated_cs != spi_chip_select(bus)) { > > - err = spi_calibration(bus); > > + err = spi_calibration(bus, hz); > > This is called after the frequency is configured in this function, this looks > really backwards. Yah, let me remove the old code. Thanks Chin Liang > > > if (err) > > return err; > > } > > Best regards, > Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
On Thursday, September 03, 2015 at 03:41:59 PM, Chin Liang See wrote: > Ensuring spi_calibration is run when there is a change of sclk > frequency. This will ensure the qspi flash access works for high > sclk frequency > > Signed-off-by: Chin Liang See> Cc: Dinh Nguyen > Cc: Dinh Nguyen > Cc: Marek Vasut > Cc: Stefan Roese > Cc: Vikas Manocha > Cc: Jagannadh Teki > Cc: Pavel Machek > --- > drivers/spi/cadence_qspi.c |8 > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index 34a0f46..300934e 100644 > --- a/drivers/spi/cadence_qspi.c > +++ b/drivers/spi/cadence_qspi.c > @@ -37,7 +37,7 @@ static int cadence_spi_write_speed(struct udevice *bus, > uint hz) } > > /* Calibration sequence to determine the read data capture delay register > */ -static int spi_calibration(struct udevice *bus) > +static int spi_calibration(struct udevice *bus, uint hz) > { > struct cadence_spi_platdata *plat = bus->platdata; > struct cadence_spi_priv *priv = dev_get_priv(bus); > @@ -64,7 +64,7 @@ static int spi_calibration(struct udevice *bus) > } > > /* use back the intended clock and find low range */ > - cadence_spi_write_speed(bus, plat->max_hz); > + cadence_spi_write_speed(bus, hz); > for (i = 0; i < CQSPI_READ_CAPTURE_MAX_DELAY; i++) { > /* Disable QSPI */ > cadence_qspi_apb_controller_disable(base); > @@ -111,7 +111,7 @@ static int spi_calibration(struct udevice *bus) > (range_hi + range_lo) / 2, range_lo, range_hi); > > /* just to ensure we do once only when speed or chip select change */ > - priv->qspi_calibrated_hz = plat->max_hz; > + priv->qspi_calibrated_hz = hz; > priv->qspi_calibrated_cs = spi_chip_select(bus); > > return 0; > @@ -131,7 +131,7 @@ static int cadence_spi_set_speed(struct udevice *bus, Hi, My impression is that the logic here should be like this: if (hz > plat->max_hz) { printf("error, freq. too high"); return -EINVAL; } if (priv->previous_hz != hz) /* Bus frequency changed, re-calibrate */ spi_calibrate(bus, hz) cadence_spi_write_speed(bus, priv->qspi_calibrated_hz); priv->previous_hz = hz; Note that you need a new private variable, previous_hz, to hold the previous value of "hz". This is needed since the calibrated frequency might not be equal to requested frequency. > uint hz) /* Calibration required for different SCLK speed or chip select > */ if (priv->qspi_calibrated_hz != plat->max_hz || > priv->qspi_calibrated_cs != spi_chip_select(bus)) { > - err = spi_calibration(bus); > + err = spi_calibration(bus, hz); This is called after the frequency is configured in this function, this looks really backwards. > if (err) > return err; > } Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot