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

Re: [PATCH 17/43] spi: ep93xx: add DT support for Cirrus EP93xx

[Alexander Sverdlin]

On Mon, 2023-04-24 at 15:34 +0300, Nikita Shubin wrote:
> - find register range from the device tree
> - provide clock access via of
> - use_dma as a DT node
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

Tested-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Acked-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>

> ---
> 
> Notes:
>     Arnd Bergmann:
>     - wildcards ep93xx to something meaningful, i.e. ep9301
>     - drop wrappers
> 
>  drivers/spi/spi-ep93xx.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)

-- 
Alexander Sverdlin.

[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

[PATCH 16/43] dt-bindings: spi: Add DT bindings ep93xx spi

[Nikita Shubin]

Add YAML bindings for ep93xx SoC.

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

diff --git a/Documentation/devicetree/bindings/spi/spi-ep93xx.yaml b/Documentation/devicetree/bindings/spi/spi-ep93xx.yaml
new file mode 100644
index 000000000000..e09ab50629fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-ep93xx.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/spi-ep93xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: EP93xx SoC SPI controller
+
+maintainers:
+  - Mark Brown <broonie@kernel.org>
+
+allOf:
+  - $ref: "spi-controller.yaml#"
+
+properties:
+  "#address-cells": true
+  "#size-cells": true
+
+  compatible:
+    items:
+      - const: cirrus,ep9301-spi
+
+  reg:
+    items:
+      - description: SPI registers region
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: SPI Controller reference clock source
+
+  clock-names:
+    items:
+      - const: ep93xx-spi.0
+
+  cs-gpios: true
+
+  use_dma:
+    type: boolean
+    items:
+      - description: Flag indicating that the SPI should use dma
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/cirrus,ep93xx-clock.h>
+    spi0: spi@808a0000 {
+        compatible = "cirrus,ep9301-spi";
+        reg = <0x808a0000 0x18>;
+        interrupt-parent = <&vic1>;
+        interrupts = <21>;
+        clocks = <&syscon EP93XX_CLK_SPI>;
+        clock-names = "ep93xx-spi.0";
+        cs-gpios = <&gpio5 2 0>;
+        use_dma;
+    };
+
+...
-- 
2.39.2

[PATCH 17/43] spi: ep93xx: add DT support for Cirrus EP93xx

[Nikita Shubin]

- find register range from the device tree
- provide clock access via of
- use_dma as a DT node

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

Notes:
    Arnd Bergmann:
    - wildcards ep93xx to something meaningful, i.e. ep9301
    - drop wrappers

 drivers/spi/spi-ep93xx.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c
index 5896a7b2fade..b37be7109161 100644
--- a/drivers/spi/spi-ep93xx.c
+++ b/drivers/spi/spi-ep93xx.c
@@ -22,6 +22,7 @@
 #include <linux/bitops.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/scatterlist.h>
@@ -644,6 +645,25 @@ static void ep93xx_spi_release_dma(struct ep93xx_spi *espi)
 		free_page((unsigned long)espi->zeropage);
 }
 
+#ifdef CONFIG_OF
+static struct ep93xx_spi_info dt_spi_info;
+
+static struct ep93xx_spi_info *ep93xx_spi_get_platdata(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	if (np && of_property_read_bool(np, "use_dma"))
+		dt_spi_info.use_dma = 1;
+
+	return &dt_spi_info;
+}
+#else
+static struct ep93xx_spi_info *ep93xx_spi_get_platdata(struct platform_device *pdev)
+{
+	return dev_get_platdata(&pdev->dev);
+}
+#endif
+
 static int ep93xx_spi_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
@@ -653,7 +673,7 @@ static int ep93xx_spi_probe(struct platform_device *pdev)
 	int irq;
 	int error;
 
-	info = dev_get_platdata(&pdev->dev);
+	info = ep93xx_spi_get_platdata(pdev);
 	if (!info) {
 		dev_err(&pdev->dev, "missing platform data\n");
 		return -EINVAL;
@@ -726,6 +746,8 @@ static int ep93xx_spi_probe(struct platform_device *pdev)
 	/* make sure that the hardware is disabled */
 	writel(0, espi->mmio + SSPCR1);
 
+	master->dev.of_node = pdev->dev.of_node;
+
 	error = devm_spi_register_master(&pdev->dev, master);
 	if (error) {
 		dev_err(&pdev->dev, "failed to register SPI master\n");
@@ -755,9 +777,16 @@ static int ep93xx_spi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id ep93xx_spi_of_ids[] = {
+	{ .compatible = "cirrus,ep9301-spi" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ep93xx_spi_of_ids);
+
 static struct platform_driver ep93xx_spi_driver = {
 	.driver		= {
 		.name	= "ep93xx-spi",
+		.of_match_table = ep93xx_spi_of_ids,
 	},
 	.probe		= ep93xx_spi_probe,
 	.remove		= ep93xx_spi_remove,
-- 
2.39.2

Re: [PATCH 16/43] dt-bindings: spi: Add DT bindings ep93xx spi

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

On Mon, Apr 24, 2023 at 03:34:32PM +0300, Nikita Shubin wrote:

> +maintainers:
> +  - Mark Brown <broonie@kernel.org>

This needs to be someone who actually knows about and works on the
device.

> +  use_dma:
> +    type: boolean
> +    items:
> +      - description: Flag indicating that the SPI should use dma

There don't seem to be any DMA properties here, and why would this not
just be done by making them optional rather than having a separate
specific property?

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

Re: [PATCH 16/43] dt-bindings: spi: Add DT bindings ep93xx spi

[Rob Herring]

On Mon, Apr 24, 2023 at 03:34:32PM +0300, Nikita Shubin wrote:
> Add YAML bindings for ep93xx SoC.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  .../devicetree/bindings/spi/spi-ep93xx.yaml   | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-ep93xx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-ep93xx.yaml b/Documentation/devicetree/bindings/spi/spi-ep93xx.yaml
> new file mode 100644
> index 000000000000..e09ab50629fb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-ep93xx.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/spi-ep93xx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: EP93xx SoC SPI controller
> +
> +maintainers:
> +  - Mark Brown <broonie@kernel.org>
> +
> +allOf:
> +  - $ref: "spi-controller.yaml#"

Drop quotes.

> +
> +properties:
> +  "#address-cells": true
> +  "#size-cells": true
> +
> +  compatible:
> +    items:
> +      - const: cirrus,ep9301-spi
> +
> +  reg:
> +    items:
> +      - description: SPI registers region
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: SPI Controller reference clock source
> +
> +  clock-names:
> +    items:
> +      - const: ep93xx-spi.0

Does not sound like a clock name... Just drop unless you expect to add 
more clocks.

> +
> +  cs-gpios: true
> +
> +  use_dma:

Don't use '_' in property names. Custom properties need a vendor prefix.

However, this looks like a driver setting rather than h/w config. Why 
not always use DMA or transfers above X size. IOW, let the driver figure 
it out.

> +    type: boolean
> +    items:

'items' is for an array, but this is a boolean.

> +      - description: Flag indicating that the SPI should use dma
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/cirrus,ep93xx-clock.h>
> +    spi0: spi@808a0000 {
> +        compatible = "cirrus,ep9301-spi";
> +        reg = <0x808a0000 0x18>;
> +        interrupt-parent = <&vic1>;
> +        interrupts = <21>;
> +        clocks = <&syscon EP93XX_CLK_SPI>;
> +        clock-names = "ep93xx-spi.0";
> +        cs-gpios = <&gpio5 2 0>;
> +        use_dma;
> +    };
> +
> +...
> -- 
> 2.39.2
> 

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

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

Re: [PATCH 17/43] spi: ep93xx: add DT support for Cirrus EP93xx

[Linus Walleij]

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

> - find register range from the device tree
> - provide clock access via of
> - use_dma as a DT node
>
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

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

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

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

[PATCH v1 17/43] dt-bindings: spi: Add Cirrus EP93xx

[Nikita Shubin]

Add YAML bindings for ep93xx SoC SPI.

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

Notes:
    v0 -> v1:
    Krzysztof Kozlowski:
    - replaced maintainers
    - removed wildcards
    - use fallback compatible and list all possible compatibles
    - drop quotes in ref
    - dropped "clock-names"
    - dropped label
    - fix ident

 .../devicetree/bindings/spi/spi-ep9301.yaml   | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-ep9301.yaml

diff --git a/Documentation/devicetree/bindings/spi/spi-ep9301.yaml b/Documentation/devicetree/bindings/spi/spi-ep9301.yaml
new file mode 100644
index 000000000000..c363b25a3074
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-ep9301.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/spi-ep9301.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: EP93xx SoC SPI controller
+
+maintainers:
+  - Alexander Sverdlin <alexander.sverdlin@gmail.com>
+  - Nikita Shubin <nikita.shubin@maquefel.me>
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+properties:
+  "#address-cells": true
+  "#size-cells": true
+
+  compatible:
+    oneOf:
+      - const: cirrus,ep9301-spi
+      - items:
+          - enum:
+              - cirrus,ep9302-spi
+              - cirrus,ep9307-spi
+              - cirrus,ep9312-spi
+              - cirrus,ep9315-spi
+          - const: cirrus,ep9301-spi
+
+  reg:
+    items:
+      - description: SPI registers region
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: SPI Controller reference clock source
+
+  cs-gpios: true
+
+  cirrus,ep9301-use-dma:
+    description: Flag indicating that the SPI should use dma
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/cirrus,ep93xx-clock.h>
+    spi@808a0000 {
+      compatible = "cirrus,ep9301-spi";
+      reg = <0x808a0000 0x18>;
+      interrupt-parent = <&vic1>;
+      interrupts = <21>;
+      clocks = <&syscon EP93XX_CLK_SPI>;
+      cs-gpios = <&gpio5 2 0>;
+      cirrus,ep9301-use-dma;
+    };
+
+...
-- 
2.37.4

[PATCH v1 18/43] spi: ep93xx: add DT support for Cirrus EP93xx

[Nikita Shubin]

- find register range from the device tree
- provide clock access via of
- use_dma as a DT node

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---

Notes:
    v0 -> v1:
    - dropped coma in ep93xx_spi_of_ids
    - renamed use_dma to "ep9301,use-dma"

 drivers/spi/spi-ep93xx.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c
index 1615fd22f9a2..bd8049fe9c91 100644
--- a/drivers/spi/spi-ep93xx.c
+++ b/drivers/spi/spi-ep93xx.c
@@ -22,6 +22,7 @@
 #include <linux/bitops.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/scatterlist.h>
@@ -644,6 +645,25 @@ static void ep93xx_spi_release_dma(struct ep93xx_spi *espi)
 		free_page((unsigned long)espi->zeropage);
 }
 
+#ifdef CONFIG_OF
+static struct ep93xx_spi_info dt_spi_info;
+
+static struct ep93xx_spi_info *ep93xx_spi_get_platdata(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	if (np && of_property_read_bool(np, "ep9301,use-dma"))
+		dt_spi_info.use_dma = 1;
+
+	return &dt_spi_info;
+}
+#else
+static struct ep93xx_spi_info *ep93xx_spi_get_platdata(struct platform_device *pdev)
+{
+	return dev_get_platdata(&pdev->dev);
+}
+#endif
+
 static int ep93xx_spi_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
@@ -653,7 +673,7 @@ static int ep93xx_spi_probe(struct platform_device *pdev)
 	int irq;
 	int error;
 
-	info = dev_get_platdata(&pdev->dev);
+	info = ep93xx_spi_get_platdata(pdev);
 	if (!info) {
 		dev_err(&pdev->dev, "missing platform data\n");
 		return -EINVAL;
@@ -726,6 +746,8 @@ static int ep93xx_spi_probe(struct platform_device *pdev)
 	/* make sure that the hardware is disabled */
 	writel(0, espi->mmio + SSPCR1);
 
+	master->dev.of_node = pdev->dev.of_node;
+
 	error = devm_spi_register_master(&pdev->dev, master);
 	if (error) {
 		dev_err(&pdev->dev, "failed to register SPI master\n");
@@ -753,9 +775,16 @@ static void ep93xx_spi_remove(struct platform_device *pdev)
 	ep93xx_spi_release_dma(espi);
 }
 
+static const struct of_device_id ep93xx_spi_of_ids[] = {
+	{ .compatible = "cirrus,ep9301-spi" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ep93xx_spi_of_ids);
+
 static struct platform_driver ep93xx_spi_driver = {
 	.driver		= {
 		.name	= "ep93xx-spi",
+		.of_match_table = ep93xx_spi_of_ids,
 	},
 	.probe		= ep93xx_spi_probe,
 	.remove_new	= ep93xx_spi_remove,
-- 
2.37.4

Re: [PATCH v1 17/43] dt-bindings: spi: Add Cirrus EP93xx

[Krzysztof Kozlowski]

On 01/06/2023 07:34, Nikita Shubin wrote:
> Add YAML bindings for ep93xx SoC SPI.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> 
> Notes:
>     v0 -> v1:
>     Krzysztof Kozlowski:
>     - replaced maintainers
>     - removed wildcards
>     - use fallback compatible and list all possible compatibles
>     - drop quotes in ref
>     - dropped "clock-names"
>     - dropped label
>     - fix ident
> 
>  .../devicetree/bindings/spi/spi-ep9301.yaml   | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-ep9301.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-ep9301.yaml b/Documentation/devicetree/bindings/spi/spi-ep9301.yaml
> new file mode 100644
> index 000000000000..c363b25a3074
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-ep9301.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/spi-ep9301.yaml#

Filename based on compatible, so missing prefix, wrong order of name
components.

This applies everywhere, not to some files only. Applied to all your
bindings.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: EP93xx SoC SPI controller
> +
> +maintainers:
> +  - Alexander Sverdlin <alexander.sverdlin@gmail.com>
> +  - Nikita Shubin <nikita.shubin@maquefel.me>
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +properties:
> +  "#address-cells": true
> +  "#size-cells": true

Drop these two.

> +
> +  compatible:

Anyway, compatible is always first.

> +    oneOf:
> +      - const: cirrus,ep9301-spi
> +      - items:
> +          - enum:
> +              - cirrus,ep9302-spi
> +              - cirrus,ep9307-spi
> +              - cirrus,ep9312-spi
> +              - cirrus,ep9315-spi
> +          - const: cirrus,ep9301-spi
> +
> +  reg:
> +    items:
> +      - description: SPI registers region
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: SPI Controller reference clock source
> +
> +  cs-gpios: true

Drop, not needed.

> +
> +  cirrus,ep9301-use-dma:
> +    description: Flag indicating that the SPI should use dma
> +    type: boolean

In such case where are dmas? Unless you meant some internal dma
controller? In such case extend the description because now it just
duplicates property name.


> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/cirrus,ep93xx-clock.h>
> +    spi@808a0000 {
> +      compatible = "cirrus,ep9301-spi";
> +      reg = <0x808a0000 0x18>;
> +      interrupt-parent = <&vic1>;
> +      interrupts = <21>;
> +      clocks = <&syscon EP93XX_CLK_SPI>;
> +      cs-gpios = <&gpio5 2 0>;

Use proper gpio defines for flags.

> +      cirrus,ep9301-use-dma;
> +    };
> +
> +...

Best regards,
Krzysztof

Re: [PATCH v1 17/43] dt-bindings: spi: Add Cirrus EP93xx

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Thu, Jun 01, 2023 at 08:34:08AM +0300, Nikita Shubin wrote:

> +  cirrus,ep9301-use-dma:
> +    description: Flag indicating that the SPI should use dma
> +    type: boolean

My previous feedback on this property still applies.

Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed.  Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing.  If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.

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

Re: [PATCH v1 17/43] dt-bindings: spi: Add Cirrus EP93xx

[Nikita Shubin]

Hello Mark!

On Thu, 1 Jun 2023 12:17:27 +0100
Mark Brown <broonie@kernel.org> wrote:

> On Thu, Jun 01, 2023 at 08:34:08AM +0300, Nikita Shubin wrote:
> 
> > +  cirrus,ep9301-use-dma:
> > +    description: Flag indicating that the SPI should use dma
> > +    type: boolean  
> 
> My previous feedback on this property still applies.
> 
> Please don't ignore review comments, people are generally making them
> for a reason and are likely to have the same concerns if issues remain
> unaddressed.  Having to repeat the same comments can get repetitive
> and make people question the value of time spent reviewing.  If you
> disagree with the review comments that's fine but you need to reply
> and discuss your concerns so that the reviewer can understand your
> decisions.

Sorry - that was totally unintentional, i was tinkering with spi and
got distracted on other part of this series (it's quite big for me,
first time tinkering with a series more than 5-6 patches).

> > +  cirrus,ep9301-use-dma:

The reason is that ep93xx DMA state is not quite device-tree ready at
this moment, and clients use it with the help of:

https://elixir.bootlin.com/linux/v6.4-rc4/source/include/linux/platform_data/dma-ep93xx.h

I was hoping to slip by without changing much in ep93xx DMA driver, so
i can deal with it later, especially seeing it's having some quirks
like:

https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/dma/ep93xx_dma.c#L471

And edb93xx and bk3 don't set use_dma with SPI for some reason.

I can move "use-dma" to module parameters, if this is acceptable.

Re: [PATCH v1 17/43] dt-bindings: spi: Add Cirrus EP93xx

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

On Thu, Jun 01, 2023 at 03:41:54PM +0300, Nikita Shubin wrote:
> Mark Brown <broonie@kernel.org> wrote:
> > On Thu, Jun 01, 2023 at 08:34:08AM +0300, Nikita Shubin wrote:

> > > +  cirrus,ep9301-use-dma:
> > > +    description: Flag indicating that the SPI should use dma
> > > +    type: boolean  

> > My previous feedback on this property still applies.

> > > +  cirrus,ep9301-use-dma:

> The reason is that ep93xx DMA state is not quite device-tree ready at
> this moment, and clients use it with the help of:

> https://elixir.bootlin.com/linux/v6.4-rc4/source/include/linux/platform_data/dma-ep93xx.h

> I was hoping to slip by without changing much in ep93xx DMA driver, so

You're definign new ABI here, that's not a good thing to do for a
temporary workaround.

> I can move "use-dma" to module parameters, if this is acceptable.

That's less bad.  I guess you could also define the bindings for the DMA
controller so that the properties are there then instead of properly
using the DMA API in the clients just check to see if the DMA properties
are present and then proceed accordingly?

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

Re: [PATCH v1 17/43] dt-bindings: spi: Add Cirrus EP93xx

[Nikita Shubin]

On Thu, 1 Jun 2023 13:55:03 +0100
Mark Brown <broonie@kernel.org> wrote:

> On Thu, Jun 01, 2023 at 03:41:54PM +0300, Nikita Shubin wrote:
> > Mark Brown <broonie@kernel.org> wrote:  
> > > On Thu, Jun 01, 2023 at 08:34:08AM +0300, Nikita Shubin wrote:  
> 
> > > > +  cirrus,ep9301-use-dma:
> > > > +    description: Flag indicating that the SPI should use dma
> > > > +    type: boolean    
> 
> > > My previous feedback on this property still applies.  
> 
> > > > +  cirrus,ep9301-use-dma:  
> 
> > The reason is that ep93xx DMA state is not quite device-tree ready
> > at this moment, and clients use it with the help of:  
> 
> > https://elixir.bootlin.com/linux/v6.4-rc4/source/include/linux/platform_data/dma-ep93xx.h
> >  
> 
> > I was hoping to slip by without changing much in ep93xx DMA driver,
> > so  
> 
> You're definign new ABI here, that's not a good thing to do for a
> temporary workaround.
> 
> > I can move "use-dma" to module parameters, if this is acceptable.  
> 
> That's less bad.  I guess you could also define the bindings for the
> DMA controller so that the properties are there then instead of
> properly using the DMA API in the clients just check to see if the
> DMA properties are present and then proceed accordingly?

This sounds like a way to go. Thank you, Mark!

Re: [PATCH v1 18/43] spi: ep93xx: add DT support for Cirrus EP93xx

[andy.shevchenko]

Thu, Jun 01, 2023 at 08:34:09AM +0300, Nikita Shubin kirjoitti:
> - find register range from the device tree
> - provide clock access via of
> - use_dma as a DT node

...

> +static struct ep93xx_spi_info *ep93xx_spi_get_platdata(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;

> +	if (np && of_property_read_bool(np, "ep9301,use-dma"))

Excessive check for np to be NULL, this is implied by the _read_bool() anyway.

> +		dt_spi_info.use_dma = 1;

> +	return &dt_spi_info;
> +}

...

> +	master->dev.of_node = pdev->dev.of_node;

device_set_node();

-- 
With Best Regards,
Andy Shevchenko