diff mbox

[v2] spi: orion: support armada extended baud rates

Message ID 1409835268-29555-1-git-send-email-gerg@uclinux.org (mailing list archive)
State New, archived
Headers show

Commit Message

Greg Ungerer Sept. 4, 2014, 12:54 p.m. UTC
From: Greg Ungerer <gerg@uclinux.org>

The Armada SoC family implementation of this SPI hardware module has
extended the configuration register to allow for a wider range of SPI
clock rates. Specifically the Serial Baud Rate Pre-selection bits in the
SPI Interface Configuration Register now also use bits 6 and 7 as well.

Modify the baud rate calculation to handle these differences for the
Armada case. Potentially a baud rate can be setup using a number of
different pre-scalar and scalar combinations. This code tries all
possible pre-scalar divisors (8 in total) to try and find the most
accuate set.

This change introduces a new device tree compatible device name
"armada-spi". This is used in place of "orion-spi" on the Armada SoC
parts.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
Changes in v2:
Introduced a new devicetree compatible name for armada SPI support.

 .../devicetree/bindings/spi/spi-orion.txt          |   2 +-
 arch/arm/boot/dts/armada-370-xp.dtsi               |   4 +-
 drivers/spi/spi-orion.c                            | 116 +++++++++++++++++----
 3 files changed, 97 insertions(+), 25 deletions(-)

Comments

Geert Uytterhoeven Sept. 4, 2014, 1:17 p.m. UTC | #1
Hi Greg,

On Thu, Sep 4, 2014 at 2:54 PM,  <gerg@uclinux.org> wrote:
> The Armada SoC family implementation of this SPI hardware module has
> extended the configuration register to allow for a wider range of SPI
> clock rates. Specifically the Serial Baud Rate Pre-selection bits in the
> SPI Interface Configuration Register now also use bits 6 and 7 as well.
>
> Modify the baud rate calculation to handle these differences for the
> Armada case. Potentially a baud rate can be setup using a number of
> different pre-scalar and scalar combinations. This code tries all
> possible pre-scalar divisors (8 in total) to try and find the most
> accuate set.

Are the Orion rates and register bits a subset of the Armada ones?
I.e. does the Armada work with the Orion setup?

> This change introduces a new device tree compatible device name
> "armada-spi". This is used in place of "orion-spi" on the Armada SoC
> parts.

> -                               compatible = "marvell,orion-spi";
> +                               compatible = "marvell,armada-spi";

If the answer to my above question is yes, you want to keep
"marvell,orion-spi" as a fallback.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Ungerer Sept. 5, 2014, 12:36 a.m. UTC | #2
Hi Geert,

On 04/09/14 23:17, Geert Uytterhoeven wrote:
> On Thu, Sep 4, 2014 at 2:54 PM,  <gerg@uclinux.org> wrote:
>> The Armada SoC family implementation of this SPI hardware module has
>> extended the configuration register to allow for a wider range of SPI
>> clock rates. Specifically the Serial Baud Rate Pre-selection bits in the
>> SPI Interface Configuration Register now also use bits 6 and 7 as well.
>>
>> Modify the baud rate calculation to handle these differences for the
>> Armada case. Potentially a baud rate can be setup using a number of
>> different pre-scalar and scalar combinations. This code tries all
>> possible pre-scalar divisors (8 in total) to try and find the most
>> accuate set.
> 
> Are the Orion rates and register bits a subset of the Armada ones?
> I.e. does the Armada work with the Orion setup?

The bits used by the orion are a subset (bits 0-4 of the SPI
configuration register) of those used by the armada (bits 0-4 and
6-7).

In practice the defined prescalar values for the orion do give
the same divisor for the same bits set on armada. The extra bits
on the armada let you get to lower SPI baudrates (which is more of
a problem on the armada with its higher core clocks).


>> This change introduces a new device tree compatible device name
>> "armada-spi". This is used in place of "orion-spi" on the Armada SoC
>> parts.
> 
>> -                               compatible = "marvell,orion-spi";
>> +                               compatible = "marvell,armada-spi";
> 
> If the answer to my above question is yes, you want to keep
> "marvell,orion-spi" as a fallback.

Do you mean like this?

  compatible = "marvell,orion-spi", "marvell,armada-spi";

Regards
Greg

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Sept. 5, 2014, 7:36 a.m. UTC | #3
Hi Greg,

On Fri, Sep 5, 2014 at 2:36 AM, Greg Ungerer <gerg@uclinux.org> wrote:
>> Are the Orion rates and register bits a subset of the Armada ones?
>> I.e. does the Armada work with the Orion setup?
>
> The bits used by the orion are a subset (bits 0-4 of the SPI
> configuration register) of those used by the armada (bits 0-4 and
> 6-7).
>
> In practice the defined prescalar values for the orion do give
> the same divisor for the same bits set on armada. The extra bits
> on the armada let you get to lower SPI baudrates (which is more of
> a problem on the armada with its higher core clocks).

OK, so the answer is yes.

>>> This change introduces a new device tree compatible device name
>>> "armada-spi". This is used in place of "orion-spi" on the Armada SoC
>>> parts.
>>
>>> -                               compatible = "marvell,orion-spi";
>>> +                               compatible = "marvell,armada-spi";
>>
>> If the answer to my above question is yes, you want to keep
>> "marvell,orion-spi" as a fallback.
>
> Do you mean like this?
>
>   compatible = "marvell,orion-spi", "marvell,armada-spi";

No, the other way around (from most-specific to least-specific):

    compatible = "marvell,armada-spi", "marvell,orion-spi"

So the Armada version is used if available, and the Orion version
is used as a fallback.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Ungerer Sept. 5, 2014, 1:11 p.m. UTC | #4
Hi Geert,

On 05/09/14 17:36, Geert Uytterhoeven wrote:
> Hi Greg,
>
> On Fri, Sep 5, 2014 at 2:36 AM, Greg Ungerer <gerg@uclinux.org> wrote:
>>> Are the Orion rates and register bits a subset of the Armada ones?
>>> I.e. does the Armada work with the Orion setup?
>> The bits used by the orion are a subset (bits 0-4 of the SPI
>> configuration register) of those used by the armada (bits 0-4 and
>> 6-7).
>>
>> In practice the defined prescalar values for the orion do give
>> the same divisor for the same bits set on armada. The extra bits
>> on the armada let you get to lower SPI baudrates (which is more of
>> a problem on the armada with its higher core clocks).
> OK, so the answer is yes.
>
>>>> This change introduces a new device tree compatible device name
>>>> "armada-spi". This is used in place of "orion-spi" on the Armada SoC
>>>> parts.
>>>> -                               compatible = "marvell,orion-spi";
>>>> +                               compatible = "marvell,armada-spi";
>>> If the answer to my above question is yes, you want to keep
>>> "marvell,orion-spi" as a fallback.
>> Do you mean like this?
>>
>>    compatible = "marvell,orion-spi", "marvell,armada-spi";
> No, the other way around (from most-specific to least-specific):
>
>      compatible = "marvell,armada-spi", "marvell,orion-spi"
>
> So the Armada version is used if available, and the Orion version
> is used as a fallback.

Ok, got it. Thanks.

Regards
Greg


> Gr{oetje,eeting}s,
>
>                          Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
>

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ezequiel Garcia Sept. 6, 2014, 12:06 p.m. UTC | #5
Hi Greg, 

(Ccing mvebu guys, full discussion here: http://www.spinics.net/lists/devicetree/msg47806.html)

On 05 Sep 11:11 PM, Greg Ungerer wrote:
> On 05/09/14 17:36, Geert Uytterhoeven wrote:
> >No, the other way around (from most-specific to least-specific):
> >
> >     compatible = "marvell,armada-spi", "marvell,orion-spi"
> >
> >So the Armada version is used if available, and the Orion version
> >is used as a fallback.

Since you are doing another spin, please split this in two patches.
The driver and devicetree binding documentation should be in one patch,
and the devicetree .dtsi change in another patch.

By following Geert's request about the compatible, you make sure both
patches can be merged through different maintainers, and that an old devicetree
blob can be used safely.

> >  Also what about armada370-xp, armada375, armada380
> >and any other variants out there?
> 
> I don't know. I don't have access to functional specs for anything
> other than the 370. I expect they are probably the same though.
> (Can anyone confirm?)
> 

Confirmed. Judging from the specs, seems the same SPI IP is on all the
Armada SoCs; you can do the 'armada-spi' compatible addition on 375 and 38x.

Did a few tests on the AXP-GP board (which has an SPI flash) using
'orion-spi' and 'armada-spi' compatibles.

Tested-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Reviewed-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

I haven't been able to spot any speed difference, though (perhaps my
test is too limited?), so I wonder if there's any improvement with this
patch?

Thanks,
Thomas Petazzoni Sept. 6, 2014, 1:16 p.m. UTC | #6
Hello,

On Sat, 6 Sep 2014 09:06:00 -0300, Ezequiel Garcia wrote:

> Confirmed. Judging from the specs, seems the same SPI IP is on all the
> Armada SoCs; you can do the 'armada-spi' compatible addition on 375 and 38x.

However, I believe a compatible string like "armada-370-spi" should be
used instead of "armada-spi". The normal rule is to use the oldest SoC
that introduced a certain feature as the compatible string.

Remember for example that Armada 1500 (Berlin) is a completely different
line of SoC, which cannot use this SPI driver.

Thanks Greg for working on this! I noticed this as well recently when
doing an SPI development, but you were faster than me in proposing a
patch!

Thanks,

Thomas
Greg Ungerer Sept. 7, 2014, 12:14 p.m. UTC | #7
Hi Ezequiel,

On 06/09/14 22:06, Ezequiel Garcia wrote:
> Hi Greg,
>
> (Ccing mvebu guys, full discussion here: http://www.spinics.net/lists/devicetree/msg47806.html)
>
> On 05 Sep 11:11 PM, Greg Ungerer wrote:
>> On 05/09/14 17:36, Geert Uytterhoeven wrote:
>>> No, the other way around (from most-specific to least-specific):
>>>
>>>      compatible = "marvell,armada-spi", "marvell,orion-spi"
>>>
>>> So the Armada version is used if available, and the Orion version
>>> is used as a fallback.
> Since you are doing another spin, please split this in two patches.
> The driver and devicetree binding documentation should be in one patch,
> and the devicetree .dtsi change in another patch.
>
> By following Geert's request about the compatible, you make sure both
> patches can be merged through different maintainers, and that an old devicetree
> blob can be used safely.

Sure, yes, I would prefer that too. I didn't do it for this because 
checkpatch
complained for the spi-orion.c change in its own:

WARNING: DT compatible string "marvell,armada-spi" appears un-documented 
-- check ./Documentation/devicetree/bindings/
#170: FILE: drivers/spi/spi-orion.c:412:
+    { .compatible = "marvell,armada-spi", .data = &armada_spi_dev_data, },

But I am happy to ignore that :-)


>>>   Also what about armada370-xp, armada375, armada380
>>> and any other variants out there?
>> I don't know. I don't have access to functional specs for anything
>> other than the 370. I expect they are probably the same though.
>> (Can anyone confirm?)
>>
> Confirmed. Judging from the specs, seems the same SPI IP is on all the
> Armada SoCs; you can do the 'armada-spi' compatible addition on 375 and 38x.
>
> Did a few tests on the AXP-GP board (which has an SPI flash) using
> 'orion-spi' and 'armada-spi' compatibles.
>
> Tested-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> Reviewed-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Thanks, appreciate that.


> I haven't been able to spot any speed difference, though (perhaps my
> test is too limited?), so I wonder if there's any improvement with this
> patch?

Try setting a speed like 2MHz for an SPI device. It will fail because that
is below the minimum divisor it can calculate with the old orion style
behavior. I have an armada 370 based board with an ethernet switch
that is controlled by SPI. Its typical SPI bus speed is 2MHz, that is how I
came up against this.

Regards
Greg

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Ungerer Sept. 7, 2014, 12:18 p.m. UTC | #8
Hi Thomas,

On 06/09/14 23:16, Thomas Petazzoni wrote:
> Hello,
>
> On Sat, 6 Sep 2014 09:06:00 -0300, Ezequiel Garcia wrote:
>
>> Confirmed. Judging from the specs, seems the same SPI IP is on all the
>> Armada SoCs; you can do the 'armada-spi' compatible addition on 375 and 38x.
> However, I believe a compatible string like "armada-370-spi" should be
> used instead of "armada-spi". The normal rule is to use the oldest SoC
> that introduced a certain feature as the compatible string.

Ok, will make it that then.


> Remember for example that Armada 1500 (Berlin) is a completely different
> line of SoC, which cannot use this SPI driver.
>
> Thanks Greg for working on this! I noticed this as well recently when
> doing an SPI development, but you were faster than me in proposing a
> patch!
>

No problem :-)

Regards
Greg

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ezequiel Garcia Sept. 7, 2014, 3:12 p.m. UTC | #9
Hi Greg,

On 07 Sep 10:14 PM, Greg Ungerer wrote:
> Hi Ezequiel,
> 
> On 06/09/14 22:06, Ezequiel Garcia wrote:
> >Hi Greg,
> >
> >(Ccing mvebu guys, full discussion here: http://www.spinics.net/lists/devicetree/msg47806.html)
> >
> >On 05 Sep 11:11 PM, Greg Ungerer wrote:
> >>On 05/09/14 17:36, Geert Uytterhoeven wrote:
> >>>No, the other way around (from most-specific to least-specific):
> >>>
> >>>     compatible = "marvell,armada-spi", "marvell,orion-spi"
> >>>
> >>>So the Armada version is used if available, and the Orion version
> >>>is used as a fallback.
> >Since you are doing another spin, please split this in two patches.
> >The driver and devicetree binding documentation should be in one patch,
> >and the devicetree .dtsi change in another patch.
> >
> >By following Geert's request about the compatible, you make sure both
> >patches can be merged through different maintainers, and that an old devicetree
> >blob can be used safely.
> 
> Sure, yes, I would prefer that too. I didn't do it for this because
> checkpatch
> complained for the spi-orion.c change in its own:
> 
> WARNING: DT compatible string "marvell,armada-spi" appears un-documented --
> check ./Documentation/devicetree/bindings/
> #170: FILE: drivers/spi/spi-orion.c:412:
> +    { .compatible = "marvell,armada-spi", .data = &armada_spi_dev_data, },
> 
> But I am happy to ignore that :-)
> 

If you split in two patches like this:

Patch #1: "spi: orion: Add a new foo feature"

 .../devicetree/bindings/spi/spi-orion.txt          |   2 +-
 drivers/spi/spi-orion.c                            | 116 +++++++++++++++++----

Patch #2: "ARM: mvebu: Add new compatible string for SPI"

 arch/arm/boot/dts/armada-370-xp.dtsi               |   4 +-

Then checkpatch shouldn't complain. As you can see, it makes more sense like
this. The change to the devicetree binding in the driver and properly
documented, but the devicetree files per-se, get modified in another patch.
Greg Ungerer Sept. 8, 2014, 1:52 a.m. UTC | #10
Hi Ezequiel,

On 08/09/14 01:12, Ezequiel Garcia wrote:
> Hi Greg,
> 
> On 07 Sep 10:14 PM, Greg Ungerer wrote:
>> Hi Ezequiel,
>>
>> On 06/09/14 22:06, Ezequiel Garcia wrote:
>>> Hi Greg,
>>>
>>> (Ccing mvebu guys, full discussion here: http://www.spinics.net/lists/devicetree/msg47806.html)
>>>
>>> On 05 Sep 11:11 PM, Greg Ungerer wrote:
>>>> On 05/09/14 17:36, Geert Uytterhoeven wrote:
>>>>> No, the other way around (from most-specific to least-specific):
>>>>>
>>>>>     compatible = "marvell,armada-spi", "marvell,orion-spi"
>>>>>
>>>>> So the Armada version is used if available, and the Orion version
>>>>> is used as a fallback.
>>> Since you are doing another spin, please split this in two patches.
>>> The driver and devicetree binding documentation should be in one patch,
>>> and the devicetree .dtsi change in another patch.
>>>
>>> By following Geert's request about the compatible, you make sure both
>>> patches can be merged through different maintainers, and that an old devicetree
>>> blob can be used safely.
>>
>> Sure, yes, I would prefer that too. I didn't do it for this because
>> checkpatch
>> complained for the spi-orion.c change in its own:
>>
>> WARNING: DT compatible string "marvell,armada-spi" appears un-documented --
>> check ./Documentation/devicetree/bindings/
>> #170: FILE: drivers/spi/spi-orion.c:412:
>> +    { .compatible = "marvell,armada-spi", .data = &armada_spi_dev_data, },
>>
>> But I am happy to ignore that :-)
>>
> 
> If you split in two patches like this:
> 
> Patch #1: "spi: orion: Add a new foo feature"
> 
>  .../devicetree/bindings/spi/spi-orion.txt          |   2 +-
>  drivers/spi/spi-orion.c                            | 116 +++++++++++++++++----
> 
> Patch #2: "ARM: mvebu: Add new compatible string for SPI"
> 
>  arch/arm/boot/dts/armada-370-xp.dtsi               |   4 +-
> 
> Then checkpatch shouldn't complain. As you can see, it makes more sense like
> this. The change to the devicetree binding in the driver and properly
> documented, but the devicetree files per-se, get modified in another patch.

Ok, I see. Thanks.

Regards
Greg



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

Patch

diff --git a/Documentation/devicetree/bindings/spi/spi-orion.txt b/Documentation/devicetree/bindings/spi/spi-orion.txt
index a3ff50f..674b64f 100644
--- a/Documentation/devicetree/bindings/spi/spi-orion.txt
+++ b/Documentation/devicetree/bindings/spi/spi-orion.txt
@@ -1,7 +1,7 @@ 
 Marvell Orion SPI device
 
 Required properties:
-- compatible : should be "marvell,orion-spi".
+- compatible : should be "marvell,orion-spi" or "marvell,armada-spi".
 - reg : offset and length of the register set for the device
 - cell-index : Which of multiple SPI controllers is this.
 Optional properties:
diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi
index 23227e0..d2a9f50 100644
--- a/arch/arm/boot/dts/armada-370-xp.dtsi
+++ b/arch/arm/boot/dts/armada-370-xp.dtsi
@@ -110,7 +110,7 @@ 
 			};
 
 			spi0: spi@10600 {
-				compatible = "marvell,orion-spi";
+				compatible = "marvell,armada-spi";
 				reg = <0x10600 0x28>;
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -121,7 +121,7 @@ 
 			};
 
 			spi1: spi@10680 {
-				compatible = "marvell,orion-spi";
+				compatible = "marvell,armada-spi";
 				reg = <0x10680 0x28>;
 				#address-cells = <1>;
 				#size-cells = <0>;
diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index c4675fa..566747f 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -18,6 +18,7 @@ 
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/clk.h>
 #include <linux/sizes.h>
 #include <asm/unaligned.h>
@@ -40,13 +41,27 @@ 
 #define ORION_SPI_MODE_CPHA		(1 << 12)
 #define ORION_SPI_IF_8_16_BIT_MODE	(1 << 5)
 #define ORION_SPI_CLK_PRESCALE_MASK	0x1F
+#define ARMADA_SPI_CLK_PRESCALE_MASK	0xDF
 #define ORION_SPI_MODE_MASK		(ORION_SPI_MODE_CPOL | \
 					 ORION_SPI_MODE_CPHA)
 
+enum orion_spi_type {
+	ORION_SPI,
+	ARMADA_SPI,
+};
+
+struct orion_spi_dev {
+	enum orion_spi_type	typ;
+	unsigned int		min_divisor;
+	unsigned int		max_divisor;
+	u32			prescale_mask;
+};
+
 struct orion_spi {
 	struct spi_master	*master;
 	void __iomem		*base;
 	struct clk              *clk;
+	const struct orion_spi_dev *devdata;
 };
 
 static inline void __iomem *spi_reg(struct orion_spi *orion_spi, u32 reg)
@@ -83,30 +98,66 @@  static int orion_spi_baudrate_set(struct spi_device *spi, unsigned int speed)
 	u32 prescale;
 	u32 reg;
 	struct orion_spi *orion_spi;
+	const struct orion_spi_dev *devdata;
 
 	orion_spi = spi_master_get_devdata(spi->master);
+	devdata = orion_spi->devdata;
 
 	tclk_hz = clk_get_rate(orion_spi->clk);
 
-	/*
-	 * the supported rates are: 4,6,8...30
-	 * round up as we look for equal or less speed
-	 */
-	rate = DIV_ROUND_UP(tclk_hz, speed);
-	rate = roundup(rate, 2);
+	if (devdata->typ == ARMADA_SPI) {
+		unsigned int clk, spr, sppr, sppr2, err;
+		unsigned int best_spr, best_sppr, best_err;
+
+		best_err = speed;
+		best_spr = 0;
+		best_sppr = 0;
+
+		/* Iterate over the valid range looking for best fit */
+		for (sppr = 0; sppr < 8; sppr++) {
+			sppr2 = 0x1 << sppr;
+
+			spr = tclk_hz / sppr2;
+			spr = DIV_ROUND_UP(spr, speed);
+			if ((spr == 0) || (spr > 15))
+				continue;
 
-	/* check if requested speed is too small */
-	if (rate > 30)
-		return -EINVAL;
+			clk = tclk_hz / (spr * sppr2);
+			err = speed - clk;
 
-	if (rate < 4)
-		rate = 4;
+			if (err < best_err) {
+				best_spr = spr;
+				best_sppr = sppr;
+				best_err = err;
+			}
+		}
+
+		if ((best_sppr == 0) && (best_spr == 0))
+			return -EINVAL;
+
+		prescale = ((best_sppr & 0x6) << 5) |
+			((best_sppr & 0x1) << 4) | best_spr;
+	} else {
+		/*
+		 * the supported rates are: 4,6,8...30
+		 * round up as we look for equal or less speed
+		 */
+		rate = DIV_ROUND_UP(tclk_hz, speed);
+		rate = roundup(rate, 2);
+
+		/* check if requested speed is too small */
+		if (rate > 30)
+			return -EINVAL;
 
-	/* Convert the rate to SPI clock divisor value.	*/
-	prescale = 0x10 + rate/2;
+		if (rate < 4)
+			rate = 4;
+
+		/* Convert the rate to SPI clock divisor value.	*/
+		prescale = 0x10 + rate/2;
+	}
 
 	reg = readl(spi_reg(orion_spi, ORION_SPI_IF_CONFIG_REG));
-	reg = ((reg & ~ORION_SPI_CLK_PRESCALE_MASK) | prescale);
+	reg = ((reg & ~devdata->prescale_mask) | prescale);
 	writel(reg, spi_reg(orion_spi, ORION_SPI_IF_CONFIG_REG));
 
 	return 0;
@@ -342,8 +393,31 @@  static int orion_spi_reset(struct orion_spi *orion_spi)
 	return 0;
 }
 
+static const struct orion_spi_dev orion_spi_dev_data = {
+	.typ = ORION_SPI,
+	.min_divisor = 4,
+	.max_divisor = 30,
+	.prescale_mask = ORION_SPI_CLK_PRESCALE_MASK,
+};
+
+static const struct orion_spi_dev armada_spi_dev_data = {
+	.typ = ARMADA_SPI,
+	.min_divisor = 1,
+	.max_divisor = 1920,
+	.prescale_mask = ARMADA_SPI_CLK_PRESCALE_MASK,
+};
+
+static const struct of_device_id orion_spi_of_match_table[] = {
+	{ .compatible = "marvell,orion-spi", .data = &orion_spi_dev_data, },
+	{ .compatible = "marvell,armada-spi", .data = &armada_spi_dev_data, },
+	{}
+};
+MODULE_DEVICE_TABLE(of, orion_spi_of_match_table);
+
 static int orion_spi_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id;
+	const struct orion_spi_dev *devdata;
 	struct spi_master *master;
 	struct orion_spi *spi;
 	struct resource *r;
@@ -378,6 +452,10 @@  static int orion_spi_probe(struct platform_device *pdev)
 	spi = spi_master_get_devdata(master);
 	spi->master = master;
 
+	of_id = of_match_device(orion_spi_of_match_table, &pdev->dev);
+	devdata = of_id->data;
+	spi->devdata = devdata;
+
 	spi->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(spi->clk)) {
 		status = PTR_ERR(spi->clk);
@@ -389,8 +467,8 @@  static int orion_spi_probe(struct platform_device *pdev)
 		goto out;
 
 	tclk_hz = clk_get_rate(spi->clk);
-	master->max_speed_hz = DIV_ROUND_UP(tclk_hz, 4);
-	master->min_speed_hz = DIV_ROUND_UP(tclk_hz, 30);
+	master->max_speed_hz = DIV_ROUND_UP(tclk_hz, devdata->min_divisor);
+	master->min_speed_hz = DIV_ROUND_UP(tclk_hz, devdata->max_divisor);
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	spi->base = devm_ioremap_resource(&pdev->dev, r);
@@ -469,12 +547,6 @@  static const struct dev_pm_ops orion_spi_pm_ops = {
 			   NULL)
 };
 
-static const struct of_device_id orion_spi_of_match_table[] = {
-	{ .compatible = "marvell,orion-spi", },
-	{}
-};
-MODULE_DEVICE_TABLE(of, orion_spi_of_match_table);
-
 static struct platform_driver orion_spi_driver = {
 	.driver = {
 		.name	= DRIVER_NAME,