[PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Sifan Naeem]

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.naeem@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);
 }
 
-- 
1.7.9.5

[PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Sifan Naeem]

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.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 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);
 }
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Jonas Gorski]

Hi,

On Wed, Jun 17, 2015 at 12:01 PM, Sifan Naeem <sifan.naeem@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.naeem@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.


Regards
Jonas

Re: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Jonas Gorski]

Hi,

On Wed, Jun 17, 2015 at 12:01 PM, Sifan Naeem <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> 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.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> ---
>  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.


Regards
Jonas
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Sifan Naeem]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2798 bytes --]

Hi Jonas,

> -----Original Message-----
> From: Jonas Gorski [mailto:jogo@openwrt.org]
> Sent: 17 June 2015 13:12
> To: Sifan Naeem
> Cc: Mark Brown; linux-spi@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.naeem@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.naeem@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>;

Thanks,
Sifan

> 
> Regards
> Jonas
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Sifan Naeem]

Hi Jonas,

> -----Original Message-----
> From: Jonas Gorski [mailto:jogo@openwrt.org]
> Sent: 17 June 2015 13:12
> To: Sifan Naeem
> Cc: Mark Brown; linux-spi@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.naeem@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.naeem@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>;

Thanks,
Sifan

> 
> Regards
> Jonas

Re: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Jonas Gorski]

Hi,

On Wed, Jun 17, 2015 at 3:36 PM, Sifan Naeem <Sifan.Naeem@imgtec.com> wrote:
> Hi Jonas,
>
>> -----Original Message-----
>> From: Jonas Gorski [mailto:jogo@openwrt.org]
>> Sent: 17 June 2015 13:12
>> To: Sifan Naeem
>> Cc: Mark Brown; linux-spi@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.naeem@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.naeem@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.


Regards
Jonas

Re: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Jonas Gorski]

Hi,

On Wed, Jun 17, 2015 at 3:36 PM, Sifan Naeem <Sifan.Naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> Hi Jonas,
>
>> -----Original Message-----
>> From: Jonas Gorski [mailto:jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org]
>> Sent: 17 June 2015 13:12
>> To: Sifan Naeem
>> Cc: Mark Brown; linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.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.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> 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.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> > ---
>> >  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.


Regards
Jonas
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Sifan Naeem]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4046 bytes --]

Hi Jonas,

> -----Original Message-----
> From: Jonas Gorski [mailto:jogo@openwrt.org]
> Sent: 17 June 2015 15:31
> To: Sifan Naeem
> Cc: Mark Brown; linux-spi@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.Naeem@imgtec.com>
> wrote:
> > Hi Jonas,
> >
> >> -----Original Message-----
> >> From: Jonas Gorski [mailto:jogo@openwrt.org]
> >> Sent: 17 June 2015 13:12
> >> To: Sifan Naeem
> >> Cc: Mark Brown; linux-spi@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.naeem@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.naeem@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.

Thanks,
Sifan
> 
> Regards
> Jonas
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Sifan Naeem]

Hi Jonas,

> -----Original Message-----
> From: Jonas Gorski [mailto:jogo@openwrt.org]
> Sent: 17 June 2015 15:31
> To: Sifan Naeem
> Cc: Mark Brown; linux-spi@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.Naeem@imgtec.com>
> wrote:
> > Hi Jonas,
> >
> >> -----Original Message-----
> >> From: Jonas Gorski [mailto:jogo@openwrt.org]
> >> Sent: 17 June 2015 13:12
> >> To: Sifan Naeem
> >> Cc: Mark Brown; linux-spi@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.naeem@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.naeem@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.

Thanks,
Sifan
> 
> Regards
> Jonas

Re: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Jonas Gorski]

Hi,

On Wed, Jun 17, 2015 at 6:08 PM, Sifan Naeem <Sifan.Naeem@imgtec.com> wrote:
> Hi Jonas,
>
>> -----Original Message-----
>> From: Jonas Gorski [mailto:jogo@openwrt.org]
>> Sent: 17 June 2015 15:31
>> To: Sifan Naeem
>> Cc: Mark Brown; linux-spi@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.Naeem@imgtec.com>
>> wrote:
>> > Hi Jonas,
>> >
>> >> -----Original Message-----
>> >> From: Jonas Gorski [mailto:jogo@openwrt.org]
>> >> Sent: 17 June 2015 13:12
>> >> To: Sifan Naeem
>> >> Cc: Mark Brown; linux-spi@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.naeem@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.naeem@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

Re: [PATCH v2] spi: img-spfi: Same Edge bit set to double supported transfer speed

[Jonas Gorski]

Hi,

On Wed, Jun 17, 2015 at 6:08 PM, Sifan Naeem <Sifan.Naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> Hi Jonas,
>
>> -----Original Message-----
>> From: Jonas Gorski [mailto:jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org]
>> Sent: 17 June 2015 15:31
>> To: Sifan Naeem
>> Cc: Mark Brown; linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.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.Naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> wrote:
>> > Hi Jonas,
>> >
>> >> -----Original Message-----
>> >> From: Jonas Gorski [mailto:jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org]
>> >> Sent: 17 June 2015 13:12
>> >> To: Sifan Naeem
>> >> Cc: Mark Brown; linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
>> >> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.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.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> >> 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.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> >> > ---
>> >> >  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-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html