openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH linux dev-4.17 v2 0/2] mtd: spi-nor: aspeed: introduce a "spi-max-frequency" property
@ 2018-06-22  7:09 Cédric Le Goater
  2018-06-22  7:09 ` [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add " Cédric Le Goater
  2018-06-22  7:09 ` [PATCH linux dev-4.17 v2 2/2] mtd: spi-nor: aspeed: limit the maximum SPI frequency Cédric Le Goater
  0 siblings, 2 replies; 9+ messages in thread
From: Cédric Le Goater @ 2018-06-22  7:09 UTC (permalink / raw)
  To: openbmc; +Cc: Andrew Jeffery, Joel Stanley, Cédric Le Goater

Hello,

The "spi-max-frequency" property protects the driver from some
optimistic settings of the optimize read algo for certain chips (old
ones). It also makes the read access to the alternate FMC chip 3 times
faster.

Thanks,

C.

Changes since v1:

 - moved to the dev-4.17 branch
 - introduced the optimize read algo on the AST2400 SoC FMC controller.

Cédric Le Goater (2):
  ARM: dts: aspeed: Add "spi-max-frequency" property
  mtd: spi-nor: aspeed: limit the maximum SPI frequency

 drivers/mtd/spi-nor/aspeed-smc.c                 | 17 +++++++++++++----
 arch/arm/boot/dts/aspeed-ast2500-evb.dts         |  2 ++
 arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts    |  2 ++
 arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts     |  2 ++
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts |  3 +++
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       |  2 ++
 arch/arm/boot/dts/aspeed-g4.dtsi                 |  2 ++
 arch/arm/boot/dts/aspeed-g5.dtsi                 |  7 +++++++
 8 files changed, 33 insertions(+), 4 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add "spi-max-frequency" property
  2018-06-22  7:09 [PATCH linux dev-4.17 v2 0/2] mtd: spi-nor: aspeed: introduce a "spi-max-frequency" property Cédric Le Goater
@ 2018-06-22  7:09 ` Cédric Le Goater
  2018-06-24 23:32   ` Andrew Jeffery
  2018-06-25  3:40   ` Joel Stanley
  2018-06-22  7:09 ` [PATCH linux dev-4.17 v2 2/2] mtd: spi-nor: aspeed: limit the maximum SPI frequency Cédric Le Goater
  1 sibling, 2 replies; 9+ messages in thread
From: Cédric Le Goater @ 2018-06-22  7:09 UTC (permalink / raw)
  To: openbmc; +Cc: Andrew Jeffery, Joel Stanley, Cédric Le Goater

Keep the FMC controller chips at a safe 50 MHz rate and use 100 MHz
for the PNOR on the machines using a AST2500 SoC.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/arm/boot/dts/aspeed-ast2500-evb.dts         | 2 ++
 arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts    | 2 ++
 arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts     | 2 ++
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 3 +++
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 2 ++
 arch/arm/boot/dts/aspeed-g4.dtsi                 | 2 ++
 arch/arm/boot/dts/aspeed-g5.dtsi                 | 7 +++++++
 7 files changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
index c0a7f51e7eb6..6b78e40d3259 100644
--- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
@@ -41,6 +41,7 @@
 		status = "okay";
 		m25p,fast-read;
 		label = "bmc";
+		spi-max-frequency = <50000000>;
 #include "openbmc-flash-layout.dtsi"
 	};
 };
@@ -51,6 +52,7 @@
 		status = "okay";
 		m25p,fast-read;
 		label = "pnor";
+		spi-max-frequency = <100000000>;
 	};
 };
 
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
index e6095f51ecf5..af41973a3882 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
@@ -66,6 +66,7 @@
 		status = "okay";
 		m25p,fast-read;
 		label = "bmc";
+		spi-max-frequency = <50000000>;
 #include "openbmc-flash-layout.dtsi"
 	};
 };
@@ -78,6 +79,7 @@
 	flash@0 {
 		status = "okay";
 		m25p,fast-read;
+		spi-max-frequency = <50000000>;
 		label = "pnor";
 	};
 };
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
index 347938673c83..ffe0c991d985 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
@@ -93,6 +93,7 @@
 		status = "okay";
 		m25p,fast-read;
 		label = "bmc";
+		spi-max-frequency = <50000000>;
 #include "openbmc-flash-layout.dtsi"
 	};
 };
@@ -106,6 +107,7 @@
 		status = "okay";
 		m25p,fast-read;
 		label = "pnor";
+		spi-max-frequency = <100000000>;
 	};
 };
 
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index d05ace220a09..c51e3e8ece62 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -190,6 +190,7 @@
 		status = "okay";
 		label = "bmc";
 		m25p,fast-read;
+		spi-max-frequency = <50000000>;
 #include "openbmc-flash-layout.dtsi"
 	};
 
@@ -197,6 +198,7 @@
 		status = "okay";
 		label = "alt";
 		m25p,fast-read;
+		spi-max-frequency = <50000000>;
 	};
 };
 
@@ -209,6 +211,7 @@
 		status = "okay";
 		label = "pnor";
 		m25p,fast-read;
+		spi-max-frequency = <100000000>;
 	};
 };
 
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index 80cc7cba163c..757d6b3eb041 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -124,6 +124,7 @@
 		status = "okay";
 		label = "bmc";
 		m25p,fast-read;
+		spi-max-frequency = <50000000>;
 #include "openbmc-flash-layout.dtsi"
 	};
 };
@@ -137,6 +138,7 @@
 		status = "okay";
 		label = "pnor";
 		m25p,fast-read;
+		spi-max-frequency = <100000000>;
 	};
 };
 
diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index 79257bf415a8..e526f54f400e 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -65,6 +65,7 @@
 			flash@0 {
 				reg = < 0 >;
 				compatible = "jedec,spi-nor";
+				spi-max-frequency = <50000000>;
 				status = "disabled";
 			};
 		};
@@ -80,6 +81,7 @@
 			flash@0 {
 				reg = < 0 >;
 				compatible = "jedec,spi-nor";
+				spi-max-frequency = <50000000>;
 				status = "disabled";
 			};
 		};
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 9cc50551c42e..afd33112c329 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -65,16 +65,19 @@
 			flash@0 {
 				reg = < 0 >;
 				compatible = "jedec,spi-nor";
+				spi-max-frequency = <50000000>;
 				status = "disabled";
 			};
 			flash@1 {
 				reg = < 1 >;
 				compatible = "jedec,spi-nor";
+				spi-max-frequency = <50000000>;
 				status = "disabled";
 			};
 			flash@2 {
 				reg = < 2 >;
 				compatible = "jedec,spi-nor";
+				spi-max-frequency = <50000000>;
 				status = "disabled";
 			};
 		};
@@ -90,11 +93,13 @@
 			flash@0 {
 				reg = < 0 >;
 				compatible = "jedec,spi-nor";
+				spi-max-frequency = <50000000>;
 				status = "disabled";
 			};
 			flash@1 {
 				reg = < 1 >;
 				compatible = "jedec,spi-nor";
+				spi-max-frequency = <50000000>;
 				status = "disabled";
 			};
 		};
@@ -110,11 +115,13 @@
 			flash@0 {
 				reg = < 0 >;
 				compatible = "jedec,spi-nor";
+				spi-max-frequency = <50000000>;
 				status = "disabled";
 			};
 			flash@1 {
 				reg = < 1 >;
 				compatible = "jedec,spi-nor";
+				spi-max-frequency = <50000000>;
 				status = "disabled";
 			};
 		};
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH linux dev-4.17 v2 2/2] mtd: spi-nor: aspeed: limit the maximum SPI frequency
  2018-06-22  7:09 [PATCH linux dev-4.17 v2 0/2] mtd: spi-nor: aspeed: introduce a "spi-max-frequency" property Cédric Le Goater
  2018-06-22  7:09 ` [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add " Cédric Le Goater
@ 2018-06-22  7:09 ` Cédric Le Goater
  2018-06-24 23:34   ` Andrew Jeffery
  1 sibling, 1 reply; 9+ messages in thread
From: Cédric Le Goater @ 2018-06-22  7:09 UTC (permalink / raw)
  To: openbmc; +Cc: Andrew Jeffery, Joel Stanley, Cédric Le Goater

The optimize read algo can choose a 100MHz SPI frequency which might
be a bit too high for dual output IO on some chips, for the W25Q256 on
palmetto for instance. The MX66L1G45G on witherspoon should be fine
though. Also, the second chip of the FMC controller does not get any
optimize settings for reads. Only the first is configured by U-Boot.

To fix these two issues, we introduce a "spi-max-frequency" property
in the device tree which will be used to cap the optimize read
algorithm and we run the algo on the FMC controller chips as well.

By default, the frequency setting is 50MHz.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 drivers/mtd/spi-nor/aspeed-smc.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
index a301895d1f06..c9cd20f199d9 100644
--- a/drivers/mtd/spi-nor/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/aspeed-smc.c
@@ -62,6 +62,7 @@ static const struct aspeed_smc_info fmc_2400_info = {
 	.ctl0 = 0x10,
 	.timing = 0x94,
 	.set_4b = aspeed_smc_chip_set_4b,
+	.optimize_read = aspeed_smc_optimize_read,
 };
 
 static const struct aspeed_smc_info spi_2400_info = {
@@ -83,6 +84,7 @@ static const struct aspeed_smc_info fmc_2500_info = {
 	.ctl0 = 0x10,
 	.timing = 0x94,
 	.set_4b = aspeed_smc_chip_set_4b,
+	.optimize_read = aspeed_smc_optimize_read,
 };
 
 static const struct aspeed_smc_info spi_2500_info = {
@@ -114,6 +116,7 @@ struct aspeed_smc_chip {
 	u32 ctl_val[smc_max];			/* control settings */
 	enum aspeed_smc_flash_type type;	/* what type of flash */
 	struct spi_nor nor;
+	u32 clk_rate;
 };
 
 struct aspeed_smc_controller {
@@ -130,6 +133,8 @@ struct aspeed_smc_controller {
 	struct aspeed_smc_chip *chips[0];	/* pointers to attached chips */
 };
 
+#define ASPEED_SPI_DEFAULT_FREQ		50000000
+
 /*
  * SPI Flash Configuration Register (AST2500 SPI)
  *     or
@@ -993,11 +998,8 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
 	dev_info(controller->dev, "read control register: %08x\n",
 		chip->ctl_val[smc_read]);
 
-	/*
-	 * TODO: get max freq from chip
-	 */
 	if (optimize_read && info->optimize_read)
-		info->optimize_read(chip, 104000000);
+		info->optimize_read(chip, chip->clk_rate);
 	return 0;
 }
 
@@ -1051,6 +1053,13 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 			break;
 		}
 
+		if (of_property_read_u32(child, "spi-max-frequency",
+					 &chip->clk_rate)) {
+			chip->clk_rate = ASPEED_SPI_DEFAULT_FREQ;
+		}
+		dev_info(dev, "Using %d MHz SPI frequency\n",
+			 chip->clk_rate / 1000000);
+
 		chip->controller = controller;
 		chip->ctl = controller->regs + info->ctl0 + cs * 4;
 		chip->cs = cs;
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add "spi-max-frequency" property
  2018-06-22  7:09 ` [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add " Cédric Le Goater
@ 2018-06-24 23:32   ` Andrew Jeffery
  2018-06-25  3:40   ` Joel Stanley
  1 sibling, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2018-06-24 23:32 UTC (permalink / raw)
  To: Cédric Le Goater, openbmc

On Fri, 22 Jun 2018, at 16:39, Cédric Le Goater wrote:
> Keep the FMC controller chips at a safe 50 MHz rate and use 100 MHz
> for the PNOR on the machines using a AST2500 SoC.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  arch/arm/boot/dts/aspeed-ast2500-evb.dts         | 2 ++
>  arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts    | 2 ++
>  arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts     | 2 ++
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 3 +++
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 2 ++
>  arch/arm/boot/dts/aspeed-g4.dtsi                 | 2 ++
>  arch/arm/boot/dts/aspeed-g5.dtsi                 | 7 +++++++
>  7 files changed, 20 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/
> dts/aspeed-ast2500-evb.dts
> index c0a7f51e7eb6..6b78e40d3259 100644
> --- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
> +++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
> @@ -41,6 +41,7 @@
>  		status = "okay";
>  		m25p,fast-read;
>  		label = "bmc";
> +		spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>  	};
>  };
> @@ -51,6 +52,7 @@
>  		status = "okay";
>  		m25p,fast-read;
>  		label = "pnor";
> +		spi-max-frequency = <100000000>;
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/
> boot/dts/aspeed-bmc-opp-palmetto.dts
> index e6095f51ecf5..af41973a3882 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> @@ -66,6 +66,7 @@
>  		status = "okay";
>  		m25p,fast-read;
>  		label = "bmc";
> +		spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>  	};
>  };
> @@ -78,6 +79,7 @@
>  	flash@0 {
>  		status = "okay";
>  		m25p,fast-read;
> +		spi-max-frequency = <50000000>;
>  		label = "pnor";
>  	};
>  };
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/
> boot/dts/aspeed-bmc-opp-romulus.dts
> index 347938673c83..ffe0c991d985 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> @@ -93,6 +93,7 @@
>  		status = "okay";
>  		m25p,fast-read;
>  		label = "bmc";
> +		spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>  	};
>  };
> @@ -106,6 +107,7 @@
>  		status = "okay";
>  		m25p,fast-read;
>  		label = "pnor";
> +		spi-max-frequency = <100000000>;
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/
> arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index d05ace220a09..c51e3e8ece62 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -190,6 +190,7 @@
>  		status = "okay";
>  		label = "bmc";
>  		m25p,fast-read;
> +		spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>  	};
>  
> @@ -197,6 +198,7 @@
>  		status = "okay";
>  		label = "alt";
>  		m25p,fast-read;
> +		spi-max-frequency = <50000000>;
>  	};
>  };
>  
> @@ -209,6 +211,7 @@
>  		status = "okay";
>  		label = "pnor";
>  		m25p,fast-read;
> +		spi-max-frequency = <100000000>;
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/
> dts/aspeed-bmc-opp-zaius.dts
> index 80cc7cba163c..757d6b3eb041 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -124,6 +124,7 @@
>  		status = "okay";
>  		label = "bmc";
>  		m25p,fast-read;
> +		spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>  	};
>  };
> @@ -137,6 +138,7 @@
>  		status = "okay";
>  		label = "pnor";
>  		m25p,fast-read;
> +		spi-max-frequency = <100000000>;
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
> index 79257bf415a8..e526f54f400e 100644
> --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> @@ -65,6 +65,7 @@
>  			flash@0 {
>  				reg = < 0 >;
>  				compatible = "jedec,spi-nor";
> +				spi-max-frequency = <50000000>;
>  				status = "disabled";
>  			};
>  		};
> @@ -80,6 +81,7 @@
>  			flash@0 {
>  				reg = < 0 >;
>  				compatible = "jedec,spi-nor";
> +				spi-max-frequency = <50000000>;
>  				status = "disabled";
>  			};
>  		};
> diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
> index 9cc50551c42e..afd33112c329 100644
> --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> @@ -65,16 +65,19 @@
>  			flash@0 {
>  				reg = < 0 >;
>  				compatible = "jedec,spi-nor";
> +				spi-max-frequency = <50000000>;
>  				status = "disabled";
>  			};
>  			flash@1 {
>  				reg = < 1 >;
>  				compatible = "jedec,spi-nor";
> +				spi-max-frequency = <50000000>;
>  				status = "disabled";
>  			};
>  			flash@2 {
>  				reg = < 2 >;
>  				compatible = "jedec,spi-nor";
> +				spi-max-frequency = <50000000>;
>  				status = "disabled";
>  			};
>  		};
> @@ -90,11 +93,13 @@
>  			flash@0 {
>  				reg = < 0 >;
>  				compatible = "jedec,spi-nor";
> +				spi-max-frequency = <50000000>;
>  				status = "disabled";
>  			};
>  			flash@1 {
>  				reg = < 1 >;
>  				compatible = "jedec,spi-nor";
> +				spi-max-frequency = <50000000>;
>  				status = "disabled";
>  			};
>  		};
> @@ -110,11 +115,13 @@
>  			flash@0 {
>  				reg = < 0 >;
>  				compatible = "jedec,spi-nor";
> +				spi-max-frequency = <50000000>;
>  				status = "disabled";
>  			};
>  			flash@1 {
>  				reg = < 1 >;
>  				compatible = "jedec,spi-nor";
> +				spi-max-frequency = <50000000>;
>  				status = "disabled";
>  			};
>  		};
> -- 
> 2.13.6
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH linux dev-4.17 v2 2/2] mtd: spi-nor: aspeed: limit the maximum SPI frequency
  2018-06-22  7:09 ` [PATCH linux dev-4.17 v2 2/2] mtd: spi-nor: aspeed: limit the maximum SPI frequency Cédric Le Goater
@ 2018-06-24 23:34   ` Andrew Jeffery
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2018-06-24 23:34 UTC (permalink / raw)
  To: Cédric Le Goater, openbmc

On Fri, 22 Jun 2018, at 16:39, Cédric Le Goater wrote:
> The optimize read algo can choose a 100MHz SPI frequency which might
> be a bit too high for dual output IO on some chips, for the W25Q256 on
> palmetto for instance. The MX66L1G45G on witherspoon should be fine
> though. Also, the second chip of the FMC controller does not get any
> optimize settings for reads. Only the first is configured by U-Boot.
> 
> To fix these two issues, we introduce a "spi-max-frequency" property
> in the device tree which will be used to cap the optimize read
> algorithm and we run the algo on the FMC controller chips as well.
> 
> By default, the frequency setting is 50MHz.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  drivers/mtd/spi-nor/aspeed-smc.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
> index a301895d1f06..c9cd20f199d9 100644
> --- a/drivers/mtd/spi-nor/aspeed-smc.c
> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
> @@ -62,6 +62,7 @@ static const struct aspeed_smc_info fmc_2400_info = {
>  	.ctl0 = 0x10,
>  	.timing = 0x94,
>  	.set_4b = aspeed_smc_chip_set_4b,
> +	.optimize_read = aspeed_smc_optimize_read,
>  };
>  
>  static const struct aspeed_smc_info spi_2400_info = {
> @@ -83,6 +84,7 @@ static const struct aspeed_smc_info fmc_2500_info = {
>  	.ctl0 = 0x10,
>  	.timing = 0x94,
>  	.set_4b = aspeed_smc_chip_set_4b,
> +	.optimize_read = aspeed_smc_optimize_read,
>  };
>  
>  static const struct aspeed_smc_info spi_2500_info = {
> @@ -114,6 +116,7 @@ struct aspeed_smc_chip {
>  	u32 ctl_val[smc_max];			/* control settings */
>  	enum aspeed_smc_flash_type type;	/* what type of flash */
>  	struct spi_nor nor;
> +	u32 clk_rate;
>  };
>  
>  struct aspeed_smc_controller {
> @@ -130,6 +133,8 @@ struct aspeed_smc_controller {
>  	struct aspeed_smc_chip *chips[0];	/* pointers to attached chips */
>  };
>  
> +#define ASPEED_SPI_DEFAULT_FREQ		50000000
> +
>  /*
>   * SPI Flash Configuration Register (AST2500 SPI)
>   *     or
> @@ -993,11 +998,8 @@ static int aspeed_smc_chip_setup_finish(struct 
> aspeed_smc_chip *chip)
>  	dev_info(controller->dev, "read control register: %08x\n",
>  		chip->ctl_val[smc_read]);
>  
> -	/*
> -	 * TODO: get max freq from chip
> -	 */
>  	if (optimize_read && info->optimize_read)
> -		info->optimize_read(chip, 104000000);
> +		info->optimize_read(chip, chip->clk_rate);
>  	return 0;
>  }
>  
> @@ -1051,6 +1053,13 @@ static int aspeed_smc_setup_flash(struct 
> aspeed_smc_controller *controller,
>  			break;
>  		}
>  
> +		if (of_property_read_u32(child, "spi-max-frequency",
> +					 &chip->clk_rate)) {
> +			chip->clk_rate = ASPEED_SPI_DEFAULT_FREQ;
> +		}
> +		dev_info(dev, "Using %d MHz SPI frequency\n",
> +			 chip->clk_rate / 1000000);
> +
>  		chip->controller = controller;
>  		chip->ctl = controller->regs + info->ctl0 + cs * 4;
>  		chip->cs = cs;
> -- 
> 2.13.6
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add "spi-max-frequency" property
  2018-06-22  7:09 ` [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add " Cédric Le Goater
  2018-06-24 23:32   ` Andrew Jeffery
@ 2018-06-25  3:40   ` Joel Stanley
  2018-06-25  4:53     ` Cédric Le Goater
  1 sibling, 1 reply; 9+ messages in thread
From: Joel Stanley @ 2018-06-25  3:40 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: OpenBMC Maillist, Andrew Jeffery

Hi Cedric,

On 22 June 2018 at 16:39, Cédric Le Goater <clg@kaod.org> wrote:
> Keep the FMC controller chips at a safe 50 MHz rate and use 100 MHz
> for the PNOR on the machines using a AST2500 SoC.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  arch/arm/boot/dts/aspeed-ast2500-evb.dts         | 2 ++
>  arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts    | 2 ++
>  arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts     | 2 ++
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 3 +++
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 2 ++

The changes look good. I notice you've only modified the openpower
platforms and the evb. Is there a reason we don't do this for all of
the ast2500 boards?

$ grep -l ast2500 arch/arm/boot/dts/aspeed-*.dts
arch/arm/boot/dts/aspeed-ast2500-evb.dts
arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts
arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts
arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts

Cheers,

Joel

>  arch/arm/boot/dts/aspeed-g4.dtsi                 | 2 ++
>  arch/arm/boot/dts/aspeed-g5.dtsi                 | 7 +++++++
>  7 files changed, 20 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
> index c0a7f51e7eb6..6b78e40d3259 100644
> --- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
> +++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
> @@ -41,6 +41,7 @@
>                 status = "okay";
>                 m25p,fast-read;
>                 label = "bmc";
> +               spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>         };
>  };
> @@ -51,6 +52,7 @@
>                 status = "okay";
>                 m25p,fast-read;
>                 label = "pnor";
> +               spi-max-frequency = <100000000>;
>         };
>  };
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> index e6095f51ecf5..af41973a3882 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> @@ -66,6 +66,7 @@
>                 status = "okay";
>                 m25p,fast-read;
>                 label = "bmc";
> +               spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>         };
>  };
> @@ -78,6 +79,7 @@
>         flash@0 {
>                 status = "okay";
>                 m25p,fast-read;
> +               spi-max-frequency = <50000000>;
>                 label = "pnor";
>         };
>  };
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> index 347938673c83..ffe0c991d985 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> @@ -93,6 +93,7 @@
>                 status = "okay";
>                 m25p,fast-read;
>                 label = "bmc";
> +               spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>         };
>  };
> @@ -106,6 +107,7 @@
>                 status = "okay";
>                 m25p,fast-read;
>                 label = "pnor";
> +               spi-max-frequency = <100000000>;
>         };
>  };
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index d05ace220a09..c51e3e8ece62 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -190,6 +190,7 @@
>                 status = "okay";
>                 label = "bmc";
>                 m25p,fast-read;
> +               spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>         };
>
> @@ -197,6 +198,7 @@
>                 status = "okay";
>                 label = "alt";
>                 m25p,fast-read;
> +               spi-max-frequency = <50000000>;
>         };
>  };
>
> @@ -209,6 +211,7 @@
>                 status = "okay";
>                 label = "pnor";
>                 m25p,fast-read;
> +               spi-max-frequency = <100000000>;
>         };
>  };
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index 80cc7cba163c..757d6b3eb041 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -124,6 +124,7 @@
>                 status = "okay";
>                 label = "bmc";
>                 m25p,fast-read;
> +               spi-max-frequency = <50000000>;
>  #include "openbmc-flash-layout.dtsi"
>         };
>  };
> @@ -137,6 +138,7 @@
>                 status = "okay";
>                 label = "pnor";
>                 m25p,fast-read;
> +               spi-max-frequency = <100000000>;
>         };
>  };
>
> diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
> index 79257bf415a8..e526f54f400e 100644
> --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> @@ -65,6 +65,7 @@
>                         flash@0 {
>                                 reg = < 0 >;
>                                 compatible = "jedec,spi-nor";
> +                               spi-max-frequency = <50000000>;
>                                 status = "disabled";
>                         };
>                 };
> @@ -80,6 +81,7 @@
>                         flash@0 {
>                                 reg = < 0 >;
>                                 compatible = "jedec,spi-nor";
> +                               spi-max-frequency = <50000000>;
>                                 status = "disabled";
>                         };
>                 };
> diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
> index 9cc50551c42e..afd33112c329 100644
> --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> @@ -65,16 +65,19 @@
>                         flash@0 {
>                                 reg = < 0 >;
>                                 compatible = "jedec,spi-nor";
> +                               spi-max-frequency = <50000000>;
>                                 status = "disabled";
>                         };
>                         flash@1 {
>                                 reg = < 1 >;
>                                 compatible = "jedec,spi-nor";
> +                               spi-max-frequency = <50000000>;
>                                 status = "disabled";
>                         };
>                         flash@2 {
>                                 reg = < 2 >;
>                                 compatible = "jedec,spi-nor";
> +                               spi-max-frequency = <50000000>;
>                                 status = "disabled";
>                         };
>                 };
> @@ -90,11 +93,13 @@
>                         flash@0 {
>                                 reg = < 0 >;
>                                 compatible = "jedec,spi-nor";
> +                               spi-max-frequency = <50000000>;
>                                 status = "disabled";
>                         };
>                         flash@1 {
>                                 reg = < 1 >;
>                                 compatible = "jedec,spi-nor";
> +                               spi-max-frequency = <50000000>;
>                                 status = "disabled";
>                         };
>                 };
> @@ -110,11 +115,13 @@
>                         flash@0 {
>                                 reg = < 0 >;
>                                 compatible = "jedec,spi-nor";
> +                               spi-max-frequency = <50000000>;
>                                 status = "disabled";
>                         };
>                         flash@1 {
>                                 reg = < 1 >;
>                                 compatible = "jedec,spi-nor";
> +                               spi-max-frequency = <50000000>;
>                                 status = "disabled";
>                         };
>                 };
> --
> 2.13.6
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add "spi-max-frequency" property
  2018-06-25  3:40   ` Joel Stanley
@ 2018-06-25  4:53     ` Cédric Le Goater
  2018-06-25  5:01       ` Joel Stanley
  0 siblings, 1 reply; 9+ messages in thread
From: Cédric Le Goater @ 2018-06-25  4:53 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist, Andrew Jeffery

On 06/25/2018 05:40 AM, Joel Stanley wrote:
> Hi Cedric,
> 
> On 22 June 2018 at 16:39, Cédric Le Goater <clg@kaod.org> wrote:
>> Keep the FMC controller chips at a safe 50 MHz rate and use 100 MHz
>> for the PNOR on the machines using a AST2500 SoC.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  arch/arm/boot/dts/aspeed-ast2500-evb.dts         | 2 ++
>>  arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts    | 2 ++
>>  arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts     | 2 ++
>>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 3 +++
>>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 2 ++
> 
> The changes look good. I notice you've only modified the openpower
> platforms and the evb. Is there a reason we don't do this for all of
> the ast2500 boards?

When I started working on this patchset, I am not sure these boards
existed yet. Should we just bump the max freq to 100MHz for all ?

Thanks,

C. 

> $ grep -l ast2500 arch/arm/boot/dts/aspeed-*.dts
> arch/arm/boot/dts/aspeed-ast2500-evb.dts
> arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts
> arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts
> arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
> arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts
> 
> Cheers,
> 
> Joel
> 
>>  arch/arm/boot/dts/aspeed-g4.dtsi                 | 2 ++
>>  arch/arm/boot/dts/aspeed-g5.dtsi                 | 7 +++++++
>>  7 files changed, 20 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
>> index c0a7f51e7eb6..6b78e40d3259 100644
>> --- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
>> +++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
>> @@ -41,6 +41,7 @@
>>                 status = "okay";
>>                 m25p,fast-read;
>>                 label = "bmc";
>> +               spi-max-frequency = <50000000>;
>>  #include "openbmc-flash-layout.dtsi"
>>         };
>>  };
>> @@ -51,6 +52,7 @@
>>                 status = "okay";
>>                 m25p,fast-read;
>>                 label = "pnor";
>> +               spi-max-frequency = <100000000>;
>>         };
>>  };
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
>> index e6095f51ecf5..af41973a3882 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
>> @@ -66,6 +66,7 @@
>>                 status = "okay";
>>                 m25p,fast-read;
>>                 label = "bmc";
>> +               spi-max-frequency = <50000000>;
>>  #include "openbmc-flash-layout.dtsi"
>>         };
>>  };
>> @@ -78,6 +79,7 @@
>>         flash@0 {
>>                 status = "okay";
>>                 m25p,fast-read;
>> +               spi-max-frequency = <50000000>;
>>                 label = "pnor";
>>         };
>>  };
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
>> index 347938673c83..ffe0c991d985 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
>> @@ -93,6 +93,7 @@
>>                 status = "okay";
>>                 m25p,fast-read;
>>                 label = "bmc";
>> +               spi-max-frequency = <50000000>;
>>  #include "openbmc-flash-layout.dtsi"
>>         };
>>  };
>> @@ -106,6 +107,7 @@
>>                 status = "okay";
>>                 m25p,fast-read;
>>                 label = "pnor";
>> +               spi-max-frequency = <100000000>;
>>         };
>>  };
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>> index d05ace220a09..c51e3e8ece62 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>> @@ -190,6 +190,7 @@
>>                 status = "okay";
>>                 label = "bmc";
>>                 m25p,fast-read;
>> +               spi-max-frequency = <50000000>;
>>  #include "openbmc-flash-layout.dtsi"
>>         };
>>
>> @@ -197,6 +198,7 @@
>>                 status = "okay";
>>                 label = "alt";
>>                 m25p,fast-read;
>> +               spi-max-frequency = <50000000>;
>>         };
>>  };
>>
>> @@ -209,6 +211,7 @@
>>                 status = "okay";
>>                 label = "pnor";
>>                 m25p,fast-read;
>> +               spi-max-frequency = <100000000>;
>>         };
>>  };
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
>> index 80cc7cba163c..757d6b3eb041 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
>> @@ -124,6 +124,7 @@
>>                 status = "okay";
>>                 label = "bmc";
>>                 m25p,fast-read;
>> +               spi-max-frequency = <50000000>;
>>  #include "openbmc-flash-layout.dtsi"
>>         };
>>  };
>> @@ -137,6 +138,7 @@
>>                 status = "okay";
>>                 label = "pnor";
>>                 m25p,fast-read;
>> +               spi-max-frequency = <100000000>;
>>         };
>>  };
>>
>> diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
>> index 79257bf415a8..e526f54f400e 100644
>> --- a/arch/arm/boot/dts/aspeed-g4.dtsi
>> +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
>> @@ -65,6 +65,7 @@
>>                         flash@0 {
>>                                 reg = < 0 >;
>>                                 compatible = "jedec,spi-nor";
>> +                               spi-max-frequency = <50000000>;
>>                                 status = "disabled";
>>                         };
>>                 };
>> @@ -80,6 +81,7 @@
>>                         flash@0 {
>>                                 reg = < 0 >;
>>                                 compatible = "jedec,spi-nor";
>> +                               spi-max-frequency = <50000000>;
>>                                 status = "disabled";
>>                         };
>>                 };
>> diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
>> index 9cc50551c42e..afd33112c329 100644
>> --- a/arch/arm/boot/dts/aspeed-g5.dtsi
>> +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
>> @@ -65,16 +65,19 @@
>>                         flash@0 {
>>                                 reg = < 0 >;
>>                                 compatible = "jedec,spi-nor";
>> +                               spi-max-frequency = <50000000>;
>>                                 status = "disabled";
>>                         };
>>                         flash@1 {
>>                                 reg = < 1 >;
>>                                 compatible = "jedec,spi-nor";
>> +                               spi-max-frequency = <50000000>;
>>                                 status = "disabled";
>>                         };
>>                         flash@2 {
>>                                 reg = < 2 >;
>>                                 compatible = "jedec,spi-nor";
>> +                               spi-max-frequency = <50000000>;
>>                                 status = "disabled";
>>                         };
>>                 };
>> @@ -90,11 +93,13 @@
>>                         flash@0 {
>>                                 reg = < 0 >;
>>                                 compatible = "jedec,spi-nor";
>> +                               spi-max-frequency = <50000000>;
>>                                 status = "disabled";
>>                         };
>>                         flash@1 {
>>                                 reg = < 1 >;
>>                                 compatible = "jedec,spi-nor";
>> +                               spi-max-frequency = <50000000>;
>>                                 status = "disabled";
>>                         };
>>                 };
>> @@ -110,11 +115,13 @@
>>                         flash@0 {
>>                                 reg = < 0 >;
>>                                 compatible = "jedec,spi-nor";
>> +                               spi-max-frequency = <50000000>;
>>                                 status = "disabled";
>>                         };
>>                         flash@1 {
>>                                 reg = < 1 >;
>>                                 compatible = "jedec,spi-nor";
>> +                               spi-max-frequency = <50000000>;
>>                                 status = "disabled";
>>                         };
>>                 };
>> --
>> 2.13.6
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add "spi-max-frequency" property
  2018-06-25  4:53     ` Cédric Le Goater
@ 2018-06-25  5:01       ` Joel Stanley
  2018-06-25  5:16         ` Cédric Le Goater
  0 siblings, 1 reply; 9+ messages in thread
From: Joel Stanley @ 2018-06-25  5:01 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: OpenBMC Maillist, Andrew Jeffery

On 25 June 2018 at 14:23, Cédric Le Goater <clg@kaod.org> wrote:
> On 06/25/2018 05:40 AM, Joel Stanley wrote:
>> Hi Cedric,
>>
>> On 22 June 2018 at 16:39, Cédric Le Goater <clg@kaod.org> wrote:
>>> Keep the FMC controller chips at a safe 50 MHz rate and use 100 MHz
>>> for the PNOR on the machines using a AST2500 SoC.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>  arch/arm/boot/dts/aspeed-ast2500-evb.dts         | 2 ++
>>>  arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts    | 2 ++
>>>  arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts     | 2 ++
>>>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 3 +++
>>>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 2 ++
>>
>> The changes look good. I notice you've only modified the openpower
>> platforms and the evb. Is there a reason we don't do this for all of
>> the ast2500 boards?
>
> When I started working on this patchset, I am not sure these boards
> existed yet. Should we just bump the max freq to 100MHz for all ?

Yes, I think that's a good option.

What are they symptoms when the speed is increased too much?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add "spi-max-frequency" property
  2018-06-25  5:01       ` Joel Stanley
@ 2018-06-25  5:16         ` Cédric Le Goater
  0 siblings, 0 replies; 9+ messages in thread
From: Cédric Le Goater @ 2018-06-25  5:16 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist, Andrew Jeffery

On 06/25/2018 07:01 AM, Joel Stanley wrote:
> On 25 June 2018 at 14:23, Cédric Le Goater <clg@kaod.org> wrote:
>> On 06/25/2018 05:40 AM, Joel Stanley wrote:
>>> Hi Cedric,
>>>
>>> On 22 June 2018 at 16:39, Cédric Le Goater <clg@kaod.org> wrote:
>>>> Keep the FMC controller chips at a safe 50 MHz rate and use 100 MHz
>>>> for the PNOR on the machines using a AST2500 SoC.
>>>>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>>  arch/arm/boot/dts/aspeed-ast2500-evb.dts         | 2 ++
>>>>  arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts    | 2 ++
>>>>  arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts     | 2 ++
>>>>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 3 +++
>>>>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 2 ++
>>>
>>> The changes look good. I notice you've only modified the openpower
>>> platforms and the evb. Is there a reason we don't do this for all of
>>> the ast2500 boards?
>>
>> When I started working on this patchset, I am not sure these boards
>> existed yet. Should we just bump the max freq to 100MHz for all ?
> 
> Yes, I think that's a good option.
> 
> What are they symptoms when the speed is increased too much?
> 

The reads become unsafe and the result can be full of surprise at 
boot time. The flash won't be corrupted though and we have the 
'aspeed.optimize_read' chicken switch to boot in a slow mode. It 
should be used in the case a really too optimistic setting was 
flashed (DT) on the board. 

I think the freqs should be kept at 50MHz for the FMC controllers, 
unless we know that a board can safely do better. Tests to be done.
For the PNOR, it is less critical. So putting 100MHz should be 
fine in all cases and this is what we have been living with.

C.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-06-25  7:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-22  7:09 [PATCH linux dev-4.17 v2 0/2] mtd: spi-nor: aspeed: introduce a "spi-max-frequency" property Cédric Le Goater
2018-06-22  7:09 ` [PATCH linux dev-4.17 v2 1/2] ARM: dts: aspeed: Add " Cédric Le Goater
2018-06-24 23:32   ` Andrew Jeffery
2018-06-25  3:40   ` Joel Stanley
2018-06-25  4:53     ` Cédric Le Goater
2018-06-25  5:01       ` Joel Stanley
2018-06-25  5:16         ` Cédric Le Goater
2018-06-22  7:09 ` [PATCH linux dev-4.17 v2 2/2] mtd: spi-nor: aspeed: limit the maximum SPI frequency Cédric Le Goater
2018-06-24 23:34   ` Andrew Jeffery

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).