__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 <[email protected]>
Cc: Ryan Mallon <[email protected]>
Cc: Mika Westerberg <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Grant Likely <[email protected]>
---
drivers/spi/spi-ep93xx.c | 43 ++++++++++---------------------------------
1 file changed, 10 insertions(+), 33 deletions(-)
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) {
--
1.8.1.4
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 <[email protected]>
> Cc: Ryan Mallon <[email protected]>
> Cc: Mika Westerberg <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: Grant Likely <[email protected]>
> ---
> + 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
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 <[email protected]>
>> Cc: Ryan Mallon <[email protected]>
>> Cc: Mika Westerberg <[email protected]>
>> Cc: Mark Brown <[email protected]>
>> Cc: Grant Likely <[email protected]>
>> ---
>
>
>> + 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
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
> Cc: Ryan Mallon <[email protected]>
> Cc: Mika Westerberg <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
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.
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
On Mon, Jul 01, 2013 at 01:15:40PM -0500, H Hartley Sweeten wrote:
> On Monday, July 01, 2013 3:28 AM, Mark Brown wrote:
> > Applied, thanks.
> 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?
That's what I said. You shouldn't resend it at all, you should base
your updates off the relevant topic branch.