All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] sunxi: F1C100s: enable MMC and SPI in U-Boot proper
@ 2022-05-03 21:20 Andre Przywara
  2022-05-03 21:20 ` [PATCH 1/7] clk: sunxi: implement clock driver for suniv f1c100s Andre Przywara
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Andre Przywara @ 2022-05-03 21:20 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

So far the U-Boot proper support for the Allwinner F1C100 family of SoCs
was really limited: we could realistically only deal with FEL booting,
as there was no storage or network device acessible from U-Boot proper.

This series enables the MMC and SPI controller, to be able to actually
load something from the device. This is made possible by the updates
to the devicetree in Linux, which now describes those devices.

The first patch is taken from George's/Yifan's older series, and adds
DM_CLK support for the F1C100s. The DM_PINCTRL support was already added
with the DM conversion a few weeks back.
Patches 2-4 fix the SPI driver clock setup, and add support for the
differing SPI clock on the F1C100.
Patch 5 updates the devicetree files, freshly synced from linux-next.
Patch 6 reverts a hack we introduced back then to fix the reset
functionality, with the DT update this is now no longer needed.
The final patch adds SPI flash support to the Licheepi Nano defconfig,
as these boards mostly ship with SPI flash soldered.

Please have a look and test!

Cheers,
Andre

Andre Przywara (6):
  spi: sunxi: refactor SPI speed/mode programming
  spi: sunxi: improve SPI clock calculation
  spi: sunxi: Add support for F1C100s SPI controller
  sunxi: F1C100s: update DT files from Linux
  Revert "sunxi: f1c100s: Drop SYSRESET to enable reset functionality"
  sunxi: licheepi_nano: enable SPI flash

George Hilliard (1):
  clk: sunxi: implement clock driver for suniv f1c100s

 arch/arm/dts/suniv-f1c100s-licheepi-nano.dts |  31 ++++++
 arch/arm/dts/suniv-f1c100s.dtsi              | 104 ++++++++++++++++--
 configs/licheepi_nano_defconfig              |   4 +-
 drivers/clk/sunxi/Kconfig                    |   7 ++
 drivers/clk/sunxi/Makefile                   |   1 +
 drivers/clk/sunxi/clk_f1c100s.c              |  74 +++++++++++++
 drivers/spi/spi-sunxi.c                      | 109 +++++++++++--------
 7 files changed, 275 insertions(+), 55 deletions(-)
 create mode 100644 drivers/clk/sunxi/clk_f1c100s.c

-- 
2.35.3


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

* [PATCH 1/7] clk: sunxi: implement clock driver for suniv f1c100s
  2022-05-03 21:20 [PATCH 0/7] sunxi: F1C100s: enable MMC and SPI in U-Boot proper Andre Przywara
@ 2022-05-03 21:20 ` Andre Przywara
  2022-05-24 16:10   ` Andre Przywara
  2022-05-03 21:20 ` [PATCH 2/7] spi: sunxi: refactor SPI speed/mode programming Andre Przywara
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Andre Przywara @ 2022-05-03 21:20 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

From: George Hilliard <thirtythreeforty@gmail.com>

The f1c100s has a clock tree similar to those of other sunxi parts.
Add support for it.

Signed-off-by: George Hilliard <thirtythreeforty@gmail.com>
Signed-off-by: Yifan Gu <me@yifangu.com>
Acked-by: Sean Anderson <seanga2@gmail.com>
[Andre: add PIO and I2C]
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/clk/sunxi/Kconfig       |  7 ++++
 drivers/clk/sunxi/Makefile      |  1 +
 drivers/clk/sunxi/clk_f1c100s.c | 74 +++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk_f1c100s.c

diff --git a/drivers/clk/sunxi/Kconfig b/drivers/clk/sunxi/Kconfig
index f19908113e1..bf11fad6eef 100644
--- a/drivers/clk/sunxi/Kconfig
+++ b/drivers/clk/sunxi/Kconfig
@@ -10,6 +10,13 @@ config CLK_SUNXI
 
 if CLK_SUNXI
 
+config CLK_SUNIV_F1C100S
+	bool "Clock driver for Allwinner F1C100s"
+	default MACH_SUNIV
+	help
+	  This enables common clock driver support for platforms based
+	  on Allwinner F1C100s SoC.
+
 config CLK_SUN4I_A10
 	bool "Clock driver for Allwinner A10/A20"
 	default MACH_SUN4I || MACH_SUN7I
diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 48a48a2f000..895da02ebea 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_CLK_SUNXI) += clk_sunxi.o
 
 obj-$(CONFIG_CLK_SUNXI) += clk_sun6i_rtc.o
 
+obj-$(CONFIG_CLK_SUNIV_F1C100S) += clk_f1c100s.o
 obj-$(CONFIG_CLK_SUN4I_A10) += clk_a10.o
 obj-$(CONFIG_CLK_SUN5I_A10S) += clk_a10s.o
 obj-$(CONFIG_CLK_SUN6I_A31) += clk_a31.o
diff --git a/drivers/clk/sunxi/clk_f1c100s.c b/drivers/clk/sunxi/clk_f1c100s.c
new file mode 100644
index 00000000000..72cf8a6e5c0
--- /dev/null
+++ b/drivers/clk/sunxi/clk_f1c100s.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2019 George Hilliard <thirtythreeforty@gmail.com>.
+ */
+
+#include <common.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <errno.h>
+#include <clk/sunxi.h>
+#include <dt-bindings/clock/suniv-ccu-f1c100s.h>
+#include <dt-bindings/reset/suniv-ccu-f1c100s.h>
+
+static struct ccu_clk_gate f1c100s_gates[] = {
+	[CLK_BUS_MMC0]		= GATE(0x060, BIT(8)),
+	[CLK_BUS_MMC1]		= GATE(0x060, BIT(9)),
+	[CLK_BUS_SPI0]		= GATE(0x060, BIT(20)),
+	[CLK_BUS_SPI1]		= GATE(0x060, BIT(21)),
+	[CLK_BUS_OTG]		= GATE(0x060, BIT(24)),
+
+	[CLK_BUS_I2C0]		= GATE(0x068, BIT(16)),
+	[CLK_BUS_I2C1]		= GATE(0x068, BIT(17)),
+	[CLK_BUS_I2C2]		= GATE(0x068, BIT(18)),
+	[CLK_BUS_PIO]		= GATE(0x068, BIT(19)),
+
+	[CLK_BUS_UART0]		= GATE(0x06c, BIT(20)),
+	[CLK_BUS_UART1]		= GATE(0x06c, BIT(21)),
+	[CLK_BUS_UART2]		= GATE(0x06c, BIT(22)),
+
+	[CLK_USB_PHY0]          = GATE(0x0cc, BIT(1)),
+};
+
+static struct ccu_reset f1c100s_resets[] = {
+	[RST_USB_PHY0]		= RESET(0x0cc, BIT(0)),
+
+	[RST_BUS_MMC0]		= RESET(0x2c0, BIT(8)),
+	[RST_BUS_MMC1]		= RESET(0x2c0, BIT(9)),
+	[RST_BUS_SPI0]		= RESET(0x2c0, BIT(20)),
+	[RST_BUS_SPI1]		= RESET(0x2c0, BIT(21)),
+	[RST_BUS_OTG]		= RESET(0x2c0, BIT(24)),
+
+	[RST_BUS_I2C0]		= RESET(0x2d0, BIT(16)),
+	[RST_BUS_I2C1]		= RESET(0x2d0, BIT(17)),
+	[RST_BUS_I2C2]		= RESET(0x2d0, BIT(18)),
+	[RST_BUS_UART0]		= RESET(0x2d0, BIT(20)),
+	[RST_BUS_UART1]		= RESET(0x2d0, BIT(21)),
+	[RST_BUS_UART2]		= RESET(0x2d0, BIT(22)),
+};
+
+static const struct ccu_desc f1c100s_ccu_desc = {
+	.gates = f1c100s_gates,
+	.resets = f1c100s_resets,
+};
+
+static int f1c100s_clk_bind(struct udevice *dev)
+{
+	return sunxi_reset_bind(dev, ARRAY_SIZE(f1c100s_resets));
+}
+
+static const struct udevice_id f1c100s_clk_ids[] = {
+	{ .compatible = "allwinner,suniv-f1c100s-ccu",
+	  .data = (ulong)&f1c100s_ccu_desc },
+	{ }
+};
+
+U_BOOT_DRIVER(clk_suniv_f1c100s) = {
+	.name		= "suniv_f1c100s_ccu",
+	.id		= UCLASS_CLK,
+	.of_match	= f1c100s_clk_ids,
+	.priv_auto	= sizeof(struct ccu_priv),
+	.ops		= &sunxi_clk_ops,
+	.probe		= sunxi_clk_probe,
+	.bind		= f1c100s_clk_bind,
+};
-- 
2.35.3


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

* [PATCH 2/7] spi: sunxi: refactor SPI speed/mode programming
  2022-05-03 21:20 [PATCH 0/7] sunxi: F1C100s: enable MMC and SPI in U-Boot proper Andre Przywara
  2022-05-03 21:20 ` [PATCH 1/7] clk: sunxi: implement clock driver for suniv f1c100s Andre Przywara
@ 2022-05-03 21:20 ` Andre Przywara
  2022-06-28  0:31   ` Andre Przywara
  2022-05-03 21:20 ` [PATCH 3/7] spi: sunxi: improve SPI clock calculation Andre Przywara
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Andre Przywara @ 2022-05-03 21:20 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

As George rightfully pointed out [1], the spi-sunxi driver programs the
speed and mode settings only when the respective functions are called,
but this gets lost over a call to release_bus(). That asserts the
reset line, thus forces each SPI register back to its default value.
Adding to that, trying to program SPI_CCR and SPI_TCR might be pointless
in the first place, when the reset line is still asserted (before
claim_bus()), so those setting won't apply most of the time. In reality
I see two nested claim_bus() calls for the first use, so settings between
the two would work (for instance for the initial "sf probe"). However
later on the speed setting is not programmed into the hardware anymore.

So far we get away with that default frequency, because that is a rather
tame 24 MHz, which most SPI flash chips can handle just fine.

Move the actual register programming into a separate function, and use
.set_speed and .set_mode just to set the variables in our priv structure.
Then we only call this new function in claim_bus(), when we are sure
that register accesses actually work and are preserved.

[1] https://lore.kernel.org/u-boot/20210725231636.879913-17-me@yifangu.com/

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reported-by: George Hilliard <thirtythreeforty@gmail.com>
---
 drivers/spi/spi-sunxi.c | 95 ++++++++++++++++++++++-------------------
 1 file changed, 52 insertions(+), 43 deletions(-)

diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index b6cd7ddafad..d6b2dd09514 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -221,6 +221,56 @@ err_ahb:
 	return ret;
 }
 
+static void sun4i_spi_set_speed_mode(struct udevice *dev)
+{
+	struct sun4i_spi_priv *priv = dev_get_priv(dev);
+	unsigned int div;
+	u32 reg;
+
+	/*
+	 * Setup clock divider.
+	 *
+	 * We have two choices there. Either we can use the clock
+	 * divide rate 1, which is calculated thanks to this formula:
+	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
+	 * Or we can use CDR2, which is calculated with the formula:
+	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
+	 * Whether we use the former or the latter is set through the
+	 * DRS bit.
+	 *
+	 * First try CDR2, and if we can't reach the expected
+	 * frequency, fall back to CDR1.
+	 */
+
+	div = SUN4I_SPI_MAX_RATE / (2 * priv->freq);
+	reg = readl(SPI_REG(priv, SPI_CCR));
+
+	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
+		if (div > 0)
+			div--;
+
+		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
+		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
+	} else {
+		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(priv->freq);
+		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
+		reg |= SUN4I_CLK_CTL_CDR1(div);
+	}
+
+	writel(reg, SPI_REG(priv, SPI_CCR));
+
+	reg = readl(SPI_REG(priv, SPI_TCR));
+	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
+
+	if (priv->mode & SPI_CPOL)
+		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
+
+	if (priv->mode & SPI_CPHA)
+		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
+
+	writel(reg, SPI_REG(priv, SPI_TCR));
+}
+
 static int sun4i_spi_claim_bus(struct udevice *dev)
 {
 	struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
@@ -240,6 +290,8 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
 	setbits_le32(SPI_REG(priv, SPI_TCR), SPI_BIT(priv, SPI_TCR_CS_MANUAL) |
 		     SPI_BIT(priv, SPI_TCR_CS_ACTIVE_LOW));
 
+	sun4i_spi_set_speed_mode(dev->parent);
+
 	return 0;
 }
 
@@ -329,46 +381,14 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
 {
 	struct sun4i_spi_plat *plat = dev_get_plat(dev);
 	struct sun4i_spi_priv *priv = dev_get_priv(dev);
-	unsigned int div;
-	u32 reg;
 
 	if (speed > plat->max_hz)
 		speed = plat->max_hz;
 
 	if (speed < SUN4I_SPI_MIN_RATE)
 		speed = SUN4I_SPI_MIN_RATE;
-	/*
-	 * Setup clock divider.
-	 *
-	 * We have two choices there. Either we can use the clock
-	 * divide rate 1, which is calculated thanks to this formula:
-	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
-	 * Or we can use CDR2, which is calculated with the formula:
-	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
-	 * Whether we use the former or the latter is set through the
-	 * DRS bit.
-	 *
-	 * First try CDR2, and if we can't reach the expected
-	 * frequency, fall back to CDR1.
-	 */
-
-	div = SUN4I_SPI_MAX_RATE / (2 * speed);
-	reg = readl(SPI_REG(priv, SPI_CCR));
-
-	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
-		if (div > 0)
-			div--;
-
-		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
-		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
-	} else {
-		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(speed);
-		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
-		reg |= SUN4I_CLK_CTL_CDR1(div);
-	}
 
 	priv->freq = speed;
-	writel(reg, SPI_REG(priv, SPI_CCR));
 
 	return 0;
 }
@@ -376,19 +396,8 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
 static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
 {
 	struct sun4i_spi_priv *priv = dev_get_priv(dev);
-	u32 reg;
-
-	reg = readl(SPI_REG(priv, SPI_TCR));
-	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
-
-	if (mode & SPI_CPOL)
-		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
-
-	if (mode & SPI_CPHA)
-		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
 
 	priv->mode = mode;
-	writel(reg, SPI_REG(priv, SPI_TCR));
 
 	return 0;
 }
-- 
2.35.3


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

* [PATCH 3/7] spi: sunxi: improve SPI clock calculation
  2022-05-03 21:20 [PATCH 0/7] sunxi: F1C100s: enable MMC and SPI in U-Boot proper Andre Przywara
  2022-05-03 21:20 ` [PATCH 1/7] clk: sunxi: implement clock driver for suniv f1c100s Andre Przywara
  2022-05-03 21:20 ` [PATCH 2/7] spi: sunxi: refactor SPI speed/mode programming Andre Przywara
@ 2022-05-03 21:20 ` Andre Przywara
  2022-05-03 21:20 ` [PATCH 4/7] spi: sunxi: Add support for F1C100s SPI controller Andre Przywara
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Andre Przywara @ 2022-05-03 21:20 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

The current SPI clock divider calculation has two problems:
- We use a normal round-down division, which results in a divider
  typically being too small, resulting in a too high frequency on the bus.
- The calculation for the power-of-two divider is very inaccurate, and
  again rounds down, which might lead to wild bus frequencies.

This wasn't a real problem so far, since most chips can handle slightly
higher bus frequencies just fine. Also the actual speed was mostly lost
anyway, due to release_bus() reseting the device. And the power-of-2
calculation was probably never used, because it only applies to
frequencies below 47 KHz.
However this will become a problem for the F1C100s support, due to its
much higher base frequency.

Calculate a safe divider correctly (using round-up), and re-use that
value when calculating the power-of-2 value. We also separate the
maximum frequency and the input clock on the way, since they will be
different for the F1C100s.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/spi/spi-sunxi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index d6b2dd09514..d1e8692ede2 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -72,7 +72,9 @@ DECLARE_GLOBAL_DATA_PTR;
 #define SUN4I_XMIT_CNT(cnt)		((cnt) & SUN4I_MAX_XFER_SIZE)
 #define SUN4I_FIFO_STA_RF_CNT_BITS	0
 
-#define SUN4I_SPI_MAX_RATE		24000000
+/* the SPI mod clock, defaulting to be 1/1 of the HOSC@24MHz */
+#define SUNXI_INPUT_CLOCK		24000000	/* 24 MHz */
+#define SUN4I_SPI_MAX_RATE		SUNXI_INPUT_CLOCK
 #define SUN4I_SPI_MIN_RATE		3000
 #define SUN4I_SPI_DEFAULT_RATE		1000000
 #define SUN4I_SPI_TIMEOUT_US		1000000
@@ -242,17 +244,18 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
 	 * frequency, fall back to CDR1.
 	 */
 
-	div = SUN4I_SPI_MAX_RATE / (2 * priv->freq);
+	div = DIV_ROUND_UP(SUNXI_INPUT_CLOCK, priv->freq);
 	reg = readl(SPI_REG(priv, SPI_CCR));
 
-	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
+	if ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
+		div /= 2;
 		if (div > 0)
 			div--;
 
 		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
 		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
 	} else {
-		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(priv->freq);
+		div = fls(div - 1);
 		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
 		reg |= SUN4I_CLK_CTL_CDR1(div);
 	}
-- 
2.35.3


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

* [PATCH 4/7] spi: sunxi: Add support for F1C100s SPI controller
  2022-05-03 21:20 [PATCH 0/7] sunxi: F1C100s: enable MMC and SPI in U-Boot proper Andre Przywara
                   ` (2 preceding siblings ...)
  2022-05-03 21:20 ` [PATCH 3/7] spi: sunxi: improve SPI clock calculation Andre Przywara
@ 2022-05-03 21:20 ` Andre Przywara
  2022-05-03 21:20 ` [PATCH 5/7] sunxi: F1C100s: update DT files from Linux Andre Przywara
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Andre Przywara @ 2022-05-03 21:20 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

The SPI controllers in the Allwinner F1Cx00 series of SoCs are
compatible to the H3 IP. The only difference in the integration is
the missing mod clock in the F1C100, instead the SPI clock is directly
derived from the AHB clock.
We *should* be able to model this through the DT, but the addition of
get_rate() requires quite some refactoring, so it's not really worth in
this simple case: We programmed both the PLL_PERIPH to 600 MHz and the
PLL/AHB divider to 3 in the SPL, so we know the SPI base clock is 200
MHz. Since we used a hard coded fixed clock rate of 24 MHz for all the
other SoCs so far, we can as well do the same for the F1C100.

Define the SPI input clock and maximum frequency differently when
compiling for the F1C100 SoC.
Also adjust the power-of-2 divider programming, because that uses a
"minus one" encoding, compared to the other SoCs.

This allows to enable SPI flash support for the F1C100 boards.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/spi/spi-sunxi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index d1e8692ede2..2f33337725e 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -72,9 +72,15 @@ DECLARE_GLOBAL_DATA_PTR;
 #define SUN4I_XMIT_CNT(cnt)		((cnt) & SUN4I_MAX_XFER_SIZE)
 #define SUN4I_FIFO_STA_RF_CNT_BITS	0
 
+#ifdef CONFIG_MACH_SUNIV
+/* the AHB clock, which we programmed to be 1/3 of PLL_PERIPH@600MHz */
+#define SUNXI_INPUT_CLOCK		200000000	/* 200 MHz */
+#define SUN4I_SPI_MAX_RATE		(SUNXI_INPUT_CLOCK / 2)
+#else
 /* the SPI mod clock, defaulting to be 1/1 of the HOSC@24MHz */
 #define SUNXI_INPUT_CLOCK		24000000	/* 24 MHz */
 #define SUN4I_SPI_MAX_RATE		SUNXI_INPUT_CLOCK
+#endif
 #define SUN4I_SPI_MIN_RATE		3000
 #define SUN4I_SPI_DEFAULT_RATE		1000000
 #define SUN4I_SPI_TIMEOUT_US		1000000
@@ -256,6 +262,9 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
 		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
 	} else {
 		div = fls(div - 1);
+		/* The F1C100s encodes the divider as 2^(n+1) */
+		if (CONFIG_IS_ENABLED(CONFIG_MACH_SUNIV))
+			div--;
 		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
 		reg |= SUN4I_CLK_CTL_CDR1(div);
 	}
-- 
2.35.3


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

* [PATCH 5/7] sunxi: F1C100s: update DT files from Linux
  2022-05-03 21:20 [PATCH 0/7] sunxi: F1C100s: enable MMC and SPI in U-Boot proper Andre Przywara
                   ` (3 preceding siblings ...)
  2022-05-03 21:20 ` [PATCH 4/7] spi: sunxi: Add support for F1C100s SPI controller Andre Przywara
@ 2022-05-03 21:20 ` Andre Przywara
  2022-05-05 11:26   ` Jesse Taube
  2022-05-24 16:11   ` Andre Przywara
  2022-05-03 21:20 ` [PATCH 6/7] Revert "sunxi: f1c100s: Drop SYSRESET to enable reset functionality" Andre Przywara
  2022-05-03 21:20 ` [PATCH 7/7] sunxi: licheepi_nano: enable SPI flash Andre Przywara
  6 siblings, 2 replies; 16+ messages in thread
From: Andre Przywara @ 2022-05-03 21:20 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

The initial U-Boot F1C100s port was based on the mainline kernel DT
files, which were quite basic and were missing the essential MMC and
SPI peripherals. While we could work around this in the SPL by
hardcoding the required information, this left U-Boot proper without SD
card or SPI flash support, so actual loading would require FEL boot.

Now the missing DT bits have been submitted and accepted in the kernel
tree, so lets sync back those files into U-Boot to enable MMC and
SPI, plus benefit from some fixes.

This is a verbatim copy of the .dts and .dtsi file from
linux-sunxi/dt-for-5.19[1], which have been part of linux-next for a
while as well.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/log/?h=sunxi/dt-for-5.19

Link: https://lore.kernel.org/linux-arm-kernel/20220317162349.739636-1-andre.przywara@arm.com/
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/dts/suniv-f1c100s-licheepi-nano.dts |  31 ++++++
 arch/arm/dts/suniv-f1c100s.dtsi              | 104 +++++++++++++++++--
 2 files changed, 125 insertions(+), 10 deletions(-)

diff --git a/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts b/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts
index a1154e6c7cb..04e59b8381c 100644
--- a/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts
+++ b/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts
@@ -11,12 +11,43 @@
 	compatible = "licheepi,licheepi-nano", "allwinner,suniv-f1c100s";
 
 	aliases {
+		mmc0 = &mmc0;
 		serial0 = &uart0;
+		spi0 = &spi0;
 	};
 
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
+
+	reg_vcc3v3: vcc3v3 {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+};
+
+&mmc0 {
+	broken-cd;
+	bus-width = <4>;
+	disable-wp;
+	status = "okay";
+	vmmc-supply = <&reg_vcc3v3>;
+};
+
+&spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi0_pc_pins>;
+	status = "okay";
+
+	flash@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "winbond,w25q128", "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <40000000>;
+	};
 };
 
 &uart0 {
diff --git a/arch/arm/dts/suniv-f1c100s.dtsi b/arch/arm/dts/suniv-f1c100s.dtsi
index 6100d3b75f6..0edc1724407 100644
--- a/arch/arm/dts/suniv-f1c100s.dtsi
+++ b/arch/arm/dts/suniv-f1c100s.dtsi
@@ -4,6 +4,9 @@
  * Copyright 2018 Mesih Kilinc <mesihkilinc@gmail.com>
  */
 
+#include <dt-bindings/clock/suniv-ccu-f1c100s.h>
+#include <dt-bindings/reset/suniv-ccu-f1c100s.h>
+
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
@@ -26,9 +29,13 @@
 	};
 
 	cpus {
-		cpu {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
 			compatible = "arm,arm926ej-s";
 			device_type = "cpu";
+			reg = <0x0>;
 		};
 	};
 
@@ -62,6 +69,70 @@
 			};
 		};
 
+		spi0: spi@1c05000 {
+			compatible = "allwinner,suniv-f1c100s-spi",
+				     "allwinner,sun8i-h3-spi";
+			reg = <0x01c05000 0x1000>;
+			interrupts = <10>;
+			clocks = <&ccu CLK_BUS_SPI0>, <&ccu CLK_BUS_SPI0>;
+			clock-names = "ahb", "mod";
+			resets = <&ccu RST_BUS_SPI0>;
+			status = "disabled";
+			num-cs = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		spi1: spi@1c06000 {
+			compatible = "allwinner,suniv-f1c100s-spi",
+				     "allwinner,sun8i-h3-spi";
+			reg = <0x01c06000 0x1000>;
+			interrupts = <11>;
+			clocks = <&ccu CLK_BUS_SPI1>, <&ccu CLK_BUS_SPI1>;
+			clock-names = "ahb", "mod";
+			resets = <&ccu RST_BUS_SPI1>;
+			status = "disabled";
+			num-cs = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		mmc0: mmc@1c0f000 {
+			compatible = "allwinner,suniv-f1c100s-mmc",
+				     "allwinner,sun7i-a20-mmc";
+			reg = <0x01c0f000 0x1000>;
+			clocks = <&ccu CLK_BUS_MMC0>,
+				 <&ccu CLK_MMC0>,
+				 <&ccu CLK_MMC0_OUTPUT>,
+				 <&ccu CLK_MMC0_SAMPLE>;
+			clock-names = "ahb", "mmc", "output", "sample";
+			resets = <&ccu RST_BUS_MMC0>;
+			reset-names = "ahb";
+			interrupts = <23>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&mmc0_pins>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		mmc1: mmc@1c10000 {
+			compatible = "allwinner,suniv-f1c100s-mmc",
+				     "allwinner,sun7i-a20-mmc";
+			reg = <0x01c10000 0x1000>;
+			clocks = <&ccu CLK_BUS_MMC1>,
+				 <&ccu CLK_MMC1>,
+				 <&ccu CLK_MMC1_OUTPUT>,
+				 <&ccu CLK_MMC1_SAMPLE>;
+			clock-names = "ahb", "mmc", "output", "sample";
+			resets = <&ccu RST_BUS_MMC1>;
+			reset-names = "ahb";
+			interrupts = <24>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		ccu: clock@1c20000 {
 			compatible = "allwinner,suniv-f1c100s-ccu";
 			reg = <0x01c20000 0x400>;
@@ -82,13 +153,24 @@
 			compatible = "allwinner,suniv-f1c100s-pinctrl";
 			reg = <0x01c20800 0x400>;
 			interrupts = <38>, <39>, <40>;
-			clocks = <&ccu 37>, <&osc24M>, <&osc32k>;
+			clocks = <&ccu CLK_BUS_PIO>, <&osc24M>, <&osc32k>;
 			clock-names = "apb", "hosc", "losc";
 			gpio-controller;
 			interrupt-controller;
 			#interrupt-cells = <3>;
 			#gpio-cells = <3>;
 
+			mmc0_pins: mmc0-pins {
+				pins = "PF0", "PF1", "PF2", "PF3", "PF4", "PF5";
+				function = "mmc0";
+				drive-strength = <30>;
+			};
+
+			spi0_pc_pins: spi0-pc-pins {
+				pins = "PC0", "PC1", "PC2", "PC3";
+				function = "spi0";
+			};
+
 			uart0_pe_pins: uart0-pe-pins {
 				pins = "PE0", "PE1";
 				function = "uart0";
@@ -98,14 +180,16 @@
 		timer@1c20c00 {
 			compatible = "allwinner,suniv-f1c100s-timer";
 			reg = <0x01c20c00 0x90>;
-			interrupts = <13>;
+			interrupts = <13>, <14>, <15>;
 			clocks = <&osc24M>;
 		};
 
 		wdt: watchdog@1c20ca0 {
 			compatible = "allwinner,suniv-f1c100s-wdt",
-				     "allwinner,sun4i-a10-wdt";
+				     "allwinner,sun6i-a31-wdt";
 			reg = <0x01c20ca0 0x20>;
+			interrupts = <16>;
+			clocks = <&osc32k>;
 		};
 
 		uart0: serial@1c25000 {
@@ -114,8 +198,8 @@
 			interrupts = <1>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
-			clocks = <&ccu 38>;
-			resets = <&ccu 24>;
+			clocks = <&ccu CLK_BUS_UART0>;
+			resets = <&ccu RST_BUS_UART0>;
 			status = "disabled";
 		};
 
@@ -125,8 +209,8 @@
 			interrupts = <2>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
-			clocks = <&ccu 39>;
-			resets = <&ccu 25>;
+			clocks = <&ccu CLK_BUS_UART1>;
+			resets = <&ccu RST_BUS_UART1>;
 			status = "disabled";
 		};
 
@@ -136,8 +220,8 @@
 			interrupts = <3>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
-			clocks = <&ccu 40>;
-			resets = <&ccu 26>;
+			clocks = <&ccu CLK_BUS_UART2>;
+			resets = <&ccu RST_BUS_UART2>;
 			status = "disabled";
 		};
 	};
-- 
2.35.3


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

* [PATCH 6/7] Revert "sunxi: f1c100s: Drop SYSRESET to enable reset functionality"
  2022-05-03 21:20 [PATCH 0/7] sunxi: F1C100s: enable MMC and SPI in U-Boot proper Andre Przywara
                   ` (4 preceding siblings ...)
  2022-05-03 21:20 ` [PATCH 5/7] sunxi: F1C100s: update DT files from Linux Andre Przywara
@ 2022-05-03 21:20 ` Andre Przywara
  2022-05-24 16:11   ` Andre Przywara
  2022-05-03 21:20 ` [PATCH 7/7] sunxi: licheepi_nano: enable SPI flash Andre Przywara
  6 siblings, 1 reply; 16+ messages in thread
From: Andre Przywara @ 2022-05-03 21:20 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

The original Allwinner F1C100 .dtsi imported from the Linux kernel tree
used the wrong compatible string for the watchdog timer, so the Allwinner
DM reset driver was not working properly. We worked around this by
disabling the SYSRESET driver, so the hardcoded SPL reset driver took
over.
Now the issue was fixed in the DTs in mainline Linux, and we synced the
fixed .dtsi file into U-Boot, so drop the hack and use the normal U-Boot
proper reset infrastructure.

This reverts commit cfcf1952c11e6ffcbbf88eb63c49edca2acf1d5e.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 configs/licheepi_nano_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configs/licheepi_nano_defconfig b/configs/licheepi_nano_defconfig
index 67b7b85c491..9fd1dcc9958 100644
--- a/configs/licheepi_nano_defconfig
+++ b/configs/licheepi_nano_defconfig
@@ -10,4 +10,3 @@ CONFIG_DRAM_CLK=156
 CONFIG_DRAM_ZQ=0
 # CONFIG_VIDEO_SUNXI is not set
 CONFIG_SPL_SPI_SUNXI=y
-# CONFIG_SYSRESET is not set
-- 
2.35.3


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

* [PATCH 7/7] sunxi: licheepi_nano: enable SPI flash
  2022-05-03 21:20 [PATCH 0/7] sunxi: F1C100s: enable MMC and SPI in U-Boot proper Andre Przywara
                   ` (5 preceding siblings ...)
  2022-05-03 21:20 ` [PATCH 6/7] Revert "sunxi: f1c100s: Drop SYSRESET to enable reset functionality" Andre Przywara
@ 2022-05-03 21:20 ` Andre Przywara
  6 siblings, 0 replies; 16+ messages in thread
From: Andre Przywara @ 2022-05-03 21:20 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

Many LicheePi Nano boards come with SPI flash soldered, which already
works for booting the SPL and loading U-Boot proper.
With the updated DTB, we can now also use the SPI flash from U-Boot
proper, so enable the bits in the defconfig, to allow loading binaries
from SPI flash.
There seem to be board revisions with a Winbond SPI chip, but also
others with an XTX chip, so include support for both: the actual chip
used will be autodetected.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 configs/licheepi_nano_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configs/licheepi_nano_defconfig b/configs/licheepi_nano_defconfig
index 9fd1dcc9958..dfdc7d46855 100644
--- a/configs/licheepi_nano_defconfig
+++ b/configs/licheepi_nano_defconfig
@@ -10,3 +10,6 @@ CONFIG_DRAM_CLK=156
 CONFIG_DRAM_ZQ=0
 # CONFIG_VIDEO_SUNXI is not set
 CONFIG_SPL_SPI_SUNXI=y
+CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_SPI_FLASH_XTX=y
+CONFIG_SPI=y
-- 
2.35.3


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

* Re: [PATCH 5/7] sunxi: F1C100s: update DT files from Linux
  2022-05-03 21:20 ` [PATCH 5/7] sunxi: F1C100s: update DT files from Linux Andre Przywara
@ 2022-05-05 11:26   ` Jesse Taube
  2022-05-24 16:11   ` Andre Przywara
  1 sibling, 0 replies; 16+ messages in thread
From: Jesse Taube @ 2022-05-05 11:26 UTC (permalink / raw)
  To: Andre Przywara, Jagan Teki
  Cc: Icenowy Zheng, Yifan Gu, Giulio Benetti, George Hilliard,
	Samuel Holland, Jernej Skrabec, linux-sunxi, u-boot



On 5/3/22 17:20, Andre Przywara wrote:
> The initial U-Boot F1C100s port was based on the mainline kernel DT
> files, which were quite basic and were missing the essential MMC and
> SPI peripherals. While we could work around this in the SPL by
> hardcoding the required information, this left U-Boot proper without SD
> card or SPI flash support, so actual loading would require FEL boot.
> 
> Now the missing DT bits have been submitted and accepted in the kernel
> tree, so lets sync back those files into U-Boot to enable MMC and
> SPI, plus benefit from some fixes.
> 
> This is a verbatim copy of the .dts and .dtsi file from
> linux-sunxi/dt-for-5.19[1], which have been part of linux-next for a
> while as well.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/log/?h=sunxi/dt-for-5.19
> 
> Link: https://lore.kernel.org/linux-arm-kernel/20220317162349.739636-1-andre.przywara@arm.com/
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>   arch/arm/dts/suniv-f1c100s-licheepi-nano.dts |  31 ++++++
>   arch/arm/dts/suniv-f1c100s.dtsi              | 104 +++++++++++++++++--
>   2 files changed, 125 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts b/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts
> index a1154e6c7cb..04e59b8381c 100644
> --- a/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts
> +++ b/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts
> @@ -11,12 +11,43 @@
>   	compatible = "licheepi,licheepi-nano", "allwinner,suniv-f1c100s";
>   
>   	aliases {
> +		mmc0 = &mmc0;
>   		serial0 = &uart0;
> +		spi0 = &spi0;
>   	};
>   
>   	chosen {
>   		stdout-path = "serial0:115200n8";
>   	};
> +
> +	reg_vcc3v3: vcc3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +};
> +
> +&mmc0 {
> +	broken-cd;
> +	bus-width = <4>;
> +	disable-wp;
> +	status = "okay";
> +	vmmc-supply = <&reg_vcc3v3>;
> +};
> +
> +&spi0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi0_pc_pins>;
> +	status = "okay";
> +
> +	flash@0 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "winbond,w25q128", "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <40000000>;
> +	};
>   };
>   
>   &uart0 {
> diff --git a/arch/arm/dts/suniv-f1c100s.dtsi b/arch/arm/dts/suniv-f1c100s.dtsi
> index 6100d3b75f6..0edc1724407 100644
> --- a/arch/arm/dts/suniv-f1c100s.dtsi
> +++ b/arch/arm/dts/suniv-f1c100s.dtsi
> @@ -4,6 +4,9 @@
>    * Copyright 2018 Mesih Kilinc <mesihkilinc@gmail.com>
>    */
>   
> +#include <dt-bindings/clock/suniv-ccu-f1c100s.h>
> +#include <dt-bindings/reset/suniv-ccu-f1c100s.h>
> +
>   / {
>   	#address-cells = <1>;
>   	#size-cells = <1>;
> @@ -26,9 +29,13 @@
>   	};
>   
>   	cpus {
> -		cpu {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
>   			compatible = "arm,arm926ej-s";
>   			device_type = "cpu";
> +			reg = <0x0>;
>   		};
>   	};
>   
> @@ -62,6 +69,70 @@
>   			};
>   		};
>   
> +		spi0: spi@1c05000 {
> +			compatible = "allwinner,suniv-f1c100s-spi",
> +				     "allwinner,sun8i-h3-spi";
> +			reg = <0x01c05000 0x1000>;
> +			interrupts = <10>;
> +			clocks = <&ccu CLK_BUS_SPI0>, <&ccu CLK_BUS_SPI0>;
> +			clock-names = "ahb", "mod";
> +			resets = <&ccu RST_BUS_SPI0>;
> +			status = "disabled";
> +			num-cs = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		spi1: spi@1c06000 {
> +			compatible = "allwinner,suniv-f1c100s-spi",
> +				     "allwinner,sun8i-h3-spi";
> +			reg = <0x01c06000 0x1000>;
> +			interrupts = <11>;
> +			clocks = <&ccu CLK_BUS_SPI1>, <&ccu CLK_BUS_SPI1>;
> +			clock-names = "ahb", "mod";
> +			resets = <&ccu RST_BUS_SPI1>;
> +			status = "disabled";
> +			num-cs = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		mmc0: mmc@1c0f000 {
> +			compatible = "allwinner,suniv-f1c100s-mmc",
> +				     "allwinner,sun7i-a20-mmc";
> +			reg = <0x01c0f000 0x1000>;
> +			clocks = <&ccu CLK_BUS_MMC0>,
> +				 <&ccu CLK_MMC0>,
> +				 <&ccu CLK_MMC0_OUTPUT>,
> +				 <&ccu CLK_MMC0_SAMPLE>;
> +			clock-names = "ahb", "mmc", "output", "sample";
> +			resets = <&ccu RST_BUS_MMC0>;
> +			reset-names = "ahb";
> +			interrupts = <23>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&mmc0_pins>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		mmc1: mmc@1c10000 {
> +			compatible = "allwinner,suniv-f1c100s-mmc",
> +				     "allwinner,sun7i-a20-mmc";
> +			reg = <0x01c10000 0x1000>;
> +			clocks = <&ccu CLK_BUS_MMC1>,
> +				 <&ccu CLK_MMC1>,
> +				 <&ccu CLK_MMC1_OUTPUT>,
> +				 <&ccu CLK_MMC1_SAMPLE>;
> +			clock-names = "ahb", "mmc", "output", "sample";
> +			resets = <&ccu RST_BUS_MMC1>;
> +			reset-names = "ahb";
> +			interrupts = <24>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
>   		ccu: clock@1c20000 {
>   			compatible = "allwinner,suniv-f1c100s-ccu";
>   			reg = <0x01c20000 0x400>;
> @@ -82,13 +153,24 @@
>   			compatible = "allwinner,suniv-f1c100s-pinctrl";
>   			reg = <0x01c20800 0x400>;
>   			interrupts = <38>, <39>, <40>;
> -			clocks = <&ccu 37>, <&osc24M>, <&osc32k>;
> +			clocks = <&ccu CLK_BUS_PIO>, <&osc24M>, <&osc32k>;
>   			clock-names = "apb", "hosc", "losc";
>   			gpio-controller;
>   			interrupt-controller;
>   			#interrupt-cells = <3>;
>   			#gpio-cells = <3>;
>   
> +			mmc0_pins: mmc0-pins {
> +				pins = "PF0", "PF1", "PF2", "PF3", "PF4", "PF5";
> +				function = "mmc0";
> +				drive-strength = <30>;
> +			};
> +
> +			spi0_pc_pins: spi0-pc-pins {
> +				pins = "PC0", "PC1", "PC2", "PC3";
> +				function = "spi0";
> +			};
> +
>   			uart0_pe_pins: uart0-pe-pins {
>   				pins = "PE0", "PE1";
>   				function = "uart0";
> @@ -98,14 +180,16 @@
>   		timer@1c20c00 {
>   			compatible = "allwinner,suniv-f1c100s-timer";
>   			reg = <0x01c20c00 0x90>;
> -			interrupts = <13>;
> +			interrupts = <13>, <14>, <15>;
>   			clocks = <&osc24M>;
>   		};
>   
>   		wdt: watchdog@1c20ca0 {
>   			compatible = "allwinner,suniv-f1c100s-wdt",
> -				     "allwinner,sun4i-a10-wdt";
> +				     "allwinner,sun6i-a31-wdt";
>   			reg = <0x01c20ca0 0x20>;
> +			interrupts = <16>;
> +			clocks = <&osc32k>;
>   		};
>   
>   		uart0: serial@1c25000 {
> @@ -114,8 +198,8 @@
>   			interrupts = <1>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
> -			clocks = <&ccu 38>;
> -			resets = <&ccu 24>;
> +			clocks = <&ccu CLK_BUS_UART0>;
> +			resets = <&ccu RST_BUS_UART0>;
>   			status = "disabled";
>   		};
>   
> @@ -125,8 +209,8 @@
>   			interrupts = <2>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
> -			clocks = <&ccu 39>;
> -			resets = <&ccu 25>;
> +			clocks = <&ccu CLK_BUS_UART1>;
> +			resets = <&ccu RST_BUS_UART1>;
>   			status = "disabled";
>   		};
>   
> @@ -136,8 +220,8 @@
>   			interrupts = <3>;
>   			reg-shift = <2>;
>   			reg-io-width = <4>;
> -			clocks = <&ccu 40>;
> -			resets = <&ccu 26>;
> +			clocks = <&ccu CLK_BUS_UART2>;
> +			resets = <&ccu RST_BUS_UART2>;
>   			status = "disabled";
>   		};
>   	};

Acked-by: Jesse Taube <Mr.Bossman075@gmail.com>

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

* Re: [PATCH 1/7] clk: sunxi: implement clock driver for suniv f1c100s
  2022-05-03 21:20 ` [PATCH 1/7] clk: sunxi: implement clock driver for suniv f1c100s Andre Przywara
@ 2022-05-24 16:10   ` Andre Przywara
  0 siblings, 0 replies; 16+ messages in thread
From: Andre Przywara @ 2022-05-24 16:10 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

On Tue,  3 May 2022 22:20:34 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

> From: George Hilliard <thirtythreeforty@gmail.com>
> 
> The f1c100s has a clock tree similar to those of other sunxi parts.
> Add support for it.
> 
> Signed-off-by: George Hilliard <thirtythreeforty@gmail.com>
> Signed-off-by: Yifan Gu <me@yifangu.com>
> Acked-by: Sean Anderson <seanga2@gmail.com>
> [Andre: add PIO and I2C]
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Applied to sunxi/master.

Thanks,
Andre

> ---
>  drivers/clk/sunxi/Kconfig       |  7 ++++
>  drivers/clk/sunxi/Makefile      |  1 +
>  drivers/clk/sunxi/clk_f1c100s.c | 74 +++++++++++++++++++++++++++++++++
>  3 files changed, 82 insertions(+)
>  create mode 100644 drivers/clk/sunxi/clk_f1c100s.c
> 
> diff --git a/drivers/clk/sunxi/Kconfig b/drivers/clk/sunxi/Kconfig
> index f19908113e1..bf11fad6eef 100644
> --- a/drivers/clk/sunxi/Kconfig
> +++ b/drivers/clk/sunxi/Kconfig
> @@ -10,6 +10,13 @@ config CLK_SUNXI
>  
>  if CLK_SUNXI
>  
> +config CLK_SUNIV_F1C100S
> +	bool "Clock driver for Allwinner F1C100s"
> +	default MACH_SUNIV
> +	help
> +	  This enables common clock driver support for platforms based
> +	  on Allwinner F1C100s SoC.
> +
>  config CLK_SUN4I_A10
>  	bool "Clock driver for Allwinner A10/A20"
>  	default MACH_SUN4I || MACH_SUN7I
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index 48a48a2f000..895da02ebea 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_CLK_SUNXI) += clk_sunxi.o
>  
>  obj-$(CONFIG_CLK_SUNXI) += clk_sun6i_rtc.o
>  
> +obj-$(CONFIG_CLK_SUNIV_F1C100S) += clk_f1c100s.o
>  obj-$(CONFIG_CLK_SUN4I_A10) += clk_a10.o
>  obj-$(CONFIG_CLK_SUN5I_A10S) += clk_a10s.o
>  obj-$(CONFIG_CLK_SUN6I_A31) += clk_a31.o
> diff --git a/drivers/clk/sunxi/clk_f1c100s.c b/drivers/clk/sunxi/clk_f1c100s.c
> new file mode 100644
> index 00000000000..72cf8a6e5c0
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk_f1c100s.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: (GPL-2.0+)
> +/*
> + * Copyright (C) 2019 George Hilliard <thirtythreeforty@gmail.com>.
> + */
> +
> +#include <common.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <clk/sunxi.h>
> +#include <dt-bindings/clock/suniv-ccu-f1c100s.h>
> +#include <dt-bindings/reset/suniv-ccu-f1c100s.h>
> +
> +static struct ccu_clk_gate f1c100s_gates[] = {
> +	[CLK_BUS_MMC0]		= GATE(0x060, BIT(8)),
> +	[CLK_BUS_MMC1]		= GATE(0x060, BIT(9)),
> +	[CLK_BUS_SPI0]		= GATE(0x060, BIT(20)),
> +	[CLK_BUS_SPI1]		= GATE(0x060, BIT(21)),
> +	[CLK_BUS_OTG]		= GATE(0x060, BIT(24)),
> +
> +	[CLK_BUS_I2C0]		= GATE(0x068, BIT(16)),
> +	[CLK_BUS_I2C1]		= GATE(0x068, BIT(17)),
> +	[CLK_BUS_I2C2]		= GATE(0x068, BIT(18)),
> +	[CLK_BUS_PIO]		= GATE(0x068, BIT(19)),
> +
> +	[CLK_BUS_UART0]		= GATE(0x06c, BIT(20)),
> +	[CLK_BUS_UART1]		= GATE(0x06c, BIT(21)),
> +	[CLK_BUS_UART2]		= GATE(0x06c, BIT(22)),
> +
> +	[CLK_USB_PHY0]          = GATE(0x0cc, BIT(1)),
> +};
> +
> +static struct ccu_reset f1c100s_resets[] = {
> +	[RST_USB_PHY0]		= RESET(0x0cc, BIT(0)),
> +
> +	[RST_BUS_MMC0]		= RESET(0x2c0, BIT(8)),
> +	[RST_BUS_MMC1]		= RESET(0x2c0, BIT(9)),
> +	[RST_BUS_SPI0]		= RESET(0x2c0, BIT(20)),
> +	[RST_BUS_SPI1]		= RESET(0x2c0, BIT(21)),
> +	[RST_BUS_OTG]		= RESET(0x2c0, BIT(24)),
> +
> +	[RST_BUS_I2C0]		= RESET(0x2d0, BIT(16)),
> +	[RST_BUS_I2C1]		= RESET(0x2d0, BIT(17)),
> +	[RST_BUS_I2C2]		= RESET(0x2d0, BIT(18)),
> +	[RST_BUS_UART0]		= RESET(0x2d0, BIT(20)),
> +	[RST_BUS_UART1]		= RESET(0x2d0, BIT(21)),
> +	[RST_BUS_UART2]		= RESET(0x2d0, BIT(22)),
> +};
> +
> +static const struct ccu_desc f1c100s_ccu_desc = {
> +	.gates = f1c100s_gates,
> +	.resets = f1c100s_resets,
> +};
> +
> +static int f1c100s_clk_bind(struct udevice *dev)
> +{
> +	return sunxi_reset_bind(dev, ARRAY_SIZE(f1c100s_resets));
> +}
> +
> +static const struct udevice_id f1c100s_clk_ids[] = {
> +	{ .compatible = "allwinner,suniv-f1c100s-ccu",
> +	  .data = (ulong)&f1c100s_ccu_desc },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(clk_suniv_f1c100s) = {
> +	.name		= "suniv_f1c100s_ccu",
> +	.id		= UCLASS_CLK,
> +	.of_match	= f1c100s_clk_ids,
> +	.priv_auto	= sizeof(struct ccu_priv),
> +	.ops		= &sunxi_clk_ops,
> +	.probe		= sunxi_clk_probe,
> +	.bind		= f1c100s_clk_bind,
> +};


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

* Re: [PATCH 5/7] sunxi: F1C100s: update DT files from Linux
  2022-05-03 21:20 ` [PATCH 5/7] sunxi: F1C100s: update DT files from Linux Andre Przywara
  2022-05-05 11:26   ` Jesse Taube
@ 2022-05-24 16:11   ` Andre Przywara
  1 sibling, 0 replies; 16+ messages in thread
From: Andre Przywara @ 2022-05-24 16:11 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

On Tue,  3 May 2022 22:20:38 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

> The initial U-Boot F1C100s port was based on the mainline kernel DT
> files, which were quite basic and were missing the essential MMC and
> SPI peripherals. While we could work around this in the SPL by
> hardcoding the required information, this left U-Boot proper without SD
> card or SPI flash support, so actual loading would require FEL boot.
> 
> Now the missing DT bits have been submitted and accepted in the kernel
> tree, so lets sync back those files into U-Boot to enable MMC and
> SPI, plus benefit from some fixes.
> 
> This is a verbatim copy of the .dts and .dtsi file from
> linux-sunxi/dt-for-5.19[1], which have been part of linux-next for a
> while as well.

Applied to sunxi/master.

Thanks,
Andre

> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/log/?h=sunxi/dt-for-5.19
> 
> Link: https://lore.kernel.org/linux-arm-kernel/20220317162349.739636-1-andre.przywara@arm.com/
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/dts/suniv-f1c100s-licheepi-nano.dts |  31 ++++++
>  arch/arm/dts/suniv-f1c100s.dtsi              | 104 +++++++++++++++++--
>  2 files changed, 125 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts b/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts
> index a1154e6c7cb..04e59b8381c 100644
> --- a/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts
> +++ b/arch/arm/dts/suniv-f1c100s-licheepi-nano.dts
> @@ -11,12 +11,43 @@
>  	compatible = "licheepi,licheepi-nano", "allwinner,suniv-f1c100s";
>  
>  	aliases {
> +		mmc0 = &mmc0;
>  		serial0 = &uart0;
> +		spi0 = &spi0;
>  	};
>  
>  	chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
> +
> +	reg_vcc3v3: vcc3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +};
> +
> +&mmc0 {
> +	broken-cd;
> +	bus-width = <4>;
> +	disable-wp;
> +	status = "okay";
> +	vmmc-supply = <&reg_vcc3v3>;
> +};
> +
> +&spi0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi0_pc_pins>;
> +	status = "okay";
> +
> +	flash@0 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "winbond,w25q128", "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <40000000>;
> +	};
>  };
>  
>  &uart0 {
> diff --git a/arch/arm/dts/suniv-f1c100s.dtsi b/arch/arm/dts/suniv-f1c100s.dtsi
> index 6100d3b75f6..0edc1724407 100644
> --- a/arch/arm/dts/suniv-f1c100s.dtsi
> +++ b/arch/arm/dts/suniv-f1c100s.dtsi
> @@ -4,6 +4,9 @@
>   * Copyright 2018 Mesih Kilinc <mesihkilinc@gmail.com>
>   */
>  
> +#include <dt-bindings/clock/suniv-ccu-f1c100s.h>
> +#include <dt-bindings/reset/suniv-ccu-f1c100s.h>
> +
>  / {
>  	#address-cells = <1>;
>  	#size-cells = <1>;
> @@ -26,9 +29,13 @@
>  	};
>  
>  	cpus {
> -		cpu {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
>  			compatible = "arm,arm926ej-s";
>  			device_type = "cpu";
> +			reg = <0x0>;
>  		};
>  	};
>  
> @@ -62,6 +69,70 @@
>  			};
>  		};
>  
> +		spi0: spi@1c05000 {
> +			compatible = "allwinner,suniv-f1c100s-spi",
> +				     "allwinner,sun8i-h3-spi";
> +			reg = <0x01c05000 0x1000>;
> +			interrupts = <10>;
> +			clocks = <&ccu CLK_BUS_SPI0>, <&ccu CLK_BUS_SPI0>;
> +			clock-names = "ahb", "mod";
> +			resets = <&ccu RST_BUS_SPI0>;
> +			status = "disabled";
> +			num-cs = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		spi1: spi@1c06000 {
> +			compatible = "allwinner,suniv-f1c100s-spi",
> +				     "allwinner,sun8i-h3-spi";
> +			reg = <0x01c06000 0x1000>;
> +			interrupts = <11>;
> +			clocks = <&ccu CLK_BUS_SPI1>, <&ccu CLK_BUS_SPI1>;
> +			clock-names = "ahb", "mod";
> +			resets = <&ccu RST_BUS_SPI1>;
> +			status = "disabled";
> +			num-cs = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		mmc0: mmc@1c0f000 {
> +			compatible = "allwinner,suniv-f1c100s-mmc",
> +				     "allwinner,sun7i-a20-mmc";
> +			reg = <0x01c0f000 0x1000>;
> +			clocks = <&ccu CLK_BUS_MMC0>,
> +				 <&ccu CLK_MMC0>,
> +				 <&ccu CLK_MMC0_OUTPUT>,
> +				 <&ccu CLK_MMC0_SAMPLE>;
> +			clock-names = "ahb", "mmc", "output", "sample";
> +			resets = <&ccu RST_BUS_MMC0>;
> +			reset-names = "ahb";
> +			interrupts = <23>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&mmc0_pins>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		mmc1: mmc@1c10000 {
> +			compatible = "allwinner,suniv-f1c100s-mmc",
> +				     "allwinner,sun7i-a20-mmc";
> +			reg = <0x01c10000 0x1000>;
> +			clocks = <&ccu CLK_BUS_MMC1>,
> +				 <&ccu CLK_MMC1>,
> +				 <&ccu CLK_MMC1_OUTPUT>,
> +				 <&ccu CLK_MMC1_SAMPLE>;
> +			clock-names = "ahb", "mmc", "output", "sample";
> +			resets = <&ccu RST_BUS_MMC1>;
> +			reset-names = "ahb";
> +			interrupts = <24>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
>  		ccu: clock@1c20000 {
>  			compatible = "allwinner,suniv-f1c100s-ccu";
>  			reg = <0x01c20000 0x400>;
> @@ -82,13 +153,24 @@
>  			compatible = "allwinner,suniv-f1c100s-pinctrl";
>  			reg = <0x01c20800 0x400>;
>  			interrupts = <38>, <39>, <40>;
> -			clocks = <&ccu 37>, <&osc24M>, <&osc32k>;
> +			clocks = <&ccu CLK_BUS_PIO>, <&osc24M>, <&osc32k>;
>  			clock-names = "apb", "hosc", "losc";
>  			gpio-controller;
>  			interrupt-controller;
>  			#interrupt-cells = <3>;
>  			#gpio-cells = <3>;
>  
> +			mmc0_pins: mmc0-pins {
> +				pins = "PF0", "PF1", "PF2", "PF3", "PF4", "PF5";
> +				function = "mmc0";
> +				drive-strength = <30>;
> +			};
> +
> +			spi0_pc_pins: spi0-pc-pins {
> +				pins = "PC0", "PC1", "PC2", "PC3";
> +				function = "spi0";
> +			};
> +
>  			uart0_pe_pins: uart0-pe-pins {
>  				pins = "PE0", "PE1";
>  				function = "uart0";
> @@ -98,14 +180,16 @@
>  		timer@1c20c00 {
>  			compatible = "allwinner,suniv-f1c100s-timer";
>  			reg = <0x01c20c00 0x90>;
> -			interrupts = <13>;
> +			interrupts = <13>, <14>, <15>;
>  			clocks = <&osc24M>;
>  		};
>  
>  		wdt: watchdog@1c20ca0 {
>  			compatible = "allwinner,suniv-f1c100s-wdt",
> -				     "allwinner,sun4i-a10-wdt";
> +				     "allwinner,sun6i-a31-wdt";
>  			reg = <0x01c20ca0 0x20>;
> +			interrupts = <16>;
> +			clocks = <&osc32k>;
>  		};
>  
>  		uart0: serial@1c25000 {
> @@ -114,8 +198,8 @@
>  			interrupts = <1>;
>  			reg-shift = <2>;
>  			reg-io-width = <4>;
> -			clocks = <&ccu 38>;
> -			resets = <&ccu 24>;
> +			clocks = <&ccu CLK_BUS_UART0>;
> +			resets = <&ccu RST_BUS_UART0>;
>  			status = "disabled";
>  		};
>  
> @@ -125,8 +209,8 @@
>  			interrupts = <2>;
>  			reg-shift = <2>;
>  			reg-io-width = <4>;
> -			clocks = <&ccu 39>;
> -			resets = <&ccu 25>;
> +			clocks = <&ccu CLK_BUS_UART1>;
> +			resets = <&ccu RST_BUS_UART1>;
>  			status = "disabled";
>  		};
>  
> @@ -136,8 +220,8 @@
>  			interrupts = <3>;
>  			reg-shift = <2>;
>  			reg-io-width = <4>;
> -			clocks = <&ccu 40>;
> -			resets = <&ccu 26>;
> +			clocks = <&ccu CLK_BUS_UART2>;
> +			resets = <&ccu RST_BUS_UART2>;
>  			status = "disabled";
>  		};
>  	};


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

* Re: [PATCH 6/7] Revert "sunxi: f1c100s: Drop SYSRESET to enable reset functionality"
  2022-05-03 21:20 ` [PATCH 6/7] Revert "sunxi: f1c100s: Drop SYSRESET to enable reset functionality" Andre Przywara
@ 2022-05-24 16:11   ` Andre Przywara
  0 siblings, 0 replies; 16+ messages in thread
From: Andre Przywara @ 2022-05-24 16:11 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	George Hilliard, Samuel Holland, Jernej Skrabec, linux-sunxi,
	u-boot

On Tue,  3 May 2022 22:20:39 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

> The original Allwinner F1C100 .dtsi imported from the Linux kernel tree
> used the wrong compatible string for the watchdog timer, so the Allwinner
> DM reset driver was not working properly. We worked around this by
> disabling the SYSRESET driver, so the hardcoded SPL reset driver took
> over.
> Now the issue was fixed in the DTs in mainline Linux, and we synced the
> fixed .dtsi file into U-Boot, so drop the hack and use the normal U-Boot
> proper reset infrastructure.
> 
> This reverts commit cfcf1952c11e6ffcbbf88eb63c49edca2acf1d5e.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Applied to sunxi/master.

Thanks,
Andre

> ---
>  configs/licheepi_nano_defconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/configs/licheepi_nano_defconfig b/configs/licheepi_nano_defconfig
> index 67b7b85c491..9fd1dcc9958 100644
> --- a/configs/licheepi_nano_defconfig
> +++ b/configs/licheepi_nano_defconfig
> @@ -10,4 +10,3 @@ CONFIG_DRAM_CLK=156
>  CONFIG_DRAM_ZQ=0
>  # CONFIG_VIDEO_SUNXI is not set
>  CONFIG_SPL_SPI_SUNXI=y
> -# CONFIG_SYSRESET is not set


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

* Re: [PATCH 2/7] spi: sunxi: refactor SPI speed/mode programming
  2022-05-03 21:20 ` [PATCH 2/7] spi: sunxi: refactor SPI speed/mode programming Andre Przywara
@ 2022-06-28  0:31   ` Andre Przywara
  2022-06-28  3:43     ` Jesse Taube
  2022-06-30  3:25     ` Jesse Taube
  0 siblings, 2 replies; 16+ messages in thread
From: Andre Przywara @ 2022-06-28  0:31 UTC (permalink / raw)
  To: Jagan Teki, George Hilliard, qianfan Zhao
  Cc: Jesse Taube, Icenowy Zheng, Yifan Gu, Giulio Benetti,
	Samuel Holland, Jernej Skrabec, linux-sunxi, u-boot

On Tue,  3 May 2022 22:20:35 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

Hi,

> As George rightfully pointed out [1], the spi-sunxi driver programs the
> speed and mode settings only when the respective functions are called,
> but this gets lost over a call to release_bus(). That asserts the
> reset line, thus forces each SPI register back to its default value.
> Adding to that, trying to program SPI_CCR and SPI_TCR might be pointless
> in the first place, when the reset line is still asserted (before
> claim_bus()), so those setting won't apply most of the time. In reality
> I see two nested claim_bus() calls for the first use, so settings between
> the two would work (for instance for the initial "sf probe"). However
> later on the speed setting is not programmed into the hardware anymore.

So this issue was addressed with patches by both George (earlier, in a
different way) and Qianfan (later, in a very similar way).

Can someone (Jagan?) please have a look at this change from the U-Boot
SPI perspective? And also the other changes in this series?
I pushed them to the sunxi/next branch:
https://source.denx.de/u-boot/custodians/u-boot-sunxi/-/commits/next/

So can people please test this and report whether this now works as
expected?

Thanks,
Andre
 
> So far we get away with that default frequency, because that is a rather
> tame 24 MHz, which most SPI flash chips can handle just fine.
> 
> Move the actual register programming into a separate function, and use
> .set_speed and .set_mode just to set the variables in our priv structure.
> Then we only call this new function in claim_bus(), when we are sure
> that register accesses actually work and are preserved.
> 
> [1] https://lore.kernel.org/u-boot/20210725231636.879913-17-me@yifangu.com/
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reported-by: George Hilliard <thirtythreeforty@gmail.com>
> ---
>  drivers/spi/spi-sunxi.c | 95 ++++++++++++++++++++++-------------------
>  1 file changed, 52 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
> index b6cd7ddafad..d6b2dd09514 100644
> --- a/drivers/spi/spi-sunxi.c
> +++ b/drivers/spi/spi-sunxi.c
> @@ -221,6 +221,56 @@ err_ahb:
>  	return ret;
>  }
>  
> +static void sun4i_spi_set_speed_mode(struct udevice *dev)
> +{
> +	struct sun4i_spi_priv *priv = dev_get_priv(dev);
> +	unsigned int div;
> +	u32 reg;
> +
> +	/*
> +	 * Setup clock divider.
> +	 *
> +	 * We have two choices there. Either we can use the clock
> +	 * divide rate 1, which is calculated thanks to this formula:
> +	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
> +	 * Or we can use CDR2, which is calculated with the formula:
> +	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
> +	 * Whether we use the former or the latter is set through the
> +	 * DRS bit.
> +	 *
> +	 * First try CDR2, and if we can't reach the expected
> +	 * frequency, fall back to CDR1.
> +	 */
> +
> +	div = SUN4I_SPI_MAX_RATE / (2 * priv->freq);
> +	reg = readl(SPI_REG(priv, SPI_CCR));
> +
> +	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> +		if (div > 0)
> +			div--;
> +
> +		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
> +		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
> +	} else {
> +		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(priv->freq);
> +		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
> +		reg |= SUN4I_CLK_CTL_CDR1(div);
> +	}
> +
> +	writel(reg, SPI_REG(priv, SPI_CCR));
> +
> +	reg = readl(SPI_REG(priv, SPI_TCR));
> +	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
> +
> +	if (priv->mode & SPI_CPOL)
> +		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
> +
> +	if (priv->mode & SPI_CPHA)
> +		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
> +
> +	writel(reg, SPI_REG(priv, SPI_TCR));
> +}
> +
>  static int sun4i_spi_claim_bus(struct udevice *dev)
>  {
>  	struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
> @@ -240,6 +290,8 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
>  	setbits_le32(SPI_REG(priv, SPI_TCR), SPI_BIT(priv, SPI_TCR_CS_MANUAL) |
>  		     SPI_BIT(priv, SPI_TCR_CS_ACTIVE_LOW));
>  
> +	sun4i_spi_set_speed_mode(dev->parent);
> +
>  	return 0;
>  }
>  
> @@ -329,46 +381,14 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
>  {
>  	struct sun4i_spi_plat *plat = dev_get_plat(dev);
>  	struct sun4i_spi_priv *priv = dev_get_priv(dev);
> -	unsigned int div;
> -	u32 reg;
>  
>  	if (speed > plat->max_hz)
>  		speed = plat->max_hz;
>  
>  	if (speed < SUN4I_SPI_MIN_RATE)
>  		speed = SUN4I_SPI_MIN_RATE;
> -	/*
> -	 * Setup clock divider.
> -	 *
> -	 * We have two choices there. Either we can use the clock
> -	 * divide rate 1, which is calculated thanks to this formula:
> -	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
> -	 * Or we can use CDR2, which is calculated with the formula:
> -	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
> -	 * Whether we use the former or the latter is set through the
> -	 * DRS bit.
> -	 *
> -	 * First try CDR2, and if we can't reach the expected
> -	 * frequency, fall back to CDR1.
> -	 */
> -
> -	div = SUN4I_SPI_MAX_RATE / (2 * speed);
> -	reg = readl(SPI_REG(priv, SPI_CCR));
> -
> -	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> -		if (div > 0)
> -			div--;
> -
> -		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
> -		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
> -	} else {
> -		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(speed);
> -		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
> -		reg |= SUN4I_CLK_CTL_CDR1(div);
> -	}
>  
>  	priv->freq = speed;
> -	writel(reg, SPI_REG(priv, SPI_CCR));
>  
>  	return 0;
>  }
> @@ -376,19 +396,8 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
>  static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
>  {
>  	struct sun4i_spi_priv *priv = dev_get_priv(dev);
> -	u32 reg;
> -
> -	reg = readl(SPI_REG(priv, SPI_TCR));
> -	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
> -
> -	if (mode & SPI_CPOL)
> -		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
> -
> -	if (mode & SPI_CPHA)
> -		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
>  
>  	priv->mode = mode;
> -	writel(reg, SPI_REG(priv, SPI_TCR));
>  
>  	return 0;
>  }


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

* Re: [PATCH 2/7] spi: sunxi: refactor SPI speed/mode programming
  2022-06-28  0:31   ` Andre Przywara
@ 2022-06-28  3:43     ` Jesse Taube
  2022-07-18 10:17       ` Andre Przywara
  2022-06-30  3:25     ` Jesse Taube
  1 sibling, 1 reply; 16+ messages in thread
From: Jesse Taube @ 2022-06-28  3:43 UTC (permalink / raw)
  To: Andre Przywara, Jagan Teki, George Hilliard, qianfan Zhao
  Cc: Icenowy Zheng, Yifan Gu, Giulio Benetti, Samuel Holland,
	Jernej Skrabec, linux-sunxi, u-boot



On 6/27/22 20:31, Andre Przywara wrote:
> On Tue,  3 May 2022 22:20:35 +0100
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
> Hi,
> 
>> As George rightfully pointed out [1], the spi-sunxi driver programs the
>> speed and mode settings only when the respective functions are called,
>> but this gets lost over a call to release_bus(). That asserts the
>> reset line, thus forces each SPI register back to its default value.
>> Adding to that, trying to program SPI_CCR and SPI_TCR might be pointless
>> in the first place, when the reset line is still asserted (before
>> claim_bus()), so those setting won't apply most of the time. In reality
>> I see two nested claim_bus() calls for the first use, so settings between
>> the two would work (for instance for the initial "sf probe"). However
>> later on the speed setting is not programmed into the hardware anymore.
> 
> So this issue was addressed with patches by both George (earlier, in a
> different way) and Qianfan (later, in a very similar way).
> 
> Can someone (Jagan?) please have a look at this change from the U-Boot
> SPI perspective? And also the other changes in this series?
> I pushed them to the sunxi/next branch:
> https://source.denx.de/u-boot/custodians/u-boot-sunxi/-/commits/next/
> 
> So can people please test this and report whether this now works as
> expected?

I'm very confused I have forgotten much about this patch set.
I'm going to test it, but why has it only been merged now?

Thanks,
Jesse
> Thanks,
> Andre
>   
>> So far we get away with that default frequency, because that is a rather
>> tame 24 MHz, which most SPI flash chips can handle just fine.
>>
>> Move the actual register programming into a separate function, and use
>> .set_speed and .set_mode just to set the variables in our priv structure.
>> Then we only call this new function in claim_bus(), when we are sure
>> that register accesses actually work and are preserved.
>>
>> [1] https://lore.kernel.org/u-boot/20210725231636.879913-17-me@yifangu.com/
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> Reported-by: George Hilliard <thirtythreeforty@gmail.com>
>> ---
>>   drivers/spi/spi-sunxi.c | 95 ++++++++++++++++++++++-------------------
>>   1 file changed, 52 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
>> index b6cd7ddafad..d6b2dd09514 100644
>> --- a/drivers/spi/spi-sunxi.c
>> +++ b/drivers/spi/spi-sunxi.c
>> @@ -221,6 +221,56 @@ err_ahb:
>>   	return ret;
>>   }
>>   
>> +static void sun4i_spi_set_speed_mode(struct udevice *dev)
>> +{
>> +	struct sun4i_spi_priv *priv = dev_get_priv(dev);
>> +	unsigned int div;
>> +	u32 reg;
>> +
>> +	/*
>> +	 * Setup clock divider.
>> +	 *
>> +	 * We have two choices there. Either we can use the clock
>> +	 * divide rate 1, which is calculated thanks to this formula:
>> +	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
>> +	 * Or we can use CDR2, which is calculated with the formula:
>> +	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
>> +	 * Whether we use the former or the latter is set through the
>> +	 * DRS bit.
>> +	 *
>> +	 * First try CDR2, and if we can't reach the expected
>> +	 * frequency, fall back to CDR1.
>> +	 */
>> +
>> +	div = SUN4I_SPI_MAX_RATE / (2 * priv->freq);
>> +	reg = readl(SPI_REG(priv, SPI_CCR));
>> +
>> +	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
>> +		if (div > 0)
>> +			div--;
>> +
>> +		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
>> +		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
>> +	} else {
>> +		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(priv->freq);
>> +		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
>> +		reg |= SUN4I_CLK_CTL_CDR1(div);
>> +	}
>> +
>> +	writel(reg, SPI_REG(priv, SPI_CCR));
>> +
>> +	reg = readl(SPI_REG(priv, SPI_TCR));
>> +	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
>> +
>> +	if (priv->mode & SPI_CPOL)
>> +		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
>> +
>> +	if (priv->mode & SPI_CPHA)
>> +		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
>> +
>> +	writel(reg, SPI_REG(priv, SPI_TCR));
>> +}
>> +
>>   static int sun4i_spi_claim_bus(struct udevice *dev)
>>   {
>>   	struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
>> @@ -240,6 +290,8 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
>>   	setbits_le32(SPI_REG(priv, SPI_TCR), SPI_BIT(priv, SPI_TCR_CS_MANUAL) |
>>   		     SPI_BIT(priv, SPI_TCR_CS_ACTIVE_LOW));
>>   
>> +	sun4i_spi_set_speed_mode(dev->parent);
>> +
>>   	return 0;
>>   }
>>   
>> @@ -329,46 +381,14 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
>>   {
>>   	struct sun4i_spi_plat *plat = dev_get_plat(dev);
>>   	struct sun4i_spi_priv *priv = dev_get_priv(dev);
>> -	unsigned int div;
>> -	u32 reg;
>>   
>>   	if (speed > plat->max_hz)
>>   		speed = plat->max_hz;
>>   
>>   	if (speed < SUN4I_SPI_MIN_RATE)
>>   		speed = SUN4I_SPI_MIN_RATE;
>> -	/*
>> -	 * Setup clock divider.
>> -	 *
>> -	 * We have two choices there. Either we can use the clock
>> -	 * divide rate 1, which is calculated thanks to this formula:
>> -	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
>> -	 * Or we can use CDR2, which is calculated with the formula:
>> -	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
>> -	 * Whether we use the former or the latter is set through the
>> -	 * DRS bit.
>> -	 *
>> -	 * First try CDR2, and if we can't reach the expected
>> -	 * frequency, fall back to CDR1.
>> -	 */
>> -
>> -	div = SUN4I_SPI_MAX_RATE / (2 * speed);
>> -	reg = readl(SPI_REG(priv, SPI_CCR));
>> -
>> -	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
>> -		if (div > 0)
>> -			div--;
>> -
>> -		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
>> -		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
>> -	} else {
>> -		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(speed);
>> -		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
>> -		reg |= SUN4I_CLK_CTL_CDR1(div);
>> -	}
>>   
>>   	priv->freq = speed;
>> -	writel(reg, SPI_REG(priv, SPI_CCR));
>>   
>>   	return 0;
>>   }
>> @@ -376,19 +396,8 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
>>   static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
>>   {
>>   	struct sun4i_spi_priv *priv = dev_get_priv(dev);
>> -	u32 reg;
>> -
>> -	reg = readl(SPI_REG(priv, SPI_TCR));
>> -	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
>> -
>> -	if (mode & SPI_CPOL)
>> -		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
>> -
>> -	if (mode & SPI_CPHA)
>> -		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
>>   
>>   	priv->mode = mode;
>> -	writel(reg, SPI_REG(priv, SPI_TCR));
>>   
>>   	return 0;
>>   }
> 

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

* Re: [PATCH 2/7] spi: sunxi: refactor SPI speed/mode programming
  2022-06-28  0:31   ` Andre Przywara
  2022-06-28  3:43     ` Jesse Taube
@ 2022-06-30  3:25     ` Jesse Taube
  1 sibling, 0 replies; 16+ messages in thread
From: Jesse Taube @ 2022-06-30  3:25 UTC (permalink / raw)
  To: Andre Przywara, Jagan Teki, George Hilliard, qianfan Zhao
  Cc: Icenowy Zheng, Yifan Gu, Giulio Benetti, Samuel Holland,
	Jernej Skrabec, linux-sunxi, u-boot



On 6/27/22 20:31, Andre Przywara wrote:
> On Tue,  3 May 2022 22:20:35 +0100
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
> Hi,
> 
>> As George rightfully pointed out [1], the spi-sunxi driver programs the
>> speed and mode settings only when the respective functions are called,
>> but this gets lost over a call to release_bus(). That asserts the
>> reset line, thus forces each SPI register back to its default value.
>> Adding to that, trying to program SPI_CCR and SPI_TCR might be pointless
>> in the first place, when the reset line is still asserted (before
>> claim_bus()), so those setting won't apply most of the time. In reality
>> I see two nested claim_bus() calls for the first use, so settings between
>> the two would work (for instance for the initial "sf probe"). However
>> later on the speed setting is not programmed into the hardware anymore.
> 
> So this issue was addressed with patches by both George (earlier, in a
> different way) and Qianfan (later, in a very similar way).
> 
> Can someone (Jagan?) please have a look at this change from the U-Boot
> SPI perspective? And also the other changes in this series?
> I pushed them to the sunxi/next branch:
> https://source.denx.de/u-boot/custodians/u-boot-sunxi/-/commits/next/
Tested-by: Jesse Taube <Mr.Bossman075@gmail.com>

I talked to Icenowy who also tested and said it worked with spi-nand.

There is one issue but not related to this set, the SPI max clock is 1Mhz.

Another note disabling the clock gates in `sun4i_spi_set_clock` will 
stop you from dumping the memory of the peripheral.

It would also be nice if i was kept in CC for other SUNIV patches.

Thanks,
Jesse
> So can people please test this and report whether this now works as
> expected?
> 
> Thanks,
> Andre
>   
>> So far we get away with that default frequency, because that is a rather
>> tame 24 MHz, which most SPI flash chips can handle just fine.
>>
>> Move the actual register programming into a separate function, and use
>> .set_speed and .set_mode just to set the variables in our priv structure.
>> Then we only call this new function in claim_bus(), when we are sure
>> that register accesses actually work and are preserved.
>>
>> [1] https://lore.kernel.org/u-boot/20210725231636.879913-17-me@yifangu.com/
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> Reported-by: George Hilliard <thirtythreeforty@gmail.com>
>> ---
>>   drivers/spi/spi-sunxi.c | 95 ++++++++++++++++++++++-------------------
>>   1 file changed, 52 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
>> index b6cd7ddafad..d6b2dd09514 100644
>> --- a/drivers/spi/spi-sunxi.c
>> +++ b/drivers/spi/spi-sunxi.c
>> @@ -221,6 +221,56 @@ err_ahb:
>>   	return ret;
>>   }
>>   
>> +static void sun4i_spi_set_speed_mode(struct udevice *dev)
>> +{
>> +	struct sun4i_spi_priv *priv = dev_get_priv(dev);
>> +	unsigned int div;
>> +	u32 reg;
>> +
>> +	/*
>> +	 * Setup clock divider.
>> +	 *
>> +	 * We have two choices there. Either we can use the clock
>> +	 * divide rate 1, which is calculated thanks to this formula:
>> +	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
>> +	 * Or we can use CDR2, which is calculated with the formula:
>> +	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
>> +	 * Whether we use the former or the latter is set through the
>> +	 * DRS bit.
>> +	 *
>> +	 * First try CDR2, and if we can't reach the expected
>> +	 * frequency, fall back to CDR1.
>> +	 */
>> +
>> +	div = SUN4I_SPI_MAX_RATE / (2 * priv->freq);
>> +	reg = readl(SPI_REG(priv, SPI_CCR));
>> +
>> +	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
>> +		if (div > 0)
>> +			div--;
>> +
>> +		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
>> +		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
>> +	} else {
>> +		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(priv->freq);
>> +		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
>> +		reg |= SUN4I_CLK_CTL_CDR1(div);
>> +	}
>> +
>> +	writel(reg, SPI_REG(priv, SPI_CCR));
>> +
>> +	reg = readl(SPI_REG(priv, SPI_TCR));
>> +	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
>> +
>> +	if (priv->mode & SPI_CPOL)
>> +		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
>> +
>> +	if (priv->mode & SPI_CPHA)
>> +		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
>> +
>> +	writel(reg, SPI_REG(priv, SPI_TCR));
>> +}
>> +
>>   static int sun4i_spi_claim_bus(struct udevice *dev)
>>   {
>>   	struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
>> @@ -240,6 +290,8 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
>>   	setbits_le32(SPI_REG(priv, SPI_TCR), SPI_BIT(priv, SPI_TCR_CS_MANUAL) |
>>   		     SPI_BIT(priv, SPI_TCR_CS_ACTIVE_LOW));
>>   
>> +	sun4i_spi_set_speed_mode(dev->parent);
>> +
>>   	return 0;
>>   }
>>   
>> @@ -329,46 +381,14 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
>>   {
>>   	struct sun4i_spi_plat *plat = dev_get_plat(dev);
>>   	struct sun4i_spi_priv *priv = dev_get_priv(dev);
>> -	unsigned int div;
>> -	u32 reg;
>>   
>>   	if (speed > plat->max_hz)
>>   		speed = plat->max_hz;
>>   
>>   	if (speed < SUN4I_SPI_MIN_RATE)
>>   		speed = SUN4I_SPI_MIN_RATE;
>> -	/*
>> -	 * Setup clock divider.
>> -	 *
>> -	 * We have two choices there. Either we can use the clock
>> -	 * divide rate 1, which is calculated thanks to this formula:
>> -	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
>> -	 * Or we can use CDR2, which is calculated with the formula:
>> -	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
>> -	 * Whether we use the former or the latter is set through the
>> -	 * DRS bit.
>> -	 *
>> -	 * First try CDR2, and if we can't reach the expected
>> -	 * frequency, fall back to CDR1.
>> -	 */
>> -
>> -	div = SUN4I_SPI_MAX_RATE / (2 * speed);
>> -	reg = readl(SPI_REG(priv, SPI_CCR));
>> -
>> -	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
>> -		if (div > 0)
>> -			div--;
>> -
>> -		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
>> -		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
>> -	} else {
>> -		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(speed);
>> -		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
>> -		reg |= SUN4I_CLK_CTL_CDR1(div);
>> -	}
>>   
>>   	priv->freq = speed;
>> -	writel(reg, SPI_REG(priv, SPI_CCR));
>>   
>>   	return 0;
>>   }
>> @@ -376,19 +396,8 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
>>   static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
>>   {
>>   	struct sun4i_spi_priv *priv = dev_get_priv(dev);
>> -	u32 reg;
>> -
>> -	reg = readl(SPI_REG(priv, SPI_TCR));
>> -	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
>> -
>> -	if (mode & SPI_CPOL)
>> -		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
>> -
>> -	if (mode & SPI_CPHA)
>> -		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
>>   
>>   	priv->mode = mode;
>> -	writel(reg, SPI_REG(priv, SPI_TCR));
>>   
>>   	return 0;
>>   }
> 

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

* Re: [PATCH 2/7] spi: sunxi: refactor SPI speed/mode programming
  2022-06-28  3:43     ` Jesse Taube
@ 2022-07-18 10:17       ` Andre Przywara
  0 siblings, 0 replies; 16+ messages in thread
From: Andre Przywara @ 2022-07-18 10:17 UTC (permalink / raw)
  To: Jesse Taube
  Cc: Jagan Teki, George Hilliard, qianfan Zhao, Icenowy Zheng,
	Yifan Gu, Giulio Benetti, Samuel Holland, Jernej Skrabec,
	linux-sunxi, u-boot

On Mon, 27 Jun 2022 23:43:25 -0400
Jesse Taube <mr.bossman075@gmail.com> wrote:

Hi Jesse,

> On 6/27/22 20:31, Andre Przywara wrote:
> > On Tue,  3 May 2022 22:20:35 +0100
> > Andre Przywara <andre.przywara@arm.com> wrote:
> > 
> > Hi,
> >   
> >> As George rightfully pointed out [1], the spi-sunxi driver programs the
> >> speed and mode settings only when the respective functions are called,
> >> but this gets lost over a call to release_bus(). That asserts the
> >> reset line, thus forces each SPI register back to its default value.
> >> Adding to that, trying to program SPI_CCR and SPI_TCR might be pointless
> >> in the first place, when the reset line is still asserted (before
> >> claim_bus()), so those setting won't apply most of the time. In reality
> >> I see two nested claim_bus() calls for the first use, so settings between
> >> the two would work (for instance for the initial "sf probe"). However
> >> later on the speed setting is not programmed into the hardware anymore.  
> > 
> > So this issue was addressed with patches by both George (earlier, in a
> > different way) and Qianfan (later, in a very similar way).
> > 
> > Can someone (Jagan?) please have a look at this change from the U-Boot
> > SPI perspective? And also the other changes in this series?
> > I pushed them to the sunxi/next branch:
> > https://source.denx.de/u-boot/custodians/u-boot-sunxi/-/commits/next/
> > 
> > So can people please test this and report whether this now works as
> > expected?  
> 
> I'm very confused I have forgotten much about this patch set.
> I'm going to test it, but why has it only been merged now?

Because no one reviewed it, or even replied to it - hence my email. And
apparently it was even broken.
The code quality in U-Boot is not really great, as the problems fixed with
this series and the other problems emerged lately ([1]) demonstrate. So I
am very reluctant to take any patches without a meaningful review, or at
least without confirmation of their functionality - regardless of the
author. For instance I just found a bug in my patch 4/7 ...

And I didn't really merge them yet, I just pushed them to the sunxi/next
branch, to expose them more widely and encourage testing - which
apparently worked out as planned ;-)

Cheers,
Andre

[1]
https://lore.kernel.org/u-boot/20220711165215.218e21ae@donnerap.cambridge.arm.com/

> >> So far we get away with that default frequency, because that is a rather
> >> tame 24 MHz, which most SPI flash chips can handle just fine.
> >>
> >> Move the actual register programming into a separate function, and use
> >> .set_speed and .set_mode just to set the variables in our priv structure.
> >> Then we only call this new function in claim_bus(), when we are sure
> >> that register accesses actually work and are preserved.
> >>
> >> [1] https://lore.kernel.org/u-boot/20210725231636.879913-17-me@yifangu.com/
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >> Reported-by: George Hilliard <thirtythreeforty@gmail.com>
> >> ---
> >>   drivers/spi/spi-sunxi.c | 95 ++++++++++++++++++++++-------------------
> >>   1 file changed, 52 insertions(+), 43 deletions(-)
> >>
> >> diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
> >> index b6cd7ddafad..d6b2dd09514 100644
> >> --- a/drivers/spi/spi-sunxi.c
> >> +++ b/drivers/spi/spi-sunxi.c
> >> @@ -221,6 +221,56 @@ err_ahb:
> >>   	return ret;
> >>   }
> >>   
> >> +static void sun4i_spi_set_speed_mode(struct udevice *dev)
> >> +{
> >> +	struct sun4i_spi_priv *priv = dev_get_priv(dev);
> >> +	unsigned int div;
> >> +	u32 reg;
> >> +
> >> +	/*
> >> +	 * Setup clock divider.
> >> +	 *
> >> +	 * We have two choices there. Either we can use the clock
> >> +	 * divide rate 1, which is calculated thanks to this formula:
> >> +	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
> >> +	 * Or we can use CDR2, which is calculated with the formula:
> >> +	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
> >> +	 * Whether we use the former or the latter is set through the
> >> +	 * DRS bit.
> >> +	 *
> >> +	 * First try CDR2, and if we can't reach the expected
> >> +	 * frequency, fall back to CDR1.
> >> +	 */
> >> +
> >> +	div = SUN4I_SPI_MAX_RATE / (2 * priv->freq);
> >> +	reg = readl(SPI_REG(priv, SPI_CCR));
> >> +
> >> +	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> >> +		if (div > 0)
> >> +			div--;
> >> +
> >> +		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
> >> +		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
> >> +	} else {
> >> +		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(priv->freq);
> >> +		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
> >> +		reg |= SUN4I_CLK_CTL_CDR1(div);
> >> +	}
> >> +
> >> +	writel(reg, SPI_REG(priv, SPI_CCR));
> >> +
> >> +	reg = readl(SPI_REG(priv, SPI_TCR));
> >> +	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
> >> +
> >> +	if (priv->mode & SPI_CPOL)
> >> +		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
> >> +
> >> +	if (priv->mode & SPI_CPHA)
> >> +		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
> >> +
> >> +	writel(reg, SPI_REG(priv, SPI_TCR));
> >> +}
> >> +
> >>   static int sun4i_spi_claim_bus(struct udevice *dev)
> >>   {
> >>   	struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
> >> @@ -240,6 +290,8 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
> >>   	setbits_le32(SPI_REG(priv, SPI_TCR), SPI_BIT(priv, SPI_TCR_CS_MANUAL) |
> >>   		     SPI_BIT(priv, SPI_TCR_CS_ACTIVE_LOW));
> >>   
> >> +	sun4i_spi_set_speed_mode(dev->parent);
> >> +
> >>   	return 0;
> >>   }
> >>   
> >> @@ -329,46 +381,14 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
> >>   {
> >>   	struct sun4i_spi_plat *plat = dev_get_plat(dev);
> >>   	struct sun4i_spi_priv *priv = dev_get_priv(dev);
> >> -	unsigned int div;
> >> -	u32 reg;
> >>   
> >>   	if (speed > plat->max_hz)
> >>   		speed = plat->max_hz;
> >>   
> >>   	if (speed < SUN4I_SPI_MIN_RATE)
> >>   		speed = SUN4I_SPI_MIN_RATE;
> >> -	/*
> >> -	 * Setup clock divider.
> >> -	 *
> >> -	 * We have two choices there. Either we can use the clock
> >> -	 * divide rate 1, which is calculated thanks to this formula:
> >> -	 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
> >> -	 * Or we can use CDR2, which is calculated with the formula:
> >> -	 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
> >> -	 * Whether we use the former or the latter is set through the
> >> -	 * DRS bit.
> >> -	 *
> >> -	 * First try CDR2, and if we can't reach the expected
> >> -	 * frequency, fall back to CDR1.
> >> -	 */
> >> -
> >> -	div = SUN4I_SPI_MAX_RATE / (2 * speed);
> >> -	reg = readl(SPI_REG(priv, SPI_CCR));
> >> -
> >> -	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> >> -		if (div > 0)
> >> -			div--;
> >> -
> >> -		reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
> >> -		reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
> >> -	} else {
> >> -		div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(speed);
> >> -		reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS);
> >> -		reg |= SUN4I_CLK_CTL_CDR1(div);
> >> -	}
> >>   
> >>   	priv->freq = speed;
> >> -	writel(reg, SPI_REG(priv, SPI_CCR));
> >>   
> >>   	return 0;
> >>   }
> >> @@ -376,19 +396,8 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
> >>   static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
> >>   {
> >>   	struct sun4i_spi_priv *priv = dev_get_priv(dev);
> >> -	u32 reg;
> >> -
> >> -	reg = readl(SPI_REG(priv, SPI_TCR));
> >> -	reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
> >> -
> >> -	if (mode & SPI_CPOL)
> >> -		reg |= SPI_BIT(priv, SPI_TCR_CPOL);
> >> -
> >> -	if (mode & SPI_CPHA)
> >> -		reg |= SPI_BIT(priv, SPI_TCR_CPHA);
> >>   
> >>   	priv->mode = mode;
> >> -	writel(reg, SPI_REG(priv, SPI_TCR));
> >>   
> >>   	return 0;
> >>   }  
> >   
> 


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

end of thread, other threads:[~2022-07-18 10:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-03 21:20 [PATCH 0/7] sunxi: F1C100s: enable MMC and SPI in U-Boot proper Andre Przywara
2022-05-03 21:20 ` [PATCH 1/7] clk: sunxi: implement clock driver for suniv f1c100s Andre Przywara
2022-05-24 16:10   ` Andre Przywara
2022-05-03 21:20 ` [PATCH 2/7] spi: sunxi: refactor SPI speed/mode programming Andre Przywara
2022-06-28  0:31   ` Andre Przywara
2022-06-28  3:43     ` Jesse Taube
2022-07-18 10:17       ` Andre Przywara
2022-06-30  3:25     ` Jesse Taube
2022-05-03 21:20 ` [PATCH 3/7] spi: sunxi: improve SPI clock calculation Andre Przywara
2022-05-03 21:20 ` [PATCH 4/7] spi: sunxi: Add support for F1C100s SPI controller Andre Przywara
2022-05-03 21:20 ` [PATCH 5/7] sunxi: F1C100s: update DT files from Linux Andre Przywara
2022-05-05 11:26   ` Jesse Taube
2022-05-24 16:11   ` Andre Przywara
2022-05-03 21:20 ` [PATCH 6/7] Revert "sunxi: f1c100s: Drop SYSRESET to enable reset functionality" Andre Przywara
2022-05-24 16:11   ` Andre Przywara
2022-05-03 21:20 ` [PATCH 7/7] sunxi: licheepi_nano: enable SPI flash Andre Przywara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.