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

[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 24/43] dt-bindings: ata: Add DT bindings ep93xx pata

[Nikita Shubin]

Add YAML bindings ep93xx SoC.

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

diff --git a/Documentation/devicetree/bindings/ata/cirrus,ep93xx-pata.yaml b/Documentation/devicetree/bindings/ata/cirrus,ep93xx-pata.yaml
new file mode 100644
index 000000000000..24ed64cfa6d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/cirrus,ep93xx-pata.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ata/cirrus,ep93xx-pata.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: EP9312 PATA controller driver
+
+maintainers:
+  - Damien Le Moal <damien.lemoal@opensource.wdc.com>
+
+properties:
+  compatible:
+    const: cirrus,ep9312-pata
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    ide: ide@800a0000 {
+        compatible = "cirrus,ep9312-pata";
+        reg = <0x800a0000 0x38>;
+        interrupt-parent = <&vic1>;
+        interrupts = <8>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&ide_default_pins>;
+    };
+
+...
-- 
2.39.2

[PATCH 25/43] pata: cirrus: add DT support for Cirrus EP93xx

[Nikita Shubin]

- find register range from the device tree
- get interrupts from device tree

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

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

 drivers/ata/pata_ep93xx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
index 47845d920075..09c95758389e 100644
--- a/drivers/ata/pata_ep93xx.c
+++ b/drivers/ata/pata_ep93xx.c
@@ -40,6 +40,8 @@
 #include <linux/ata.h>
 #include <linux/libata.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/delay.h>
 #include <linux/dmaengine.h>
 #include <linux/ktime.h>
@@ -1016,9 +1018,16 @@ static int ep93xx_pata_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id ep93xx_pata_of_ids[] = {
+	{ .compatible = "cirrus,ep9312-pata" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ep93xx_pata_of_ids);
+
 static struct platform_driver ep93xx_pata_platform_driver = {
 	.driver = {
 		.name = DRV_NAME,
+		.of_match_table = ep93xx_pata_of_ids,
 	},
 	.probe = ep93xx_pata_probe,
 	.remove = ep93xx_pata_remove,
-- 
2.39.2

Re: [PATCH 24/43] dt-bindings: ata: Add DT bindings ep93xx pata

[Damien Le Moal]

On 4/24/23 21:34, Nikita Shubin wrote:
> Add YAML bindings ep93xx SoC.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  .../bindings/ata/cirrus,ep93xx-pata.yaml      | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/cirrus,ep93xx-pata.yaml
> 
> diff --git a/Documentation/devicetree/bindings/ata/cirrus,ep93xx-pata.yaml b/Documentation/devicetree/bindings/ata/cirrus,ep93xx-pata.yaml
> new file mode 100644
> index 000000000000..24ed64cfa6d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/cirrus,ep93xx-pata.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/cirrus,ep93xx-pata.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: EP9312 PATA controller driver
> +
> +maintainers:
> +  - Damien Le Moal <damien.lemoal@opensource.wdc.com>

Wrong email address. Please use dlemoal@kernel.org as mentioned in the
MAINTAINERS file.

(other schema files need to be changed as well. Will send a patch for that)

> +
> +properties:
> +  compatible:
> +    const: cirrus,ep9312-pata
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ide: ide@800a0000 {
> +        compatible = "cirrus,ep9312-pata";
> +        reg = <0x800a0000 0x38>;
> +        interrupt-parent = <&vic1>;
> +        interrupts = <8>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&ide_default_pins>;
> +    };
> +
> +...

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 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 25/43] dt-bindings: ata: Add Cirrus EP93xx

[Nikita Shubin]

Add YAML bindings for ep93xx SoC PATA.

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

Notes:
    v0 -> v1:
    
    - renamed file to ep9312-pata
    - changed email to dlemoal@kernel.org
    - dropped label
    - fixed ident

 .../bindings/ata/cirrus,ep9312-pata.yaml      | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml

diff --git a/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
new file mode 100644
index 000000000000..3489be55a6fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ata/cirrus,ep9312-pata.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic EP9312 PATA controller
+
+maintainers:
+  - Damien Le Moal <dlemoal@kernel.org>
+
+properties:
+  compatible:
+    oneOf:
+      - const: cirrus,ep9312-pata
+      - items:
+          - const: cirrus,ep9315-pata
+          - const: cirrus,ep9312-pata
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    ide@800a0000 {
+      compatible = "cirrus,ep9312-pata";
+      reg = <0x800a0000 0x38>;
+      interrupt-parent = <&vic1>;
+      interrupts = <8>;
+      pinctrl-names = "default";
+      pinctrl-0 = <&ide_default_pins>;
+    };
+
+...
-- 
2.37.4

[PATCH v1 26/43] pata: cirrus: add DT support for Cirrus EP93xx

[Nikita Shubin]

- find register range from the device tree
- get interrupts from device tree

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

Notes:
    v0 -> v1:
    
    - fixed headers
    - dropped coma in id table

 drivers/ata/pata_ep93xx.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
index c6e043e05d43..8d363bc71342 100644
--- a/drivers/ata/pata_ep93xx.c
+++ b/drivers/ata/pata_ep93xx.c
@@ -40,6 +40,7 @@
 #include <linux/ata.h>
 #include <linux/libata.h>
 #include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
 #include <linux/delay.h>
 #include <linux/dmaengine.h>
 #include <linux/ktime.h>
@@ -1016,9 +1017,16 @@ static int ep93xx_pata_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id ep93xx_pata_of_ids[] = {
+	{ .compatible = "cirrus,ep9312-pata" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ep93xx_pata_of_ids);
+
 static struct platform_driver ep93xx_pata_platform_driver = {
 	.driver = {
 		.name = DRV_NAME,
+		.of_match_table = ep93xx_pata_of_ids,
 	},
 	.probe = ep93xx_pata_probe,
 	.remove = ep93xx_pata_remove,
-- 
2.37.4

[PATCH v1 38/43] pata: cirrus: drop legacy pinctrl

[Nikita Shubin]

Drop legacy acquire/release since we are using
pinctrl for this now.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/ata/pata_ep93xx.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
index 8d363bc71342..88cf31000000 100644
--- a/drivers/ata/pata_ep93xx.c
+++ b/drivers/ata/pata_ep93xx.c
@@ -921,28 +921,18 @@ static int ep93xx_pata_probe(struct platform_device *pdev)
 	void __iomem *ide_base;
 	int err;
 
-	err = ep93xx_ide_acquire_gpio(pdev);
-	if (err)
-		return err;
-
 	/* INT[3] (IRQ_EP93XX_EXT3) line connected as pull down */
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		err = irq;
-		goto err_rel_gpio;
-	}
+	if (irq < 0)
+		return irq;
 
 	ide_base = devm_platform_get_and_ioremap_resource(pdev, 0, &mem_res);
-	if (IS_ERR(ide_base)) {
-		err = PTR_ERR(ide_base);
-		goto err_rel_gpio;
-	}
+	if (IS_ERR(ide_base))
+		return PTR_ERR(ide_base);
 
 	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
-	if (!drv_data) {
-		err = -ENXIO;
-		goto err_rel_gpio;
-	}
+	if (!drv_data)
+		return -ENXIO;
 
 	drv_data->pdev = pdev;
 	drv_data->ide_base = ide_base;
@@ -1000,8 +990,6 @@ static int ep93xx_pata_probe(struct platform_device *pdev)
 
 err_rel_dma:
 	ep93xx_pata_release_dma(drv_data);
-err_rel_gpio:
-	ep93xx_ide_release_gpio(pdev);
 	return err;
 }
 
@@ -1013,7 +1001,6 @@ static int ep93xx_pata_remove(struct platform_device *pdev)
 	ata_host_detach(host);
 	ep93xx_pata_release_dma(drv_data);
 	ep93xx_pata_clear_regs(drv_data->ide_base);
-	ep93xx_ide_release_gpio(pdev);
 	return 0;
 }
 
-- 
2.37.4

Re: [PATCH v1 26/43] pata: cirrus: add DT support for Cirrus EP93xx

[Damien Le Moal]

On 6/1/23 14:45, Nikita Shubin wrote:
> - find register range from the device tree
> - get interrupts from device tree
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

Please change the commit title to:

ata: pata_ep93xx: add device tree support

> ---
> 
> Notes:
>     v0 -> v1:
>     
>     - fixed headers
>     - dropped coma in id table
> 
>  drivers/ata/pata_ep93xx.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> index c6e043e05d43..8d363bc71342 100644
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c
> @@ -40,6 +40,7 @@
>  #include <linux/ata.h>
>  #include <linux/libata.h>
>  #include <linux/platform_device.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/delay.h>
>  #include <linux/dmaengine.h>
>  #include <linux/ktime.h>
> @@ -1016,9 +1017,16 @@ static int ep93xx_pata_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id ep93xx_pata_of_ids[] = {
> +	{ .compatible = "cirrus,ep9312-pata" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ep93xx_pata_of_ids);
> +
>  static struct platform_driver ep93xx_pata_platform_driver = {
>  	.driver = {
>  		.name = DRV_NAME,
> +		.of_match_table = ep93xx_pata_of_ids,
>  	},
>  	.probe = ep93xx_pata_probe,
>  	.remove = ep93xx_pata_remove,

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v1 38/43] pata: cirrus: drop legacy pinctrl

[Damien Le Moal]

On 6/1/23 14:45, Nikita Shubin wrote:
> Drop legacy acquire/release since we are using
> pinctrl for this now.

Please use full 72 chars lines.

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

Please change the commit title to:

ata: pata_ep93xx: remove legacy pinctrl use

Also, are the functions ep93xx_ide_acquire_gpio() & ep93xx_ide_release_gpio()
used somewhere else ? If not, this patch should remove these functions as well.

> ---
>  drivers/ata/pata_ep93xx.c | 25 ++++++-------------------
>  1 file changed, 6 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> index 8d363bc71342..88cf31000000 100644
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c
> @@ -921,28 +921,18 @@ static int ep93xx_pata_probe(struct platform_device *pdev)
>  	void __iomem *ide_base;
>  	int err;
>  
> -	err = ep93xx_ide_acquire_gpio(pdev);
> -	if (err)
> -		return err;
> -
>  	/* INT[3] (IRQ_EP93XX_EXT3) line connected as pull down */
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		err = irq;
> -		goto err_rel_gpio;
> -	}
> +	if (irq < 0)
> +		return irq;
>  
>  	ide_base = devm_platform_get_and_ioremap_resource(pdev, 0, &mem_res);
> -	if (IS_ERR(ide_base)) {
> -		err = PTR_ERR(ide_base);
> -		goto err_rel_gpio;
> -	}
> +	if (IS_ERR(ide_base))
> +		return PTR_ERR(ide_base);
>  
>  	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> -	if (!drv_data) {
> -		err = -ENXIO;
> -		goto err_rel_gpio;
> -	}
> +	if (!drv_data)
> +		return -ENXIO;
>  
>  	drv_data->pdev = pdev;
>  	drv_data->ide_base = ide_base;
> @@ -1000,8 +990,6 @@ static int ep93xx_pata_probe(struct platform_device *pdev)
>  
>  err_rel_dma:
>  	ep93xx_pata_release_dma(drv_data);
> -err_rel_gpio:
> -	ep93xx_ide_release_gpio(pdev);
>  	return err;
>  }
>  
> @@ -1013,7 +1001,6 @@ static int ep93xx_pata_remove(struct platform_device *pdev)
>  	ata_host_detach(host);
>  	ep93xx_pata_release_dma(drv_data);
>  	ep93xx_pata_clear_regs(drv_data->ide_base);
> -	ep93xx_ide_release_gpio(pdev);
>  	return 0;
>  }
>  

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v1 25/43] dt-bindings: ata: Add Cirrus EP93xx

[Damien Le Moal]

On 6/1/23 14:45, Nikita Shubin wrote:
> Add YAML bindings for ep93xx SoC PATA.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> 
> Notes:
>     v0 -> v1:
>     
>     - renamed file to ep9312-pata

Looks OK to me but given that this is both for the cirrus,ep9315-pata and
cirrus,ep9312-pata, wouldn't it be better to name the file
cirrus,ep931x-pata.yaml ?


>     - changed email to dlemoal@kernel.org
>     - dropped label
>     - fixed ident
> 
>  .../bindings/ata/cirrus,ep9312-pata.yaml      | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
> 
> diff --git a/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
> new file mode 100644
> index 000000000000..3489be55a6fe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/cirrus,ep9312-pata.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus Logic EP9312 PATA controller
> +
> +maintainers:
> +  - Damien Le Moal <dlemoal@kernel.org>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: cirrus,ep9312-pata

I am not a DT specialist, but isn't this line superfluous since it is listed in
the items ?

> +      - items:
> +          - const: cirrus,ep9315-pata
> +          - const: cirrus,ep9312-pata
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ide@800a0000 {
> +      compatible = "cirrus,ep9312-pata";
> +      reg = <0x800a0000 0x38>;
> +      interrupt-parent = <&vic1>;
> +      interrupts = <8>;
> +      pinctrl-names = "default";
> +      pinctrl-0 = <&ide_default_pins>;
> +    };
> +
> +...

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v1 38/43] pata: cirrus: drop legacy pinctrl

[andy.shevchenko]

Thu, Jun 01, 2023 at 08:45:43AM +0300, Nikita Shubin kirjoitti:
> Drop legacy acquire/release since we are using
> pinctrl for this now.

...

>  	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> -	if (!drv_data) {
> -		err = -ENXIO;

Wondering why it's not ENOMEM even in the original code?

> -		goto err_rel_gpio;
> -	}
> +	if (!drv_data)
> +		return -ENXIO;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 26/43] pata: cirrus: add DT support for Cirrus EP93xx

[andy.shevchenko]

Thu, Jun 01, 2023 at 08:45:31AM +0300, Nikita Shubin kirjoitti:
> - find register range from the device tree
> - get interrupts from device tree

...

>  #include <linux/ata.h>
>  #include <linux/libata.h>
>  #include <linux/platform_device.h>

> +#include <linux/mod_devicetable.h>

In all patches, can you try to put a new header inclusions to be ordered (to
some extent)?

>  #include <linux/delay.h>
>  #include <linux/dmaengine.h>
>  #include <linux/ktime.h>

With the context above a new one is logically suits this location.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 38/43] pata: cirrus: drop legacy pinctrl

[Damien Le Moal]

On 6/2/23 10:52, andy.shevchenko@gmail.com wrote:
> Thu, Jun 01, 2023 at 08:45:43AM +0300, Nikita Shubin kirjoitti:
>> Drop legacy acquire/release since we are using
>> pinctrl for this now.
> 
> ...
> 
>>  	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
>> -	if (!drv_data) {
>> -		err = -ENXIO;
> 
> Wondering why it's not ENOMEM even in the original code?

Indeed...

> 
>> -		goto err_rel_gpio;
>> -	}
>> +	if (!drv_data)
>> +		return -ENXIO;
> 

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v1 26/43] pata: cirrus: add DT support for Cirrus EP93xx

[Sergey Shtylyov]

Hello!

On 6/1/23 8:45 AM, Nikita Shubin wrote:

> - find register range from the device tree
> - get interrupts from device tree

   Hm... I'm not seeing such changes in your patch?

> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> 
> Notes:
>     v0 -> v1:
>     
>     - fixed headers
>     - dropped coma in id table
> 
>  drivers/ata/pata_ep93xx.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> index c6e043e05d43..8d363bc71342 100644
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c
> @@ -40,6 +40,7 @@
>  #include <linux/ata.h>
>  #include <linux/libata.h>
>  #include <linux/platform_device.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/delay.h>
>  #include <linux/dmaengine.h>
>  #include <linux/ktime.h>
> @@ -1016,9 +1017,16 @@ static int ep93xx_pata_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id ep93xx_pata_of_ids[] = {
> +	{ .compatible = "cirrus,ep9312-pata" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ep93xx_pata_of_ids);
> +
>  static struct platform_driver ep93xx_pata_platform_driver = {
>  	.driver = {
>  		.name = DRV_NAME,
> +		.of_match_table = ep93xx_pata_of_ids,
>  	},
>  	.probe = ep93xx_pata_probe,
>  	.remove = ep93xx_pata_remove,

MBR, Sergey

Re: [PATCH v1 38/43] pata: cirrus: drop legacy pinctrl

[Sergey Shtylyov]

On 6/1/23 8:45 AM, Nikita Shubin wrote:

> Drop legacy acquire/release since we are using
> pinctrl for this now.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>

[...]

MBR, Sergey

Re: [PATCH v1 25/43] dt-bindings: ata: Add Cirrus EP93xx

[Nikita Shubin]

Hello Damien!

On Fri, 2023-06-02 at 08:57 +0900, Damien Le Moal wrote:
> On 6/1/23 14:45, Nikita Shubin wrote:
> > Add YAML bindings for ep93xx SoC PATA.
> > 
> > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> > ---
> > 
> > Notes:
> >     v0 -> v1:
> >     
> >     - renamed file to ep9312-pata
> 
> Looks OK to me but given that this is both for the cirrus,ep9315-pata
> and
> cirrus,ep9312-pata, wouldn't it be better to name the file
> cirrus,ep931x-pata.yaml ?

I was advised against using wildcards by Arnd and Krzysztof.

See 
https://lore.kernel.org/all/c981e048-8925-deba-6916-9199844976b9@linaro.org/

As i understood we should have at least one fallback, in out case it's
"cirrus,ep9312-pata" and one for each SoC variant that supports it.

All other comments acknowledged and agreed.

I will also change 

```
>> +	if (!drv_data)
>> +		return -ENXIO;
```

To ENOMEM, as a part of dt conversion patch in v2.


> 
> 
> >     - changed email to dlemoal@kernel.org
> >     - dropped label
> >     - fixed ident
> > 
> >  .../bindings/ata/cirrus,ep9312-pata.yaml      | 44
> > +++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/ata/cirrus,ep9312-
> > pata.yaml b/Documentation/devicetree/bindings/ata/cirrus,ep9312-
> > pata.yaml
> > new file mode 100644
> > index 000000000000..3489be55a6fe
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
> > @@ -0,0 +1,44 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/ata/cirrus,ep9312-pata.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Cirrus Logic EP9312 PATA controller
> > +
> > +maintainers:
> > +  - Damien Le Moal <dlemoal@kernel.org>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: cirrus,ep9312-pata
> 
> I am not a DT specialist, but isn't this line superfluous since it is
> listed in
> the items ?
> 
> > +      - items:
> > +          - const: cirrus,ep9315-pata
> > +          - const: cirrus,ep9312-pata
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    ide@800a0000 {
> > +      compatible = "cirrus,ep9312-pata";
> > +      reg = <0x800a0000 0x38>;
> > +      interrupt-parent = <&vic1>;
> > +      interrupts = <8>;
> > +      pinctrl-names = "default";
> > +      pinctrl-0 = <&ide_default_pins>;
> > +    };
> > +
> > +...
> 

Re: [PATCH v1 26/43] pata: cirrus: add DT support for Cirrus EP93xx

[Nikita Shubin]

Hello Sergey!

On Fri, 2023-06-02 at 23:03 +0300, Sergey Shtylyov wrote:
> Hello!
> 
> On 6/1/23 8:45 AM, Nikita Shubin wrote:
> 
> > - find register range from the device tree
> > - get interrupts from device tree
> 
>    Hm... I'm not seeing such changes in your patch?

I meant that from now on it will take irq's and register from device
tree:
https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/ata/pata_ep93xx.c#L928

If CONFIG_OF_IRQ is enabled.

You are right

> > get interrupts from device tree

Is confusing.


> 
> > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> > ---
> > 
> > Notes:
> >     v0 -> v1:
> >     
> >     - fixed headers
> >     - dropped coma in id table
> > 
> >  drivers/ata/pata_ep93xx.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> > index c6e043e05d43..8d363bc71342 100644
> > --- a/drivers/ata/pata_ep93xx.c
> > +++ b/drivers/ata/pata_ep93xx.c
> > @@ -40,6 +40,7 @@
> >  #include <linux/ata.h>
> >  #include <linux/libata.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/mod_devicetable.h>
> >  #include <linux/delay.h>
> >  #include <linux/dmaengine.h>
> >  #include <linux/ktime.h>
> > @@ -1016,9 +1017,16 @@ static int ep93xx_pata_remove(struct
> > platform_device *pdev)
> >         return 0;
> >  }
> >  
> > +static const struct of_device_id ep93xx_pata_of_ids[] = {
> > +       { .compatible = "cirrus,ep9312-pata" },
> > +       { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, ep93xx_pata_of_ids);
> > +
> >  static struct platform_driver ep93xx_pata_platform_driver = {
> >         .driver = {
> >                 .name = DRV_NAME,
> > +               .of_match_table = ep93xx_pata_of_ids,
> >         },
> >         .probe = ep93xx_pata_probe,
> >         .remove = ep93xx_pata_remove,
> 
> MBR, Sergey

Re: [PATCH v1 25/43] dt-bindings: ata: Add Cirrus EP93xx

[Rob Herring]

On Fri, Jun 02, 2023 at 08:57:37AM +0900, Damien Le Moal wrote:
> On 6/1/23 14:45, Nikita Shubin wrote:
> > Add YAML bindings for ep93xx SoC PATA.
> > 
> > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> > ---
> > 
> > Notes:
> >     v0 -> v1:
> >     
> >     - renamed file to ep9312-pata
> 
> Looks OK to me but given that this is both for the cirrus,ep9315-pata and
> cirrus,ep9312-pata, wouldn't it be better to name the file
> cirrus,ep931x-pata.yaml ?

cirrus,ep9312-pata makes sense given that is the common fallback.

Wildcards are okay in filenames (only) when there's not a common 
fallback.

> >     - changed email to dlemoal@kernel.org
> >     - dropped label
> >     - fixed ident
> > 
> >  .../bindings/ata/cirrus,ep9312-pata.yaml      | 44 +++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
> > new file mode 100644
> > index 000000000000..3489be55a6fe
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
> > @@ -0,0 +1,44 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/ata/cirrus,ep9312-pata.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Cirrus Logic EP9312 PATA controller
> > +
> > +maintainers:
> > +  - Damien Le Moal <dlemoal@kernel.org>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: cirrus,ep9312-pata
> 
> I am not a DT specialist, but isn't this line superfluous since it is listed in
> the items ?

No, this entry is for ep9312. The next entry is for ep9315 which is 
compatible with ep9312 version. The cirrus,ep9315-pata is there in case 
a distinction (e.g. quirk/errata) needs to be made by the driver 
without having to change the DT.

Rob

> 
> > +      - items:
> > +          - const: cirrus,ep9315-pata
> > +          - const: cirrus,ep9312-pata

Re: [PATCH v1 25/43] dt-bindings: ata: Add Cirrus EP93xx

[Damien Le Moal]

On 6/15/23 04:00, Rob Herring wrote:
> On Fri, Jun 02, 2023 at 08:57:37AM +0900, Damien Le Moal wrote:
>> On 6/1/23 14:45, Nikita Shubin wrote:
>>> Add YAML bindings for ep93xx SoC PATA.
>>>
>>> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
>>> ---
>>>
>>> Notes:
>>>     v0 -> v1:
>>>     
>>>     - renamed file to ep9312-pata
>>
>> Looks OK to me but given that this is both for the cirrus,ep9315-pata and
>> cirrus,ep9312-pata, wouldn't it be better to name the file
>> cirrus,ep931x-pata.yaml ?
> 
> cirrus,ep9312-pata makes sense given that is the common fallback.
> 
> Wildcards are okay in filenames (only) when there's not a common 
> fallback.

Got it.

> 
>>>     - changed email to dlemoal@kernel.org
>>>     - dropped label
>>>     - fixed ident
>>>
>>>  .../bindings/ata/cirrus,ep9312-pata.yaml      | 44 +++++++++++++++++++
>>>  1 file changed, 44 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
>>> new file mode 100644
>>> index 000000000000..3489be55a6fe
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml
>>> @@ -0,0 +1,44 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/ata/cirrus,ep9312-pata.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Cirrus Logic EP9312 PATA controller
>>> +
>>> +maintainers:
>>> +  - Damien Le Moal <dlemoal@kernel.org>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - const: cirrus,ep9312-pata
>>
>> I am not a DT specialist, but isn't this line superfluous since it is listed in
>> the items ?
> 
> No, this entry is for ep9312. The next entry is for ep9315 which is 
> compatible with ep9312 version. The cirrus,ep9315-pata is there in case 
> a distinction (e.g. quirk/errata) needs to be made by the driver 
> without having to change the DT.

Thanks for all the clarification.

Looks good then.

Acked-by: Damien Le Moal <dlemoal@kernel.org>

I can take this patch through the ata tree, but it may be better to have it with
the entire series (arm tree ?)

-- 
Damien Le Moal
Western Digital Research