Hi, On Wed, Jun 17, 2015 at 6:08 PM, Sifan Naeem <sifan.na...@imgtec.com> wrote: > Hi Jonas, > >> -----Original Message----- >> From: Jonas Gorski [mailto:j...@openwrt.org] >> Sent: 17 June 2015 15:31 >> To: Sifan Naeem >> Cc: Mark Brown; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; >> Andrew Bresticker >> Subject: Re: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported >> transfer speed >> >> Hi, >> >> On Wed, Jun 17, 2015 at 3:36 PM, Sifan Naeem <sifan.na...@imgtec.com> >> wrote: >> > Hi Jonas, >> > >> >> -----Original Message----- >> >> From: Jonas Gorski [mailto:j...@openwrt.org] >> >> Sent: 17 June 2015 13:12 >> >> To: Sifan Naeem >> >> Cc: Mark Brown; linux-...@vger.kernel.org; >> >> linux-kernel@vger.kernel.org; Andrew Bresticker >> >> Subject: Re: [PATCH v2] spi: img-spfi: Same Edge bit set to double >> >> supported transfer speed >> >> >> >> Hi, >> >> >> >> On Wed, Jun 17, 2015 at 12:01 PM, Sifan Naeem >> >> <sifan.na...@imgtec.com> >> >> wrote: >> >> > Same edge bit set in SPFI Control register to double the supported >> >> > spfi clock speed. Setting this bit increases the supported spfi >> >> > frequency from 1/8 to 1/4 of the core clock frequency. >> >> > >> >> > Without this bit set the maximum speed supported was 25MHz on >> >> > Pistachio. >> >> > >> >> > Change-Id: I26782ea88ac3567e72dac11e46c5b5f5f52c5e3d >> >> > Signed-off-by: Sifan Naeem <sifan.na...@imgtec.com> >> >> > --- >> >> > drivers/spi/spi-img-spfi.c | 2 ++ >> >> > 1 file changed, 2 insertions(+) >> >> > >> >> > diff --git a/drivers/spi/spi-img-spfi.c >> >> > b/drivers/spi/spi-img-spfi.c index 788e2b1..acce90a 100644 >> >> > --- a/drivers/spi/spi-img-spfi.c >> >> > +++ b/drivers/spi/spi-img-spfi.c >> >> > @@ -40,6 +40,7 @@ >> >> > #define SPFI_CONTROL_SOFT_RESET BIT(11) >> >> > #define SPFI_CONTROL_SEND_DMA BIT(10) >> >> > #define SPFI_CONTROL_GET_DMA BIT(9) >> >> > +#define SPFI_CONTROL_SE BIT(8) >> >> > #define SPFI_CONTROL_TMODE_SHIFT 5 >> >> > #define SPFI_CONTROL_TMODE_MASK 0x7 >> >> > #define SPFI_CONTROL_TMODE_SINGLE 0 >> >> > @@ -491,6 +492,7 @@ static void img_spfi_config(struct spi_master >> >> *master, struct spi_device *spi, >> >> > else if (xfer->tx_nbits == SPI_NBITS_QUAD && >> >> > xfer->rx_nbits == SPI_NBITS_QUAD) >> >> > val |= SPFI_CONTROL_TMODE_QUAD << >> >> > SPFI_CONTROL_TMODE_SHIFT; >> >> > + val |= SPFI_CONTROL_SE; >> >> > spfi_writel(spfi, val, SPFI_CONTROL); } >> >> >> >> Don't you also need to update master->max_speed_hz? And if it doubles >> >> the clock speed, don't you need to reflect that in the calculation for the >> devider? >> >> Currently it looks like it would just cause all transfers to go with >> >> the doubled requested rate. But maybe I'm missing something. >> >> >> > max_speed_hz is already set to 1/4 of the Core clock. So I don't think we >> need to change that. >> > In Pistachio SoC the max speed of SPFI block is limited to 1/4 of the >> > core clock or to 50Mhz, to achieve these values the change suggested >> > in this patch should have always been set. In the device tree node for >> > SPFI we have to mention the limit of 50Mhz as spi-max-frequency = >> > <50000000>; >> >> So it calculated the wrong divisor before, and the resulting speed was half >> of >> that and setting this bit actually fixes it? Or does the block just treat >> speeds > >> 25 MHz as 25 MHz when this bit is set? I'm still trying to wrap my head >> around >> that setting a single bit in a register allows going from 0~25 MHz to 0~50 >> MHz >> without needing to update the calculation of the divisor. >> > > Yes, if the requested speed is 50Mhz without the SE bit set, the divisor > calculated would still request 50Mhz from the spfi block, which is correct, > but the transfer would fail as SE bit is not set and the maximum speed > supported would be 25Mhz.
So this is actually a bug fix, and it should be noted as such so that stable maintainers can properly decide to pick it up. >From the original commit message it sounds like this is an enhancement that increases the max speed from 25 to 50 MHz, and not that speeds 25~50 MHz are currently broken (i.e. transfers will fail) and require this to be set to work. Regards Jonas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
