| Message ID | 201306281143.35130.hartleys@visionengravers.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 29/06/13 04:43, H Hartley Sweeten wrote: > __spi_async(), which starts every SPI message transfer, initializes > the bits_per_word and max speed for every transfer in the message. > Since the conditional test in ep93xx_spi_process_transfer() will > always succeed just remove it and always call ep93xx_spi_chip_setup() > to configure the hardware for each transfer in the message. > > Remove the redundant ep93xx_spi_chp_setup() in ep93xx_spi_process_transfer() > which just initializes the hardware to the "default" based on the SPI > device. > > Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> > Cc: Ryan Mallon <rmallon@gmail.com> > Cc: Mika Westerberg <mika.westerberg@iki.fi> > Cc: Mark Brown <broonie@kernel.org> > Cc: Grant Likely <grant.likely@linaro.org> > --- > + err = ep93xx_spi_calc_divisors(espi, chip, t->speed_hz); > + if (err) { > + dev_err(&espi->pdev->dev, "failed to adjust speed\n"); Printing out the speed it was trying to set might be useful here? ~Ryan ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev
On Friday, June 28, 2013 4:18 PM, Ryan Mallon wrote: > On 29/06/13 04:43, H Hartley Sweeten wrote: >> __spi_async(), which starts every SPI message transfer, initializes >> the bits_per_word and max speed for every transfer in the message. >> Since the conditional test in ep93xx_spi_process_transfer() will >> always succeed just remove it and always call ep93xx_spi_chip_setup() >> to configure the hardware for each transfer in the message. >> >> Remove the redundant ep93xx_spi_chp_setup() in ep93xx_spi_process_transfer() >> which just initializes the hardware to the "default" based on the SPI >> device. >> >> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> >> Cc: Ryan Mallon <rmallon@gmail.com> >> Cc: Mika Westerberg <mika.westerberg@iki.fi> >> Cc: Mark Brown <broonie@kernel.org> >> Cc: Grant Likely <grant.likely@linaro.org> >> --- > > >> + err = ep93xx_spi_calc_divisors(espi, chip, t->speed_hz); >> + if (err) { >> + dev_err(&espi->pdev->dev, "failed to adjust speed\n"); > > > Printing out the speed it was trying to set might be useful here? Technically I don't think this can ever happen. The minimum and maximum possible speeds are determined during the probe. espi->max_rate = clk_get_rate(espi->clk) / 2; espi->min_rate = clk_get_rate(espi->clk) / (254 * 256); Each transfer is validated to make sure the speed is greater than the minimum. if (t->speed_hz < espi->min_rate) return -EINVAL; Then the rate is clamped to the min/max rates. rate = clamp(rate, espi->min_rate, espi->max_rate); The calculations for the div_csr and div_cpsr values needed to produce the desired rate should always succeed. Patch 7/8 actually replaces that dev_err() message with a more generic one. In reality, even the new message, and the error checking, could probably be removed. Regards, Hartley ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev
On Fri, Jun 28, 2013 at 11:43:34AM -0700, H Hartley Sweeten wrote: > __spi_async(), which starts every SPI message transfer, initializes > the bits_per_word and max speed for every transfer in the message. > Since the conditional test in ep93xx_spi_process_transfer() will > always succeed just remove it and always call ep93xx_spi_chip_setup() > to configure the hardware for each transfer in the message. > > Remove the redundant ep93xx_spi_chp_setup() in ep93xx_spi_process_transfer() > which just initializes the hardware to the "default" based on the SPI > device. > > Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> > Cc: Ryan Mallon <rmallon@gmail.com> > Cc: Mika Westerberg <mika.westerberg@iki.fi> Acked-by: Mika Westerberg <mika.westerberg@iki.fi> ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev
On Monday, July 01, 2013 3:28 AM, Mark Brown wrote: > On Fri, Jun 28, 2013 at 11:43:34AM -0700, H Hartley Sweeten wrote: >> __spi_async(), which starts every SPI message transfer, initializes >> the bits_per_word and max speed for every transfer in the message. >> Since the conditional test in ep93xx_spi_process_transfer() will >> always succeed just remove it and always call ep93xx_spi_chip_setup() >> to configure the hardware for each transfer in the message. > > Applied, thanks. Mark, I was going to redo this series based on the comments I have received so far. Did you already apply this one? Should I push it to the front of my patchset? Regards, Hartley ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev
diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c index 93ae7b6..bcfd35a 100644 --- a/drivers/spi/spi-ep93xx.c +++ b/drivers/spi/spi-ep93xx.c @@ -684,38 +684,20 @@ static void ep93xx_spi_process_transfer(struct ep93xx_spi *espi, struct spi_transfer *t) { struct ep93xx_spi_chip *chip = spi_get_ctldata(msg->spi); + int err; msg->state = t; - /* - * Handle any transfer specific settings if needed. We use - * temporary chip settings here and restore original later when - * the transfer is finished. - */ - if (t->speed_hz || t->bits_per_word) { - struct ep93xx_spi_chip tmp_chip = *chip; - - if (t->speed_hz) { - int err; - - err = ep93xx_spi_calc_divisors(espi, &tmp_chip, - t->speed_hz); - if (err) { - dev_err(&espi->pdev->dev, - "failed to adjust speed\n"); - msg->status = err; - return; - } - } + err = ep93xx_spi_calc_divisors(espi, chip, t->speed_hz); + if (err) { + dev_err(&espi->pdev->dev, "failed to adjust speed\n"); + msg->status = err; + return; + } - if (t->bits_per_word) - tmp_chip.dss = bits_per_word_to_dss(t->bits_per_word); + chip->dss = bits_per_word_to_dss(t->bits_per_word); - /* - * Set up temporary new hw settings for this transfer. - */ - ep93xx_spi_chip_setup(espi, &tmp_chip); - } + ep93xx_spi_chip_setup(espi, chip); espi->rx = 0; espi->tx = 0; @@ -759,9 +741,6 @@ static void ep93xx_spi_process_transfer(struct ep93xx_spi *espi, ep93xx_spi_cs_control(msg->spi, true); } } - - if (t->speed_hz || t->bits_per_word) - ep93xx_spi_chip_setup(espi, chip); } /* @@ -814,10 +793,8 @@ static void ep93xx_spi_process_message(struct ep93xx_spi *espi, espi->fifo_level = 0; /* - * Update SPI controller registers according to spi device and assert - * the chipselect. + * Assert the chipselect. */ - ep93xx_spi_chip_setup(espi, spi_get_ctldata(msg->spi)); ep93xx_spi_cs_control(msg->spi, true); list_for_each_entry(t, &msg->transfers, transfer_list) {
__spi_async(), which starts every SPI message transfer, initializes the bits_per_word and max speed for every transfer in the message. Since the conditional test in ep93xx_spi_process_transfer() will always succeed just remove it and always call ep93xx_spi_chip_setup() to configure the hardware for each transfer in the message. Remove the redundant ep93xx_spi_chp_setup() in ep93xx_spi_process_transfer() which just initializes the hardware to the "default" based on the SPI device. Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> Cc: Ryan Mallon <rmallon@gmail.com> Cc: Mika Westerberg <mika.westerberg@iki.fi> Cc: Mark Brown <broonie@kernel.org> Cc: Grant Likely <grant.likely@linaro.org> --- drivers/spi/spi-ep93xx.c | 43 ++++++++++--------------------------------- 1 file changed, 10 insertions(+), 33 deletions(-)