Re: [PATCH 00/43] ep93xx device tree conversion

Re: [PATCH 00/43] ep93xx device tree conversion

[Arnd Bergmann]

On Mon, Apr 24, 2023, at 14:34, Nikita Shubin wrote:
> This series aims to convert ep93xx from platform to full device tree support.
>
> Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.
>
> Thank you Linus and Arnd for your support, review and comments, sorry 
> if i missed something -
> these series are quite big for me.
>
> Big thanks to Alexander Sverdlin for his testing, support, review, 
> fixes and patches.

Thanks a lot for your continued work. I can't merge any of this at
the moment since the upstream merge window just opened, but I'm
happy to take this all through the soc tree for 6.5, provided we
get the sufficient Acks from the subsystem maintainers. Merging
it through each individual tree would take a lot longer, so I
hope we can avoid that.

      Arnd

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 00/43] ep93xx device tree conversion

[Nikita Shubin]

This series aims to convert ep93xx from platform to full device tree support.

Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.

Thank you Linus and Arnd for your support, review and comments, sorry if i missed something -
these series are quite big for me.

Big thanks to Alexander Sverdlin for his testing, support, review, fixes and patches.

Alexander Sverdlin (4):
  ARM: dts: ep93xx: Add ADC node
  ARM: dts: ep93xx: Add I2S and AC97 nodes
  ARM: dts: ep93xx: Add EDB9302 DT
  ASoC: cirrus: edb93xx: Delete driver

Nikita Shubin (39):
  gpio: ep93xx: split device in multiple
  soc: Add SoC driver for Cirrus ep93xx
  dt-bindings: pinctrl: Add DT bindings ep93xx pinctrl
  pinctrl: add a Cirrus ep93xx SoC pin controller
  dt-bindings: timers: add DT bindings for Cirrus EP93xx
  clocksource: ep93xx: Add driver for Cirrus Logic EP93xx
  dt-bindings: rtc: add DT bindings for Cirrus EP93xx
  rtc: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: watchdog: add DT bindings for Cirrus EP93x
  watchdog: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: clock: add DT bindings for Cirrus EP93xx
  clk: ep93xx: add DT support for Cirrus EP93xx
  power: reset: Add a driver for the ep93xx reset
  dt-bindings: pwm: Add DT bindings ep93xx PWM
  pwm: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: spi: Add DT bindings ep93xx spi
  spi: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: net: Add DT bindings ep93xx eth
  net: cirrus: add DT support for Cirrus EP93xx
  dt-bindings: dma: Add DT bindings ep93xx dma
  dma: cirrus: add DT support for Cirrus EP93xx
  dt-bindings: mtd: add DT bindings for ts7250 nand
  mtd: ts72xx_nand: add platform helper
  dt-bindings: ata: Add DT bindings ep93xx pata
  pata: cirrus: add DT support for Cirrus EP93xx
  dt-bindings: input: Add DT bindings ep93xx keypad
  input: keypad: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: rtc: Add DT binding m48t86 rtc
  rtc: m48t86: add DT support for m48t86
  dt-bindings: wdt: Add DT binding ts72xx wdt
  wdt: ts72xx: add DT support for ts72xx
  dt-bindings: gpio: Add DT bindings ep93xx gpio
  gpio: ep93xx: add DT support for gpio-ep93xx
  ARM: dts: add device tree for ep93xx Soc
  ARM: ep93xx: DT for the Cirrus ep93xx SoC platforms
  pwm: ep93xx: drop legacy pinctrl
  input: keypad: ep93xx: drop legacy pinctrl
  ARM: ep93xx: soc: drop defines
  ARM: ep93xx: delete all boardfiles

 .../devicetree/bindings/arm/ep93xx.yaml       |   99 +
 .../bindings/ata/cirrus,ep93xx-pata.yaml      |   40 +
 .../bindings/dma/cirrus,ep93xx-dma-m2m.yaml   |   66 +
 .../bindings/dma/cirrus,ep93xx-dma-m2p.yaml   |  102 +
 .../devicetree/bindings/gpio/gpio-ep93xx.yaml |  161 ++
 .../bindings/input/cirrus,ep93xx-keypad.yaml  |  123 ++
 .../bindings/mtd/technologic,nand.yaml        |   56 +
 .../bindings/net/cirrus,ep93xx_eth.yaml       |   51 +
 .../pinctrl/cirrus,ep93xx-pinctrl.yaml        |   66 +
 .../bindings/pwm/cirrus,ep93xx-pwm.yaml       |   45 +
 .../bindings/rtc/cirrus,ep93xx-rtc.yaml       |   32 +
 .../bindings/rtc/dallas,rtc-m48t86.yaml       |   33 +
 .../devicetree/bindings/spi/spi-ep93xx.yaml   |   68 +
 .../bindings/timer/cirrus,ep93xx-timer.yaml   |   41 +
 .../bindings/watchdog/cirrus,ep93xx-wdt.yaml  |   38 +
 .../watchdog/technologic,ts72xx-wdt.yaml      |   39 +
 arch/arm/Makefile                             |    1 -
 arch/arm/boot/dts/Makefile                    |    1 +
 arch/arm/boot/dts/ep93xx-bk3.dts              |   96 +
 arch/arm/boot/dts/ep93xx-edb9302.dts          |  150 ++
 arch/arm/boot/dts/ep93xx-ts7250.dts           |  113 ++
 arch/arm/boot/dts/ep93xx.dtsi                 |  466 +++++
 arch/arm/mach-ep93xx/Kconfig                  |   20 +-
 arch/arm/mach-ep93xx/Makefile                 |   11 -
 arch/arm/mach-ep93xx/core.c                   | 1017 ----------
 arch/arm/mach-ep93xx/dma.c                    |  114 --
 arch/arm/mach-ep93xx/edb93xx.c                |  344 ----
 arch/arm/mach-ep93xx/ep93xx-regs.h            |   38 -
 arch/arm/mach-ep93xx/gpio-ep93xx.h            |  111 --
 arch/arm/mach-ep93xx/hardware.h               |   25 -
 arch/arm/mach-ep93xx/irqs.h                   |   76 -
 arch/arm/mach-ep93xx/platform.h               |   42 -
 arch/arm/mach-ep93xx/soc.h                    |  212 --
 arch/arm/mach-ep93xx/ts72xx.c                 |  422 ----
 arch/arm/mach-ep93xx/ts72xx.h                 |   94 -
 arch/arm/mach-ep93xx/vision_ep9307.c          |  311 ---
 drivers/ata/pata_ep93xx.c                     |    9 +
 drivers/clk/Kconfig                           |    8 +
 drivers/clk/Makefile                          |    1 +
 .../clock.c => drivers/clk/clk-ep93xx.c       |  491 +++--
 drivers/clocksource/Kconfig                   |   11 +
 drivers/clocksource/Makefile                  |    1 +
 .../clocksource}/timer-ep93xx.c               |  143 +-
 drivers/dma/ep93xx_dma.c                      |  119 +-
 drivers/gpio/gpio-ep93xx.c                    |  329 ++--
 drivers/input/keyboard/ep93xx_keypad.c        |   25 +-
 drivers/mtd/nand/raw/Kconfig                  |    8 +
 drivers/mtd/nand/raw/Makefile                 |    1 +
 drivers/mtd/nand/raw/ts72xx_nand.c            |   94 +
 drivers/net/ethernet/cirrus/ep93xx_eth.c      |   49 +-
 drivers/pinctrl/Kconfig                       |    7 +
 drivers/pinctrl/Makefile                      |    1 +
 drivers/pinctrl/pinctrl-ep93xx.c              | 1698 +++++++++++++++++
 drivers/power/reset/Kconfig                   |   10 +
 drivers/power/reset/Makefile                  |    1 +
 drivers/power/reset/ep93xx-restart.c          |   65 +
 drivers/pwm/pwm-ep93xx.c                      |   24 +-
 drivers/rtc/rtc-ep93xx.c                      |    8 +
 drivers/rtc/rtc-m48t86.c                      |   10 +
 drivers/soc/Kconfig                           |    1 +
 drivers/soc/Makefile                          |    1 +
 drivers/soc/cirrus/Kconfig                    |   11 +
 drivers/soc/cirrus/Makefile                   |    2 +
 drivers/soc/cirrus/soc-ep93xx.c               |  134 ++
 drivers/spi/spi-ep93xx.c                      |   31 +-
 drivers/watchdog/ep93xx_wdt.c                 |    8 +
 drivers/watchdog/ts72xx_wdt.c                 |    8 +
 .../dt-bindings/clock/cirrus,ep93xx-clock.h   |   53 +
 include/linux/platform_data/dma-ep93xx.h      |    3 +
 include/linux/soc/cirrus/ep93xx.h             |   28 +-
 sound/soc/cirrus/Kconfig                      |    9 -
 sound/soc/cirrus/Makefile                     |    4 -
 sound/soc/cirrus/edb93xx.c                    |  119 --
 73 files changed, 4796 insertions(+), 3453 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/ep93xx.yaml
 create mode 100644 Documentation/devicetree/bindings/ata/cirrus,ep93xx-pata.yaml
 create mode 100644 Documentation/devicetree/bindings/dma/cirrus,ep93xx-dma-m2m.yaml
 create mode 100644 Documentation/devicetree/bindings/dma/cirrus,ep93xx-dma-m2p.yaml
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-ep93xx.yaml
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml
 create mode 100644 Documentation/devicetree/bindings/mtd/technologic,nand.yaml
 create mode 100644 Documentation/devicetree/bindings/net/cirrus,ep93xx_eth.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/cirrus,ep93xx-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pwm/cirrus,ep93xx-pwm.yaml
 create mode 100644 Documentation/devicetree/bindings/rtc/cirrus,ep93xx-rtc.yaml
 create mode 100644 Documentation/devicetree/bindings/rtc/dallas,rtc-m48t86.yaml
 create mode 100644 Documentation/devicetree/bindings/spi/spi-ep93xx.yaml
 create mode 100644 Documentation/devicetree/bindings/timer/cirrus,ep93xx-timer.yaml
 create mode 100644 Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml
 create mode 100644 Documentation/devicetree/bindings/watchdog/technologic,ts72xx-wdt.yaml
 create mode 100644 arch/arm/boot/dts/ep93xx-bk3.dts
 create mode 100644 arch/arm/boot/dts/ep93xx-edb9302.dts
 create mode 100644 arch/arm/boot/dts/ep93xx-ts7250.dts
 create mode 100644 arch/arm/boot/dts/ep93xx.dtsi
 delete mode 100644 arch/arm/mach-ep93xx/Makefile
 delete mode 100644 arch/arm/mach-ep93xx/core.c
 delete mode 100644 arch/arm/mach-ep93xx/dma.c
 delete mode 100644 arch/arm/mach-ep93xx/edb93xx.c
 delete mode 100644 arch/arm/mach-ep93xx/ep93xx-regs.h
 delete mode 100644 arch/arm/mach-ep93xx/gpio-ep93xx.h
 delete mode 100644 arch/arm/mach-ep93xx/hardware.h
 delete mode 100644 arch/arm/mach-ep93xx/irqs.h
 delete mode 100644 arch/arm/mach-ep93xx/platform.h
 delete mode 100644 arch/arm/mach-ep93xx/soc.h
 delete mode 100644 arch/arm/mach-ep93xx/ts72xx.c
 delete mode 100644 arch/arm/mach-ep93xx/ts72xx.h
 delete mode 100644 arch/arm/mach-ep93xx/vision_ep9307.c
 rename arch/arm/mach-ep93xx/clock.c => drivers/clk/clk-ep93xx.c (60%)
 rename {arch/arm/mach-ep93xx => drivers/clocksource}/timer-ep93xx.c (51%)
 create mode 100644 drivers/mtd/nand/raw/ts72xx_nand.c
 create mode 100644 drivers/pinctrl/pinctrl-ep93xx.c
 create mode 100644 drivers/power/reset/ep93xx-restart.c
 create mode 100644 drivers/soc/cirrus/Kconfig
 create mode 100644 drivers/soc/cirrus/Makefile
 create mode 100644 drivers/soc/cirrus/soc-ep93xx.c
 create mode 100644 include/dt-bindings/clock/cirrus,ep93xx-clock.h
 delete mode 100644 sound/soc/cirrus/edb93xx.c

-- 
2.39.2


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 22/43] dt-bindings: mtd: add DT bindings for ts7250 nand

[Nikita Shubin]

Add YAML bindings for ts7250 NAND.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 .../bindings/mtd/technologic,nand.yaml        | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/technologic,nand.yaml

diff --git a/Documentation/devicetree/bindings/mtd/technologic,nand.yaml b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
new file mode 100644
index 000000000000..3234d93a1c21
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/technologic,nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Technologic Systems NAND controller
+
+maintainers:
+  - Lukasz Majewski <lukma@denx.de>
+
+properties:
+  compatible:
+    items:
+      - const: technologic,ts7200-nand
+      - const: gen_nand
+
+  reg:
+    maxItems: 1
+
+  '#address-cells': true
+  '#size-cells': true
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: true
+
+examples:
+  - |
+    nand-parts@0 {
+      compatible = "technologic,ts7200-nand", "gen_nand";
+      reg = <0x60000000 0x8000000>;
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      partition@0 {
+        label = "TS-BOOTROM";
+        reg = <0x00000000 0x00020000>;
+        read-only;
+      };
+
+      partition@20000 {
+        label = "Linux";
+        reg = <0x00020000 0x07d00000>;
+      };
+
+      partition@7d20000 {
+        label = "RedBoot";
+        reg = <0x07d20000 0x002e0000>;
+        read-only;
+      };
+    };
+
+...
-- 
2.39.2


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 23/43] mtd: ts72xx_nand: add platform helper

[Nikita Shubin]

The only purpose of this code to provide platform_nand_data to gen_nand
driver which requires .cmd_ctrl and .dev_ready.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---

Notes:
    rfc->v0
    Alexander Sverdlin:
    - platfrom -> platform
    
    Linus Walleij:
    - __raw_read/write[w|l] -> readb/writeb
    
    Arnd Bergmann:
    - name match node ts7250 instead of wildcard

 drivers/mtd/nand/raw/Kconfig       |  8 +++
 drivers/mtd/nand/raw/Makefile      |  1 +
 drivers/mtd/nand/raw/ts72xx_nand.c | 94 ++++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+)
 create mode 100644 drivers/mtd/nand/raw/ts72xx_nand.c

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 170f1185ddc4..08f88c34ce53 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -456,6 +456,14 @@ config MTD_NAND_RENESAS
 	  Enables support for the NAND controller found on Renesas R-Car
 	  Gen3 and RZ/N1 SoC families.
 
+config MTD_NAND_TS72XX
+	bool "ts72xx NAND controller"
+	depends on ARCH_EP93XX && HAS_IOMEM
+	select MTD_NAND_PLATFORM
+	help
+	  Enables support for NAND controller on ts72xx SBCs.
+	  These only set's platform data. The real driver is gen_nand.
+
 comment "Misc"
 
 config MTD_SM_COMMON
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 917cdfb815b9..b1d5cec84519 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -23,6 +23,7 @@ omap2_nand-objs := omap2.o
 obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap2_nand.o
 obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)	+= omap_elm.o
 obj-$(CONFIG_MTD_NAND_MARVELL)		+= marvell_nand.o
+obj-$(CONFIG_MTD_NAND_TS72XX)		+= ts72xx_nand.o
 obj-$(CONFIG_MTD_NAND_PLATFORM)		+= plat_nand.o
 obj-$(CONFIG_MTD_NAND_PASEMI)		+= pasemi_nand.o
 obj-$(CONFIG_MTD_NAND_ORION)		+= orion_nand.o
diff --git a/drivers/mtd/nand/raw/ts72xx_nand.c b/drivers/mtd/nand/raw/ts72xx_nand.c
new file mode 100644
index 000000000000..1c1c06c6e71a
--- /dev/null
+++ b/drivers/mtd/nand/raw/ts72xx_nand.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Technologic Systems TS72xx NAND driver platform code
+ * Copyright (C) 2021 Nikita Shubin <nikita.shubin@maquefel.me>
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/clk.h>
+#include <linux/reset.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/rawnand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/platnand.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+
+#define TS72XX_NAND_CONTROL_ADDR_LINE	22	/* 0xN0400000 */
+#define TS72XX_NAND_BUSY_ADDR_LINE	23	/* 0xN0800000 */
+
+static void ts72xx_nand_hwcontrol(struct nand_chip *chip,
+				  int cmd, unsigned int ctrl)
+{
+	if (ctrl & NAND_CTRL_CHANGE) {
+		void __iomem *addr = chip->legacy.IO_ADDR_R;
+		unsigned char bits;
+
+		addr += (1 << TS72XX_NAND_CONTROL_ADDR_LINE);
+
+		bits = readb(addr) & ~0x07;
+		bits |= (ctrl & NAND_NCE) << 2;	/* bit 0 -> bit 2 */
+		bits |= (ctrl & NAND_CLE);	/* bit 1 -> bit 1 */
+		bits |= (ctrl & NAND_ALE) >> 2;	/* bit 2 -> bit 0 */
+
+		writeb(bits, addr);
+	}
+
+	if (cmd != NAND_CMD_NONE)
+		writeb(cmd, chip->legacy.IO_ADDR_W);
+}
+
+static int ts72xx_nand_device_ready(struct nand_chip *chip)
+{
+	void __iomem *addr = chip->legacy.IO_ADDR_R;
+
+	addr += (1 << TS72XX_NAND_BUSY_ADDR_LINE);
+
+	return !!(readb(addr) & 0x20);
+}
+
+static struct platform_nand_data ts72xx_nand_data = {
+	.chip = {
+		.nr_chips	= 1,
+		.chip_offset	= 0,
+		.chip_delay	= 15,
+	},
+	.ctrl = {
+		.cmd_ctrl	= ts72xx_nand_hwcontrol,
+		.dev_ready	= ts72xx_nand_device_ready,
+	},
+};
+
+static int __init ts72xx_nand_setup(void)
+{
+	struct device_node *np;
+	struct platform_device *pdev;
+
+	/* Multiplatform guard, only proceed on ts7250 */
+	if (!of_machine_is_compatible("technologic,ts7250"))
+		return 0;
+
+	np = of_find_compatible_node(NULL, NULL, "technologic,ts7200-nand");
+	if (!np) {
+		pr_err("%s : nand device tree node not found.\n", __func__);
+		return -EINVAL;
+	}
+
+	pdev = of_find_device_by_node(np);
+	if (!pdev) {
+		pr_err("%s : nand device not found.\n", __func__);
+		return -EINVAL;
+	}
+
+	pdev->dev.platform_data = &ts72xx_nand_data;
+	put_device(&pdev->dev);
+	of_node_put(np);
+
+	return 0;
+}
+
+subsys_initcall(ts72xx_nand_setup);
-- 
2.39.2


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 22/43] dt-bindings: mtd: add DT bindings for ts7250 nand

[Rob Herring]

On Mon, Apr 24, 2023 at 03:34:38PM +0300, Nikita Shubin wrote:
> Add YAML bindings for ts7250 NAND.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  .../bindings/mtd/technologic,nand.yaml        | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/technologic,nand.yaml b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> new file mode 100644
> index 000000000000..3234d93a1c21
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/technologic,nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Technologic Systems NAND controller
> +
> +maintainers:
> +  - Lukasz Majewski <lukma@denx.de>
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: technologic,ts7200-nand
> +      - const: gen_nand

Not a useful compatible.

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells': true
> +  '#size-cells': true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: true

No, 'true' is not allowed here.

> +
> +examples:
> +  - |
> +    nand-parts@0 {
> +      compatible = "technologic,ts7200-nand", "gen_nand";
> +      reg = <0x60000000 0x8000000>;
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +
> +      partition@0 {
> +        label = "TS-BOOTROM";
> +        reg = <0x00000000 0x00020000>;
> +        read-only;
> +      };
> +
> +      partition@20000 {
> +        label = "Linux";
> +        reg = <0x00020000 0x07d00000>;
> +      };
> +
> +      partition@7d20000 {
> +        label = "RedBoot";
> +        reg = <0x07d20000 0x002e0000>;
> +        read-only;
> +      };
> +    };
> +
> +...
> -- 
> 2.39.2
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/43] ep93xx device tree conversion

[Jakub Kicinski]

On Mon, 24 Apr 2023 13:31:25 +0200 Arnd Bergmann wrote:
> Thanks a lot for your continued work. I can't merge any of this at
> the moment since the upstream merge window just opened, but I'm
> happy to take this all through the soc tree for 6.5, provided we
> get the sufficient Acks from the subsystem maintainers. Merging
> it through each individual tree would take a lot longer, so I
> hope we can avoid that.

Is there a dependency between the patches?
-- 
pw-bot: nap

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/43] ep93xx device tree conversion

[Krzysztof Kozlowski]

On 25/04/2023 00:29, Jakub Kicinski wrote:
> On Mon, 24 Apr 2023 13:31:25 +0200 Arnd Bergmann wrote:
>> Thanks a lot for your continued work. I can't merge any of this at
>> the moment since the upstream merge window just opened, but I'm
>> happy to take this all through the soc tree for 6.5, provided we
>> get the sufficient Acks from the subsystem maintainers. Merging
>> it through each individual tree would take a lot longer, so I
>> hope we can avoid that.
> 
> Is there a dependency between the patches?

I didn't get entire patchset and cover letter does not mention
dependencies, but usually there shouldn't be such. Maybe for the next
versions this should be split per subsystem?

Best regards,
Krzysztof


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/43] ep93xx device tree conversion

[Arnd Bergmann]

On Tue, Apr 25, 2023, at 10:20, Krzysztof Kozlowski wrote:
> On 25/04/2023 00:29, Jakub Kicinski wrote:
>> On Mon, 24 Apr 2023 13:31:25 +0200 Arnd Bergmann wrote:
>>> Thanks a lot for your continued work. I can't merge any of this at
>>> the moment since the upstream merge window just opened, but I'm
>>> happy to take this all through the soc tree for 6.5, provided we
>>> get the sufficient Acks from the subsystem maintainers. Merging
>>> it through each individual tree would take a lot longer, so I
>>> hope we can avoid that.
>> 
>> Is there a dependency between the patches?
>
> I didn't get entire patchset and cover letter does not mention
> dependencies, but usually there shouldn't be such. Maybe for the next
> versions this should be split per subsystem?

Clearly the last patch that removes the board files depends on
all the previous patches, but I assume that the other ones
are all independent.

We don't do complete conversions from boardfiles to DT that often
any more, but in the past we tended to do this through a cross-
subsystem branch in the soc tree, which helps do it more quickly
and is less work for Nikita. In this case, I would make it a
separate top-level branch in the soc tree.

If anyone strongly feels that the patches should go through
the subsystem trees here, we'll take the longer path and
do the changes separately, with the boardfile removal
coming a release later.

     Arnd

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/43] ep93xx device tree conversion

[Linus Walleij]

On Mon, Apr 24, 2023 at 11:35 AM Nikita Shubin
<nikita.shubin@maquefel.me> wrote:

> This series aims to convert ep93xx from platform to full device tree support.
>
> Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.

Neat, I'd say let's merge this for 6.5 once the final rough edges are
off. The DT bindings should be easy to fix.

This is a big patch set and the improvement to the ARM kernel it
brings is great, so I am a bit worried about over-review stalling the
merged. If there start to be nitpicky comments I would prefer that
we merge it and let minor comments and "nice-to-haves" be
addressed in-tree during the development cycle.

I encourage you to use b4 to manage the patch series if you
have time to learn it, it could help you:
https://people.kernel.org/monsieuricon/sending-a-kernel-patch-with-b4-part-1

Yours,
Linus Walleij

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/43] ep93xx device tree conversion

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 454 bytes --]

On Wed, Apr 26, 2023 at 10:56:53PM +0200, Linus Walleij wrote:

> This is a big patch set and the improvement to the ARM kernel it
> brings is great, so I am a bit worried about over-review stalling the
> merged. If there start to be nitpicky comments I would prefer that
> we merge it and let minor comments and "nice-to-haves" be
> addressed in-tree during the development cycle.

I'm really not enthusiastic about the SPI bindings being merged as-is.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/43] ep93xx device tree conversion

[Linus Walleij]

On Wed, Apr 26, 2023 at 11:02 PM Mark Brown <broonie@kernel.org> wrote:
> On Wed, Apr 26, 2023 at 10:56:53PM +0200, Linus Walleij wrote:
>
> > This is a big patch set and the improvement to the ARM kernel it
> > brings is great, so I am a bit worried about over-review stalling the
> > merged. If there start to be nitpicky comments I would prefer that
> > we merge it and let minor comments and "nice-to-haves" be
> > addressed in-tree during the development cycle.
>
> I'm really not enthusiastic about the SPI bindings being merged as-is.

Agree, the bindings are more important than the code IMO,
they tend to get written in stone.

Yours,
Linus Walleij

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 22/43] dt-bindings: mtd: add DT bindings for ts7250 nand

[Miquel Raynal]

Hi Nikita,

nikita.shubin@maquefel.me wrote on Mon, 24 Apr 2023 15:34:38 +0300:

> Add YAML bindings for ts7250 NAND.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  .../bindings/mtd/technologic,nand.yaml        | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/technologic,nand.yaml b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> new file mode 100644
> index 000000000000..3234d93a1c21
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/technologic,nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Technologic Systems NAND controller
> +
> +maintainers:
> +  - Lukasz Majewski <lukma@denx.de>
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: technologic,ts7200-nand

would -nand-controller instead of -nand work as a suffix here?

You mention ts7250 in the title, should we have a more specific
compatible than ts7200 as well?

I see by looking at the mtd patch that you actually try to match both,
so they should both be defined in the bindings.

> +      - const: gen_nand

This is a old hack for very simple controllers (converted to DT probing
12 years ago). The logic used by this driver has been deprecated for
like 10 years and does not really apply to modern APIs. I would really
like to keep this driver contained with platform data coming from arch/
data only.

I suggest you create a real NAND controller driver based on the
generic one (should not be very complex, just duplicate the code so the
migration to the up-to-date API is eased) and you flag it as "must be
updated to ->exec_op() somehow. This way if someone starts the
conversion, it does not need to cope with the 5 other users of the
generic driver which anyway share nothing in common besides the
deprecated ->cmd_ctrl() backbone.

I read the comments on the cover letter, people are kind of pushing on
having this merged quickly. I am fine accepting a legacy controller
driver and migrating it to ->exec_op() later, but the current driver
conversion does not fit the approach taken years ago towards a cleaner
mtd tree.

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells': true
> +  '#size-cells': true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: true
> +
> +examples:
> +  - |
> +    nand-parts@0 {
> +      compatible = "technologic,ts7200-nand", "gen_nand";
> +      reg = <0x60000000 0x8000000>;
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +
> +      partition@0 {
> +        label = "TS-BOOTROM";
> +        reg = <0x00000000 0x00020000>;
> +        read-only;
> +      };

Partitions are not useful here, but if you want them, use the
partitions container instead, please.

> +
> +      partition@20000 {
> +        label = "Linux";
> +        reg = <0x00020000 0x07d00000>;
> +      };
> +
> +      partition@7d20000 {
> +        label = "RedBoot";
> +        reg = <0x07d20000 0x002e0000>;
> +        read-only;
> +      };
> +    };
> +
> +...


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 22/43] dt-bindings: mtd: add DT bindings for ts7250 nand

[Nikita Shubin]

Hello Miquel!

Thank you for looking into it.

On Tue, 2023-05-02 at 11:48 +0200, Miquel Raynal wrote:
> Hi Nikita,
> 
> nikita.shubin@maquefel.me wrote on Mon, 24 Apr 2023 15:34:38 +0300:
> 
> > Add YAML bindings for ts7250 NAND.
> > 
> > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> > ---
> >  .../bindings/mtd/technologic,nand.yaml        | 56
> > +++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> > b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> > new file mode 100644
> > index 000000000000..3234d93a1c21
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> > @@ -0,0 +1,56 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mtd/technologic,nand.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Technologic Systems NAND controller
> > +
> > +maintainers:
> > +  - Lukasz Majewski <lukma@denx.de>
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: technologic,ts7200-nand
> 
> would -nand-controller instead of -nand work as a suffix here?
> 
> You mention ts7250 in the title, should we have a more specific
> compatible than ts7200 as well?
> 
> I see by looking at the mtd patch that you actually try to match
> both,
> so they should both be defined in the bindings.
> 
> > +      - const: gen_nand
> 
> This is a old hack for very simple controllers (converted to DT
> probing
> 12 years ago). The logic used by this driver has been deprecated for
> like 10 years and does not really apply to modern APIs. I would
> really
> like to keep this driver contained with platform data coming from
> arch/
> data only.
> 
> I suggest you create a real NAND controller driver based on the
> generic one (should not be very complex, just duplicate the code so
> the
> migration to the up-to-date API is eased) and you flag it as "must be
> updated to ->exec_op() somehow. This way if someone starts the
> conversion, it does not need to cope with the 5 other users of the
> generic driver which anyway share nothing in common besides the
> deprecated ->cmd_ctrl() backbone.
> 
> I read the comments on the cover letter, people are kind of pushing
> on
> having this merged quickly. I am fine accepting a legacy controller
> driver and migrating it to ->exec_op() later, but the current driver
> conversion does not fit the approach taken years ago towards a
> cleaner
> mtd tree.

Did you mean that i should at least implement legacy nand controller,
like, for example, Xway (xway_nand.c) ?:

        data->chip.legacy.cmd_ctrl = xway_cmd_ctrl;
        data->chip.legacy.dev_ready = xway_dev_ready;
        data->chip.legacy.select_chip = xway_select_chip;
        data->chip.legacy.write_buf = xway_write_buf;
        data->chip.legacy.read_buf = xway_read_buf;
        data->chip.legacy.read_byte = xway_read_byte;
        data->chip.legacy.chip_delay = 30;

And the best solution would be switching to exec_op completely ?

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#address-cells': true
> > +  '#size-cells': true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +unevaluatedProperties: true
> > +
> > +examples:
> > +  - |
> > +    nand-parts@0 {
> > +      compatible = "technologic,ts7200-nand", "gen_nand";
> > +      reg = <0x60000000 0x8000000>;
> > +      #address-cells = <1>;
> > +      #size-cells = <1>;
> > +
> > +      partition@0 {
> > +        label = "TS-BOOTROM";
> > +        reg = <0x00000000 0x00020000>;
> > +        read-only;
> > +      };
> 
> Partitions are not useful here, but if you want them, use the
> partitions container instead, please.
> 
> > +
> > +      partition@20000 {
> > +        label = "Linux";
> > +        reg = <0x00020000 0x07d00000>;
> > +      };
> > +
> > +      partition@7d20000 {
> > +        label = "RedBoot";
> > +        reg = <0x07d20000 0x002e0000>;
> > +        read-only;
> > +      };
> > +    };
> > +
> > +...
> 
> 
> Thanks,
> Miquèl


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/43] ep93xx device tree conversion

[Florian Fainelli]

On 4/24/2023 5:34 AM, Nikita Shubin wrote:
> This series aims to convert ep93xx from platform to full device tree support.
> 
> Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.
> 
> Thank you Linus and Arnd for your support, review and comments, sorry if i missed something -
> these series are quite big for me.
> 
> Big thanks to Alexander Sverdlin for his testing, support, review, fixes and patches.

If anyone is interested I still have a TS-7300 board [1] that is fully 
functional and could be sent out to a new home.

https://www.embeddedts.com/products/TS-7300
-- 
Florian

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/43] ep93xx device tree conversion

[Nikita Shubin]

Hello Florian!

On Mon, 2023-05-15 at 20:47 -0700, Florian Fainelli wrote:
> 
> 
> On 4/24/2023 5:34 AM, Nikita Shubin wrote:
> > This series aims to convert ep93xx from platform to full device
> > tree support.
> > 
> > Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.
> > 
> > Thank you Linus and Arnd for your support, review and comments,
> > sorry if i missed something -
> > these series are quite big for me.
> > 
> > Big thanks to Alexander Sverdlin for his testing, support, review,
> > fixes and patches.
> 
> If anyone is interested I still have a TS-7300 board [1] that is
> fully 
> functional and could be sent out to a new home.

Thank you kindly, i'll keep this in mind !

> 
> https://www.embeddedts.com/products/TS-7300


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 22/43] dt-bindings: mtd: add DT bindings for ts7250 nand

[Miquel Raynal]

Hi Nikita,

nikita.shubin@maquefel.me wrote on Mon, 15 May 2023 18:48:31 +0300:

> Hello Miquel!
> 
> Thank you for looking into it.
> 
> On Tue, 2023-05-02 at 11:48 +0200, Miquel Raynal wrote:
> > Hi Nikita,
> > 
> > nikita.shubin@maquefel.me wrote on Mon, 24 Apr 2023 15:34:38 +0300:
> >   
> > > Add YAML bindings for ts7250 NAND.
> > > 
> > > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> > > ---
> > >  .../bindings/mtd/technologic,nand.yaml        | 56
> > > +++++++++++++++++++
> > >  1 file changed, 56 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> > > b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> > > new file mode 100644
> > > index 000000000000..3234d93a1c21
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> > > @@ -0,0 +1,56 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mtd/technologic,nand.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Technologic Systems NAND controller
> > > +
> > > +maintainers:
> > > +  - Lukasz Majewski <lukma@denx.de>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - const: technologic,ts7200-nand  
> > 
> > would -nand-controller instead of -nand work as a suffix here?
> > 
> > You mention ts7250 in the title, should we have a more specific
> > compatible than ts7200 as well?
> > 
> > I see by looking at the mtd patch that you actually try to match
> > both,
> > so they should both be defined in the bindings.
> >   
> > > +      - const: gen_nand  
> > 
> > This is a old hack for very simple controllers (converted to DT
> > probing
> > 12 years ago). The logic used by this driver has been deprecated for
> > like 10 years and does not really apply to modern APIs. I would
> > really
> > like to keep this driver contained with platform data coming from
> > arch/
> > data only.
> > 
> > I suggest you create a real NAND controller driver based on the
> > generic one (should not be very complex, just duplicate the code so
> > the
> > migration to the up-to-date API is eased) and you flag it as "must be
> > updated to ->exec_op() somehow. This way if someone starts the
> > conversion, it does not need to cope with the 5 other users of the
> > generic driver which anyway share nothing in common besides the
> > deprecated ->cmd_ctrl() backbone.
> > 
> > I read the comments on the cover letter, people are kind of pushing
> > on
> > having this merged quickly. I am fine accepting a legacy controller
> > driver and migrating it to ->exec_op() later, but the current driver
> > conversion does not fit the approach taken years ago towards a
> > cleaner
> > mtd tree.  
> 
> Did you mean that i should at least implement legacy nand controller,
> like, for example, Xway (xway_nand.c) ?:
> 
>         data->chip.legacy.cmd_ctrl = xway_cmd_ctrl;
>         data->chip.legacy.dev_ready = xway_dev_ready;
>         data->chip.legacy.select_chip = xway_select_chip;
>         data->chip.legacy.write_buf = xway_write_buf;
>         data->chip.legacy.read_buf = xway_read_buf;
>         data->chip.legacy.read_byte = xway_read_byte;
>         data->chip.legacy.chip_delay = 30;

I don't know how urgent this conversion is, this is really the minimal
step...

> And the best solution would be switching to exec_op completely ?

...and this is what I would really prefer, yes. I don't think it's
huge, the controller being very simple and straightforward.

> 
> >   
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  '#address-cells': true
> > > +  '#size-cells': true
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +
> > > +unevaluatedProperties: true
> > > +
> > > +examples:
> > > +  - |
> > > +    nand-parts@0 {
> > > +      compatible = "technologic,ts7200-nand", "gen_nand";
> > > +      reg = <0x60000000 0x8000000>;
> > > +      #address-cells = <1>;
> > > +      #size-cells = <1>;
> > > +
> > > +      partition@0 {
> > > +        label = "TS-BOOTROM";
> > > +        reg = <0x00000000 0x00020000>;
> > > +        read-only;
> > > +      };  
> > 
> > Partitions are not useful here, but if you want them, use the
> > partitions container instead, please.
> >   
> > > +
> > > +      partition@20000 {
> > > +        label = "Linux";
> > > +        reg = <0x00020000 0x07d00000>;
> > > +      };
> > > +
> > > +      partition@7d20000 {
> > > +        label = "RedBoot";
> > > +        reg = <0x07d20000 0x002e0000>;
> > > +        read-only;
> > > +      };
> > > +    };
> > > +
> > > +...  
> > 
> > 
> > Thanks,
> > Miquèl  
> 


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v1 23/43] dt-bindings: mtd: Add ts7250 nand-controller

[Nikita Shubin]

Add YAML bindings for ts7250 NAND Controller.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---

Notes:
    v0 -> v1:
    
    make it a nand contoller

 .../bindings/mtd/technologic,nand.yaml        | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/technologic,nand.yaml

diff --git a/Documentation/devicetree/bindings/mtd/technologic,nand.yaml b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
new file mode 100644
index 000000000000..26d1d9c3331d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/technologic,nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Technologic Systems NAND controller
+
+maintainers:
+  - Nikita Shubin <nikita.shubin@maquefel.me>
+
+allOf:
+  - $ref: nand-controller.yaml
+
+properties:
+  compatible:
+    oneOf:
+      - const: technologic,ts7200-nand
+      - items:
+          - enum:
+              - technologic,ts7300-nand
+              - technologic,ts7260-nand
+              - technologic,ts7250-nand
+          - const: technologic,ts7200-nand
+
+  reg:
+    maxItems: 1
+
+  '#address-cells': true
+  '#size-cells': true
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: true
+
+examples:
+  - |
+    nand-controller@60000000 {
+      compatible = "technologic,ts7200-nand";
+      reg = <0x60000000 0x8000000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+    };
+
+...
-- 
2.37.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v1 24/43] mtd: nand: add support for ts72xx

[Nikita Shubin]

Technologic Systems has it's own nand controller implementation in CPLD.

This patch adds support for TS-72XX boards family.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/mtd/nand/raw/Kconfig                  |   7 +
 drivers/mtd/nand/raw/Makefile                 |   1 +
 .../nand/raw/technologic-nand-controller.c    | 151 ++++++++++++++++++
 3 files changed, 159 insertions(+)
 create mode 100644 drivers/mtd/nand/raw/technologic-nand-controller.c

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index b523354dfb00..94788da1a169 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -456,6 +456,13 @@ config MTD_NAND_RENESAS
 	  Enables support for the NAND controller found on Renesas R-Car
 	  Gen3 and RZ/N1 SoC families.
 
+config MTD_NAND_TS72XX
+	bool "ts72xx NAND controller"
+	depends on ARCH_EP93XX && HAS_IOMEM
+	help
+	  Enables support for NAND controller on ts72xx SBCs.
+	  This is a legacy driver based on gen_nand.
+
 comment "Misc"
 
 config MTD_SM_COMMON
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 917cdfb815b9..783e990a0078 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -23,6 +23,7 @@ omap2_nand-objs := omap2.o
 obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap2_nand.o
 obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)	+= omap_elm.o
 obj-$(CONFIG_MTD_NAND_MARVELL)		+= marvell_nand.o
+obj-$(CONFIG_MTD_NAND_TS72XX)		+= technologic-nand-controller.o
 obj-$(CONFIG_MTD_NAND_PLATFORM)		+= plat_nand.o
 obj-$(CONFIG_MTD_NAND_PASEMI)		+= pasemi_nand.o
 obj-$(CONFIG_MTD_NAND_ORION)		+= orion_nand.o
diff --git a/drivers/mtd/nand/raw/technologic-nand-controller.c b/drivers/mtd/nand/raw/technologic-nand-controller.c
new file mode 100644
index 000000000000..09aeada933a1
--- /dev/null
+++ b/drivers/mtd/nand/raw/technologic-nand-controller.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Technologic Systems TS72xx NAND controller driver
+ *
+ * Copyright (C) 2023 Nikita Shubin <nikita.shubin@maquefel.me>
+ *
+ * derived: plat_nand.c
+ *  Author: Vitaly Wool <vitalywool@gmail.com>
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/platnand.h>
+
+#define TS72XX_NAND_CONTROL_ADDR_LINE	22	/* 0xN0400000 */
+#define TS72XX_NAND_BUSY_ADDR_LINE	23	/* 0xN0800000 */
+
+struct ts72xx_nand_data {
+	struct nand_controller	controller;
+	struct nand_chip	chip;
+	void __iomem		*io_base;
+};
+
+static int ts72xx_nand_attach_chip(struct nand_chip *chip)
+{
+	if (chip->ecc.engine_type == NAND_ECC_ENGINE_TYPE_SOFT &&
+	    chip->ecc.algo == NAND_ECC_ALGO_UNKNOWN)
+		chip->ecc.algo = NAND_ECC_ALGO_HAMMING;
+
+	return 0;
+}
+
+static const struct nand_controller_ops ts72xx_nand_ops = {
+	.attach_chip = ts72xx_nand_attach_chip,
+};
+
+static void ts72xx_nand_hwcontrol(struct nand_chip *chip,
+				  int cmd, unsigned int ctrl)
+{
+	if (ctrl & NAND_CTRL_CHANGE) {
+		void __iomem *addr = chip->legacy.IO_ADDR_R;
+		unsigned char bits;
+
+		addr += (1 << TS72XX_NAND_CONTROL_ADDR_LINE);
+
+		bits = readb(addr) & ~0x07;
+		bits |= (ctrl & NAND_NCE) << 2;	/* bit 0 -> bit 2 */
+		bits |= (ctrl & NAND_CLE);	/* bit 1 -> bit 1 */
+		bits |= (ctrl & NAND_ALE) >> 2;	/* bit 2 -> bit 0 */
+
+		writeb(bits, addr);
+	}
+
+	if (cmd != NAND_CMD_NONE)
+		writeb(cmd, chip->legacy.IO_ADDR_W);
+}
+
+static int ts72xx_nand_device_ready(struct nand_chip *chip)
+{
+	void __iomem *addr = chip->legacy.IO_ADDR_R;
+
+	addr += (1 << TS72XX_NAND_BUSY_ADDR_LINE);
+
+	return !!(readb(addr) & 0x20);
+}
+
+static int ts72xx_nand_probe(struct platform_device *pdev)
+{
+	struct ts72xx_nand_data *data;
+	struct mtd_info *mtd;
+	int err = 0;
+
+	/* Allocate memory for the device structure (and zero it) */
+	data = devm_kzalloc(&pdev->dev, sizeof(struct ts72xx_nand_data),
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->controller.ops = &ts72xx_nand_ops;
+	nand_controller_init(&data->controller);
+	data->chip.controller = &data->controller;
+
+	data->io_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(data->io_base))
+		return PTR_ERR(data->io_base);
+
+	nand_set_flash_node(&data->chip, pdev->dev.of_node);
+	mtd = nand_to_mtd(&data->chip);
+	mtd->dev.parent = &pdev->dev;
+
+	data->chip.legacy.IO_ADDR_R = data->io_base;
+	data->chip.legacy.IO_ADDR_W = data->io_base;
+	data->chip.legacy.cmd_ctrl = ts72xx_nand_hwcontrol;
+	data->chip.legacy.dev_ready = ts72xx_nand_device_ready;
+
+	platform_set_drvdata(pdev, data);
+
+	/*
+	 * This driver assumes that the default ECC engine should be TYPE_SOFT.
+	 * Set ->engine_type before registering the NAND devices in order to
+	 * provide a driver specific default value.
+	 */
+	data->chip.ecc.engine_type = NAND_ECC_ENGINE_TYPE_SOFT;
+
+	/* Scan to find existence of the device */
+	err = nand_scan(&data->chip, 1);
+	if (err)
+		return err;
+
+	err = mtd_device_parse_register(mtd, NULL, NULL,
+					NULL, 0);
+
+	if (!err)
+		return err;
+
+	nand_cleanup(&data->chip);
+
+	return 0;
+}
+
+static void ts72xx_nand_remove(struct platform_device *pdev)
+{
+	struct ts72xx_nand_data *data = platform_get_drvdata(pdev);
+	struct nand_chip *chip = &data->chip;
+	int ret;
+
+	ret = mtd_device_unregister(nand_to_mtd(chip));
+	WARN_ON(ret);
+	nand_cleanup(chip);
+}
+
+static const struct of_device_id ts72xx_id_table[] = {
+	{ .compatible = "technologic,ts7200-nand" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ts72xx_id_table);
+
+static struct platform_driver ts72xx_nand_driver = {
+	.driver = {
+		.name = "ts72xx-nand",
+		.of_match_table = ts72xx_id_table,
+	},
+	.probe = ts72xx_nand_probe,
+	.remove_new = ts72xx_nand_remove,
+};
+module_platform_driver(ts72xx_nand_driver);
+
-- 
2.37.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 23/43] dt-bindings: mtd: Add ts7250 nand-controller

[Miquel Raynal]

Hi Nikita,

nikita.shubin@maquefel.me wrote on Thu,  1 Jun 2023 08:45:28 +0300:

> Add YAML bindings for ts7250 NAND Controller.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> 
> Notes:
>     v0 -> v1:
>     
>     make it a nand contoller
> 
>  .../bindings/mtd/technologic,nand.yaml        | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/technologic,nand.yaml b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> new file mode 100644
> index 000000000000..26d1d9c3331d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/technologic,nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Technologic Systems NAND controller
> +
> +maintainers:
> +  - Nikita Shubin <nikita.shubin@maquefel.me>
> +
> +allOf:
> +  - $ref: nand-controller.yaml
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: technologic,ts7200-nand
> +      - items:
> +          - enum:
> +              - technologic,ts7300-nand
> +              - technologic,ts7260-nand
> +              - technologic,ts7250-nand
> +          - const: technologic,ts7200-nand
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells': true
> +  '#size-cells': true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: true

Should be false I guess.

> +
> +examples:
> +  - |
> +    nand-controller@60000000 {
> +      compatible = "technologic,ts7200-nand";
> +      reg = <0x60000000 0x8000000>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +    };
> +
> +...


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 24/43] mtd: nand: add support for ts72xx

[Miquel Raynal]

Hi Nikita,

nikita.shubin@maquefel.me wrote on Thu,  1 Jun 2023 08:45:29 +0300:

> Technologic Systems has it's own nand controller implementation in CPLD.
> 
> This patch adds support for TS-72XX boards family.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  drivers/mtd/nand/raw/Kconfig                  |   7 +
>  drivers/mtd/nand/raw/Makefile                 |   1 +
>  .../nand/raw/technologic-nand-controller.c    | 151 ++++++++++++++++++
>  3 files changed, 159 insertions(+)
>  create mode 100644 drivers/mtd/nand/raw/technologic-nand-controller.c
> 
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index b523354dfb00..94788da1a169 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -456,6 +456,13 @@ config MTD_NAND_RENESAS
>  	  Enables support for the NAND controller found on Renesas R-Car
>  	  Gen3 and RZ/N1 SoC families.
>  
> +config MTD_NAND_TS72XX
> +	bool "ts72xx NAND controller"
> +	depends on ARCH_EP93XX && HAS_IOMEM
> +	help
> +	  Enables support for NAND controller on ts72xx SBCs.
> +	  This is a legacy driver based on gen_nand.
> +
>  comment "Misc"
>  
>  config MTD_SM_COMMON
> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
> index 917cdfb815b9..783e990a0078 100644
> --- a/drivers/mtd/nand/raw/Makefile
> +++ b/drivers/mtd/nand/raw/Makefile
> @@ -23,6 +23,7 @@ omap2_nand-objs := omap2.o
>  obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap2_nand.o
>  obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)	+= omap_elm.o
>  obj-$(CONFIG_MTD_NAND_MARVELL)		+= marvell_nand.o
> +obj-$(CONFIG_MTD_NAND_TS72XX)		+= technologic-nand-controller.o
>  obj-$(CONFIG_MTD_NAND_PLATFORM)		+= plat_nand.o
>  obj-$(CONFIG_MTD_NAND_PASEMI)		+= pasemi_nand.o
>  obj-$(CONFIG_MTD_NAND_ORION)		+= orion_nand.o
> diff --git a/drivers/mtd/nand/raw/technologic-nand-controller.c b/drivers/mtd/nand/raw/technologic-nand-controller.c
> new file mode 100644
> index 000000000000..09aeada933a1
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/technologic-nand-controller.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Technologic Systems TS72xx NAND controller driver
> + *
> + * Copyright (C) 2023 Nikita Shubin <nikita.shubin@maquefel.me>
> + *
> + * derived: plat_nand.c
> + *  Author: Vitaly Wool <vitalywool@gmail.com>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/platnand.h>
> +
> +#define TS72XX_NAND_CONTROL_ADDR_LINE	22	/* 0xN0400000 */
> +#define TS72XX_NAND_BUSY_ADDR_LINE	23	/* 0xN0800000 */
> +
> +struct ts72xx_nand_data {
> +	struct nand_controller	controller;
> +	struct nand_chip	chip;
> +	void __iomem		*io_base;
> +};
> +
> +static int ts72xx_nand_attach_chip(struct nand_chip *chip)
> +{
> +	if (chip->ecc.engine_type == NAND_ECC_ENGINE_TYPE_SOFT &&
> +	    chip->ecc.algo == NAND_ECC_ALGO_UNKNOWN)
> +		chip->ecc.algo = NAND_ECC_ALGO_HAMMING;

I believe engine_type == ON_HOST should return an error.

> +
> +	return 0;
> +}
> +
> +static const struct nand_controller_ops ts72xx_nand_ops = {
> +	.attach_chip = ts72xx_nand_attach_chip,
> +};
> +
> +static void ts72xx_nand_hwcontrol(struct nand_chip *chip,
> +				  int cmd, unsigned int ctrl)
> +{
> +	if (ctrl & NAND_CTRL_CHANGE) {
> +		void __iomem *addr = chip->legacy.IO_ADDR_R;
> +		unsigned char bits;
> +
> +		addr += (1 << TS72XX_NAND_CONTROL_ADDR_LINE);
> +
> +		bits = readb(addr) & ~0x07;
> +		bits |= (ctrl & NAND_NCE) << 2;	/* bit 0 -> bit 2 */
> +		bits |= (ctrl & NAND_CLE);	/* bit 1 -> bit 1 */
> +		bits |= (ctrl & NAND_ALE) >> 2;	/* bit 2 -> bit 0 */
> +
> +		writeb(bits, addr);
> +	}
> +
> +	if (cmd != NAND_CMD_NONE)
> +		writeb(cmd, chip->legacy.IO_ADDR_W);
> +}
> +
> +static int ts72xx_nand_device_ready(struct nand_chip *chip)
> +{
> +	void __iomem *addr = chip->legacy.IO_ADDR_R;
> +
> +	addr += (1 << TS72XX_NAND_BUSY_ADDR_LINE);
> +
> +	return !!(readb(addr) & 0x20);
> +}
> +
> +static int ts72xx_nand_probe(struct platform_device *pdev)
> +{
> +	struct ts72xx_nand_data *data;
> +	struct mtd_info *mtd;
> +	int err = 0;
> +
> +	/* Allocate memory for the device structure (and zero it) */
> +	data = devm_kzalloc(&pdev->dev, sizeof(struct ts72xx_nand_data),
> +			    GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->controller.ops = &ts72xx_nand_ops;
> +	nand_controller_init(&data->controller);
> +	data->chip.controller = &data->controller;
> +
> +	data->io_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(data->io_base))
> +		return PTR_ERR(data->io_base);
> +
> +	nand_set_flash_node(&data->chip, pdev->dev.of_node);
> +	mtd = nand_to_mtd(&data->chip);
> +	mtd->dev.parent = &pdev->dev;
> +
> +	data->chip.legacy.IO_ADDR_R = data->io_base;
> +	data->chip.legacy.IO_ADDR_W = data->io_base;
> +	data->chip.legacy.cmd_ctrl = ts72xx_nand_hwcontrol;
> +	data->chip.legacy.dev_ready = ts72xx_nand_device_ready;
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	/*
> +	 * This driver assumes that the default ECC engine should be TYPE_SOFT.
> +	 * Set ->engine_type before registering the NAND devices in order to
> +	 * provide a driver specific default value.
> +	 */
> +	data->chip.ecc.engine_type = NAND_ECC_ENGINE_TYPE_SOFT;
> +
> +	/* Scan to find existence of the device */
> +	err = nand_scan(&data->chip, 1);
> +	if (err)
> +		return err;
> +
> +	err = mtd_device_parse_register(mtd, NULL, NULL,
> +					NULL, 0);

The usual way -and more readable- is to jump to a goto label upon
error, and return 0 in the normal path.
> +
> +	if (!err)
> +		return err;
> +
> +	nand_cleanup(&data->chip);
> +
> +	return 0;
> +}
> +
> +static void ts72xx_nand_remove(struct platform_device *pdev)
> +{
> +	struct ts72xx_nand_data *data = platform_get_drvdata(pdev);
> +	struct nand_chip *chip = &data->chip;
> +	int ret;
> +
> +	ret = mtd_device_unregister(nand_to_mtd(chip));
> +	WARN_ON(ret);
> +	nand_cleanup(chip);
> +}
> +
> +static const struct of_device_id ts72xx_id_table[] = {
> +	{ .compatible = "technologic,ts7200-nand" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ts72xx_id_table);
> +
> +static struct platform_driver ts72xx_nand_driver = {
> +	.driver = {
> +		.name = "ts72xx-nand",
> +		.of_match_table = ts72xx_id_table,
> +	},
> +	.probe = ts72xx_nand_probe,
> +	.remove_new = ts72xx_nand_remove,
> +};
> +module_platform_driver(ts72xx_nand_driver);
> +


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 23/43] dt-bindings: mtd: Add ts7250 nand-controller

[Krzysztof Kozlowski]

On 01/06/2023 07:45, Nikita Shubin wrote:
> Add YAML bindings for ts7250 NAND Controller.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> 
> Notes:
>     v0 -> v1:
>     
>     make it a nand contoller
> 
>  .../bindings/mtd/technologic,nand.yaml        | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/technologic,nand.yaml b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> new file mode 100644
> index 000000000000..26d1d9c3331d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/technologic,nand.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/technologic,nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Technologic Systems NAND controller
> +
> +maintainers:
> +  - Nikita Shubin <nikita.shubin@maquefel.me>
> +
> +allOf:
> +  - $ref: nand-controller.yaml
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: technologic,ts7200-nand
> +      - items:
> +          - enum:
> +              - technologic,ts7300-nand
> +              - technologic,ts7260-nand
> +              - technologic,ts7250-nand
> +          - const: technologic,ts7200-nand
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells': true
> +  '#size-cells': true

Except what Miquel wrote - drop these two.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: true
> +
> +examples:
> +  - |
> +    nand-controller@60000000 {
> +      compatible = "technologic,ts7200-nand";
> +      reg = <0x60000000 0x8000000>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;

Incomplete example. address/size cells do not make sense here alone.
Finish the example.

> +    };
> +
> +...

Best regards,
Krzysztof


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 24/43] mtd: nand: add support for ts72xx

[andy.shevchenko]

Thu, Jun 01, 2023 at 08:45:29AM +0300, Nikita Shubin kirjoitti:
> Technologic Systems has it's own nand controller implementation in CPLD.
> 
> This patch adds support for TS-72XX boards family.

Use imperative mode, this is documented in the Submitting Patches,

...

> +/*
> + * Technologic Systems TS72xx NAND controller driver
> + *
> + * Copyright (C) 2023 Nikita Shubin <nikita.shubin@maquefel.me>
> + *
> + * derived: plat_nand.c

Derived from:

> + *  Author: Vitaly Wool <vitalywool@gmail.com>
> + */

...

> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>

+ Blank line?

> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/platnand.h>

...

> +		bits = readb(addr) & ~0x07;

GENMASK()?

...

> +	addr += (1 << TS72XX_NAND_BUSY_ADDR_LINE);

BIT() ?

...

> +	return !!(readb(addr) & 0x20);

BIT() ?

...

> +	struct ts72xx_nand_data *data;
> +	struct mtd_info *mtd;
> +	int err = 0;

Redundant assignment.

> +	/* Allocate memory for the device structure (and zero it) */
> +	data = devm_kzalloc(&pdev->dev, sizeof(struct ts72xx_nand_data),

sizeof(*data) and make it a single line.

> +			    GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;

...

> +	nand_set_flash_node(&data->chip, pdev->dev.of_node);

Hmm... wondering why this uses OF node instead of fwnode... But okay, this is
question to the subsystem maintaners.


> +	err = mtd_device_parse_register(mtd, NULL, NULL,
> +					NULL, 0);

There is plenty of space on the previous line.

> +

Redundant blank line.

> +	if (!err)
> +		return err;
> +
> +	nand_cleanup(&data->chip);
> +
> +	return 0;

This seems at least weird and rather broken.
To me it looks like

	if (err) {
		nand_cleanup(&data->chip);
		return err;
	}

	return 0;

has to be here.

> +}

...

> +	ret = mtd_device_unregister(nand_to_mtd(chip));
> +	WARN_ON(ret);

WARN_ON()?! Why?

> +	nand_cleanup(chip);
> +}

-- 
With Best Regards,
Andy Shevchenko



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 24/43] mtd: nand: add support for ts72xx

[Miquel Raynal]

Hi andy.shevchenko@gmail.com,

andy.shevchenko@gmail.com wrote on Sat, 3 Jun 2023 23:20:57 +0300:

> Thu, Jun 01, 2023 at 08:45:29AM +0300, Nikita Shubin kirjoitti:
> > Technologic Systems has it's own nand controller implementation in CPLD.
> > 
> > This patch adds support for TS-72XX boards family.  
> 
> Use imperative mode, this is documented in the Submitting Patches,
> 
> ...
> 
> > +/*
> > + * Technologic Systems TS72xx NAND controller driver
> > + *
> > + * Copyright (C) 2023 Nikita Shubin <nikita.shubin@maquefel.me>
> > + *
> > + * derived: plat_nand.c  
> 
> Derived from:
> 
> > + *  Author: Vitaly Wool <vitalywool@gmail.com>
> > + */  
> 
> ...
> 
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>  
> 
> + Blank line?
> 
> > +#include <linux/mtd/mtd.h>
> > +#include <linux/mtd/platnand.h>  
> 
> ...
> 
> > +		bits = readb(addr) & ~0x07;  
> 
> GENMASK()?
> 
> ...
> 
> > +	addr += (1 << TS72XX_NAND_BUSY_ADDR_LINE);  
> 
> BIT() ?
> 
> ...
> 
> > +	return !!(readb(addr) & 0x20);  
> 
> BIT() ?
> 
> ...
> 
> > +	struct ts72xx_nand_data *data;
> > +	struct mtd_info *mtd;
> > +	int err = 0;  
> 
> Redundant assignment.
> 
> > +	/* Allocate memory for the device structure (and zero it) */
> > +	data = devm_kzalloc(&pdev->dev, sizeof(struct ts72xx_nand_data),  
> 
> sizeof(*data) and make it a single line.
> 
> > +			    GFP_KERNEL);
> > +	if (!data)
> > +		return -ENOMEM;  
> 
> ...
> 
> > +	nand_set_flash_node(&data->chip, pdev->dev.of_node);  
> 
> Hmm... wondering why this uses OF node instead of fwnode... But okay, this is
> question to the subsystem maintaners.
> 
> 
> > +	err = mtd_device_parse_register(mtd, NULL, NULL,
> > +					NULL, 0);  
> 
> There is plenty of space on the previous line.
> 
> > +  
> 
> Redundant blank line.
> 
> > +	if (!err)
> > +		return err;
> > +
> > +	nand_cleanup(&data->chip);
> > +
> > +	return 0;  
> 
> This seems at least weird and rather broken.

Yeah, I made the same comment.

> To me it looks like
> 
> 	if (err) {
> 		nand_cleanup(&data->chip);
> 		return err;
> 	}
> 
> 	return 0;
> 
> has to be here.
> 
> > +}  
> 
> ...
> 
> > +	ret = mtd_device_unregister(nand_to_mtd(chip));
> > +	WARN_ON(ret);  
> 
> WARN_ON()?! Why?

This is actually something that is expected for now, the device
unregistration should not fail and the return value should not be used
to skip other cleanups. I cannot find the original discussion anymore
but we decided to use that construction. We might actually switch that
one to void someday.

> 
> > +	nand_cleanup(chip);
> > +}  
> 


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/