2017-07-28 22:07:23

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH 0/5] Initial support for Adaptrum Anarion SOC

We've reached the point where we can boot a basic linux system on the
new SOC, and I'm very excited to share the code I've developed in the
process.

This series includes a new driver for the QSPI controller on the chip,
although the flash layout is not specified in the devicetree. The
flash layout is still being worked on, as it depends on finalizing
other components.

The scope of this series is to add enough support to get to a working
system. Keep in mind that the SOC is emulated on an FPGA, and things
are not running too fast. For this reason, as there's yet no way to
get any meaningful speed measurements with the CPU clock at 12 MHz.
Once the silicon arrives, I'll look at the performance aspect and
other aspects that we simply can't support on an FPGA.

Alex

Alexandru Gagniuc (5):
of: Add vendor prefix for Adaptrum, Inc.
ARC: [plat-anarion] Add early boot workarounds for Anarion SOC
net: stmmac: Add Adaptrum Anarion GMAC glue layer
mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller
ARC: DTS: Add device-tree for Anarion-based development board

.../devicetree/bindings/mtd/anarion-quadspi.txt | 22 +
.../devicetree/bindings/net/anarion-gmac.txt | 25 ++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
arch/arc/Kconfig | 1 +
arch/arc/Makefile | 1 +
arch/arc/boot/dts/adaptrum_anarion.dtsi | 107 +++++
arch/arc/boot/dts/adaptrum_anarion_fpga.dts | 49 +++
arch/arc/plat-anarion/Kconfig | 10 +
arch/arc/plat-anarion/Makefile | 7 +
arch/arc/plat-anarion/platform.c | 39 ++
drivers/mtd/spi-nor/Kconfig | 7 +
drivers/mtd/spi-nor/Makefile | 1 +
drivers/mtd/spi-nor/anarion-quadspi.c | 490 +++++++++++++++++++++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 9 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../net/ethernet/stmicro/stmmac/dwmac-anarion.c | 151 +++++++
16 files changed, 921 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
create mode 100644 Documentation/devicetree/bindings/net/anarion-gmac.txt
create mode 100644 arch/arc/boot/dts/adaptrum_anarion.dtsi
create mode 100644 arch/arc/boot/dts/adaptrum_anarion_fpga.dts
create mode 100644 arch/arc/plat-anarion/Kconfig
create mode 100644 arch/arc/plat-anarion/Makefile
create mode 100644 arch/arc/plat-anarion/platform.c
create mode 100644 drivers/mtd/spi-nor/anarion-quadspi.c
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-anarion.c

--
2.9.3


2017-07-28 22:07:32

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH 1/5] of: Add vendor prefix for Adaptrum, Inc.

Signed-off-by: Alexandru Gagniuc <[email protected]>
---
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index daf465be..33ee112 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -9,6 +9,7 @@ actions Actions Semiconductor Co., Ltd.
active-semi Active-Semi International Inc
ad Avionic Design GmbH
adapteva Adapteva, Inc.
+adaptrum Adaptrum, Inc.
adh AD Holdings Plc.
adi Analog Devices, Inc.
advantech Advantech Corporation
--
2.9.3

2017-07-28 22:07:37

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH 2/5] ARC: [plat-anarion] Add early boot workarounds for Anarion SOC

An ARC, the interrupts are enabled globally, rather than per-line, as
drivers request it. Thus, we need to make sure that peripherals don't
generate any before the respective drivers are probed.
The GMAC is infamous for spamming interrupts, so it must be kept in
reset until the driver is probed and interrupt mapping established.

Signed-off-by: Alexandru Gagniuc <[email protected]>
---
arch/arc/Kconfig | 1 +
arch/arc/Makefile | 1 +
arch/arc/plat-anarion/Kconfig | 10 ++++++++++
arch/arc/plat-anarion/Makefile | 7 +++++++
arch/arc/plat-anarion/platform.c | 39 +++++++++++++++++++++++++++++++++++++++
5 files changed, 58 insertions(+)
create mode 100644 arch/arc/plat-anarion/Kconfig
create mode 100644 arch/arc/plat-anarion/Makefile
create mode 100644 arch/arc/plat-anarion/platform.c

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index a545969..dff8423 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -100,6 +100,7 @@ source "arch/arc/plat-sim/Kconfig"
source "arch/arc/plat-tb10x/Kconfig"
source "arch/arc/plat-axs10x/Kconfig"
#New platform adds here
+source "arch/arc/plat-anarion/Kconfig"
source "arch/arc/plat-eznps/Kconfig"

endmenu
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index 44ef35d..9bc0048 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -109,6 +109,7 @@ core-y += arch/arc/boot/dts/

core-$(CONFIG_ARC_PLAT_SIM) += arch/arc/plat-sim/
core-$(CONFIG_ARC_PLAT_TB10X) += arch/arc/plat-tb10x/
+core-$(CONFIG_ARC_PLAT_ANARION) += arch/arc/plat-anarion/
core-$(CONFIG_ARC_PLAT_AXS10X) += arch/arc/plat-axs10x/
core-$(CONFIG_ARC_PLAT_EZNPS) += arch/arc/plat-eznps/

diff --git a/arch/arc/plat-anarion/Kconfig b/arch/arc/plat-anarion/Kconfig
new file mode 100644
index 0000000..632c7be
--- /dev/null
+++ b/arch/arc/plat-anarion/Kconfig
@@ -0,0 +1,10 @@
+#
+# (C) Copyright 2017 Adaptrum, Inc.
+# Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
+# Licensed under the GPLv2 or (at your option) any later version.
+#
+
+menuconfig ARC_PLAT_ANARION
+ bool "Adaptrum Anarion based platforms"
+ help
+ Support for Adaptrum Anarion based ARC platforms.
diff --git a/arch/arc/plat-anarion/Makefile b/arch/arc/plat-anarion/Makefile
new file mode 100644
index 0000000..9596a41
--- /dev/null
+++ b/arch/arc/plat-anarion/Makefile
@@ -0,0 +1,7 @@
+#
+# (C) Copyright 2017 Adaptrum, Inc.
+# Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
+# Licensed under the GPLv2 or (at your option) any later version.
+#
+
+obj-y := platform.o
diff --git a/arch/arc/plat-anarion/platform.c b/arch/arc/plat-anarion/platform.c
new file mode 100644
index 0000000..ef0d381
--- /dev/null
+++ b/arch/arc/plat-anarion/platform.c
@@ -0,0 +1,39 @@
+/*
+ * Workarounds for Adaptrum Anarion SOC
+ *
+ * Copyright (C) 2017, Adaptrum, Inc.
+ * (Written by Alexandru Gagniuc <alex.g at adaptrum.com> for Adaptrum, Inc.)
+ * Licensed under the GPLv2 or (at your option) any later version.
+ */
+
+#include <asm/io.h>
+#include <linux/init.h>
+#include <asm/mach_desc.h>
+
+#define GMAC0_RESET 0xf2018000
+#define GMAC1_RESET 0xf2018100
+
+/* This works around an issue where the GMAC will generate interrupts before
+ * the driver is probed, confusing the heck out of the early boot.
+ */
+static void __init anarion_gmac_irq_storm_workaround(void)
+{
+ writel(1, (void *)GMAC0_RESET);
+ writel(1, (void *)GMAC1_RESET);
+}
+
+static void __init anarion_early_init(void)
+{
+ anarion_gmac_irq_storm_workaround();
+ /* Please, no more workarounds!!! */
+}
+
+static const char *anarion_compat[] __initconst = {
+ "adaptrum,anarion",
+ NULL,
+};
+
+MACHINE_START(ANARION, "anarion")
+ .dt_compat = anarion_compat,
+ .init_early = anarion_early_init,
+MACHINE_END
--
2.9.3

2017-07-28 22:07:45

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH 3/5] net: stmmac: Add Adaptrum Anarion GMAC glue layer

Before the GMAC on the Anarion chip can be used, the PHY interface
selection must be configured with the DWMAC block in reset.

This layer covers a block containing only two registers. Although it
is possible to model this as a reset controller and use the "resets"
property of stmmac, it's much more intuitive to include this in the
glue layer instead.

At this time only RGMII is supported, because it is the only mode
which has been validated hardware-wise.

Signed-off-by: Alexandru Gagniuc <[email protected]>
---
.../devicetree/bindings/net/anarion-gmac.txt | 25 ++++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 9 ++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../net/ethernet/stmicro/stmmac/dwmac-anarion.c | 151 +++++++++++++++++++++
4 files changed, 186 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/anarion-gmac.txt
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-anarion.c

diff --git a/Documentation/devicetree/bindings/net/anarion-gmac.txt b/Documentation/devicetree/bindings/net/anarion-gmac.txt
new file mode 100644
index 0000000..fe67896
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/anarion-gmac.txt
@@ -0,0 +1,25 @@
+* Adaptrum Anarion ethernet controller
+
+This device is a platform glue layer for stmmac.
+Please see stmmac.txt for the other unchanged properties.
+
+Required properties:
+ - compatible: Should be "adaptrum,anarion-gmac", "snps,dwmac"
+ - phy-mode: Should be "rgmii". Other modes are not currently supported.
+
+
+Examples:
+
+ gmac1: ethernet@f2014000 {
+ compatible = "adaptrum,anarion-gmac", "snps,dwmac";
+ reg = <0xf2014000 0x4000>, <0xf2018100 8>;
+
+ interrupt-parent = <&core_intc>;
+ interrupts = <21>;
+ interrupt-names = "macirq";
+
+ clocks = <&core_clk>;
+ clock-names = "stmmaceth";
+
+ phy-mode = "rgmii";
+ };
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 85c0e41..9703576 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -45,6 +45,15 @@ config DWMAC_GENERIC
platform specific code to function or is using platform
data for setup.

+config DWMAC_ANARION
+ tristate "Adaptrum Anarion GMAC support"
+ default ARC
+ depends on OF && (ARC || COMPILE_TEST)
+ help
+ Support for Adaptrum Anarion GMAC Ethernet controller.
+
+ This selects the Anarion SoC glue layer support for the stmmac driver.
+
config DWMAC_IPQ806X
tristate "QCA IPQ806x DWMAC support"
default ARCH_QCOM
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index fd4937a..238307f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -7,6 +7,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \

# Ordering matters. Generic driver must be last.
obj-$(CONFIG_STMMAC_PLATFORM) += stmmac-platform.o
+obj-$(CONFIG_DWMAC_ANARION) += dwmac-anarion.o
obj-$(CONFIG_DWMAC_IPQ806X) += dwmac-ipq806x.o
obj-$(CONFIG_DWMAC_LPC18XX) += dwmac-lpc18xx.o
obj-$(CONFIG_DWMAC_MESON) += dwmac-meson.o dwmac-meson8b.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-anarion.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-anarion.c
new file mode 100644
index 0000000..6da4d9a
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-anarion.c
@@ -0,0 +1,151 @@
+/*
+ * Adaptrum Anarion DWMAC glue layer
+ *
+ * Copyright (C) 2017, Adaptrum, Inc.
+ * (Written by Alexandru Gagniuc <alex.g at adaptrum.com> for Adaptrum, Inc.)
+ * Licensed under the GPLv2 or (at your option) any later version.
+ */
+
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/stmmac.h>
+
+#include "stmmac.h"
+#include "stmmac_platform.h"
+
+#define GMAC_RESET_CONTROL_REG 0
+#define GMAC_SW_CONFIG_REG 4
+#define GMAC_CONFIG_INTF_SEL_MASK (0x7 << 0)
+#define GMAC_CONFIG_INTF_RGMII (0x1 << 0)
+
+struct anarion_gmac {
+ uintptr_t ctl_block;
+ uint32_t phy_intf_sel;
+};
+
+static uint32_t gmac_read_reg(struct anarion_gmac *gmac, uint8_t reg)
+{
+ return readl((void *)(gmac->ctl_block + reg));
+};
+
+static void gmac_write_reg(struct anarion_gmac *gmac, uint8_t reg, uint32_t val)
+{
+ writel(val, (void *)(gmac->ctl_block + reg));
+}
+
+static int anarion_gmac_init(struct platform_device *pdev, void *priv)
+{
+ uint32_t sw_config;
+ struct anarion_gmac *gmac = priv;
+
+ /* Reset logic, configure interface mode, then release reset. SIMPLE! */
+ gmac_write_reg(gmac, GMAC_RESET_CONTROL_REG, 1);
+
+ sw_config = gmac_read_reg(gmac, GMAC_SW_CONFIG_REG);
+ sw_config &= ~GMAC_CONFIG_INTF_SEL_MASK;
+ sw_config |= (gmac->phy_intf_sel & GMAC_CONFIG_INTF_SEL_MASK);
+ gmac_write_reg(gmac, GMAC_SW_CONFIG_REG, sw_config);
+
+ gmac_write_reg(gmac, GMAC_RESET_CONTROL_REG, 0);
+
+ return 0;
+}
+
+static void anarion_gmac_exit(struct platform_device *pdev, void *priv)
+{
+ struct anarion_gmac *gmac = priv;
+
+ gmac_write_reg(gmac, GMAC_RESET_CONTROL_REG, 1);
+}
+
+static struct anarion_gmac *anarion_config_dt(struct platform_device *pdev)
+{
+ int phy_mode;
+ struct resource *res;
+ void __iomem *ctl_block;
+ struct anarion_gmac *gmac;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ ctl_block = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(ctl_block)) {
+ dev_err(&pdev->dev, "Cannot get reset region (%ld)!\n",
+ PTR_ERR(ctl_block));
+ return ctl_block;
+ }
+
+ gmac = devm_kzalloc(&pdev->dev, sizeof(*gmac), GFP_KERNEL);
+ if (!gmac)
+ return ERR_PTR(-ENOMEM);
+
+ gmac->ctl_block = (uintptr_t)ctl_block;
+
+ phy_mode = of_get_phy_mode(pdev->dev.of_node);
+ switch (phy_mode) {
+ case PHY_INTERFACE_MODE_RGMII: /* Fall through */
+ case PHY_INTERFACE_MODE_RGMII_ID /* Fall through */:
+ case PHY_INTERFACE_MODE_RGMII_RXID: /* Fall through */
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ gmac->phy_intf_sel = GMAC_CONFIG_INTF_RGMII;
+ break;
+ default:
+ dev_err(&pdev->dev, "Unsupported phy-mode (%d)\n",
+ phy_mode);
+ return ERR_PTR(-ENOTSUPP);
+ }
+
+ return gmac;
+}
+
+static int anarion_dwmac_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct anarion_gmac *gmac;
+ struct plat_stmmacenet_data *plat_dat;
+ struct stmmac_resources stmmac_res;
+
+ ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+ if (ret)
+ return ret;
+
+ gmac = anarion_config_dt(pdev);
+ if (IS_ERR(gmac))
+ return PTR_ERR(gmac);
+
+ plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
+ if (IS_ERR(plat_dat))
+ return PTR_ERR(plat_dat);
+
+ plat_dat->init = anarion_gmac_init;
+ plat_dat->exit = anarion_gmac_exit;
+ anarion_gmac_init(pdev, gmac);
+
+ ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+ if (ret) {
+ stmmac_remove_config_dt(pdev, plat_dat);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id anarion_dwmac_match[] = {
+ { .compatible = "adaptrum,anarion-gmac" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, anarion_dwmac_match);
+
+static struct platform_driver anarion_dwmac_driver = {
+ .probe = anarion_dwmac_probe,
+ .remove = stmmac_pltfr_remove,
+ .driver = {
+ .name = "anarion-dwmac",
+ .pm = &stmmac_pltfr_pm_ops,
+ .of_match_table = anarion_dwmac_match,
+ },
+};
+module_platform_driver(anarion_dwmac_driver);
+
+MODULE_DESCRIPTION("Adaptrum Anarion DWMAC specific glue layer");
+MODULE_AUTHOR("Alexandru Gagniuc <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.9.3

2017-07-28 22:07:51

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH 5/5] ARC: DTS: Add device-tree for Anarion-based development board

Signed-off-by: Alexandru Gagniuc <[email protected]>
---
arch/arc/boot/dts/adaptrum_anarion.dtsi | 107 ++++++++++++++++++++++++++++
arch/arc/boot/dts/adaptrum_anarion_fpga.dts | 49 +++++++++++++
2 files changed, 156 insertions(+)
create mode 100644 arch/arc/boot/dts/adaptrum_anarion.dtsi
create mode 100644 arch/arc/boot/dts/adaptrum_anarion_fpga.dts

diff --git a/arch/arc/boot/dts/adaptrum_anarion.dtsi b/arch/arc/boot/dts/adaptrum_anarion.dtsi
new file mode 100644
index 0000000..f50958f
--- /dev/null
+++ b/arch/arc/boot/dts/adaptrum_anarion.dtsi
@@ -0,0 +1,107 @@
+/*
+ * (C) Copyright 2017 Adaptrum, Inc.
+ * Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
+ * Licensed under the GPLv2 or (at your option) any later version
+ */
+
+#include "skeleton.dtsi"
+
+/ {
+ compatible = "adaptrum,anarion";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ soc {
+ compatible = "simple-bus";
+ device_type = "soc";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ interrupt-parent = <&core_intc>;
+
+ core_intc: interrupt-controller {
+ compatible = "snps,arc700-intc";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
+
+ uart0: serial@f2202100 {
+ compatible = "ns16550";
+ reg = <0xf2202100 0x20>;
+ interrupts = <8>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ clock-frequency = <192000000>;
+ status = "disabled";
+ };
+
+ uart1: serial@f2202200 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0xf2202200 0x20>;
+ interrupts = <8>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ clock-frequency = <192000000>;
+ status = "disabled";
+ };
+
+ uart2: serial@f2202300 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0xf2202300 0x20>;
+ interrupts = <8>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ clock-frequency = <192000000>;
+ status = "disabled";
+ };
+
+ uart3: serial@f2202400 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0xf2202400 0x20>;
+ interrupts = <8>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ clock-frequency = <192000000>;
+ status = "disabled";
+ };
+
+ qspi: qspi@f200f000 {
+ compatible = "adaptrum,anarion-qspi";
+ reg = <0xf200f000 0x1000>,
+ <0x20000000 0x08000000>;
+
+ interrupts = <10>;
+ status = "disabled";
+ };
+
+ gmac0: ethernet@f2010000 {
+ compatible = "snps,dwmac";
+ reg = <0xf2010000 0x4000>;
+
+ interrupt-parent = <&core_intc>;
+ interrupts = <20>;
+ interrupt-names = "macirq";
+
+ clocks = <&core_clk>;
+ clock-names = "stmmaceth";
+
+ snps,pbl = <32>;
+ status = "disabled";
+ };
+
+ gmac1: ethernet@f2014000 {
+ compatible = "adaptrum,anarion-gmac", "snps,dwmac";
+ reg = <0xf2014000 0x4000>, <0xf2018100 8>;
+
+ interrupt-parent = <&core_intc>;
+ interrupts = <21>;
+ interrupt-names = "macirq";
+
+ clocks = <&core_clk>;
+ clock-names = "stmmaceth";
+
+ snps,pbl = <32>;
+ status = "disabled";
+ };
+ };
+};
diff --git a/arch/arc/boot/dts/adaptrum_anarion_fpga.dts b/arch/arc/boot/dts/adaptrum_anarion_fpga.dts
new file mode 100644
index 0000000..36173b2
--- /dev/null
+++ b/arch/arc/boot/dts/adaptrum_anarion_fpga.dts
@@ -0,0 +1,49 @@
+/*
+ * (C) Copyright 2017 Adaptrum, Inc.
+ * Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
+ * Licensed under the GPLv2 or (at your option) any later version
+ */
+
+/dts-v1/;
+
+#include "adaptrum_anarion.dtsi"
+
+/ {
+ model = "adaptrum,anarion";
+ compatible = "adaptrum,anarion";
+
+ chosen {
+ bootargs = "earlycon console=ttyS0,115200n8";
+ stdout-path = "serial0:115200n8";
+ };
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ core_clk: core_clk {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <12000000>;
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
+
+&qspi {
+ status = "okay";
+ flash0: w25q128fvn@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "winbond,w25q128", "jedec,spi-nor";
+ spi-max-frequency = <70000000>;
+ m25p,fast-read;
+ };
+};
+
+&gmac1 {
+ phy-mode = "rgmii";
+ status = "okay";
+};
--
2.9.3

2017-07-28 22:08:23

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

Add support for the QSPI controller found in Adaptrum Anarion SOCs.
This controller is designed specifically to handle SPI NOR chips, and
the driver is modeled as such.

Because the system is emulated on an FPGA, we don't have a way to test
all the hardware adjustemts, so only basic features are implemented at
this time.

Signed-off-by: Alexandru Gagniuc <[email protected]>
---
.../devicetree/bindings/mtd/anarion-quadspi.txt | 22 +
drivers/mtd/spi-nor/Kconfig | 7 +
drivers/mtd/spi-nor/Makefile | 1 +
drivers/mtd/spi-nor/anarion-quadspi.c | 490 +++++++++++++++++++++
4 files changed, 520 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
create mode 100644 drivers/mtd/spi-nor/anarion-quadspi.c

diff --git a/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt b/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
new file mode 100644
index 0000000..b4971e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
@@ -0,0 +1,22 @@
+* Adaptrum Anarion Quad SPI controller
+
+Required properties:
+- compatible : Should be "adaptrum,anarion-qspi".
+- reg : Contains two entries, each of which is a tuple consisting of a
+ physical address and length. The first entry is the address and
+ length of the controller register set. The second entry is the
+ address and length of the memory-mapped flash. This second region is
+ the region where the controller responds to XIP requests, and may be
+ larger than the size of the attached flash.
+
+Example:
+
+ qspi: qspi@f200f000 {
+ compatible = "adaptrum,anarion-qspi";
+ reg = <0xf200f000 0x1000>,
+ <0x20000000 0x08000000>;
+
+ flash0: w25q128fvn@0 {
+ ...
+ }
+ };
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 293c8a4..98dc012 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -48,6 +48,13 @@ config SPI_ATMEL_QUADSPI
This driver does not support generic SPI. The implementation only
supports SPI NOR.

+config SPI_ANARION_QUADSPI
+ tristate "Adaptrum Anarion Quad SPI Controller"
+ depends on OF && HAS_IOMEM
+ help
+ Enable support for the Adaptrum Anarion Quad SPI controller.
+ This driver does not support generic SPI. It only supports SPI NOR.
+
config SPI_CADENCE_QUADSPI
tristate "Cadence Quad SPI controller"
depends on OF && (ARM || COMPILE_TEST)
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 285aab8..53635f6 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,6 +1,7 @@
obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
obj-$(CONFIG_SPI_ASPEED_SMC) += aspeed-smc.o
obj-$(CONFIG_SPI_ATMEL_QUADSPI) += atmel-quadspi.o
+obj-$(CONFIG_SPI_ANARION_QUADSPI) += anarion-quadspi.o
obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o
obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
obj-$(CONFIG_SPI_HISI_SFC) += hisi-sfc.o
diff --git a/drivers/mtd/spi-nor/anarion-quadspi.c b/drivers/mtd/spi-nor/anarion-quadspi.c
new file mode 100644
index 0000000..d981356
--- /dev/null
+++ b/drivers/mtd/spi-nor/anarion-quadspi.c
@@ -0,0 +1,490 @@
+/*
+ * Adaptrum Anarion Quad SPI controller driver
+ *
+ * Copyright (C) 2017, Adaptrum, Inc.
+ * (Written by Alexandru Gagniuc <alex.g at adaptrum.com> for Adaptrum, Inc.)
+ * Licensed under the GPLv2 or (at your option) any later version.
+ */
+
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define ASPI_REG_CLOCK 0x00
+#define ASPI_REG_GO 0x04
+#define ASPI_REG_CHAIN 0x08
+#define ASPI_REG_CMD1 0x0c
+#define ASPI_REG_CMD2 0x10
+#define ASPI_REG_ADDR1 0x14
+#define ASPI_REG_ADDR2 0x18
+#define ASPI_REG_PERF1 0x1c
+#define ASPI_REG_PERF2 0x20
+#define ASPI_REG_HI_Z 0x24
+#define ASPI_REG_BYTE_COUNT 0x28
+#define ASPI_REG_DATA1 0x2c
+#define ASPI_REG_DATA2 0x30
+#define ASPI_REG_FINISH 0x34
+#define ASPI_REG_XIP 0x38
+#define ASPI_REG_FIFO_STATUS 0x3c
+#define ASPI_REG_LAT 0x40
+#define ASPI_REG_OUT_DELAY_0 0x44
+#define ASPI_REG_OUT_DELAY_1 0x48
+#define ASPI_REG_IN_DELAY_0 0x4c
+#define ASPI_REG_IN_DELAY_1 0x50
+#define ASPI_REG_DQS_DELAY 0x54
+#define ASPI_REG_STATUS 0x58
+#define ASPI_REG_IRQ_ENABLE 0x5c
+#define ASPI_REG_IRQ_STATUS 0x60
+#define ASPI_REG_AXI_BAR 0x64
+#define ASPI_REG_READ_CFG 0x6c
+
+#define ASPI_CLK_SW_RESET (1 << 0)
+#define ASPI_CLK_RESET_BUF (1 << 1)
+#define ASPI_CLK_RESET_ALL (ASPI_CLK_SW_RESET | ASPI_CLK_RESET_BUF)
+#define ASPI_CLK_SPI_MODE3 (1 << 2)
+#define ASPI_CLOCK_DIV_MASK (0xff << 8)
+#define ASPI_CLOCK_DIV(d) (((d) << 8) & ASPI_CLOCK_DIV_MASK)
+
+#define ASPI_TIMEOUT_US 100000
+
+#define ASPI_DATA_LEN_MASK 0x3fff
+#define ASPI_MAX_XFER_LEN (size_t)(ASPI_DATA_LEN_MASK + 1)
+
+#define MODE_IO_X1 (0 << 16)
+#define MODE_IO_X2 (1 << 16)
+#define MODE_IO_X4 (2 << 16)
+#define MODE_IO_SDR_POS_SKEW (0 << 20)
+#define MODE_IO_SDR_NEG_SKEW (1 << 20)
+#define MODE_IO_DDR_34_SKEW (2 << 20)
+#define MODE_IO_DDR_PN_SKEW (3 << 20)
+#define MODE_IO_DDR_DQS (5 << 20)
+
+#define ASPI_STATUS_BUSY (1 << 2)
+
+/*
+ * This mask does not match reality. Get over it:
+ * DATA2: 0x3fff
+ * CMD2: 0x0003
+ * ADDR2: 0x0007
+ * PERF2: 0x0000
+ * HI_Z: 0x003f
+ * BCNT: 0x0007
+ */
+#define CHAIN_LEN(x) ((x - 1) & ASPI_DATA_LEN_MASK)
+
+struct anarion_qspi {
+ struct spi_nor nor;
+ struct device *dev;
+ uintptr_t regbase;
+ uintptr_t xipbase;
+ uint32_t xfer_mode_cmd;
+ uint32_t xfer_mode_addr;
+ uint32_t xfer_mode_data;
+ uint8_t num_hi_z_clocks;
+};
+
+struct qspi_io_chain {
+ uint8_t action;
+ uint32_t data;
+ uint16_t data_len;
+ uint32_t mode;
+};
+
+enum chain_code {
+ CHAIN_NOP = 0,
+ CHAIN_CMD = 1,
+ CHAIN_ADDR = 2,
+ CHAIN_WTFIUM = 3,
+ CHAIN_HI_Z = 4,
+ CHAIN_DATA_OUT = 5,
+ CHAIN_DATA_IN = 6,
+ CHAIN_FINISH = 7,
+};
+
+static const struct chain_to_reg {
+ uint8_t data_reg;
+ uint8_t ctl_reg;
+} chain_to_reg_map[] = {
+ [CHAIN_NOP] = {0, 0},
+ [CHAIN_CMD] = {ASPI_REG_CMD1, ASPI_REG_CMD2},
+ [CHAIN_ADDR] = {ASPI_REG_ADDR1, ASPI_REG_ADDR2},
+ [CHAIN_WTFIUM] = {0, 0},
+ [CHAIN_HI_Z] = {0, ASPI_REG_HI_Z},
+ [CHAIN_DATA_OUT] = {0, ASPI_REG_DATA2},
+ [CHAIN_DATA_IN] = {0, ASPI_REG_DATA2},
+ [CHAIN_FINISH] = {0, ASPI_REG_FINISH},
+};
+
+static uint32_t aspi_read_reg(struct anarion_qspi *spi, uint8_t reg)
+{
+ return readl((void *)(spi->regbase + reg));
+};
+
+static void aspi_write_reg(struct anarion_qspi *spi, uint8_t reg, uint32_t val)
+{
+ writel(val, (void *)(spi->regbase + reg));
+};
+
+static size_t aspi_get_fifo_level(struct anarion_qspi *spi)
+{
+ return aspi_read_reg(spi, ASPI_REG_FIFO_STATUS) & 0xff;
+}
+
+static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf, size_t len)
+{
+ uint32_t data;
+
+ aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
+ while (len >= 4) {
+ data = aspi_read_reg(aspi, ASPI_REG_DATA1);
+ memcpy(buf, &data, sizeof(data));
+ buf += 4;
+ len -= 4;
+ }
+
+ if (len) {
+ aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, len);
+ data = aspi_read_reg(aspi, ASPI_REG_DATA1);
+ memcpy(buf, &data, len);
+ }
+}
+
+static void aspi_seed_fifo(struct anarion_qspi *spi,
+ const uint8_t *buf, size_t len)
+{
+ uint32_t data;
+
+ aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
+ while (len >= 4) {
+ memcpy(&data, buf, sizeof(data));
+ aspi_write_reg(spi, ASPI_REG_DATA1, data);
+ buf += 4;
+ len -= 4;
+ }
+
+ if (len) {
+ aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
+ memcpy(&data, buf, len);
+ aspi_write_reg(spi, ASPI_REG_DATA1, data);
+ }
+}
+
+static int aspi_wait_idle(struct anarion_qspi *aspi)
+{
+ uint32_t status;
+ void *status_reg = (void *)(aspi->regbase + ASPI_REG_STATUS);
+
+ return readl_poll_timeout(status_reg, status,
+ !(status & ASPI_STATUS_BUSY),
+ 1, ASPI_TIMEOUT_US);
+}
+
+static int aspi_poll_and_seed_fifo(struct anarion_qspi *spi,
+ const void *src_addr, size_t len)
+{
+ size_t wait_us, fifo_space = 0, xfer_len;
+ const uint8_t *src = src_addr;
+
+ while (len > 0) {
+ wait_us = 0;
+ while (wait_us++ < ASPI_TIMEOUT_US) {
+ fifo_space = 64 - aspi_get_fifo_level(spi);
+ if (fifo_space)
+ break;
+ udelay(1);
+ }
+
+ xfer_len = min(len, fifo_space);
+ aspi_seed_fifo(spi, src, xfer_len);
+ src += xfer_len;
+ len -= xfer_len;
+ }
+
+ return 0;
+}
+
+static void aspi_setup_chain(struct anarion_qspi *aspi,
+ const struct qspi_io_chain *chain,
+ size_t chain_len)
+{
+ size_t i;
+ uint32_t chain_reg = 0;
+ const struct qspi_io_chain *link;
+ const struct chain_to_reg *regs;
+
+ for (link = chain, i = 0; i < chain_len; i++, link++) {
+ regs = &chain_to_reg_map[link->action];
+
+ if (link->data_len && regs->data_reg)
+ aspi_write_reg(aspi, regs->data_reg, link->data);
+
+ if (regs->ctl_reg)
+ aspi_write_reg(aspi, regs->ctl_reg,
+ CHAIN_LEN(link->data_len) | link->mode);
+
+ chain_reg |= link->action << (i * 4);
+ }
+
+ chain_reg |= CHAIN_FINISH << (i * 4);
+
+ aspi_write_reg(aspi, ASPI_REG_CHAIN, chain_reg);
+}
+
+static int aspi_execute_chain(struct anarion_qspi *aspi)
+{
+ /* Go, johnny go */
+ aspi_write_reg(aspi, ASPI_REG_GO, 1);
+ return aspi_wait_idle(aspi);
+}
+
+static int anarion_spi_read_nor_reg(struct spi_nor *nor, uint8_t opcode,
+ uint8_t *buf, int len)
+{
+ struct anarion_qspi *aspi = nor->priv;
+ struct qspi_io_chain chain[] = {
+ {CHAIN_CMD, opcode, 1, MODE_IO_X1},
+ {CHAIN_DATA_IN, 0, (uint16_t)len, MODE_IO_X1},
+ };
+
+ if (len >= 8)
+ return -EMSGSIZE;
+
+ aspi_setup_chain(aspi, chain, ARRAY_SIZE(chain));
+ aspi_execute_chain(aspi);
+
+ aspi_drain_fifo(aspi, buf, len);
+
+ return 0;
+}
+
+static int anarion_qspi_cmd_addr(struct anarion_qspi *aspi, uint16_t cmd,
+ uint32_t addr, int addr_len)
+{
+ size_t chain_size;
+ const struct qspi_io_chain chain[] = {
+ {CHAIN_CMD, cmd, 1, MODE_IO_X1},
+ {CHAIN_ADDR, addr, addr_len, MODE_IO_X1},
+ };
+
+ chain_size = addr_len ? ARRAY_SIZE(chain) : (ARRAY_SIZE(chain) - 1);
+ aspi_setup_chain(aspi, chain, chain_size);
+ return aspi_execute_chain(aspi);
+}
+
+static int anarion_spi_write_nor_reg(struct spi_nor *nor, uint8_t opcode,
+ uint8_t *buf, int len)
+{
+ uint32_t addr, i;
+ struct anarion_qspi *aspi = nor->priv;
+
+ if (len > sizeof(uint32_t))
+ return -ENOTSUPP;
+
+ for (i = 0, addr = 0; i < len; i++)
+ addr |= buf[len - 1 - i] << (i * 8);
+
+ return anarion_qspi_cmd_addr(aspi, opcode, addr, len);
+}
+
+/* After every operation, we need to restore the IO chain for XIP to work. */
+static void aspi_setup_xip_read_chain(struct anarion_qspi *spi,
+ struct spi_nor *nor)
+{
+ struct qspi_io_chain chain[] = {
+ {CHAIN_CMD, nor->read_opcode, 1, spi->xfer_mode_cmd},
+ {CHAIN_ADDR, 0, nor->addr_width, spi->xfer_mode_addr},
+ {CHAIN_HI_Z, 0, spi->num_hi_z_clocks, spi->xfer_mode_addr},
+ {CHAIN_DATA_IN, 0, ASPI_DATA_LEN_MASK, spi->xfer_mode_data},
+ };
+
+ aspi_setup_chain(spi, chain, ARRAY_SIZE(chain));
+}
+
+static int aspi_do_write_xfer(struct anarion_qspi *spi,
+ struct spi_nor *nor, uint32_t addr,
+ const void *buf, size_t len)
+{
+ struct qspi_io_chain chain[] = {
+ {CHAIN_CMD, nor->program_opcode, 1, MODE_IO_X1},
+ {CHAIN_ADDR, addr, nor->addr_width, MODE_IO_X1},
+ {CHAIN_DATA_OUT, 0, len, MODE_IO_X1},
+ };
+
+ aspi_setup_chain(spi, chain, ARRAY_SIZE(chain));
+
+ /* Go, johnny go */
+ aspi_write_reg(spi, ASPI_REG_GO, 1);
+
+ aspi_poll_and_seed_fifo(spi, buf, len);
+ return aspi_wait_idle(spi);
+}
+
+/* While we could send read commands manually to the flash chip, we'd have to
+ * get data back through the DATA2 register. That is on the AHB bus, whereas
+ * XIP reads go over AXI. Hence, we use the memory-mapped flash space for read.
+ * TODO: Look at using DMA instead of memcpy().
+ */
+static ssize_t anarion_spi_nor_read(struct spi_nor *nor, loff_t from,
+ size_t len, uint8_t *read_buf)
+{
+ struct anarion_qspi *aspi = nor->priv;
+ void *from_xip = (void *)(aspi->xipbase + from);
+
+ aspi_setup_xip_read_chain(aspi, nor);
+ memcpy(read_buf, from_xip, len);
+
+ return len;
+}
+
+static ssize_t anarion_spi_nor_write(struct spi_nor *nor, loff_t to,
+ size_t len, const uint8_t *src)
+{
+ int ret;
+ struct anarion_qspi *aspi = nor->priv;
+
+ dev_err(aspi->dev, "%s, @0x%llx + %zu\n", __func__, to, len);
+
+ if (len > nor->page_size)
+ return -EINVAL;
+
+ ret = aspi_do_write_xfer(aspi, nor, to, src, len);
+ return (ret < 0) ? ret : len;
+}
+
+/* TODO: Revisit this when we get actual HW. Right now max speed is 6 MHz. */
+static void aspi_configure_clocks(struct anarion_qspi *aspi)
+{
+ uint8_t div = 0;
+ uint32_t ck_ctl = aspi_read_reg(aspi, ASPI_REG_CLOCK);
+
+ ck_ctl &= ~ASPI_CLOCK_DIV_MASK;
+ ck_ctl |= ASPI_CLOCK_DIV(div);
+ aspi_write_reg(aspi, ASPI_REG_CLOCK, ck_ctl);
+}
+
+static int anarion_qspi_drv_probe(struct platform_device *pdev)
+{
+ int ret;
+ void __iomem *mmiobase;
+ struct resource *res;
+ struct anarion_qspi *aspi;
+ struct device_node *flash_node;
+ struct spi_nor *nor;
+
+ aspi = devm_kzalloc(&pdev->dev, sizeof(*aspi), GFP_KERNEL);
+ if (!aspi)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, aspi);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ mmiobase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(mmiobase)) {
+ dev_err(&pdev->dev, "Cannot get base addresses (%ld)!\n",
+ PTR_ERR(mmiobase));
+ return PTR_ERR(mmiobase);
+ }
+ aspi->regbase = (uintptr_t)mmiobase;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ mmiobase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(mmiobase)) {
+ dev_err(&pdev->dev, "Cannot get XIP addresses (%ld)!\n",
+ PTR_ERR(mmiobase));
+ return PTR_ERR(mmiobase);
+ }
+ aspi->xipbase = (uintptr_t)mmiobase;
+
+ aspi->dev = &pdev->dev;
+
+ /* only support one attached flash */
+ flash_node = of_get_next_available_child(pdev->dev.of_node, NULL);
+ if (!flash_node) {
+ dev_err(&pdev->dev, "no SPI flash device to configure\n");
+ return -ENODEV;
+ }
+
+ /* Reset the controller */
+ aspi_write_reg(aspi, ASPI_REG_CLOCK, ASPI_CLK_RESET_ALL);
+ aspi_write_reg(aspi, ASPI_REG_LAT, 0x010);
+ aspi_configure_clocks(aspi);
+
+ nor = &aspi->nor;
+ nor->priv = aspi;
+ nor->dev = aspi->dev;
+ nor->read = anarion_spi_nor_read;
+ nor->write = anarion_spi_nor_write;
+ nor->read_reg = anarion_spi_read_nor_reg;
+ nor->write_reg = anarion_spi_write_nor_reg;
+
+ spi_nor_set_flash_node(nor, flash_node);
+
+ ret = spi_nor_scan(&aspi->nor, NULL, SPI_NOR_DUAL);
+ if (ret)
+ return ret;
+
+ switch (nor->flash_read) {
+ default: /* Fall through */
+ case SPI_NOR_NORMAL:
+ aspi->num_hi_z_clocks = nor->read_dummy;
+ aspi->xfer_mode_cmd = MODE_IO_X1;
+ aspi->xfer_mode_addr = MODE_IO_X1;
+ aspi->xfer_mode_data = MODE_IO_X1;
+ break;
+ case SPI_NOR_FAST:
+ aspi->num_hi_z_clocks = nor->read_dummy;
+ aspi->xfer_mode_cmd = MODE_IO_X1;
+ aspi->xfer_mode_addr = MODE_IO_X1;
+ aspi->xfer_mode_data = MODE_IO_X1;
+ break;
+ case SPI_NOR_DUAL:
+ aspi->num_hi_z_clocks = nor->read_dummy;
+ aspi->xfer_mode_cmd = MODE_IO_X1;
+ aspi->xfer_mode_addr = MODE_IO_X1;
+ aspi->xfer_mode_data = MODE_IO_X2;
+ break;
+ case SPI_NOR_QUAD:
+ aspi->num_hi_z_clocks = nor->read_dummy;
+ aspi->xfer_mode_cmd = MODE_IO_X1;
+ aspi->xfer_mode_addr = MODE_IO_X1;
+ aspi->xfer_mode_data = MODE_IO_X4;
+ break;
+ }
+
+ aspi_setup_xip_read_chain(aspi, nor);
+
+ mtd_device_register(&aspi->nor.mtd, NULL, 0);
+
+ return 0;
+}
+
+static int anarion_qspi_drv_remove(struct platform_device *pdev)
+{
+ struct anarion_qspi *aspi = platform_get_drvdata(pdev);
+
+ mtd_device_unregister(&aspi->nor.mtd);
+ return 0;
+}
+
+static const struct of_device_id anarion_qspi_of_match[] = {
+ { .compatible = "adaptrum,anarion-qspi" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, anarion_qspi_of_match);
+
+static struct platform_driver anarion_qspi_driver = {
+ .driver = {
+ .name = "anarion-qspi",
+ .of_match_table = anarion_qspi_of_match,
+ },
+ .probe = anarion_qspi_drv_probe,
+ .remove = anarion_qspi_drv_remove,
+};
+module_platform_driver(anarion_qspi_driver);
+
+MODULE_DESCRIPTION("Adaptrum Anarion Quad SPI Controller Driver");
+MODULE_AUTHOR("Alexandru Gagniuc <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.9.3

2017-07-29 02:01:58

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 3/5] net: stmmac: Add Adaptrum Anarion GMAC glue layer

From: Alexandru Gagniuc <[email protected]>
Date: Fri, 28 Jul 2017 15:07:03 -0700

> Before the GMAC on the Anarion chip can be used, the PHY interface
> selection must be configured with the DWMAC block in reset.
>
> This layer covers a block containing only two registers. Although it
> is possible to model this as a reset controller and use the "resets"
> property of stmmac, it's much more intuitive to include this in the
> glue layer instead.
>
> At this time only RGMII is supported, because it is the only mode
> which has been validated hardware-wise.
>
> Signed-off-by: Alexandru Gagniuc <[email protected]>

I don't see how this fits into any patch series at all. If this is
part of a series you posted elsewhere, you should keep netdev@ on
all such postings so people there can review the change in-context.

Thanks.

2017-07-29 09:34:35

by Marek Vasut

[permalink] [raw]
Subject: Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

On 07/29/2017 12:07 AM, Alexandru Gagniuc wrote:
> Add support for the QSPI controller found in Adaptrum Anarion SOCs.
> This controller is designed specifically to handle SPI NOR chips, and
> the driver is modeled as such.
>
> Because the system is emulated on an FPGA, we don't have a way to test
> all the hardware adjustemts, so only basic features are implemented at
> this time.
>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> .../devicetree/bindings/mtd/anarion-quadspi.txt | 22 +
> drivers/mtd/spi-nor/Kconfig | 7 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/anarion-quadspi.c | 490 +++++++++++++++++++++
> 4 files changed, 520 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
> create mode 100644 drivers/mtd/spi-nor/anarion-quadspi.c
>
> diff --git a/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt b/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
> new file mode 100644
> index 0000000..b4971e1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
> @@ -0,0 +1,22 @@
> +* Adaptrum Anarion Quad SPI controller
> +
> +Required properties:
> +- compatible : Should be "adaptrum,anarion-qspi".
> +- reg : Contains two entries, each of which is a tuple consisting of a
> + physical address and length. The first entry is the address and
> + length of the controller register set. The second entry is the
> + address and length of the memory-mapped flash. This second region is
> + the region where the controller responds to XIP requests, and may be
> + larger than the size of the attached flash.

You want to split the bindings into separate patch and CC Rob to review
them.

> +Example:
> +
> + qspi: qspi@f200f000 {
> + compatible = "adaptrum,anarion-qspi";
> + reg = <0xf200f000 0x1000>,
> + <0x20000000 0x08000000>;
> +
> + flash0: w25q128fvn@0 {
> + ...
> + }
> + };
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 293c8a4..98dc012 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -48,6 +48,13 @@ config SPI_ATMEL_QUADSPI
> This driver does not support generic SPI. The implementation only
> supports SPI NOR.
>
> +config SPI_ANARION_QUADSPI
> + tristate "Adaptrum Anarion Quad SPI Controller"
> + depends on OF && HAS_IOMEM
> + help
> + Enable support for the Adaptrum Anarion Quad SPI controller.
> + This driver does not support generic SPI. It only supports SPI NOR.

Keep the list sorted.

> config SPI_CADENCE_QUADSPI
> tristate "Cadence Quad SPI controller"
> depends on OF && (ARM || COMPILE_TEST)
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 285aab8..53635f6 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,6 +1,7 @@
> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
> obj-$(CONFIG_SPI_ASPEED_SMC) += aspeed-smc.o
> obj-$(CONFIG_SPI_ATMEL_QUADSPI) += atmel-quadspi.o
> +obj-$(CONFIG_SPI_ANARION_QUADSPI) += anarion-quadspi.o

DTTO, N is before S and T .

> obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o
> obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
> obj-$(CONFIG_SPI_HISI_SFC) += hisi-sfc.o
> diff --git a/drivers/mtd/spi-nor/anarion-quadspi.c b/drivers/mtd/spi-nor/anarion-quadspi.c
> new file mode 100644
> index 0000000..d981356
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/anarion-quadspi.c
> @@ -0,0 +1,490 @@
> +/*
> + * Adaptrum Anarion Quad SPI controller driver
> + *
> + * Copyright (C) 2017, Adaptrum, Inc.
> + * (Written by Alexandru Gagniuc <alex.g at adaptrum.com> for Adaptrum, Inc.)
> + * Licensed under the GPLv2 or (at your option) any later version.

The GPL boilerplate should be here.

> + */
> +
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/mtd/spi-nor.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define ASPI_REG_CLOCK 0x00
> +#define ASPI_REG_GO 0x04
> +#define ASPI_REG_CHAIN 0x08
> +#define ASPI_REG_CMD1 0x0c
> +#define ASPI_REG_CMD2 0x10
> +#define ASPI_REG_ADDR1 0x14
> +#define ASPI_REG_ADDR2 0x18
> +#define ASPI_REG_PERF1 0x1c
> +#define ASPI_REG_PERF2 0x20
> +#define ASPI_REG_HI_Z 0x24
> +#define ASPI_REG_BYTE_COUNT 0x28
> +#define ASPI_REG_DATA1 0x2c
> +#define ASPI_REG_DATA2 0x30
> +#define ASPI_REG_FINISH 0x34
> +#define ASPI_REG_XIP 0x38
> +#define ASPI_REG_FIFO_STATUS 0x3c
> +#define ASPI_REG_LAT 0x40
> +#define ASPI_REG_OUT_DELAY_0 0x44
> +#define ASPI_REG_OUT_DELAY_1 0x48
> +#define ASPI_REG_IN_DELAY_0 0x4c
> +#define ASPI_REG_IN_DELAY_1 0x50
> +#define ASPI_REG_DQS_DELAY 0x54
> +#define ASPI_REG_STATUS 0x58
> +#define ASPI_REG_IRQ_ENABLE 0x5c
> +#define ASPI_REG_IRQ_STATUS 0x60
> +#define ASPI_REG_AXI_BAR 0x64
> +#define ASPI_REG_READ_CFG 0x6c
> +
> +#define ASPI_CLK_SW_RESET (1 << 0)

BIT(0) , fix globally

> +#define ASPI_CLK_RESET_BUF (1 << 1)
> +#define ASPI_CLK_RESET_ALL (ASPI_CLK_SW_RESET | ASPI_CLK_RESET_BUF)
> +#define ASPI_CLK_SPI_MODE3 (1 << 2)
> +#define ASPI_CLOCK_DIV_MASK (0xff << 8)
> +#define ASPI_CLOCK_DIV(d) (((d) << 8) & ASPI_CLOCK_DIV_MASK)
> +
> +#define ASPI_TIMEOUT_US 100000
> +
> +#define ASPI_DATA_LEN_MASK 0x3fff
> +#define ASPI_MAX_XFER_LEN (size_t)(ASPI_DATA_LEN_MASK + 1)
> +
> +#define MODE_IO_X1 (0 << 16)
> +#define MODE_IO_X2 (1 << 16)
> +#define MODE_IO_X4 (2 << 16)
> +#define MODE_IO_SDR_POS_SKEW (0 << 20)
> +#define MODE_IO_SDR_NEG_SKEW (1 << 20)
> +#define MODE_IO_DDR_34_SKEW (2 << 20)
> +#define MODE_IO_DDR_PN_SKEW (3 << 20)
> +#define MODE_IO_DDR_DQS (5 << 20)
> +
> +#define ASPI_STATUS_BUSY (1 << 2)
> +
> +/*
> + * This mask does not match reality. Get over it:

What is this about ?

> + * DATA2: 0x3fff
> + * CMD2: 0x0003
> + * ADDR2: 0x0007
> + * PERF2: 0x0000
> + * HI_Z: 0x003f
> + * BCNT: 0x0007
> + */
> +#define CHAIN_LEN(x) ((x - 1) & ASPI_DATA_LEN_MASK)
> +
> +struct anarion_qspi {
> + struct spi_nor nor;
> + struct device *dev;
> + uintptr_t regbase;

Should be void __iomem * I guess ?

> + uintptr_t xipbase;
> + uint32_t xfer_mode_cmd;

u32 etc, fix globally, this is not userspace.

> + uint32_t xfer_mode_addr;
> + uint32_t xfer_mode_data;
> + uint8_t num_hi_z_clocks;
> +};
> +
> +struct qspi_io_chain {
> + uint8_t action;
> + uint32_t data;
> + uint16_t data_len;
> + uint32_t mode;
> +};
> +
> +enum chain_code {
> + CHAIN_NOP = 0,
> + CHAIN_CMD = 1,
> + CHAIN_ADDR = 2,
> + CHAIN_WTFIUM = 3,
> + CHAIN_HI_Z = 4,
> + CHAIN_DATA_OUT = 5,
> + CHAIN_DATA_IN = 6,
> + CHAIN_FINISH = 7,
> +};
> +
> +static const struct chain_to_reg {
> + uint8_t data_reg;
> + uint8_t ctl_reg;
> +} chain_to_reg_map[] = {
> + [CHAIN_NOP] = {0, 0},
> + [CHAIN_CMD] = {ASPI_REG_CMD1, ASPI_REG_CMD2},
> + [CHAIN_ADDR] = {ASPI_REG_ADDR1, ASPI_REG_ADDR2},
> + [CHAIN_WTFIUM] = {0, 0},
> + [CHAIN_HI_Z] = {0, ASPI_REG_HI_Z},
> + [CHAIN_DATA_OUT] = {0, ASPI_REG_DATA2},
> + [CHAIN_DATA_IN] = {0, ASPI_REG_DATA2},
> + [CHAIN_FINISH] = {0, ASPI_REG_FINISH},
> +};
> +
> +static uint32_t aspi_read_reg(struct anarion_qspi *spi, uint8_t reg)
> +{
> + return readl((void *)(spi->regbase + reg));
> +};
> +
> +static void aspi_write_reg(struct anarion_qspi *spi, uint8_t reg, uint32_t val)
> +{
> + writel(val, (void *)(spi->regbase + reg));
> +};
> +
> +static size_t aspi_get_fifo_level(struct anarion_qspi *spi)
> +{
> + return aspi_read_reg(spi, ASPI_REG_FIFO_STATUS) & 0xff;
> +}
> +
> +static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf, size_t len)
> +{
> + uint32_t data;

Is this stuff below something like ioread32_rep() ?

> + aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
> + while (len >= 4) {
> + data = aspi_read_reg(aspi, ASPI_REG_DATA1);
> + memcpy(buf, &data, sizeof(data));
> + buf += 4;
> + len -= 4;
> + }
> +
> + if (len) {
> + aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, len);
> + data = aspi_read_reg(aspi, ASPI_REG_DATA1);
> + memcpy(buf, &data, len);
> + }
> +}
> +
> +static void aspi_seed_fifo(struct anarion_qspi *spi,
> + const uint8_t *buf, size_t len)
> +{
> + uint32_t data;
> +
> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
> + while (len >= 4) {
> + memcpy(&data, buf, sizeof(data));
> + aspi_write_reg(spi, ASPI_REG_DATA1, data);

iowrite32_rep ?

> + buf += 4;
> + len -= 4;
> + }
> +
> + if (len) {
> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
> + memcpy(&data, buf, len);
> + aspi_write_reg(spi, ASPI_REG_DATA1, data);
> + }
> +}
> +
> +static int aspi_wait_idle(struct anarion_qspi *aspi)
> +{
> + uint32_t status;
> + void *status_reg = (void *)(aspi->regbase + ASPI_REG_STATUS);
> +
> + return readl_poll_timeout(status_reg, status,
> + !(status & ASPI_STATUS_BUSY),
> + 1, ASPI_TIMEOUT_US);
> +}
> +
> +static int aspi_poll_and_seed_fifo(struct anarion_qspi *spi,
> + const void *src_addr, size_t len)
> +{
> + size_t wait_us, fifo_space = 0, xfer_len;
> + const uint8_t *src = src_addr;
> +
> + while (len > 0) {
> + wait_us = 0;
> + while (wait_us++ < ASPI_TIMEOUT_US) {
> + fifo_space = 64 - aspi_get_fifo_level(spi);
> + if (fifo_space)
> + break;
> + udelay(1);
> + }
> +
> + xfer_len = min(len, fifo_space);
> + aspi_seed_fifo(spi, src, xfer_len);
> + src += xfer_len;
> + len -= xfer_len;
> + }
> +
> + return 0;
> +}
> +
> +static void aspi_setup_chain(struct anarion_qspi *aspi,
> + const struct qspi_io_chain *chain,
> + size_t chain_len)
> +{
> + size_t i;
> + uint32_t chain_reg = 0;
> + const struct qspi_io_chain *link;
> + const struct chain_to_reg *regs;
> +
> + for (link = chain, i = 0; i < chain_len; i++, link++) {
> + regs = &chain_to_reg_map[link->action];
> +
> + if (link->data_len && regs->data_reg)
> + aspi_write_reg(aspi, regs->data_reg, link->data);
> +
> + if (regs->ctl_reg)
> + aspi_write_reg(aspi, regs->ctl_reg,
> + CHAIN_LEN(link->data_len) | link->mode);
> +
> + chain_reg |= link->action << (i * 4);
> + }
> +
> + chain_reg |= CHAIN_FINISH << (i * 4);
> +
> + aspi_write_reg(aspi, ASPI_REG_CHAIN, chain_reg);
> +}
> +
> +static int aspi_execute_chain(struct anarion_qspi *aspi)
> +{
> + /* Go, johnny go */
> + aspi_write_reg(aspi, ASPI_REG_GO, 1);
> + return aspi_wait_idle(aspi);
> +}
> +
> +static int anarion_spi_read_nor_reg(struct spi_nor *nor, uint8_t opcode,
> + uint8_t *buf, int len)
> +{
> + struct anarion_qspi *aspi = nor->priv;
> + struct qspi_io_chain chain[] = {
> + {CHAIN_CMD, opcode, 1, MODE_IO_X1},
> + {CHAIN_DATA_IN, 0, (uint16_t)len, MODE_IO_X1},
> + };
> +
> + if (len >= 8)
> + return -EMSGSIZE;
> +
> + aspi_setup_chain(aspi, chain, ARRAY_SIZE(chain));
> + aspi_execute_chain(aspi);
> +
> + aspi_drain_fifo(aspi, buf, len);
> +
> + return 0;
> +}
> +
> +static int anarion_qspi_cmd_addr(struct anarion_qspi *aspi, uint16_t cmd,
> + uint32_t addr, int addr_len)
> +{
> + size_t chain_size;
> + const struct qspi_io_chain chain[] = {
> + {CHAIN_CMD, cmd, 1, MODE_IO_X1},
> + {CHAIN_ADDR, addr, addr_len, MODE_IO_X1},
> + };
> +
> + chain_size = addr_len ? ARRAY_SIZE(chain) : (ARRAY_SIZE(chain) - 1);
> + aspi_setup_chain(aspi, chain, chain_size);
> + return aspi_execute_chain(aspi);
> +}
> +
> +static int anarion_spi_write_nor_reg(struct spi_nor *nor, uint8_t opcode,
> + uint8_t *buf, int len)
> +{
> + uint32_t addr, i;
> + struct anarion_qspi *aspi = nor->priv;
> +
> + if (len > sizeof(uint32_t))
> + return -ENOTSUPP;
> +
> + for (i = 0, addr = 0; i < len; i++)
> + addr |= buf[len - 1 - i] << (i * 8);
> +
> + return anarion_qspi_cmd_addr(aspi, opcode, addr, len);
> +}
> +
> +/* After every operation, we need to restore the IO chain for XIP to work. */
> +static void aspi_setup_xip_read_chain(struct anarion_qspi *spi,
> + struct spi_nor *nor)
> +{
> + struct qspi_io_chain chain[] = {
> + {CHAIN_CMD, nor->read_opcode, 1, spi->xfer_mode_cmd},
> + {CHAIN_ADDR, 0, nor->addr_width, spi->xfer_mode_addr},
> + {CHAIN_HI_Z, 0, spi->num_hi_z_clocks, spi->xfer_mode_addr},
> + {CHAIN_DATA_IN, 0, ASPI_DATA_LEN_MASK, spi->xfer_mode_data},
> + };
> +
> + aspi_setup_chain(spi, chain, ARRAY_SIZE(chain));
> +}
> +
> +static int aspi_do_write_xfer(struct anarion_qspi *spi,
> + struct spi_nor *nor, uint32_t addr,
> + const void *buf, size_t len)
> +{
> + struct qspi_io_chain chain[] = {
> + {CHAIN_CMD, nor->program_opcode, 1, MODE_IO_X1},
> + {CHAIN_ADDR, addr, nor->addr_width, MODE_IO_X1},
> + {CHAIN_DATA_OUT, 0, len, MODE_IO_X1},
> + };
> +
> + aspi_setup_chain(spi, chain, ARRAY_SIZE(chain));
> +
> + /* Go, johnny go */
> + aspi_write_reg(spi, ASPI_REG_GO, 1);
> +
> + aspi_poll_and_seed_fifo(spi, buf, len);
> + return aspi_wait_idle(spi);
> +}
> +
> +/* While we could send read commands manually to the flash chip, we'd have to
> + * get data back through the DATA2 register. That is on the AHB bus, whereas
> + * XIP reads go over AXI. Hence, we use the memory-mapped flash space for read.
> + * TODO: Look at using DMA instead of memcpy().
> + */

Multiline comment looks like this,
/*
* foo
* bar
*/

> +static ssize_t anarion_spi_nor_read(struct spi_nor *nor, loff_t from,
> + size_t len, uint8_t *read_buf)
> +{
> + struct anarion_qspi *aspi = nor->priv;
> + void *from_xip = (void *)(aspi->xipbase + from);
> +
> + aspi_setup_xip_read_chain(aspi, nor);
> + memcpy(read_buf, from_xip, len);
> +
> + return len;
> +}
> +
> +static ssize_t anarion_spi_nor_write(struct spi_nor *nor, loff_t to,
> + size_t len, const uint8_t *src)
> +{
> + int ret;
> + struct anarion_qspi *aspi = nor->priv;
> +
> + dev_err(aspi->dev, "%s, @0x%llx + %zu\n", __func__, to, len);

Drop this.

> + if (len > nor->page_size)
> + return -EINVAL;
> +
> + ret = aspi_do_write_xfer(aspi, nor, to, src, len);
> + return (ret < 0) ? ret : len;
> +}
> +
> +/* TODO: Revisit this when we get actual HW. Right now max speed is 6 MHz. */
> +static void aspi_configure_clocks(struct anarion_qspi *aspi)
> +{
> + uint8_t div = 0;
> + uint32_t ck_ctl = aspi_read_reg(aspi, ASPI_REG_CLOCK);
> +
> + ck_ctl &= ~ASPI_CLOCK_DIV_MASK;
> + ck_ctl |= ASPI_CLOCK_DIV(div);
> + aspi_write_reg(aspi, ASPI_REG_CLOCK, ck_ctl);
> +}
> +
> +static int anarion_qspi_drv_probe(struct platform_device *pdev)
> +{
> + int ret;
> + void __iomem *mmiobase;
> + struct resource *res;
> + struct anarion_qspi *aspi;
> + struct device_node *flash_node;
> + struct spi_nor *nor;
> +
> + aspi = devm_kzalloc(&pdev->dev, sizeof(*aspi), GFP_KERNEL);
> + if (!aspi)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, aspi);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mmiobase = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(mmiobase)) {
> + dev_err(&pdev->dev, "Cannot get base addresses (%ld)!\n",
> + PTR_ERR(mmiobase));
> + return PTR_ERR(mmiobase);
> + }
> + aspi->regbase = (uintptr_t)mmiobase;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + mmiobase = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(mmiobase)) {
> + dev_err(&pdev->dev, "Cannot get XIP addresses (%ld)!\n",
> + PTR_ERR(mmiobase));
> + return PTR_ERR(mmiobase);
> + }
> + aspi->xipbase = (uintptr_t)mmiobase;
> +
> + aspi->dev = &pdev->dev;
> +
> + /* only support one attached flash */
> + flash_node = of_get_next_available_child(pdev->dev.of_node, NULL);
> + if (!flash_node) {
> + dev_err(&pdev->dev, "no SPI flash device to configure\n");
> + return -ENODEV;
> + }
> +
> + /* Reset the controller */
> + aspi_write_reg(aspi, ASPI_REG_CLOCK, ASPI_CLK_RESET_ALL);
> + aspi_write_reg(aspi, ASPI_REG_LAT, 0x010);
> + aspi_configure_clocks(aspi);
> +
> + nor = &aspi->nor;
> + nor->priv = aspi;
> + nor->dev = aspi->dev;
> + nor->read = anarion_spi_nor_read;
> + nor->write = anarion_spi_nor_write;
> + nor->read_reg = anarion_spi_read_nor_reg;
> + nor->write_reg = anarion_spi_write_nor_reg;
> +
> + spi_nor_set_flash_node(nor, flash_node);
> +
> + ret = spi_nor_scan(&aspi->nor, NULL, SPI_NOR_DUAL);
> + if (ret)
> + return ret;
> +
> + switch (nor->flash_read) {
> + default: /* Fall through */

This will break once we add OSPI support ...

> + case SPI_NOR_NORMAL:
> + aspi->num_hi_z_clocks = nor->read_dummy;
> + aspi->xfer_mode_cmd = MODE_IO_X1;
> + aspi->xfer_mode_addr = MODE_IO_X1;
> + aspi->xfer_mode_data = MODE_IO_X1;
> + break;
> + case SPI_NOR_FAST:
> + aspi->num_hi_z_clocks = nor->read_dummy;
> + aspi->xfer_mode_cmd = MODE_IO_X1;
> + aspi->xfer_mode_addr = MODE_IO_X1;
> + aspi->xfer_mode_data = MODE_IO_X1;
> + break;
> + case SPI_NOR_DUAL:
> + aspi->num_hi_z_clocks = nor->read_dummy;
> + aspi->xfer_mode_cmd = MODE_IO_X1;
> + aspi->xfer_mode_addr = MODE_IO_X1;
> + aspi->xfer_mode_data = MODE_IO_X2;
> + break;
> + case SPI_NOR_QUAD:
> + aspi->num_hi_z_clocks = nor->read_dummy;
> + aspi->xfer_mode_cmd = MODE_IO_X1;
> + aspi->xfer_mode_addr = MODE_IO_X1;
> + aspi->xfer_mode_data = MODE_IO_X4;
> + break;
> + }
> +
> + aspi_setup_xip_read_chain(aspi, nor);
> +
> + mtd_device_register(&aspi->nor.mtd, NULL, 0);
> +
> + return 0;
> +}
> +
> +static int anarion_qspi_drv_remove(struct platform_device *pdev)
> +{
> + struct anarion_qspi *aspi = platform_get_drvdata(pdev);
> +
> + mtd_device_unregister(&aspi->nor.mtd);
> + return 0;
> +}
> +
> +static const struct of_device_id anarion_qspi_of_match[] = {
> + { .compatible = "adaptrum,anarion-qspi" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, anarion_qspi_of_match);
> +
> +static struct platform_driver anarion_qspi_driver = {
> + .driver = {
> + .name = "anarion-qspi",
> + .of_match_table = anarion_qspi_of_match,
> + },
> + .probe = anarion_qspi_drv_probe,
> + .remove = anarion_qspi_drv_remove,
> +};
> +module_platform_driver(anarion_qspi_driver);
> +
> +MODULE_DESCRIPTION("Adaptrum Anarion Quad SPI Controller Driver");
> +MODULE_AUTHOR("Alexandru Gagniuc <[email protected]>");
> +MODULE_LICENSE("GPL v2");
>


--
Best regards,
Marek Vasut

2017-07-29 14:48:34

by Andreas Färber

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add vendor prefix for Adaptrum, Inc.

Hi Alexandru,

Am 29.07.2017 um 00:07 schrieb Alexandru Gagniuc:
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> 1 file changed, 1 insertion(+)

Not sure why I'm CC'ed here, but it looks sane, so

Reviewed-by: Andreas Färber <[email protected]>

If this patch gets respun, please add a sentence about your company to
the commit message. I also think "dt-bindings:" is a more common prefix?

Cheers,
Andreas

--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

2017-07-29 19:04:11

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

Hi Alexandru,

[auto build test WARNING on arc/for-next]
[also build test WARNING on v4.13-rc2 next-20170728]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Alexandru-Gagniuc/Initial-support-for-Adaptrum-Anarion-SOC/20170730-013701
base: https://git.kernel.org/pub/scm/linux/kernel/git/vgupta/arc.git for-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

All warnings (new ones prefixed by >>):

drivers/mtd/spi-nor/anarion-quadspi.c: In function 'anarion_spi_nor_read':
>> drivers/mtd/spi-nor/anarion-quadspi.c:335:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
void *from_xip = (void *)(aspi->xipbase + from);
^

vim +335 drivers/mtd/spi-nor/anarion-quadspi.c

325
326 /* While we could send read commands manually to the flash chip, we'd have to
327 * get data back through the DATA2 register. That is on the AHB bus, whereas
328 * XIP reads go over AXI. Hence, we use the memory-mapped flash space for read.
329 * TODO: Look at using DMA instead of memcpy().
330 */
331 static ssize_t anarion_spi_nor_read(struct spi_nor *nor, loff_t from,
332 size_t len, uint8_t *read_buf)
333 {
334 struct anarion_qspi *aspi = nor->priv;
> 335 void *from_xip = (void *)(aspi->xipbase + from);
336
337 aspi_setup_xip_read_chain(aspi, nor);
338 memcpy(read_buf, from_xip, len);
339
340 return len;
341 }
342

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.80 kB)
.config.gz (58.68 kB)
Download all attachments

2017-07-31 06:33:19

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH 5/5] ARC: DTS: Add device-tree for Anarion-based development board

On 07/29/2017 03:37 AM, Alexandru Gagniuc wrote:
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> arch/arc/boot/dts/adaptrum_anarion.dtsi | 107 ++++++++++++++++++++++++++++
> arch/arc/boot/dts/adaptrum_anarion_fpga.dts | 49 +++++++++++++
> 2 files changed, 156 insertions(+)
> create mode 100644 arch/arc/boot/dts/adaptrum_anarion.dtsi
> create mode 100644 arch/arc/boot/dts/adaptrum_anarion_fpga.dts

So you really need to upstream the fpga dts - if this just for initial bringup and
you will eventually switch to silicon.
The reason I say is every additional file is a maintenance burden so better to
avoid things which are only temporary.
But if you plan to support this config in long run I'm fine !

Looking further it seems first one is a "common" include style dts while fpga is
for actual platform and the SoC one will follow once you get it running ?

>
> diff --git a/arch/arc/boot/dts/adaptrum_anarion.dtsi b/arch/arc/boot/dts/adaptrum_anarion.dtsi
> new file mode 100644
> index 0000000..f50958f
> --- /dev/null
> +++ b/arch/arc/boot/dts/adaptrum_anarion.dtsi
> @@ -0,0 +1,107 @@
> +/*
> + * (C) Copyright 2017 Adaptrum, Inc.
> + * Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
> + * Licensed under the GPLv2 or (at your option) any later version
> + */
> +
> +#include "skeleton.dtsi"

Perhaps put a one liner that this is based on SNPS ARC700 cpu !

> +
> +/ {
> + compatible = "adaptrum,anarion";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + soc {
> + compatible = "simple-bus";
> + device_type = "soc";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + interrupt-parent = <&core_intc>;
> +
> + core_intc: interrupt-controller {
> + compatible = "snps,arc700-intc";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + };
> +
> + uart0: serial@f2202100 {
> + compatible = "ns16550";
> + reg = <0xf2202100 0x20>;
> + interrupts = <8>;
> + reg-shift = <2>;
> + reg-io-width = <4>;
> + clock-frequency = <192000000>;
> + status = "disabled";
> + };
> +
> + uart1: serial@f2202200 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0xf2202200 0x20>;
> + interrupts = <8>;
> + reg-shift = <2>;
> + reg-io-width = <4>;
> + clock-frequency = <192000000>;
> + status = "disabled";
> + };
> +
> + uart2: serial@f2202300 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0xf2202300 0x20>;
> + interrupts = <8>;
> + reg-shift = <2>;
> + reg-io-width = <4>;
> + clock-frequency = <192000000>;
> + status = "disabled";
> + };
> +
> + uart3: serial@f2202400 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0xf2202400 0x20>;
> + interrupts = <8>;
> + reg-shift = <2>;
> + reg-io-width = <4>;
> + clock-frequency = <192000000>;
> + status = "disabled";
> + };
> +
> + qspi: qspi@f200f000 {
> + compatible = "adaptrum,anarion-qspi";
> + reg = <0xf200f000 0x1000>,
> + <0x20000000 0x08000000>;
> +
> + interrupts = <10>;
> + status = "disabled";
> + };
> +
> + gmac0: ethernet@f2010000 {
> + compatible = "snps,dwmac";
> + reg = <0xf2010000 0x4000>;
> +
> + interrupt-parent = <&core_intc>;
> + interrupts = <20>;
> + interrupt-names = "macirq";
> +
> + clocks = <&core_clk>;
> + clock-names = "stmmaceth";
> +
> + snps,pbl = <32>;
> + status = "disabled";
> + };
> +
> + gmac1: ethernet@f2014000 {
> + compatible = "adaptrum,anarion-gmac", "snps,dwmac";
> + reg = <0xf2014000 0x4000>, <0xf2018100 8>;
> +
> + interrupt-parent = <&core_intc>;
> + interrupts = <21>;
> + interrupt-names = "macirq";
> +
> + clocks = <&core_clk>;
> + clock-names = "stmmaceth";
> +
> + snps,pbl = <32>;
> + status = "disabled";
> + };
> + };
> +};
> diff --git a/arch/arc/boot/dts/adaptrum_anarion_fpga.dts b/arch/arc/boot/dts/adaptrum_anarion_fpga.dts
> new file mode 100644
> index 0000000..36173b2
> --- /dev/null
> +++ b/arch/arc/boot/dts/adaptrum_anarion_fpga.dts
> @@ -0,0 +1,49 @@
> +/*
> + * (C) Copyright 2017 Adaptrum, Inc.
> + * Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
> + * Licensed under the GPLv2 or (at your option) any later version
> + */
> +
> +/dts-v1/;
> +
> +#include "adaptrum_anarion.dtsi"
> +
> +/ {
> + model = "adaptrum,anarion";
> + compatible = "adaptrum,anarion";
> +
> + chosen {
> + bootargs = "earlycon console=ttyS0,115200n8";
> + stdout-path = "serial0:115200n8";
> + };
> +
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + core_clk: core_clk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <12000000>;
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&qspi {
> + status = "okay";
> + flash0: w25q128fvn@0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "winbond,w25q128", "jedec,spi-nor";
> + spi-max-frequency = <70000000>;
> + m25p,fast-read;
> + };
> +};
> +
> +&gmac1 {
> + phy-mode = "rgmii";
> + status = "okay";
> +};

2017-07-31 15:08:58

by Alexandru Gagniuc

[permalink] [raw]
Subject: Re: [PATCH 5/5] ARC: DTS: Add device-tree for Anarion-based development board

On 07/30/2017 11:32 PM, Vineet Gupta wrote:
> On 07/29/2017 03:37 AM, Alexandru Gagniuc wrote:
>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>> ---
>> arch/arc/boot/dts/adaptrum_anarion.dtsi | 107
>> ++++++++++++++++++++++++++++
>> arch/arc/boot/dts/adaptrum_anarion_fpga.dts | 49 +++++++++++++
>> 2 files changed, 156 insertions(+)
>> create mode 100644 arch/arc/boot/dts/adaptrum_anarion.dtsi
>> create mode 100644 arch/arc/boot/dts/adaptrum_anarion_fpga.dts
>
> So you really need to upstream the fpga dts - if this just for initial
> bringup and you will eventually switch to silicon.
> The reason I say is every additional file is a maintenance burden so
> better to avoid things which are only temporary.
> But if you plan to support this config in long run I'm fine !
>
> Looking further it seems first one is a "common" include style dts while
> fpga is for actual platform and the SoC one will follow once you get it
> running ?

That is correct. the '_fpga.dts' is the tree for the current emulation
board. The plan is to use the fpga platform until the actual silicon
arrives and is brought up. I included the _fpga dts in order to have a
way to verify that the anarion DTSI compiles. I also plan to later
remove the fpga dts once the actual silicon works.

[snip]

>> +#include "skeleton.dtsi"
>
> Perhaps put a one liner that this is based on SNPS ARC700 cpu !

Staged for [PATCH v2].

Alex

>> +
>> +/ {
>> + compatible = "adaptrum,anarion";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + soc {
>> + compatible = "simple-bus";
>> + device_type = "soc";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> + interrupt-parent = <&core_intc>;
>> +
>> + core_intc: interrupt-controller {
>> + compatible = "snps,arc700-intc";
>> + interrupt-controller;
>> + #interrupt-cells = <1>;
>> + };
>> +
>> + uart0: serial@f2202100 {
>> + compatible = "ns16550";
>> + reg = <0xf2202100 0x20>;
>> + interrupts = <8>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clock-frequency = <192000000>;
>> + status = "disabled";
>> + };
>> +
>> + uart1: serial@f2202200 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0xf2202200 0x20>;
>> + interrupts = <8>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clock-frequency = <192000000>;
>> + status = "disabled";
>> + };
>> +
>> + uart2: serial@f2202300 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0xf2202300 0x20>;
>> + interrupts = <8>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clock-frequency = <192000000>;
>> + status = "disabled";
>> + };
>> +
>> + uart3: serial@f2202400 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0xf2202400 0x20>;
>> + interrupts = <8>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clock-frequency = <192000000>;
>> + status = "disabled";
>> + };
>> +
>> + qspi: qspi@f200f000 {
>> + compatible = "adaptrum,anarion-qspi";
>> + reg = <0xf200f000 0x1000>,
>> + <0x20000000 0x08000000>;
>> +
>> + interrupts = <10>;
>> + status = "disabled";
>> + };
>> +
>> + gmac0: ethernet@f2010000 {
>> + compatible = "snps,dwmac";
>> + reg = <0xf2010000 0x4000>;
>> +
>> + interrupt-parent = <&core_intc>;
>> + interrupts = <20>;
>> + interrupt-names = "macirq";
>> +
>> + clocks = <&core_clk>;
>> + clock-names = "stmmaceth";
>> +
>> + snps,pbl = <32>;
>> + status = "disabled";
>> + };
>> +
>> + gmac1: ethernet@f2014000 {
>> + compatible = "adaptrum,anarion-gmac", "snps,dwmac";
>> + reg = <0xf2014000 0x4000>, <0xf2018100 8>;
>> +
>> + interrupt-parent = <&core_intc>;
>> + interrupts = <21>;
>> + interrupt-names = "macirq";
>> +
>> + clocks = <&core_clk>;
>> + clock-names = "stmmaceth";
>> +
>> + snps,pbl = <32>;
>> + status = "disabled";
>> + };
>> + };
>> +};
>> diff --git a/arch/arc/boot/dts/adaptrum_anarion_fpga.dts
>> b/arch/arc/boot/dts/adaptrum_anarion_fpga.dts
>> new file mode 100644
>> index 0000000..36173b2
>> --- /dev/null
>> +++ b/arch/arc/boot/dts/adaptrum_anarion_fpga.dts
>> @@ -0,0 +1,49 @@
>> +/*
>> + * (C) Copyright 2017 Adaptrum, Inc.
>> + * Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
>> + * Licensed under the GPLv2 or (at your option) any later version
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "adaptrum_anarion.dtsi"
>> +
>> +/ {
>> + model = "adaptrum,anarion";
>> + compatible = "adaptrum,anarion";
>> +
>> + chosen {
>> + bootargs = "earlycon console=ttyS0,115200n8";
>> + stdout-path = "serial0:115200n8";
>> + };
>> +
>> + aliases {
>> + serial0 = &uart0;
>> + };
>> +
>> + core_clk: core_clk {
>> + #clock-cells = <0>;
>> + compatible = "fixed-clock";
>> + clock-frequency = <12000000>;
>> + };
>> +};
>> +
>> +&uart0 {
>> + status = "okay";
>> +};
>> +
>> +&qspi {
>> + status = "okay";
>> + flash0: w25q128fvn@0 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + compatible = "winbond,w25q128", "jedec,spi-nor";
>> + spi-max-frequency = <70000000>;
>> + m25p,fast-read;
>> + };
>> +};
>> +
>> +&gmac1 {
>> + phy-mode = "rgmii";
>> + status = "okay";
>> +};
>

2017-07-31 15:11:23

by Alexandru Gagniuc

[permalink] [raw]
Subject: Re: [PATCH 3/5] net: stmmac: Add Adaptrum Anarion GMAC glue layer

Hi David,

On 07/28/2017 07:01 PM, David Miller wrote:
> From: Alexandru Gagniuc <[email protected]>
> Date: Fri, 28 Jul 2017 15:07:03 -0700
>
>> Before the GMAC on the Anarion chip can be used, the PHY interface
>> selection must be configured with the DWMAC block in reset.
>>
>> This layer covers a block containing only two registers. Although it
>> is possible to model this as a reset controller and use the "resets"
>> property of stmmac, it's much more intuitive to include this in the
>> glue layer instead.
>>
>> At this time only RGMII is supported, because it is the only mode
>> which has been validated hardware-wise.
>>
>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>
> I don't see how this fits into any patch series at all. If this is
> part of a series you posted elsewhere, you should keep netdev@ on
> all such postings so people there can review the change in-context.

I used the --cc-cmd option to send-email. I'll be sure to CC netdev@ on
[PATCH v2].

Alex

2017-07-31 17:17:31

by Alexandru Gagniuc

[permalink] [raw]
Subject: Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

On 07/29/2017 02:34 AM, Marek Vasut wrote:
> On 07/29/2017 12:07 AM, Alexandru Gagniuc wrote:
>> Add support for the QSPI controller found in Adaptrum Anarion SOCs.
>> This controller is designed specifically to handle SPI NOR chips, and
>> the driver is modeled as such.
>>
>> Because the system is emulated on an FPGA, we don't have a way to test
>> all the hardware adjustemts, so only basic features are implemented at
>> this time.
>>
Hi Marek,

Thank you very much for the comments. I am going to fix most issues you
pointed out in v2. I do have some follow-up question on some of your
comments, so please bear with me. I hope I have responded to all your
comments, but feel free to nudge me if I missed any.

[snip]

>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
>> @@ -0,0 +1,22 @@
>> +* Adaptrum Anarion Quad SPI controller
>> +
>> +Required properties:
>> +- compatible : Should be "adaptrum,anarion-qspi".
>> +- reg : Contains two entries, each of which is a tuple consisting of a
>> + physical address and length. The first entry is the address and
>> + length of the controller register set. The second entry is the
>> + address and length of the memory-mapped flash. This second region is
>> + the region where the controller responds to XIP requests, and may be
>> + larger than the size of the attached flash.
>
> You want to split the bindings into separate patch and CC Rob to review
> them.

Yes, I do want that now!

[snip]

>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 293c8a4..98dc012 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -48,6 +48,13 @@ config SPI_ATMEL_QUADSPI
>> This driver does not support generic SPI. The implementation only
>> supports SPI NOR.
>>
>> +config SPI_ANARION_QUADSPI
>> + tristate "Adaptrum Anarion Quad SPI Controller"
>> + depends on OF && HAS_IOMEM
>> + help
>> + Enable support for the Adaptrum Anarion Quad SPI controller.
>> + This driver does not support generic SPI. It only supports SPI NOR.
>
> Keep the list sorted.

Staged for [PATCH v2].

>> config SPI_CADENCE_QUADSPI
>> tristate "Cadence Quad SPI controller"
>> depends on OF && (ARM || COMPILE_TEST)
>> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
>> index 285aab8..53635f6 100644
>> --- a/drivers/mtd/spi-nor/Makefile
>> +++ b/drivers/mtd/spi-nor/Makefile
>> @@ -1,6 +1,7 @@
>> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
>> obj-$(CONFIG_SPI_ASPEED_SMC) += aspeed-smc.o
>> obj-$(CONFIG_SPI_ATMEL_QUADSPI) += atmel-quadspi.o
>> +obj-$(CONFIG_SPI_ANARION_QUADSPI) += anarion-quadspi.o
>
> DTTO, N is before S and T .

Staged for [PATCH v2].

>> obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o
>> obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
>> obj-$(CONFIG_SPI_HISI_SFC) += hisi-sfc.o
>> diff --git a/drivers/mtd/spi-nor/anarion-quadspi.c b/drivers/mtd/spi-nor/anarion-quadspi.c
>> new file mode 100644
>> index 0000000..d981356
>> --- /dev/null
>> +++ b/drivers/mtd/spi-nor/anarion-quadspi.c
>> @@ -0,0 +1,490 @@
>> +/*
>> + * Adaptrum Anarion Quad SPI controller driver
>> + *
>> + * Copyright (C) 2017, Adaptrum, Inc.
>> + * (Written by Alexandru Gagniuc <alex.g at adaptrum.com> for Adaptrum, Inc.)
>> + * Licensed under the GPLv2 or (at your option) any later version.
>
> The GPL boilerplate should be here.

I chose this form of the boilerplate because it seems to be quite used
in other places. I am assuming the fatter boilerplate the requirement
for drivers/mtd, correct?

[snip]

>> +#define ASPI_CLK_SW_RESET (1 << 0)
>
> BIT(0) , fix globally

Staged for [PATCH v2].

>> +#define ASPI_CLK_RESET_BUF (1 << 1)
>> +#define ASPI_CLK_RESET_ALL (ASPI_CLK_SW_RESET | ASPI_CLK_RESET_BUF)
>> +#define ASPI_CLK_SPI_MODE3 (1 << 2)
>> +#define ASPI_CLOCK_DIV_MASK (0xff << 8)
>> +#define ASPI_CLOCK_DIV(d) (((d) << 8) & ASPI_CLOCK_DIV_MASK)
>> +
>> +#define ASPI_TIMEOUT_US 100000
>> +
>> +#define ASPI_DATA_LEN_MASK 0x3fff
>> +#define ASPI_MAX_XFER_LEN (size_t)(ASPI_DATA_LEN_MASK + 1)
>> +
>> +#define MODE_IO_X1 (0 << 16)
>> +#define MODE_IO_X2 (1 << 16)
>> +#define MODE_IO_X4 (2 << 16)
>> +#define MODE_IO_SDR_POS_SKEW (0 << 20)
>> +#define MODE_IO_SDR_NEG_SKEW (1 << 20)
>> +#define MODE_IO_DDR_34_SKEW (2 << 20)
>> +#define MODE_IO_DDR_PN_SKEW (3 << 20)
>> +#define MODE_IO_DDR_DQS (5 << 20)
>> +
>> +#define ASPI_STATUS_BUSY (1 << 2)
>> +
>> +/*
>> + * This mask does not match reality. Get over it:
>
> What is this about ?

Each stage of the QSPI chain has two registers. The second register has
a bitfield which takes in the length of the stage. For example, for
DATA2, we can set the length up to 0x4000, but for ADDR2, we can only
set a max of 4 bytes. I wrote this comment as a reminder to myself to be
careful about using this mask. I'll rephrase the comment for [v2]

>> + * DATA2: 0x3fff
>> + * CMD2: 0x0003
>> + * ADDR2: 0x0007
>> + * PERF2: 0x0000
>> + * HI_Z: 0x003f
>> + * BCNT: 0x0007
>> + */
>> +#define CHAIN_LEN(x) ((x - 1) & ASPI_DATA_LEN_MASK)
>> +
>> +struct anarion_qspi {
>> + struct spi_nor nor;
>> + struct device *dev;
>> + uintptr_t regbase;
>
> Should be void __iomem * I guess ?

I chose uintptr_t as opposed to void *, because arithmetic on void * is
not valid in C. What is the right answer hen, without risking undefined
behavior?

>
>> + uintptr_t xipbase;
>> + uint32_t xfer_mode_cmd;
>
> u32 etc, fix globally, this is not userspace.

From coding-style, section 5.(d), my understanding is that
"Linux-specific u8/u16/u32/u64 types [...] are not mandatory in new
code". Most of the code in this driver is shared between Linux, u-boot,
openocd, ASIC validation tests, and manufacturing tests. Unlike,
shortint types, stdint types are available in all cases.

Therefore, having to use a different set of primitive types makes code
sharing much more difficult, and increases the maintenance burden, hence
the strong preference for standard types. Is this reasonable?

[snip]

>> +static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf, size_t len)
>> +{
>> + uint32_t data;
>
> Is this stuff below something like ioread32_rep() ?
>
>> + aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>> + while (len >= 4) {
>> + data = aspi_read_reg(aspi, ASPI_REG_DATA1);
>> + memcpy(buf, &data, sizeof(data));
>> + buf += 4;
>> + len -= 4;
>> + }

That is very similar to ioread32_rep, yes. I kept this as for the
reasons outlined above, but changing this to _rep() seems innocent enough.

>> + if (len) {
>> + aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, len);
>> + data = aspi_read_reg(aspi, ASPI_REG_DATA1);
>> + memcpy(buf, &data, len);
>> + }
>> +}
>> +
>> +static void aspi_seed_fifo(struct anarion_qspi *spi,
>> + const uint8_t *buf, size_t len)
>> +{
>> + uint32_t data;
>> +
>> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>> + while (len >= 4) {
>> + memcpy(&data, buf, sizeof(data));
>> + aspi_write_reg(spi, ASPI_REG_DATA1, data);
>
> iowrite32_rep ?
>

[snip]
>> +/* While we could send read commands manually to the flash chip, we'd have to
>> + * get data back through the DATA2 register. That is on the AHB bus, whereas
>> + * XIP reads go over AXI. Hence, we use the memory-mapped flash space for read.
>> + * TODO: Look at using DMA instead of memcpy().
>> + */
>
> Multiline comment looks like this,
> /*
> * foo
> * bar
> */
>

Staged for [PATCH v2].

[snip]
>> +static ssize_t anarion_spi_nor_write(struct spi_nor *nor, loff_t to,
>> + size_t len, const uint8_t *src)
>> +{
>> + int ret;
>> + struct anarion_qspi *aspi = nor->priv;
>> +
>> + dev_err(aspi->dev, "%s, @0x%llx + %zu\n", __func__, to, len);
>
> Drop this.
>
OOPS! That wasn't supposed to be there.
Staged for [PATCH v2].

[snip]

>> + switch (nor->flash_read) {
>> + default: /* Fall through */
>
> This will break once we add OSPI support ...

Ooh, I see the API here has changed significantly from the 4.9 LTS
branch where we originally developed the driver. I will add and test
normal and FAST_READ support, but I won't have the bandwidth to test
other modes yet. Those will have to remain as a TODO.

>> + case SPI_NOR_NORMAL:
>> + aspi->num_hi_z_clocks = nor->read_dummy;
>> + aspi->xfer_mode_cmd = MODE_IO_X1;
>> + aspi->xfer_mode_addr = MODE_IO_X1;
>> + aspi->xfer_mode_data = MODE_IO_X1;
>> + break;
>> + case SPI_NOR_FAST:
>> + aspi->num_hi_z_clocks = nor->read_dummy;
>> + aspi->xfer_mode_cmd = MODE_IO_X1;
>> + aspi->xfer_mode_addr = MODE_IO_X1;
>> + aspi->xfer_mode_data = MODE_IO_X1;
>> + break;
>> + case SPI_NOR_DUAL:
>> + aspi->num_hi_z_clocks = nor->read_dummy;
>> + aspi->xfer_mode_cmd = MODE_IO_X1;
>> + aspi->xfer_mode_addr = MODE_IO_X1;
>> + aspi->xfer_mode_data = MODE_IO_X2;
>> + break;
>> + case SPI_NOR_QUAD:
>> + aspi->num_hi_z_clocks = nor->read_dummy;
>> + aspi->xfer_mode_cmd = MODE_IO_X1;
>> + aspi->xfer_mode_addr = MODE_IO_X1;
>> + aspi->xfer_mode_data = MODE_IO_X4;
>> + break;
>> + }
>> +
>> + aspi_setup_xip_read_chain(aspi, nor);
>> +
>> + mtd_device_register(&aspi->nor.mtd, NULL, 0);
>> +
>> + return 0;
>> +}

[snip]

2017-07-31 20:55:10

by Alexandru Gagniuc

[permalink] [raw]
Subject: Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

Hi Marek,

Me again!

On 07/29/2017 02:34 AM, Marek Vasut wrote:
> On 07/29/2017 12:07 AM, Alexandru Gagniuc wrote:
>> +static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf, size_t len)
>> +{
>> + uint32_t data;
>
> Is this stuff below something like ioread32_rep() ?
>
[snip]
>> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>> + while (len >= 4) {
>> + memcpy(&data, buf, sizeof(data));
>> + aspi_write_reg(spi, ASPI_REG_DATA1, data);
>
> iowrite32_rep ?
>
>> + buf += 4;
>> + len -= 4;
>> + }

I looked at using io(read|write)32_rep in these two places, and I've run
into some issues.

First, I'm seeing unaligned MMIO accesses, which are not supported on
ARC. Note that 'buf' has an alignment of 1, while the register requires
an alignment of 4. The memcpy() in-between takes care of that, which was
the original intent.

Other than that, we still need to break off the tail because we need to
update ASPI_REG_BYTE_COUNT before writing/reading any more data from the
FIFO. We have to keep track of the remainder, so we're not really saving
any SLOC.

I'd like to keep the original version as I find it to be much more
symmetrical and readable.

Thanks,
Alex

>> +
>> + if (len) {
>> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
>> + memcpy(&data, buf, len);
>> + aspi_write_reg(spi, ASPI_REG_DATA1, data);
>> + }
>> +}


Exhibit A: aspi_seed_fifo with writesl()

static void aspi_seed_fifo(struct anarion_qspi *spi,
const uint8_t *buf, size_t len)
{
uint32_t data;
void __iomem *data_reg = (void *)(spi->regbase + ASPI_REG_DATA1);

aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
writesl(data_reg, buf, len / 4);
buf += len & ~0x03;
len &= 0x03;

if (len) {
aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
memcpy(&data, buf, len);
aspi_write_reg(spi, ASPI_REG_DATA1, data);
}
}

2017-07-31 21:35:15

by Marek Vasut

[permalink] [raw]
Subject: Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

On 07/31/2017 07:17 PM, Alexandru Gagniuc wrote:
[...]

>>> +++ b/drivers/mtd/spi-nor/anarion-quadspi.c
>>> @@ -0,0 +1,490 @@
>>> +/*
>>> + * Adaptrum Anarion Quad SPI controller driver
>>> + *
>>> + * Copyright (C) 2017, Adaptrum, Inc.
>>> + * (Written by Alexandru Gagniuc <alex.g at adaptrum.com> for
>>> Adaptrum, Inc.)
>>> + * Licensed under the GPLv2 or (at your option) any later version.
>>
>> The GPL boilerplate should be here.
>
> I chose this form of the boilerplate because it seems to be quite used
> in other places. I am assuming the fatter boilerplate the requirement
> for drivers/mtd, correct?

AFAIK the regular GPLv2 boilerplate is the standard throughout the kernel.

> [snip]
>
>>> +#define ASPI_CLK_SW_RESET (1 << 0)
>>
>> BIT(0) , fix globally
>
> Staged for [PATCH v2].
>
>>> +#define ASPI_CLK_RESET_BUF (1 << 1)
>>> +#define ASPI_CLK_RESET_ALL (ASPI_CLK_SW_RESET |
>>> ASPI_CLK_RESET_BUF)
>>> +#define ASPI_CLK_SPI_MODE3 (1 << 2)
>>> +#define ASPI_CLOCK_DIV_MASK (0xff << 8)
>>> +#define ASPI_CLOCK_DIV(d) (((d) << 8) & ASPI_CLOCK_DIV_MASK)
>>> +
>>> +#define ASPI_TIMEOUT_US 100000
>>> +
>>> +#define ASPI_DATA_LEN_MASK 0x3fff
>>> +#define ASPI_MAX_XFER_LEN (size_t)(ASPI_DATA_LEN_MASK + 1)
>>> +
>>> +#define MODE_IO_X1 (0 << 16)
>>> +#define MODE_IO_X2 (1 << 16)
>>> +#define MODE_IO_X4 (2 << 16)
>>> +#define MODE_IO_SDR_POS_SKEW (0 << 20)
>>> +#define MODE_IO_SDR_NEG_SKEW (1 << 20)
>>> +#define MODE_IO_DDR_34_SKEW (2 << 20)
>>> +#define MODE_IO_DDR_PN_SKEW (3 << 20)
>>> +#define MODE_IO_DDR_DQS (5 << 20)
>>> +
>>> +#define ASPI_STATUS_BUSY (1 << 2)
>>> +
>>> +/*
>>> + * This mask does not match reality. Get over it:
>>
>> What is this about ?
>
> Each stage of the QSPI chain has two registers. The second register has
> a bitfield which takes in the length of the stage. For example, for
> DATA2, we can set the length up to 0x4000, but for ADDR2, we can only
> set a max of 4 bytes. I wrote this comment as a reminder to myself to be
> careful about using this mask. I'll rephrase the comment for [v2]

Please do.

>>> + * DATA2: 0x3fff
>>> + * CMD2: 0x0003
>>> + * ADDR2: 0x0007
>>> + * PERF2: 0x0000
>>> + * HI_Z: 0x003f
>>> + * BCNT: 0x0007
>>> + */
>>> +#define CHAIN_LEN(x) ((x - 1) & ASPI_DATA_LEN_MASK)
>>> +
>>> +struct anarion_qspi {
>>> + struct spi_nor nor;
>>> + struct device *dev;
>>> + uintptr_t regbase;
>>
>> Should be void __iomem * I guess ?
>
> I chose uintptr_t as opposed to void *, because arithmetic on void * is
> not valid in C. What is the right answer hen, without risking undefined
> behavior?

What sort of arithmetic ? It's perfectly valid in general ...

>>> + uintptr_t xipbase;
>>> + uint32_t xfer_mode_cmd;
>>
>> u32 etc, fix globally, this is not userspace.
>
> From coding-style, section 5.(d), my understanding is that
> "Linux-specific u8/u16/u32/u64 types [...] are not mandatory in new
> code". Most of the code in this driver is shared between Linux, u-boot,
> openocd, ASIC validation tests, and manufacturing tests. Unlike,
> shortint types, stdint types are available in all cases.
>
> Therefore, having to use a different set of primitive types makes code
> sharing much more difficult, and increases the maintenance burden, hence
> the strong preference for standard types. Is this reasonable?

The uXX is still prevalent in drivers/mtd/ according to git grep , so
I'd stick with that. Using uXX in U-Boot is perfectly fine and in fact
recommended.

> [snip]
>
>>> +static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf,
>>> size_t len)
>>> +{
>>> + uint32_t data;
>>
>> Is this stuff below something like ioread32_rep() ?
>>
>>> + aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>>> + while (len >= 4) {
>>> + data = aspi_read_reg(aspi, ASPI_REG_DATA1);
>>> + memcpy(buf, &data, sizeof(data));
>>> + buf += 4;
>>> + len -= 4;
>>> + }
>
> That is very similar to ioread32_rep, yes. I kept this as for the
> reasons outlined above, but changing this to _rep() seems innocent enough.

What reason ?

>>> + if (len) {
>>> + aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, len);
>>> + data = aspi_read_reg(aspi, ASPI_REG_DATA1);
>>> + memcpy(buf, &data, len);
>>> + }
>>> +}

[...]

>>> + switch (nor->flash_read) {
>>> + default: /* Fall through */
>>
>> This will break once we add OSPI support ...
>
> Ooh, I see the API here has changed significantly from the 4.9 LTS
> branch where we originally developed the driver.I will add and test
> normal and FAST_READ support, but I won't have the bandwidth to test
> other modes yet. Those will have to remain as a TODO.

Sigh, please be so kind and use -next for your development next time ...

>>> + case SPI_NOR_NORMAL:
>>> + aspi->num_hi_z_clocks = nor->read_dummy;
>>> + aspi->xfer_mode_cmd = MODE_IO_X1;
>>> + aspi->xfer_mode_addr = MODE_IO_X1;
>>> + aspi->xfer_mode_data = MODE_IO_X1;
>>> + break;
>>> + case SPI_NOR_FAST:
>>> + aspi->num_hi_z_clocks = nor->read_dummy;
>>> + aspi->xfer_mode_cmd = MODE_IO_X1;
>>> + aspi->xfer_mode_addr = MODE_IO_X1;
>>> + aspi->xfer_mode_data = MODE_IO_X1;
>>> + break;
>>> + case SPI_NOR_DUAL:
>>> + aspi->num_hi_z_clocks = nor->read_dummy;
>>> + aspi->xfer_mode_cmd = MODE_IO_X1;
>>> + aspi->xfer_mode_addr = MODE_IO_X1;
>>> + aspi->xfer_mode_data = MODE_IO_X2;
>>> + break;
>>> + case SPI_NOR_QUAD:
>>> + aspi->num_hi_z_clocks = nor->read_dummy;
>>> + aspi->xfer_mode_cmd = MODE_IO_X1;
>>> + aspi->xfer_mode_addr = MODE_IO_X1;
>>> + aspi->xfer_mode_data = MODE_IO_X4;
>>> + break;
>>> + }
>>> +
>>> + aspi_setup_xip_read_chain(aspi, nor);
>>> +
>>> + mtd_device_register(&aspi->nor.mtd, NULL, 0);
>>> +
>>> + return 0;
>>> +}
>
> [snip]


--
Best regards,
Marek Vasut

2017-07-31 21:35:26

by Marek Vasut

[permalink] [raw]
Subject: Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

On 07/31/2017 10:54 PM, Alexandru Gagniuc wrote:
> Hi Marek,
>
> Me again!
>
> On 07/29/2017 02:34 AM, Marek Vasut wrote:
>> On 07/29/2017 12:07 AM, Alexandru Gagniuc wrote:
>>> +static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf,
>>> size_t len)
>>> +{
>>> + uint32_t data;
>>
>> Is this stuff below something like ioread32_rep() ?
>>
> [snip]
>>> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>>> + while (len >= 4) {
>>> + memcpy(&data, buf, sizeof(data));
>>> + aspi_write_reg(spi, ASPI_REG_DATA1, data);
>>
>> iowrite32_rep ?
>>
>>> + buf += 4;
>>> + len -= 4;
>>> + }
>
> I looked at using io(read|write)32_rep in these two places, and I've run
> into some issues.
>
> First, I'm seeing unaligned MMIO accesses, which are not supported on
> ARC. Note that 'buf' has an alignment of 1, while the register requires
> an alignment of 4. The memcpy() in-between takes care of that, which was
> the original intent.
>
> Other than that, we still need to break off the tail because we need to
> update ASPI_REG_BYTE_COUNT before writing/reading any more data from the
> FIFO. We have to keep track of the remainder, so we're not really saving
> any SLOC.
>
> I'd like to keep the original version as I find it to be much more
> symmetrical and readable.

Look at the other drivers, AFAIR the io*_rep() issue was solved over and
over again, maybe you can factor that code out.

> Thanks,
> Alex
>
>>> +
>>> + if (len) {
>>> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
>>> + memcpy(&data, buf, len);
>>> + aspi_write_reg(spi, ASPI_REG_DATA1, data);
>>> + }
>>> +}
>
>
> Exhibit A: aspi_seed_fifo with writesl()
>
> static void aspi_seed_fifo(struct anarion_qspi *spi,
> const uint8_t *buf, size_t len)
> {
> uint32_t data;
> void __iomem *data_reg = (void *)(spi->regbase + ASPI_REG_DATA1);
>
> aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
> writesl(data_reg, buf, len / 4);
> buf += len & ~0x03;
> len &= 0x03;
>
> if (len) {
> aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
> memcpy(&data, buf, len);
> aspi_write_reg(spi, ASPI_REG_DATA1, data);
> }
> }


--
Best regards,
Marek Vasut

2017-07-31 22:20:51

by Alexandru Gagniuc

[permalink] [raw]
Subject: Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

On 07/31/2017 02:33 PM, Marek Vasut wrote:
> On 07/31/2017 07:17 PM, Alexandru Gagniuc wrote:

Hi Marek,

Thank you again for your feedback. I've applied a majority of your
suggestions, and I am very happy with the result. I should have v2
posted within a day or so.

[snip]
>>>> +/*
>>>> + * This mask does not match reality. Get over it:
>>>
>>> What is this about ?
>>
>> Each stage of the QSPI chain has two registers. The second register has
>> a bitfield which takes in the length of the stage. For example, for
>> DATA2, we can set the length up to 0x4000, but for ADDR2, we can only
>> set a max of 4 bytes. I wrote this comment as a reminder to myself to be
>> careful about using this mask. I'll rephrase the comment for [v2]
>
> Please do.
>
Staged for [PATCH v2]

>>>> + * DATA2: 0x3fff
>>>> + * CMD2: 0x0003
>>>> + * ADDR2: 0x0007
>>>> + * PERF2: 0x0000
>>>> + * HI_Z: 0x003f
>>>> + * BCNT: 0x0007
>>>> + */
>>>> +#define CHAIN_LEN(x) ((x - 1) & ASPI_DATA_LEN_MASK)
>>>> +
>>>> +struct anarion_qspi {
>>>> + struct spi_nor nor;
>>>> + struct device *dev;
>>>> + uintptr_t regbase;
>>>
>>> Should be void __iomem * I guess ?
>>
>> I chose uintptr_t as opposed to void *, because arithmetic on void * is
>> not valid in C. What is the right answer hen, without risking undefined
>> behavior?
>
> What sort of arithmetic ? It's perfectly valid in general ...

ISO/IEC 9899:201x, Section 6.5.6, constraint(2) is not met when the one
of the operands to addition is a void pointer.
Section 6.2.5 (19) defines void to be an incomplete type.

[snip]

>>> Is this stuff below something like ioread32_rep() ?
>>>
>>>> + aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>>>> + while (len >= 4) {
>>>> + data = aspi_read_reg(aspi, ASPI_REG_DATA1);
>>>> + memcpy(buf, &data, sizeof(data));
>>>> + buf += 4;
>>>> + len -= 4;
>>>> + }
>>
>> That is very similar to ioread32_rep, yes. I kept this as for the
>> reasons outlined above, but changing this to _rep() seems innocent enough.
>
> What reason ?

Being able to share the code between the different codebases where it is
used.

Alex

2017-07-31 22:44:21

by Marek Vasut

[permalink] [raw]
Subject: Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

On 08/01/2017 12:20 AM, Alexandru Gagniuc wrote:
> On 07/31/2017 02:33 PM, Marek Vasut wrote:
>> On 07/31/2017 07:17 PM, Alexandru Gagniuc wrote:
>
> Hi Marek,
>
> Thank you again for your feedback. I've applied a majority of your
> suggestions, and I am very happy with the result. I should have v2
> posted within a day or so.

No. You should have v2 out in about a week or so after people have time
to review v1 some more.

> [snip]
>>>>> +/*
>>>>> + * This mask does not match reality. Get over it:
>>>>
>>>> What is this about ?
>>>
>>> Each stage of the QSPI chain has two registers. The second register has
>>> a bitfield which takes in the length of the stage. For example, for
>>> DATA2, we can set the length up to 0x4000, but for ADDR2, we can only
>>> set a max of 4 bytes. I wrote this comment as a reminder to myself to be
>>> careful about using this mask. I'll rephrase the comment for [v2]
>>
>> Please do.
>>
> Staged for [PATCH v2]
>
>>>>> + * DATA2: 0x3fff
>>>>> + * CMD2: 0x0003
>>>>> + * ADDR2: 0x0007
>>>>> + * PERF2: 0x0000
>>>>> + * HI_Z: 0x003f
>>>>> + * BCNT: 0x0007
>>>>> + */
>>>>> +#define CHAIN_LEN(x) ((x - 1) & ASPI_DATA_LEN_MASK)

btw parenthesis around (x) missing, although this is like GEN_MASK() or
something here ...

>>>>> +struct anarion_qspi {
>>>>> + struct spi_nor nor;
>>>>> + struct device *dev;
>>>>> + uintptr_t regbase;
>>>>
>>>> Should be void __iomem * I guess ?
>>>
>>> I chose uintptr_t as opposed to void *, because arithmetic on void * is
>>> not valid in C. What is the right answer hen, without risking undefined
>>> behavior?
>>
>> What sort of arithmetic ? It's perfectly valid in general ...
>
> ISO/IEC 9899:201x, Section 6.5.6, constraint(2) is not met when the one
> of the operands to addition is a void pointer.
> Section 6.2.5 (19) defines void to be an incomplete type.

Is that something new in C 201x draft ? Anyway, this would mean half of
the drivers are broken, so I'm not convinced.

> [snip]
>
>>>> Is this stuff below something like ioread32_rep() ?
>>>>
>>>>> + aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>>>>> + while (len >= 4) {
>>>>> + data = aspi_read_reg(aspi, ASPI_REG_DATA1);
>>>>> + memcpy(buf, &data, sizeof(data));
>>>>> + buf += 4;
>>>>> + len -= 4;
>>>>> + }
>>>
>>> That is very similar to ioread32_rep, yes. I kept this as for the
>>> reasons outlined above, but changing this to _rep() seems innocent
>>> enough.
>>
>> What reason ?
>
> Being able to share the code between the different codebases where it is
> used.

Yes, that argument isn't gonna work, it'd make things impossible to
maintain in the kernel.

--
Best regards,
Marek Vasut

2017-07-31 22:59:40

by Alexandru Gagniuc

[permalink] [raw]
Subject: Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

On 07/31/2017 03:43 PM, Marek Vasut wrote:
> On 08/01/2017 12:20 AM, Alexandru Gagniuc wrote:
>> On 07/31/2017 02:33 PM, Marek Vasut wrote:
>>> On 07/31/2017 07:17 PM, Alexandru Gagniuc wrote:
>>>>>> +struct anarion_qspi {
>>>>>> + struct spi_nor nor;
>>>>>> + struct device *dev;
>>>>>> + uintptr_t regbase;
>>>>>
>>>>> Should be void __iomem * I guess ?
>>>>
>>>> I chose uintptr_t as opposed to void *, because arithmetic on void * is
>>>> not valid in C. What is the right answer hen, without risking undefined
>>>> behavior?
>>>
>>> What sort of arithmetic ? It's perfectly valid in general ...
>>
>> ISO/IEC 9899:201x, Section 6.5.6, constraint(2) is not met when the one
>> of the operands to addition is a void pointer.
>> Section 6.2.5 (19) defines void to be an incomplete type.
>
> Is that something new in C 201x draft ?

C99 had similar restrictions.

> Anyway, this would mean half of the drivers are broken, so I'm not convinced.

They are. Feel free to send me a private email if you want to discuss
this further.

Alex

2017-08-03 23:22:53

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 3/5] net: stmmac: Add Adaptrum Anarion GMAC glue layer

On Mon, Jul 31, 2017 at 08:11:00AM -0700, Alex wrote:
> Hi David,
>
> On 07/28/2017 07:01 PM, David Miller wrote:
> > From: Alexandru Gagniuc <[email protected]>
> > Date: Fri, 28 Jul 2017 15:07:03 -0700
> >
> > > Before the GMAC on the Anarion chip can be used, the PHY interface
> > > selection must be configured with the DWMAC block in reset.
> > >
> > > This layer covers a block containing only two registers. Although it
> > > is possible to model this as a reset controller and use the "resets"
> > > property of stmmac, it's much more intuitive to include this in the
> > > glue layer instead.
> > >
> > > At this time only RGMII is supported, because it is the only mode
> > > which has been validated hardware-wise.
> > >
> > > Signed-off-by: Alexandru Gagniuc <[email protected]>
> >
> > I don't see how this fits into any patch series at all. If this is
> > part of a series you posted elsewhere, you should keep netdev@ on
> > all such postings so people there can review the change in-context.
>
> I used the --cc-cmd option to send-email. I'll be sure to CC netdev@ on
> [PATCH v2].

The problem is your series spans several subsystems and it's not
clear who you intend to apply these. There aren't really any hard
dependencies between the patches, so they could all go thru different
trees. But you need to state that at least implicitly by sending the
patches TO who should apply them and CC the rest (and get_maintainers.pl
doesn't really help with that aspect). Or just don't send them in a
series if there's not an inter-dependency of the patches. Normally
bindings and a driver do go together and I'll ack the binding.

Rob

2017-08-03 23:25:29

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add vendor prefix for Adaptrum, Inc.

On Sat, Jul 29, 2017 at 04:48:28PM +0200, Andreas F?rber wrote:
> Hi Alexandru,
>
> Am 29.07.2017 um 00:07 schrieb Alexandru Gagniuc:
> > Signed-off-by: Alexandru Gagniuc <[email protected]>
> > ---
> > Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> > 1 file changed, 1 insertion(+)
>
> Not sure why I'm CC'ed here, but it looks sane, so
>
> Reviewed-by: Andreas F?rber <[email protected]>
>
> If this patch gets respun, please add a sentence about your company to
> the commit message. I also think "dt-bindings:" is a more common prefix?

Please respin with the above and just send by itself. This can go
separately as there's no need to merge via the same tree just in the
name of satisfying checkpatch.

Rob

2017-08-03 23:26:47

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 3/5] net: stmmac: Add Adaptrum Anarion GMAC glue layer

On Fri, Jul 28, 2017 at 03:07:03PM -0700, Alexandru Gagniuc wrote:
> Before the GMAC on the Anarion chip can be used, the PHY interface
> selection must be configured with the DWMAC block in reset.
>
> This layer covers a block containing only two registers. Although it
> is possible to model this as a reset controller and use the "resets"
> property of stmmac, it's much more intuitive to include this in the
> glue layer instead.
>
> At this time only RGMII is supported, because it is the only mode
> which has been validated hardware-wise.
>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> .../devicetree/bindings/net/anarion-gmac.txt | 25 ++++

The binding looks fine, but please split to separate patch.

> drivers/net/ethernet/stmicro/stmmac/Kconfig | 9 ++
> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> .../net/ethernet/stmicro/stmmac/dwmac-anarion.c | 151 +++++++++++++++++++++
> 4 files changed, 186 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/anarion-gmac.txt
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-anarion.c

2017-08-04 20:04:08

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH v2 1/2] ARC: [plat-anarion] Add early boot workarounds for Anarion SOC

An ARC, the interrupts are enabled globally, rather than per-line, as
drivers request it. Thus, we need to make sure that peripherals don't
generate any before the respective drivers are probed.
The GMAC is infamous for spamming interrupts, so it must be kept in
reset until the driver is probed and interrupt mapping established.

Signed-off-by: Alexandru Gagniuc <[email protected]>
---
Changes since v1:
* None

arch/arc/Kconfig | 1 +
arch/arc/Makefile | 1 +
arch/arc/plat-anarion/Kconfig | 10 ++++++++++
arch/arc/plat-anarion/Makefile | 7 +++++++
arch/arc/plat-anarion/platform.c | 39 +++++++++++++++++++++++++++++++++++++++
5 files changed, 58 insertions(+)
create mode 100644 arch/arc/plat-anarion/Kconfig
create mode 100644 arch/arc/plat-anarion/Makefile
create mode 100644 arch/arc/plat-anarion/platform.c

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index a545969..dff8423 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -100,6 +100,7 @@ source "arch/arc/plat-sim/Kconfig"
source "arch/arc/plat-tb10x/Kconfig"
source "arch/arc/plat-axs10x/Kconfig"
#New platform adds here
+source "arch/arc/plat-anarion/Kconfig"
source "arch/arc/plat-eznps/Kconfig"

endmenu
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index 44ef35d..9bc0048 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -109,6 +109,7 @@ core-y += arch/arc/boot/dts/

core-$(CONFIG_ARC_PLAT_SIM) += arch/arc/plat-sim/
core-$(CONFIG_ARC_PLAT_TB10X) += arch/arc/plat-tb10x/
+core-$(CONFIG_ARC_PLAT_ANARION) += arch/arc/plat-anarion/
core-$(CONFIG_ARC_PLAT_AXS10X) += arch/arc/plat-axs10x/
core-$(CONFIG_ARC_PLAT_EZNPS) += arch/arc/plat-eznps/

diff --git a/arch/arc/plat-anarion/Kconfig b/arch/arc/plat-anarion/Kconfig
new file mode 100644
index 0000000..632c7be
--- /dev/null
+++ b/arch/arc/plat-anarion/Kconfig
@@ -0,0 +1,10 @@
+#
+# (C) Copyright 2017 Adaptrum, Inc.
+# Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
+# Licensed under the GPLv2 or (at your option) any later version.
+#
+
+menuconfig ARC_PLAT_ANARION
+ bool "Adaptrum Anarion based platforms"
+ help
+ Support for Adaptrum Anarion based ARC platforms.
diff --git a/arch/arc/plat-anarion/Makefile b/arch/arc/plat-anarion/Makefile
new file mode 100644
index 0000000..9596a41
--- /dev/null
+++ b/arch/arc/plat-anarion/Makefile
@@ -0,0 +1,7 @@
+#
+# (C) Copyright 2017 Adaptrum, Inc.
+# Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
+# Licensed under the GPLv2 or (at your option) any later version.
+#
+
+obj-y := platform.o
diff --git a/arch/arc/plat-anarion/platform.c b/arch/arc/plat-anarion/platform.c
new file mode 100644
index 0000000..ef0d381
--- /dev/null
+++ b/arch/arc/plat-anarion/platform.c
@@ -0,0 +1,39 @@
+/*
+ * Workarounds for Adaptrum Anarion SOC
+ *
+ * Copyright (C) 2017, Adaptrum, Inc.
+ * (Written by Alexandru Gagniuc <alex.g at adaptrum.com> for Adaptrum, Inc.)
+ * Licensed under the GPLv2 or (at your option) any later version.
+ */
+
+#include <asm/io.h>
+#include <linux/init.h>
+#include <asm/mach_desc.h>
+
+#define GMAC0_RESET 0xf2018000
+#define GMAC1_RESET 0xf2018100
+
+/* This works around an issue where the GMAC will generate interrupts before
+ * the driver is probed, confusing the heck out of the early boot.
+ */
+static void __init anarion_gmac_irq_storm_workaround(void)
+{
+ writel(1, (void *)GMAC0_RESET);
+ writel(1, (void *)GMAC1_RESET);
+}
+
+static void __init anarion_early_init(void)
+{
+ anarion_gmac_irq_storm_workaround();
+ /* Please, no more workarounds!!! */
+}
+
+static const char *anarion_compat[] __initconst = {
+ "adaptrum,anarion",
+ NULL,
+};
+
+MACHINE_START(ANARION, "anarion")
+ .dt_compat = anarion_compat,
+ .init_early = anarion_early_init,
+MACHINE_END
--
2.9.3

2017-08-04 20:04:26

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH v2 2/2] ARC: DTS: Add device-tree for Anarion-based development board

Signed-off-by: Alexandru Gagniuc <[email protected]>
---
Changes since v1:
* Updated CPU core clock to 24 MHz to match HW changes.

arch/arc/boot/dts/adaptrum_anarion.dtsi | 108 ++++++++++++++++++++++++++++
arch/arc/boot/dts/adaptrum_anarion_fpga.dts | 49 +++++++++++++
2 files changed, 157 insertions(+)
create mode 100644 arch/arc/boot/dts/adaptrum_anarion.dtsi
create mode 100644 arch/arc/boot/dts/adaptrum_anarion_fpga.dts

diff --git a/arch/arc/boot/dts/adaptrum_anarion.dtsi b/arch/arc/boot/dts/adaptrum_anarion.dtsi
new file mode 100644
index 0000000..31ed204
--- /dev/null
+++ b/arch/arc/boot/dts/adaptrum_anarion.dtsi
@@ -0,0 +1,108 @@
+/*
+ * (C) Copyright 2017 Adaptrum, Inc.
+ * Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
+ * Licensed under the GPLv2 or (at your option) any later version
+ */
+
+/* This skeleton is based on the ARC700 CPU */
+#include "skeleton.dtsi"
+
+/ {
+ compatible = "adaptrum,anarion";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ soc {
+ compatible = "simple-bus";
+ device_type = "soc";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ interrupt-parent = <&core_intc>;
+
+ core_intc: interrupt-controller {
+ compatible = "snps,arc700-intc";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
+
+ uart0: serial@f2202100 {
+ compatible = "ns16550";
+ reg = <0xf2202100 0x20>;
+ interrupts = <8>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ clock-frequency = <192000000>;
+ status = "disabled";
+ };
+
+ uart1: serial@f2202200 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0xf2202200 0x20>;
+ interrupts = <8>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ clock-frequency = <192000000>;
+ status = "disabled";
+ };
+
+ uart2: serial@f2202300 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0xf2202300 0x20>;
+ interrupts = <8>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ clock-frequency = <192000000>;
+ status = "disabled";
+ };
+
+ uart3: serial@f2202400 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0xf2202400 0x20>;
+ interrupts = <8>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ clock-frequency = <192000000>;
+ status = "disabled";
+ };
+
+ qspi: qspi@f200f000 {
+ compatible = "adaptrum,anarion-qspi";
+ reg = <0xf200f000 0x1000>,
+ <0x20000000 0x08000000>;
+
+ interrupts = <10>;
+ status = "disabled";
+ };
+
+ gmac0: ethernet@f2010000 {
+ compatible = "adaptrum,anarion-gmac", "snps,dwmac";
+ reg = <0xf2010000 0x4000>;
+
+ interrupt-parent = <&core_intc>;
+ interrupts = <20>;
+ interrupt-names = "macirq";
+
+ clocks = <&core_clk>;
+ clock-names = "stmmaceth";
+
+ snps,pbl = <32>;
+ status = "disabled";
+ };
+
+ gmac1: ethernet@f2014000 {
+ compatible = "adaptrum,anarion-gmac", "snps,dwmac";
+ reg = <0xf2014000 0x4000>, <0xf2018100 8>;
+
+ interrupt-parent = <&core_intc>;
+ interrupts = <21>;
+ interrupt-names = "macirq";
+
+ clocks = <&core_clk>;
+ clock-names = "stmmaceth";
+
+ snps,pbl = <32>;
+ status = "disabled";
+ };
+ };
+};
diff --git a/arch/arc/boot/dts/adaptrum_anarion_fpga.dts b/arch/arc/boot/dts/adaptrum_anarion_fpga.dts
new file mode 100644
index 0000000..02b838f
--- /dev/null
+++ b/arch/arc/boot/dts/adaptrum_anarion_fpga.dts
@@ -0,0 +1,49 @@
+/*
+ * (C) Copyright 2017 Adaptrum, Inc.
+ * Written by Alexandru Gagniuc <[email protected]> for Adaptrum, Inc.
+ * Licensed under the GPLv2 or (at your option) any later version
+ */
+
+/dts-v1/;
+
+#include "adaptrum_anarion.dtsi"
+
+/ {
+ model = "adaptrum,anarion";
+ compatible = "adaptrum,anarion";
+
+ chosen {
+ bootargs = "earlycon console=ttyS0,115200n8";
+ stdout-path = "serial0:115200n8";
+ };
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ core_clk: core_clk {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <24000000>;
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
+
+&qspi {
+ status = "okay";
+ flash0: w25q128fvn@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "winbond,w25q128", "jedec,spi-nor";
+ spi-max-frequency = <70000000>;
+ m25p,fast-read;
+ };
+};
+
+&gmac1 {
+ phy-mode = "rgmii";
+ status = "okay";
+};
--
2.9.3

2017-08-04 20:15:09

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH v2 6/7] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller

Add support for the QSPI controller found in Adaptrum Anarion SOCs.
This controller is designed specifically to handle SPI NOR chips, and
the driver is modeled as such.

Because the system is emulated on an FPGA, we don't have a way to test
all the hardware adjustemts, so only basic features are implemented at
this time.

Signed-off-by: Alexandru Gagniuc <[email protected]>
---
Changes since v1:
* Moved documentation for bindings to separate patch
* Removed idiotic dev_err() message
* Changed bit macros to use BIT(n) instead of (1 << n)
* Fixed alphabetical ordering in Kconfig and makefiles
* Rebased on v4.13-rc3 and re-tested READ and FAST_READ modes

drivers/mtd/spi-nor/Kconfig | 7 +
drivers/mtd/spi-nor/Makefile | 1 +
drivers/mtd/spi-nor/anarion-quadspi.c | 482 ++++++++++++++++++++++++++++++++++
3 files changed, 490 insertions(+)
create mode 100644 drivers/mtd/spi-nor/anarion-quadspi.c

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 293c8a4..6f36e13 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -29,6 +29,13 @@ config MTD_SPI_NOR_USE_4K_SECTORS
Please note that some tools/drivers/filesystems may not work with
4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).

+config SPI_ANARION_QUADSPI
+ tristate "Adaptrum Anarion Quad SPI Controller"
+ depends on OF && HAS_IOMEM
+ help
+ Enable support for the Adaptrum Anarion Quad SPI controller.
+ This driver does not support generic SPI. It only supports SPI NOR.
+
config SPI_ASPEED_SMC
tristate "Aspeed flash controllers in SPI mode"
depends on ARCH_ASPEED || COMPILE_TEST
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 285aab8..0ea797b 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
+obj-$(CONFIG_SPI_ANARION_QUADSPI) += anarion-quadspi.o
obj-$(CONFIG_SPI_ASPEED_SMC) += aspeed-smc.o
obj-$(CONFIG_SPI_ATMEL_QUADSPI) += atmel-quadspi.o
obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o
diff --git a/drivers/mtd/spi-nor/anarion-quadspi.c b/drivers/mtd/spi-nor/anarion-quadspi.c
new file mode 100644
index 0000000..c250a75
--- /dev/null
+++ b/drivers/mtd/spi-nor/anarion-quadspi.c
@@ -0,0 +1,482 @@
+/*
+ * Adaptrum Anarion Quad SPI controller driver
+ *
+ * Copyright (C) 2017, Adaptrum, Inc.
+ * (Written by Alexandru Gagniuc <alex.g at adaptrum.com> for Adaptrum, Inc.)
+ * Licensed under the GPLv2 or (at your option) any later version.
+ */
+
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define ASPI_REG_CLOCK 0x00
+#define ASPI_REG_GO 0x04
+#define ASPI_REG_CHAIN 0x08
+#define ASPI_REG_CMD1 0x0c
+#define ASPI_REG_CMD2 0x10
+#define ASPI_REG_ADDR1 0x14
+#define ASPI_REG_ADDR2 0x18
+#define ASPI_REG_PERF1 0x1c
+#define ASPI_REG_PERF2 0x20
+#define ASPI_REG_HI_Z 0x24
+#define ASPI_REG_BYTE_COUNT 0x28
+#define ASPI_REG_DATA1 0x2c
+#define ASPI_REG_DATA2 0x30
+#define ASPI_REG_FINISH 0x34
+#define ASPI_REG_XIP 0x38
+#define ASPI_REG_FIFO_STATUS 0x3c
+#define ASPI_REG_LAT 0x40
+#define ASPI_REG_OUT_DELAY_0 0x44
+#define ASPI_REG_OUT_DELAY_1 0x48
+#define ASPI_REG_IN_DELAY_0 0x4c
+#define ASPI_REG_IN_DELAY_1 0x50
+#define ASPI_REG_DQS_DELAY 0x54
+#define ASPI_REG_STATUS 0x58
+#define ASPI_REG_IRQ_ENABLE 0x5c
+#define ASPI_REG_IRQ_STATUS 0x60
+#define ASPI_REG_AXI_BAR 0x64
+#define ASPI_REG_READ_CFG 0x6c
+
+#define ASPI_CLK_SW_RESET BIT(0)
+#define ASPI_CLK_RESET_BUF BIT(1)
+#define ASPI_CLK_RESET_ALL (ASPI_CLK_SW_RESET | ASPI_CLK_RESET_BUF)
+#define ASPI_CLK_SPI_MODE3 BIT(2)
+#define ASPI_CLOCK_DIV_MASK (0xff << 8)
+#define ASPI_CLOCK_DIV(div) (((div) << 8) & ASPI_CLOCK_DIV_MASK)
+
+#define ASPI_TIMEOUT_US 100000
+
+#define ASPI_DATA_LEN_MASK 0x3fff
+#define ASPI_MAX_XFER_LEN (size_t)(ASPI_DATA_LEN_MASK + 1)
+
+#define MODE_IO_X1 (0 << 16)
+#define MODE_IO_X2 (1 << 16)
+#define MODE_IO_X4 (2 << 16)
+#define MODE_IO_SDR_POS_SKEW (0 << 20)
+#define MODE_IO_SDR_NEG_SKEW (1 << 20)
+#define MODE_IO_DDR_34_SKEW (2 << 20)
+#define MODE_IO_DDR_PN_SKEW (3 << 20)
+#define MODE_IO_DDR_DQS (5 << 20)
+
+#define ASPI_STATUS_BUSY BIT(2)
+
+/*
+ * This mask only matches the chain length for the DATA2 register. That's okay
+ * since we only need to use it with DATA2. The other masks are:
+ * DATA2: 0x3fff
+ * CMD2: 0x0003
+ * ADDR2: 0x0007
+ * PERF2: 0x0000
+ * HI_Z: 0x003f
+ * BCNT: 0x0007
+ */
+#define CHAIN_LEN(x) (((x) - 1) & ASPI_DATA_LEN_MASK)
+
+struct anarion_qspi {
+ struct spi_nor nor;
+ struct device *dev;
+ uintptr_t regbase;
+ uintptr_t xipbase;
+ uint32_t xfer_mode_cmd;
+ uint32_t xfer_mode_addr;
+ uint32_t xfer_mode_data;
+ uint8_t num_hi_z_clocks;
+};
+
+struct qspi_io_chain {
+ uint8_t action;
+ uint32_t data;
+ uint16_t data_len;
+ uint32_t mode;
+};
+
+enum chain_code {
+ CHAIN_NOP = 0,
+ CHAIN_CMD = 1,
+ CHAIN_ADDR = 2,
+ CHAIN_WTFIUM = 3,
+ CHAIN_HI_Z = 4,
+ CHAIN_DATA_OUT = 5,
+ CHAIN_DATA_IN = 6,
+ CHAIN_FINISH = 7,
+};
+
+static const struct chain_to_reg {
+ uint8_t data_reg;
+ uint8_t ctl_reg;
+} chain_to_reg_map[] = {
+ [CHAIN_NOP] = {0, 0},
+ [CHAIN_CMD] = {ASPI_REG_CMD1, ASPI_REG_CMD2},
+ [CHAIN_ADDR] = {ASPI_REG_ADDR1, ASPI_REG_ADDR2},
+ [CHAIN_WTFIUM] = {0, 0},
+ [CHAIN_HI_Z] = {0, ASPI_REG_HI_Z},
+ [CHAIN_DATA_OUT] = {0, ASPI_REG_DATA2},
+ [CHAIN_DATA_IN] = {0, ASPI_REG_DATA2},
+ [CHAIN_FINISH] = {0, ASPI_REG_FINISH},
+};
+
+static uint32_t aspi_read_reg(struct anarion_qspi *aspi, uint8_t reg)
+{
+ return readl((void *)(aspi->regbase + reg));
+};
+
+static void aspi_write_reg(struct anarion_qspi *aspi, uint8_t reg, uint32_t val)
+{
+ writel(val, (void *)(aspi->regbase + reg));
+};
+
+static size_t aspi_get_fifo_level(struct anarion_qspi *aspi)
+{
+ return aspi_read_reg(aspi, ASPI_REG_FIFO_STATUS) & 0xff;
+}
+
+static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf, size_t len)
+{
+ uint32_t data;
+
+ aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
+ while (len >= 4) {
+ data = aspi_read_reg(aspi, ASPI_REG_DATA1);
+ memcpy(buf, &data, sizeof(data));
+ buf += 4;
+ len -= 4;
+ }
+
+ if (len) {
+ aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, len);
+ data = aspi_read_reg(aspi, ASPI_REG_DATA1);
+ memcpy(buf, &data, len);
+ }
+}
+
+static void aspi_seed_fifo(struct anarion_qspi *aspi,
+ const uint8_t *buf, size_t len)
+{
+ uint32_t data;
+
+ aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
+ while (len >= 4) {
+ memcpy(&data, buf, sizeof(data));
+ aspi_write_reg(aspi, ASPI_REG_DATA1, data);
+ buf += 4;
+ len -= 4;
+ }
+
+ if (len) {
+ aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, len);
+ memcpy(&data, buf, len);
+ aspi_write_reg(aspi, ASPI_REG_DATA1, data);
+ }
+}
+
+static int aspi_wait_idle(struct anarion_qspi *aspi)
+{
+ uint32_t status;
+ void *status_reg = (void *)(aspi->regbase + ASPI_REG_STATUS);
+
+ return readl_poll_timeout(status_reg, status,
+ !(status & ASPI_STATUS_BUSY),
+ 1, ASPI_TIMEOUT_US);
+}
+
+static int aspi_poll_and_seed_fifo(struct anarion_qspi *aspi,
+ const void *src_addr, size_t len)
+{
+ size_t wait_us, fifo_space = 0, xfer_len;
+ const uint8_t *src = src_addr;
+
+ while (len > 0) {
+ wait_us = 0;
+ while (wait_us++ < ASPI_TIMEOUT_US) {
+ fifo_space = 64 - aspi_get_fifo_level(aspi);
+ if (fifo_space)
+ break;
+ udelay(1);
+ }
+
+ xfer_len = min(len, fifo_space);
+ aspi_seed_fifo(aspi, src, xfer_len);
+ src += xfer_len;
+ len -= xfer_len;
+ }
+
+ return 0;
+}
+
+static void aspi_setup_chain(struct anarion_qspi *aspi,
+ const struct qspi_io_chain *chain,
+ size_t chain_len)
+{
+ size_t i;
+ uint32_t chain_reg = 0;
+ const struct qspi_io_chain *link;
+ const struct chain_to_reg *regs;
+
+ for (link = chain, i = 0; i < chain_len; i++, link++) {
+ regs = &chain_to_reg_map[link->action];
+
+ if (link->data_len && regs->data_reg)
+ aspi_write_reg(aspi, regs->data_reg, link->data);
+
+ if (regs->ctl_reg)
+ aspi_write_reg(aspi, regs->ctl_reg,
+ CHAIN_LEN(link->data_len) | link->mode);
+
+ chain_reg |= link->action << (i * 4);
+ }
+
+ chain_reg |= CHAIN_FINISH << (i * 4);
+
+ aspi_write_reg(aspi, ASPI_REG_CHAIN, chain_reg);
+}
+
+static int aspi_execute_chain(struct anarion_qspi *aspi)
+{
+ /* Go, johnny go */
+ aspi_write_reg(aspi, ASPI_REG_GO, 1);
+ return aspi_wait_idle(aspi);
+}
+
+static int anarion_spi_read_nor_reg(struct spi_nor *nor, uint8_t opcode,
+ uint8_t *buf, int len)
+{
+ struct anarion_qspi *aspi = nor->priv;
+ struct qspi_io_chain chain[] = {
+ {CHAIN_CMD, opcode, 1, MODE_IO_X1},
+ {CHAIN_DATA_IN, 0, (uint16_t)len, MODE_IO_X1},
+ };
+
+ if (len >= 8)
+ return -EMSGSIZE;
+
+ aspi_setup_chain(aspi, chain, ARRAY_SIZE(chain));
+ aspi_execute_chain(aspi);
+
+ aspi_drain_fifo(aspi, buf, len);
+
+ return 0;
+}
+
+static int anarion_qspi_cmd_addr(struct anarion_qspi *aspi, uint16_t cmd,
+ uint32_t addr, int addr_len)
+{
+ size_t chain_size;
+ const struct qspi_io_chain chain[] = {
+ {CHAIN_CMD, cmd, 1, MODE_IO_X1},
+ {CHAIN_ADDR, addr, addr_len, MODE_IO_X1},
+ };
+
+ chain_size = addr_len ? ARRAY_SIZE(chain) : (ARRAY_SIZE(chain) - 1);
+ aspi_setup_chain(aspi, chain, chain_size);
+ return aspi_execute_chain(aspi);
+}
+
+static int anarion_spi_write_nor_reg(struct spi_nor *nor, uint8_t opcode,
+ uint8_t *buf, int len)
+{
+ uint32_t addr, i;
+ struct anarion_qspi *aspi = nor->priv;
+
+ if (len > sizeof(uint32_t))
+ return -ENOTSUPP;
+
+ for (i = 0, addr = 0; i < len; i++)
+ addr |= buf[len - 1 - i] << (i * 8);
+
+ return anarion_qspi_cmd_addr(aspi, opcode, addr, len);
+}
+
+/* After every operation, we need to restore the IO chain for XIP to work. */
+static void aspi_setup_xip_read_chain(struct anarion_qspi *aspi,
+ struct spi_nor *nor)
+{
+ struct qspi_io_chain chain[] = {
+ {CHAIN_CMD, nor->read_opcode, 1, aspi->xfer_mode_cmd},
+ {CHAIN_ADDR, 0, nor->addr_width, aspi->xfer_mode_addr},
+ {CHAIN_HI_Z, 0, aspi->num_hi_z_clocks, aspi->xfer_mode_addr},
+ {CHAIN_DATA_IN, 0, ASPI_DATA_LEN_MASK, aspi->xfer_mode_data},
+ };
+
+ aspi_setup_chain(aspi, chain, ARRAY_SIZE(chain));
+}
+
+static int aspi_do_write_xfer(struct anarion_qspi *aspi,
+ struct spi_nor *nor, uint32_t addr,
+ const void *buf, size_t len)
+{
+ struct qspi_io_chain chain[] = {
+ {CHAIN_CMD, nor->program_opcode, 1, MODE_IO_X1},
+ {CHAIN_ADDR, addr, nor->addr_width, MODE_IO_X1},
+ {CHAIN_DATA_OUT, 0, len, MODE_IO_X1},
+ };
+
+ aspi_setup_chain(aspi, chain, ARRAY_SIZE(chain));
+
+ /* Go, johnny go */
+ aspi_write_reg(aspi, ASPI_REG_GO, 1);
+
+ aspi_poll_and_seed_fifo(aspi, buf, len);
+ return aspi_wait_idle(aspi);
+}
+
+/*
+ * While we could send read commands manually to the flash chip, we'd have to
+ * get data back through the DATA2 register. That is on the AHB bus, whereas
+ * XIP reads go over AXI. Hence, we use the memory-mapped flash space for read.
+ * TODO: Look at using DMA instead of memcpy().
+ */
+static ssize_t anarion_spi_nor_read(struct spi_nor *nor, loff_t from,
+ size_t len, uint8_t *read_buf)
+{
+ struct anarion_qspi *aspi = nor->priv;
+ void *from_xip = (void *)(aspi->xipbase + from);
+
+ aspi_setup_xip_read_chain(aspi, nor);
+ memcpy(read_buf, from_xip, len);
+
+ return len;
+}
+
+static ssize_t anarion_spi_nor_write(struct spi_nor *nor, loff_t to,
+ size_t len, const uint8_t *src)
+{
+ int ret;
+ struct anarion_qspi *aspi = nor->priv;
+
+ if (len > nor->page_size)
+ return -EINVAL;
+
+ ret = aspi_do_write_xfer(aspi, nor, to, src, len);
+ return (ret < 0) ? ret : len;
+}
+
+/* TODO: Revisit this when we get actual HW. Right now max speed is 6 MHz. */
+static void aspi_configure_clocks(struct anarion_qspi *aspi)
+{
+ uint8_t div = 0;
+ uint32_t ck_ctl = aspi_read_reg(aspi, ASPI_REG_CLOCK);
+
+ ck_ctl &= ~ASPI_CLOCK_DIV_MASK;
+ ck_ctl |= ASPI_CLOCK_DIV(div);
+ aspi_write_reg(aspi, ASPI_REG_CLOCK, ck_ctl);
+}
+
+static int anarion_qspi_drv_probe(struct platform_device *pdev)
+{
+ int ret;
+ void __iomem *mmiobase;
+ struct resource *res;
+ struct anarion_qspi *aspi;
+ struct device_node *flash_node;
+ struct spi_nor *nor;
+
+ /* TODO: Test faster modes. Controller supports DUAL/QUAD/DTR modes. */
+ const struct spi_nor_hwcaps hwcaps = {
+ .mask = SNOR_HWCAPS_READ | SNOR_HWCAPS_READ_FAST |
+ SNOR_HWCAPS_PP,
+ };
+
+ aspi = devm_kzalloc(&pdev->dev, sizeof(*aspi), GFP_KERNEL);
+ if (!aspi)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, aspi);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ mmiobase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(mmiobase)) {
+ dev_err(&pdev->dev, "Cannot get base addresses (%ld)!\n",
+ PTR_ERR(mmiobase));
+ return PTR_ERR(mmiobase);
+ }
+ aspi->regbase = (uintptr_t)mmiobase;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ mmiobase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(mmiobase)) {
+ dev_err(&pdev->dev, "Cannot get XIP addresses (%ld)!\n",
+ PTR_ERR(mmiobase));
+ return PTR_ERR(mmiobase);
+ }
+ aspi->xipbase = (uintptr_t)mmiobase;
+
+ aspi->dev = &pdev->dev;
+
+ /* only support one attached flash */
+ flash_node = of_get_next_available_child(pdev->dev.of_node, NULL);
+ if (!flash_node) {
+ dev_err(&pdev->dev, "no SPI flash device to configure\n");
+ return -ENODEV;
+ }
+
+ /* Reset the controller */
+ aspi_write_reg(aspi, ASPI_REG_CLOCK, ASPI_CLK_RESET_ALL);
+ aspi_write_reg(aspi, ASPI_REG_LAT, 0x010);
+ aspi_configure_clocks(aspi);
+
+ nor = &aspi->nor;
+ nor->priv = aspi;
+ nor->dev = aspi->dev;
+ nor->read = anarion_spi_nor_read;
+ nor->write = anarion_spi_nor_write;
+ nor->read_reg = anarion_spi_read_nor_reg;
+ nor->write_reg = anarion_spi_write_nor_reg;
+
+ spi_nor_set_flash_node(nor, flash_node);
+
+ ret = spi_nor_scan(&aspi->nor, NULL, &hwcaps);
+ if (ret)
+ return ret;
+
+ /* TODO: Add _and_test_ support for DUAL/QUAD modes */
+ switch (nor->read_proto) {
+ case SNOR_PROTO_1_1_1:
+ aspi->num_hi_z_clocks = nor->read_dummy;
+ aspi->xfer_mode_cmd = MODE_IO_X1;
+ aspi->xfer_mode_addr = MODE_IO_X1;
+ aspi->xfer_mode_data = MODE_IO_X1;
+ break;
+ default:
+ dev_err(&pdev->dev, "Unsupported QSPI protocol (%x)\n",
+ nor->read_proto);
+ return -ENOTSUPP;
+ }
+
+ aspi_setup_xip_read_chain(aspi, nor);
+
+ mtd_device_register(&aspi->nor.mtd, NULL, 0);
+
+ return 0;
+}
+
+static int anarion_qspi_drv_remove(struct platform_device *pdev)
+{
+ struct anarion_qspi *aspi = platform_get_drvdata(pdev);
+
+ mtd_device_unregister(&aspi->nor.mtd);
+ return 0;
+}
+
+static const struct of_device_id anarion_qspi_of_match[] = {
+ { .compatible = "adaptrum,anarion-qspi" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, anarion_qspi_of_match);
+
+static struct platform_driver anarion_qspi_driver = {
+ .driver = {
+ .name = "anarion-qspi",
+ .of_match_table = anarion_qspi_of_match,
+ },
+ .probe = anarion_qspi_drv_probe,
+ .remove = anarion_qspi_drv_remove,
+};
+module_platform_driver(anarion_qspi_driver);
+
+MODULE_DESCRIPTION("Adaptrum Anarion Quad SPI Controller Driver");
+MODULE_AUTHOR("Alexandru Gagniuc <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.9.3

2017-08-04 20:15:23

by Alexandru Gagniuc

[permalink] [raw]
Subject: [PATCH v2 7/7] dt-bindings: Add documentation for adaptrum,anarion-quadspi

Add devicetree binding documentation for the Anarion QSPI controller.

Signed-off-by: Alexandru Gagniuc <[email protected]>
---
.../devicetree/bindings/mtd/anarion-quadspi.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/anarion-quadspi.txt

diff --git a/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt b/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
new file mode 100644
index 0000000..b4971e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/anarion-quadspi.txt
@@ -0,0 +1,22 @@
+* Adaptrum Anarion Quad SPI controller
+
+Required properties:
+- compatible : Should be "adaptrum,anarion-qspi".
+- reg : Contains two entries, each of which is a tuple consisting of a
+ physical address and length. The first entry is the address and
+ length of the controller register set. The second entry is the
+ address and length of the memory-mapped flash. This second region is
+ the region where the controller responds to XIP requests, and may be
+ larger than the size of the attached flash.
+
+Example:
+
+ qspi: qspi@f200f000 {
+ compatible = "adaptrum,anarion-qspi";
+ reg = <0xf200f000 0x1000>,
+ <0x20000000 0x08000000>;
+
+ flash0: w25q128fvn@0 {
+ ...
+ }
+ };
--
2.9.3