On Fri, Oct 01, 2021 at 11:27:26PM +0200, Michael Walle wrote:
Make the workaround more reliable and just drop the unneeded sysclk
lookup.
For reference, the error during the bootup is the following:
[ 4.898400] nxp-fspi 20c0000.spi: Errata cannot be executed. Read via IP bus may not work
Well, in Kuldeep's defence, at least this part is sane, right? I mean we
cannot prove an issue => we don't disable reads via the AHB. So it's
just the error message (which I didn't notice TBH, sorry).
On the other hand, is anyone using LS1028A with a platform clock of 300 MHz? :)
Fixes: 82ce7d0e74b6 ("spi: spi-nxp-fspi: Implement errata workaround for LS1028A")
Cc: Vladimir Oltean <vladimir.oltean@xxxxxxx>
Signed-off-by: Michael Walle <michael@xxxxxxxx>
---
drivers/spi/spi-nxp-fspi.c | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index a66fa97046ee..2b0301fc971c 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -33,6 +33,7 @@
#include <linux/acpi.h>
#include <linux/bitops.h>
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/delay.h>
@@ -315,6 +316,7 @@
#define NXP_FSPI_MIN_IOMAP SZ_4M
#define DCFG_RCWSR1 0x100
+#define SYS_PLL_RAT GENMASK(6, 2)
Ugh. So your solution still makes a raw read of the platform PLL value
from the DCFG, now it just adds a nice definition for it. Not nice.
/* Access flash memory using IP bus only */
#define FSPI_QUIRK_USE_IP_ONLY BIT(0)
@@ -926,9 +928,8 @@ static void erratum_err050568(struct nxp_fspi *f)
{ .family = "QorIQ LS1028A" },
{ /* sentinel */ }
};
- struct device_node *np;
struct regmap *map;
- u32 val = 0, sysclk = 0;
+ u32 val, sys_pll_ratio;
int ret;
/* Check for LS1028A family */
@@ -937,7 +938,6 @@ static void erratum_err050568(struct nxp_fspi *f)
return;
}
- /* Compute system clock frequency multiplier ratio */
map = syscon_regmap_lookup_by_compatible("fsl,ls1028a-dcfg");
if (IS_ERR(map)) {
dev_err(f->dev, "No syscon regmap\n");
@@ -948,23 +948,11 @@ static void erratum_err050568(struct nxp_fspi *f)
if (ret < 0)
goto err;
- /* Strap bits 6:2 define SYS_PLL_RAT i.e frequency multiplier ratio */
- val = (val >> 2) & 0x1F;
- WARN(val == 0, "Strapping is zero: Cannot determine ratio");
+ sys_pll_ratio = FIELD_GET(SYS_PLL_RAT, val);
+ dev_dbg(f->dev, "val: 0x%08x, sys_pll_ratio: %d\n", val, sys_pll_ratio);
Do we really feel that this dev_dbg is valuable?
- /* Compute system clock frequency */
- np = of_find_node_by_name(NULL, "clock-sysclk");
- if (!np)
- goto err;
-
- if (of_property_read_u32(np, "clock-frequency", &sysclk))
- goto err;
-
- sysclk = (sysclk * val) / 1000000; /* Convert sysclk to Mhz */
- dev_dbg(f->dev, "val: 0x%08x, sysclk: %dMhz\n", val, sysclk);
-
- /* Use IP bus only if PLL is 300MHz */
- if (sysclk == 300)
+ /* Use IP bus only if platform clock is 300MHz */
+ if (sys_pll_ratio == 3)
f->devtype_data->quirks |= FSPI_QUIRK_USE_IP_ONLY;
return;
--
2.30.2
How about:
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 343ecf0e8973..ffe820c22719 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -326,15 +326,17 @@ i2c7: i2c@2070000 {
};
fspi: spi@20c0000 {
- compatible = "nxp,lx2160a-fspi";
+ compatible = "nxp,ls1028a-fspi";
#address-cells = <1>;
#size-cells = <0>;
reg = <0x0 0x20c0000 0x0 0x10000>,
<0x0 0x20000000 0x0 0x10000000>;
reg-names = "fspi_base", "fspi_mmap";
interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&fspi_clk>, <&fspi_clk>;
- clock-names = "fspi_en", "fspi";
+ clocks = <&fspi_clk>, <&fspi_clk>,
+ <&clockgen QORIQ_CLK_PLATFORM_PLL
+ QORIQ_CLK_PLL_DIV(2)>;
+ clock-names = "fspi_en", "fspi", "base";
status = "disabled";
};
diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index a66fa97046ee..f2815e6cae2c 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -314,8 +314,6 @@
#define NXP_FSPI_MAX_CHIPSELECT 4
#define NXP_FSPI_MIN_IOMAP SZ_4M
-#define DCFG_RCWSR1 0x100
-
/* Access flash memory using IP bus only */
#define FSPI_QUIRK_USE_IP_ONLY BIT(0)
@@ -922,55 +920,18 @@ static int nxp_fspi_adjust_op_size(struct
spi_mem *mem, struct spi_mem_op *op)
static void erratum_err050568(struct nxp_fspi *f)
{
- const struct soc_device_attribute ls1028a_soc_attr[] = {
- { .family = "QorIQ LS1028A" },
- { /* sentinel */ }
- };
- struct device_node *np;
- struct regmap *map;
- u32 val = 0, sysclk = 0;
- int ret;
+ struct clk *clk_base;
- /* Check for LS1028A family */
- if (!soc_device_match(ls1028a_soc_attr)) {
- dev_dbg(f->dev, "Errata applicable only for LS1028A\n");
+ clk_base = clk_get(f->dev, "base");
+ if (IS_ERR(clk_base)) {
+ dev_err(f->dev, "Errata cannot be executed. Read via IP bus may not work\n");
return;
}
- /* Compute system clock frequency multiplier ratio */
- map = syscon_regmap_lookup_by_compatible("fsl,ls1028a-dcfg");
- if (IS_ERR(map)) {
- dev_err(f->dev, "No syscon regmap\n");
- goto err;
- }
-
- ret = regmap_read(map, DCFG_RCWSR1, &val);
- if (ret < 0)
- goto err;
-
- /* Strap bits 6:2 define SYS_PLL_RAT i.e frequency multiplier ratio */
- val = (val >> 2) & 0x1F;
- WARN(val == 0, "Strapping is zero: Cannot determine ratio");
-
- /* Compute system clock frequency */
- np = of_find_node_by_name(NULL, "clock-sysclk");
- if (!np)
- goto err;
-
- if (of_property_read_u32(np, "clock-frequency", &sysclk))
- goto err;
-
- sysclk = (sysclk * val) / 1000000; /* Convert sysclk to Mhz */
- dev_dbg(f->dev, "val: 0x%08x, sysclk: %dMhz\n", val, sysclk);
-
- /* Use IP bus only if PLL is 300MHz */
- if (sysclk == 300)
+ /* Use IP bus only if platform PLL is configured for 300 MHz, hence
we get 150 MHz */
+ if (clk_get_rate(clk_base) == 150000000)
f->devtype_data->quirks |= FSPI_QUIRK_USE_IP_ONLY;
-
- return;
-
-err:
- dev_err(f->dev, "Errata cannot be executed. Read via IP bus may not work\n");
+ clk_put(clk_base);
}
static int nxp_fspi_default_setup(struct nxp_fspi *f)
@@ -994,10 +955,8 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f)
/*
* ERR050568: Flash access by FlexSPI AHB command may not work with
* platform frequency equal to 300 MHz on LS1028A.
- * LS1028A reuses LX2160A compatible entry. Make errata applicable for
- * Layerscape LS1028A platform.
*/
- if (of_device_is_compatible(f->dev->of_node, "nxp,lx2160a-fspi"))
+ if (of_device_is_compatible(f->dev->of_node, "nxp,ls1028a-fspi"))
erratum_err050568(f);
/* Reset the module */