[PATCH 00/10] mips: ralink: add complete clock and reset driver for mtmips SoCs

[PATCH 00/10] mips: ralink: add complete clock and reset driver for mtmips SoCs

[Sergio Paracuellos]

This patchset is a big effort to properly implement a clock and reset
driver for old ralink SoCs. This allow to properly define clocks in 
device tree and avoid to use fixed-clocks directly from 'arch/mips/ralink'
architecture directory code.

Device tree 'sysc' node will be both clock and reset provider using 
'clock-cells' and 'reset-cells' properties.

The ralink SoCs we are taking about are RT2880, RT3050, RT3052, RT3350,
RT3352, RT3883, RT5350, MT7620, MT7628 and MT7688. Mostly the code in
this new driver has been extracted from 'arch/mips/ralink' and cleanly
put using kernel clock and reset driver APIs. The clock plans for this
SoCs only talks about relation between CPU frequency and BUS frequency.
This relation is different depending on the particular SoC. CPU clock is
derived from XTAL frequencies.

 Depending on the SoC we have the following frequencies:
 * RT2880 SoC:
     - XTAL: 40 MHz.
     - CPU: 250, 266, 280 or 300 MHz.
     - BUS: CPU / 2 MHz.
  * RT3050, RT3052, RT3350:
     - XTAL: 40 MHz.
     - CPU: 320 or 384 MHz.
     - BUS: CPU / 3 MHz.
  * RT3352:
     - XTAL: 40 MHz.
     - CPU: 384 or 400 MHz.
     - BUS: CPU / 3 MHz.
     - PERIPH: 40 MHz.
  * RT3383:
     - XTAL: 40 MHz.
     - CPU: 250, 384, 480 or 500 MHz.
     - BUS: Depends on RAM Type and CPU:
       + RAM DDR2: 125. ELSE 83 MHz.
       + RAM DDR2: 128. ELSE 96 MHz.
       + RAM DDR2: 160. ELSE 120 MHz.
       + RAM DDR2: 166. ELSE 125 MHz.
  * RT5350:
      - XTAL: 40 MHz.
      - CPU: 300, 320 or 360 MHz.
      - BUS: CPU / 3, CPU / 4, CPU / 3 MHz.
      - PERIPH: 40 MHz.
  * MT7628 and MT7688:
     - XTAL: 20 MHz or 40 MHz.
     - CPU: 575 or 580 MHz.
     - BUS: CPU / 3.
     - PCMI2S: 480 MHz.
     - PERIPH: 40 MHz.
  * MT7620:
     - XTAL: 20 MHz or 40 MHz.
     - PLL: XTAL, 480, 600 MHz.
     - CPU: depends on PLL and some mult and dividers.
     - BUS: depends on PLL and some mult and dividers.
     - PERIPH: 40 or XTAL MHz.

MT7620 is a bit more complex deriving CPU clock from a PLL and an bunch of
register reads and predividers. To derive CPU and BUS frequencies in the
MT7620 SoC 'mt7620_calc_rate()' helper is used.
In the case XTAL can have different frequencies and we need a different
clock frequency for peripherals 'periph' clock in introduced.
The rest of the peripherals present in the SoC just follow their parent
frequencies.

I am using 'mtmips' inside for ralink clock driver. This is aligned with
pinctrl series recently merged through pinctrl git tree [0].

Changes have been compile tested for:
- RT2880
- RT3883
- MT7620

Changes have been properly tested in RT5350 SoC based board (ALL5003 board)
resulting in a working platform.

Dts files for these SoCs in-tree except MT7621 are incomplete. We are
planning to align with openWRT files at some point and add extra needed
changes. Hence I am not touching them at all in these series. If this is
a problem, please let me know and I will update them.

Talking about merging this series I'd like all of the patches going through
the MIPS tree if possible.

Thanks in advance for your time.

Best regards,
    Sergio Paracuellos

[0]: https://lore.kernel.org/linux-gpio/e9e6ad87-2db5-9767-ff39-64a302b06185@arinc9.com/T/#t

Sergio Paracuellos (10):
  dt: bindings: clock: add mtmips SoCs clock device tree binding
    documentation
  clk: ralink: add clock and reset driver for MTMIPS SoCs
  mips: ralink: rt288x: remove clock related code
  mips: ralink: rt305x: remove clock related code
  mips: ralink: rt3883: remove clock related code
  mips: ralink: mt7620: remove clock related code
  mips: ralink: remove clock related function prototypes
  mips: ralink: remove reset related code
  mips: ralink: get cpu rate from new driver code
  MAINTAINERS: add Mediatek MTMIPS Clock maintainer

 .../bindings/clock/mtmips-clock.yaml          |  68 ++
 MAINTAINERS                                   |   6 +
 arch/mips/include/asm/mach-ralink/mt7620.h    |  35 -
 arch/mips/include/asm/mach-ralink/rt288x.h    |  10 -
 arch/mips/include/asm/mach-ralink/rt305x.h    |  21 -
 arch/mips/include/asm/mach-ralink/rt3883.h    |   8 -
 arch/mips/ralink/clk.c                        |  26 +-
 arch/mips/ralink/common.h                     |   5 -
 arch/mips/ralink/mt7620.c                     | 226 ----
 arch/mips/ralink/of.c                         |   4 -
 arch/mips/ralink/reset.c                      |  61 --
 arch/mips/ralink/rt288x.c                     |  31 -
 arch/mips/ralink/rt305x.c                     |  78 --
 arch/mips/ralink/rt3883.c                     |  44 -
 drivers/clk/ralink/Kconfig                    |   7 +
 drivers/clk/ralink/Makefile                   |   1 +
 drivers/clk/ralink/clk-mtmips.c               | 985 ++++++++++++++++++
 17 files changed, 1086 insertions(+), 530 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml
 create mode 100644 drivers/clk/ralink/clk-mtmips.c

-- 
2.25.1

[PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Sergio Paracuellos]

Adds device tree binding documentation for clocks and resets in the
Mediatek MIPS and Ralink SOCs. This covers RT2880, RT3050, RT3052, RT3350,
RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../bindings/clock/mtmips-clock.yaml          | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/mtmips-clock.yaml b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
new file mode 100644
index 000000000000..c92969ce231d
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/mtmips-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MTMIPS SoCs Clock
+
+maintainers:
+  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
+
+description: |
+  MediaTek MIPS and Ralink SoCs have an XTAL from where the cpu clock is
+  provided as well as derived clocks for the bus and the peripherals.
+
+  Each clock is assigned an identifier and client nodes use this identifier
+  to specify the clock which they consume.
+
+  The clocks are provided inside a system controller node.
+
+  This node is also a reset provider for all the peripherals.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - ralink,rt2880-sysc
+          - ralink,rt3050-sysc
+          - ralink,rt3052-sysc
+          - ralink,rt3352-sysc
+          - ralink,rt3883-sysc
+          - ralink,rt5350-sysc
+          - ralink,mt7620-sysc
+          - ralink,mt7620a-sysc
+          - ralink,mt7628-sysc
+          - ralink,mt7688-sysc
+          - ralink,rt2880-reset
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    description:
+      The first cell indicates the clock number.
+    const: 1
+
+  '#reset-cells':
+    description:
+      The first cell indicates the reset bit within the register.
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    sysc: sysc@0 {
+      compatible = "ralink,rt5350-sysc", "syscon";
+      reg = <0x0 0x100>;
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+    };
-- 
2.25.1

[PATCH 02/10] clk: ralink: add clock and reset driver for MTMIPS SoCs

[Sergio Paracuellos]

Until now, clock related code for old ralink SoCs was based in fixed clocks
using 'clk_register_fixed_rate' and 'clkdev_create' directly doing in code
and not using device tree at all for their definition. Including this driver
is an effort to be able to define proper clocks using device tree and also
cleaning all the clock and reset related code from 'arch/mips/ralink' dir.
This clock and reset driver covers all the ralink SoCs but MT7621 which is
the newest and provides gating and some differences that make it different
from its predecesors. It has its own driver since some time ago. The ralink
SoCs we are taking about are RT2880, RT3050, RT3052, RT3350, RT3352, RT3883,
RT5350, MT7620, MT7628 and MT7688. Mostly the code in this new driver has
been extracted from 'arch/mips/ralink' and cleanly put using kernel clock
driver APIs. The clock plans for this SoCs only talks about relation between
CPU frequency and BUS frequency. This relation is different depending on the
particular SoC. CPU clock is derived from XTAL frequencies.

Depending on the SoC we have the following frequencies:
* RT2880 SoC:
    - XTAL: 40 MHz.
    - CPU: 250, 266, 280 or 300 MHz.
    - BUS: CPU / 2 MHz.
* RT3050, RT3052, RT3350:
    - XTAL: 40 MHz.
    - CPU: 320 or 384 MHz.
    - BUS: CPU / 3 MHz.
* RT3352:
    - XTAL: 40 MHz.
    - CPU: 384 or 400 MHz.
    - BUS: CPU / 3 MHz.
    - PERIPH: 40 MHz.
* RT3383:
    - XTAL: 40 MHz.
    - CPU: 250, 384, 480 or 500 MHz.
    - BUS: Depends on RAM Type and CPU:
        + RAM DDR2: 125. ELSE 83 MHz.
        + RAM DDR2: 128. ELSE 96 MHz.
        + RAM DDR2: 160. ELSE 120 MHz.
        + RAM DDR2: 166. ELSE 125 MHz.
* RT5350:
    - XTAL: 40 MHz.
    - CPU: 300, 320 or 360 MHz.
    - BUS: CPU / 3, CPU / 4, CPU / 3 MHz.
    - PERIPH: 40 MHz.
* MT7628 and MT7688:
    - XTAL: 20 MHz or 40 MHz.
    - CPU: 575 or 580 MHz.
    - BUS: CPU / 3.
    - PCMI2S: 480 MHz.
    - PERIPH: 40 MHz.
* MT7620:
    - XTAL: 20 MHz or 40 MHz.
    - PLL: XTAL, 480, 600 MHz.
    - CPU: depends on PLL and some mult and dividers.
    - BUS: depends on PLL and some mult and dividers.
    - PERIPH: 40 or XTAL MHz.

MT7620 is a bit more complex deriving CPU clock from a PLL and an bunch of
register reads and predividers. To derive CPU and BUS frequencies in the
MT7620 SoC 'mt7620_calc_rate()' helper is used.

In the case XTAL can have different frequencies and we need a different
clock frequency for peripherals 'periph' clock in introduced.

The rest of the peripherals present in the SoC just follow their parent
frequencies.

With this information the clk driver will provide all the clock and reset
functionality from a set of hardcoded clocks allowing to define a nice
device tree without fixed clocks.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/clk/ralink/Kconfig      |   7 +
 drivers/clk/ralink/Makefile     |   1 +
 drivers/clk/ralink/clk-mtmips.c | 985 ++++++++++++++++++++++++++++++++
 3 files changed, 993 insertions(+)
 create mode 100644 drivers/clk/ralink/clk-mtmips.c

diff --git a/drivers/clk/ralink/Kconfig b/drivers/clk/ralink/Kconfig
index 6580d5edc676..7c4f335864a8 100644
--- a/drivers/clk/ralink/Kconfig
+++ b/drivers/clk/ralink/Kconfig
@@ -9,3 +9,10 @@ config CLK_MT7621
 	select MFD_SYSCON
 	help
 	  This driver supports MediaTek MT7621 basic clocks.
+
+config CLK_MTMIPS
+	bool "Clock driver for MTMIPS SoCs"
+	depends on SOC_RT305X || SOC_RT288X || SOC_RT3883 || SOC_MT7620 || COMPILE_TEST
+	select MFD_SYSCON
+	help
+	  This driver supports MTMIPS basic clocks.
diff --git a/drivers/clk/ralink/Makefile b/drivers/clk/ralink/Makefile
index cf6f9216379d..398c1bf8cbc1 100644
--- a/drivers/clk/ralink/Makefile
+++ b/drivers/clk/ralink/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_CLK_MT7621) += clk-mt7621.o
+obj-$(CONFIG_CLK_MTMIPS) += clk-mtmips.o
diff --git a/drivers/clk/ralink/clk-mtmips.c b/drivers/clk/ralink/clk-mtmips.c
new file mode 100644
index 000000000000..6b4b5ae9384d
--- /dev/null
+++ b/drivers/clk/ralink/clk-mtmips.c
@@ -0,0 +1,985 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MTMIPS SoCs Clock Driver
+ * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+
+/* Configuration registers */
+#define SYSC_REG_SYSTEM_CONFIG		0x10
+#define SYSC_REG_CLKCFG0		0x2c
+#define SYSC_REG_RESET_CTRL		0x34
+#define SYSC_REG_CPU_SYS_CLKCFG		0x3c
+#define SYSC_REG_CPLL_CONFIG0		0x54
+#define SYSC_REG_CPLL_CONFIG1		0x58
+
+/* RT2880 SoC */
+#define RT2880_CONFIG_CPUCLK_SHIFT	20
+#define RT2880_CONFIG_CPUCLK_MASK	0x3
+#define RT2880_CONFIG_CPUCLK_250	0x0
+#define RT2880_CONFIG_CPUCLK_266	0x1
+#define RT2880_CONFIG_CPUCLK_280	0x2
+#define RT2880_CONFIG_CPUCLK_300	0x3
+
+/* RT305X SoC */
+#define RT305X_SYSCFG_CPUCLK_SHIFT	18
+#define RT305X_SYSCFG_CPUCLK_MASK	0x1
+#define RT305X_SYSCFG_CPUCLK_LOW	0x0
+#define RT305X_SYSCFG_CPUCLK_HIGH	0x1
+
+/* RT3352 SoC */
+#define RT3352_SYSCFG0_CPUCLK_SHIFT	8
+#define RT3352_SYSCFG0_CPUCLK_MASK	0x1
+#define RT3352_SYSCFG0_CPUCLK_LOW	0x0
+#define RT3352_SYSCFG0_CPUCLK_HIGH	0x1
+
+/* RT3383 SoC */
+#define RT3883_SYSCFG0_DRAM_TYPE_DDR2	BIT(17)
+#define RT3883_SYSCFG0_CPUCLK_SHIFT	8
+#define RT3883_SYSCFG0_CPUCLK_MASK	0x3
+#define RT3883_SYSCFG0_CPUCLK_250	0x0
+#define RT3883_SYSCFG0_CPUCLK_384	0x1
+#define RT3883_SYSCFG0_CPUCLK_480	0x2
+#define RT3883_SYSCFG0_CPUCLK_500	0x3
+
+/* RT5350 SoC */
+#define RT5350_CLKCFG0_XTAL_SEL		BIT(20)
+#define RT5350_SYSCFG0_CPUCLK_SHIFT	8
+#define RT5350_SYSCFG0_CPUCLK_MASK	0x3
+#define RT5350_SYSCFG0_CPUCLK_360	0x0
+#define RT5350_SYSCFG0_CPUCLK_320	0x2
+#define RT5350_SYSCFG0_CPUCLK_300	0x3
+
+/* MT7620 and MT76x8 SoCs */
+#define MT7620_XTAL_FREQ_SEL		BIT(6)
+#define CPLL_CFG0_SW_CFG		BIT(31)
+#define CPLL_CFG0_PLL_MULT_RATIO_SHIFT	16
+#define CPLL_CFG0_PLL_MULT_RATIO_MASK   0x7
+#define CPLL_CFG0_LC_CURFCK		BIT(15)
+#define CPLL_CFG0_BYPASS_REF_CLK	BIT(14)
+#define CPLL_CFG0_PLL_DIV_RATIO_SHIFT	10
+#define CPLL_CFG0_PLL_DIV_RATIO_MASK	0x3
+#define CPLL_CFG1_CPU_AUX1		BIT(25)
+#define CPLL_CFG1_CPU_AUX0		BIT(24)
+#define CLKCFG0_PERI_CLK_SEL		BIT(4)
+#define CPU_SYS_CLKCFG_OCP_RATIO_SHIFT	16
+#define CPU_SYS_CLKCFG_OCP_RATIO_MASK	0xf
+#define CPU_SYS_CLKCFG_OCP_RATIO_1	0	/* 1:1   (Reserved) */
+#define CPU_SYS_CLKCFG_OCP_RATIO_1_5	1	/* 1:1.5 (Reserved) */
+#define CPU_SYS_CLKCFG_OCP_RATIO_2	2	/* 1:2   */
+#define CPU_SYS_CLKCFG_OCP_RATIO_2_5	3       /* 1:2.5 (Reserved) */
+#define CPU_SYS_CLKCFG_OCP_RATIO_3	4	/* 1:3   */
+#define CPU_SYS_CLKCFG_OCP_RATIO_3_5	5	/* 1:3.5 (Reserved) */
+#define CPU_SYS_CLKCFG_OCP_RATIO_4	6	/* 1:4   */
+#define CPU_SYS_CLKCFG_OCP_RATIO_5	7	/* 1:5   */
+#define CPU_SYS_CLKCFG_OCP_RATIO_10	8	/* 1:10  */
+#define CPU_SYS_CLKCFG_CPU_FDIV_SHIFT	8
+#define CPU_SYS_CLKCFG_CPU_FDIV_MASK	0x1f
+#define CPU_SYS_CLKCFG_CPU_FFRAC_SHIFT	0
+#define CPU_SYS_CLKCFG_CPU_FFRAC_MASK	0x1f
+
+/* clock scaling */
+#define CLKCFG_FDIV_MASK		0x1f00
+#define CLKCFG_FDIV_USB_VAL		0x0300
+#define CLKCFG_FFRAC_MASK		0x001f
+#define CLKCFG_FFRAC_USB_VAL		0x0003
+
+struct mtmips_clk;
+
+struct mtmips_clk_data {
+	struct mtmips_clk *clk_base;
+	size_t num_clk_base;
+	struct mtmips_clk *clk_periph;
+	size_t num_clk_periph;
+};
+
+struct mtmips_clk_priv {
+	struct regmap *sysc;
+	const struct mtmips_clk_data *data;
+};
+
+struct mtmips_clk {
+	struct clk_hw hw;
+	struct mtmips_clk_priv *priv;
+};
+
+static unsigned long mtmips_pherip_clk_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	return parent_rate;
+}
+
+/*
+ * There are drivers for these SoCs that are older than clock driver
+ * and are not prepared for the clock. We don't want the kernel to
+ * disable anything so we add CLK_IS_CRITICAL flag here.
+ */
+#define CLK_PERIPH(_name, _parent) {				\
+	.init = &(struct clk_init_data) {			\
+		.name = _name,					\
+		.ops = &(const struct clk_ops) {                \
+			.recalc_rate = mtmips_pherip_clk_rate	\
+		},						\
+		.parent_data = &(const struct clk_parent_data) {\
+			.name = _parent,			\
+			.fw_name = _parent			\
+		},						\
+		.num_parents = 1,				\
+		.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL	\
+	},							\
+}
+
+static struct mtmips_clk rt2880_pherip_clks[] = {
+	{ CLK_PERIPH("300100.timer", "bus") },
+	{ CLK_PERIPH("300120.watchdog", "bus") },
+	{ CLK_PERIPH("300500.uart", "bus") },
+	{ CLK_PERIPH("300900.i2c", "bus") },
+	{ CLK_PERIPH("300c00.uartlite", "bus") },
+	{ CLK_PERIPH("400000.ethernet", "bus") },
+	{ CLK_PERIPH("480000.wmac", "xtal") }
+};
+
+static struct mtmips_clk rt305x_pherip_clks[] = {
+	{ CLK_PERIPH("10000900.i2c", "bus") },
+	{ CLK_PERIPH("10000a00.i2s", "bus") },
+	{ CLK_PERIPH("10000b00.spi", "bus") },
+	{ CLK_PERIPH("10000b40.spi", "bus") },
+	{ CLK_PERIPH("10000100.timer", "bus") },
+	{ CLK_PERIPH("10000120.watchdog", "bus") },
+	{ CLK_PERIPH("10000500.uart", "bus") },
+	{ CLK_PERIPH("10000c00.uartlite", "bus") },
+	{ CLK_PERIPH("10100000.ethernet", "bus") },
+	{ CLK_PERIPH("10180000.wmac", "xtal") }
+};
+
+static struct mtmips_clk rt5350_pherip_clks[] = {
+	{ CLK_PERIPH("10000900.i2c", "periph") },
+	{ CLK_PERIPH("10000a00.i2s", "periph") },
+	{ CLK_PERIPH("10000b00.spi", "bus") },
+	{ CLK_PERIPH("10000b40.spi", "bus") },
+	{ CLK_PERIPH("10000100.timer", "bus") },
+	{ CLK_PERIPH("10000120.watchdog", "bus") },
+	{ CLK_PERIPH("10000500.uart", "periph") },
+	{ CLK_PERIPH("10000c00.uartlite", "periph") },
+	{ CLK_PERIPH("10100000.ethernet", "bus") },
+	{ CLK_PERIPH("10180000.wmac", "xtal") }
+};
+
+static struct mtmips_clk mt7620_pherip_clks[] = {
+	{ CLK_PERIPH("10000100.timer", "periph") },
+	{ CLK_PERIPH("10000120.watchdog", "periph") },
+	{ CLK_PERIPH("10000900.i2c", "periph") },
+	{ CLK_PERIPH("10000a00.i2s", "periph") },
+	{ CLK_PERIPH("10000b00.spi", "bus") },
+	{ CLK_PERIPH("10000b40.spi", "bus") },
+	{ CLK_PERIPH("10000c00.uartlite", "periph") },
+	{ CLK_PERIPH("10000d00.uart1", "periph") },
+	{ CLK_PERIPH("10000e00.uart2", "periph") },
+	{ CLK_PERIPH("10180000.wmac", "xtal") }
+};
+
+static struct mtmips_clk mt76x8_pherip_clks[] = {
+	{ CLK_PERIPH("10000100.timer", "periph") },
+	{ CLK_PERIPH("10000120.watchdog", "periph") },
+	{ CLK_PERIPH("10000900.i2c", "periph") },
+	{ CLK_PERIPH("10000a00.i2s", "pcmi2s") },
+	{ CLK_PERIPH("10000b00.spi", "bus") },
+	{ CLK_PERIPH("10000b40.spi", "bus") },
+	{ CLK_PERIPH("10000c00.uartlite", "periph") },
+	{ CLK_PERIPH("10000d00.uartlite", "periph") },
+	{ CLK_PERIPH("10000e00.uartlite", "periph") },
+	{ CLK_PERIPH("10000d00.uart1", "periph") },
+	{ CLK_PERIPH("10000e00.uart2", "periph") },
+	{ CLK_PERIPH("10180000.wmac", "xtal") }
+};
+
+static int mtmips_register_pherip_clocks(struct device_node *np,
+					 struct clk_hw_onecell_data *clk_data,
+					 struct mtmips_clk_priv *priv)
+{
+	struct clk_hw **hws = clk_data->hws;
+	struct mtmips_clk *sclk;
+	int ret, i;
+
+	for (i = 0; i < priv->data->num_clk_periph; i++) {
+		int idx = (priv->data->num_clk_base - 1) + i;
+
+		sclk = &priv->data->clk_periph[i];
+		ret = of_clk_hw_register(np, &sclk->hw);
+		if (ret) {
+			pr_err("Couldn't register peripheral clock %d\n", idx);
+			goto err_clk_unreg;
+		}
+
+		hws[idx] = &sclk->hw;
+	}
+
+	return 0;
+
+err_clk_unreg:
+	while (--i >= 0) {
+		sclk = &priv->data->clk_periph[i];
+		clk_hw_unregister(&sclk->hw);
+	}
+	return ret;
+}
+
+static inline struct mtmips_clk *to_mtmips_clk(struct clk_hw *hw)
+{
+	return container_of(hw, struct mtmips_clk, hw);
+}
+
+static unsigned long rt5350_xtal_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 val;
+
+	regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG, &val);
+	if (!(val & RT5350_CLKCFG0_XTAL_SEL))
+		return 20000000;
+
+	return 40000000;
+}
+
+static unsigned long rt5350_cpu_recalc_rate(struct clk_hw *hw,
+					    unsigned long xtal_clk)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 t;
+
+	regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG, &t);
+	t = (t >> RT5350_SYSCFG0_CPUCLK_SHIFT) & RT5350_SYSCFG0_CPUCLK_MASK;
+
+	switch (t) {
+	case RT5350_SYSCFG0_CPUCLK_360:
+		return 360000000;
+	case RT5350_SYSCFG0_CPUCLK_320:
+		return 320000000;
+	case RT5350_SYSCFG0_CPUCLK_300:
+		return 300000000;
+	default:
+		BUG();
+	}
+}
+
+static unsigned long rt5350_bus_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	if (parent_rate == 320000000)
+		return parent_rate / 4;
+
+	return parent_rate / 3;
+}
+
+static unsigned long rt3352_cpu_recalc_rate(struct clk_hw *hw,
+					    unsigned long xtal_clk)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 t;
+
+	regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG, &t);
+	t = (t >> RT3352_SYSCFG0_CPUCLK_SHIFT) & RT3352_SYSCFG0_CPUCLK_MASK;
+
+	switch (t) {
+	case RT3352_SYSCFG0_CPUCLK_LOW:
+		return 384000000;
+	case RT3352_SYSCFG0_CPUCLK_HIGH:
+		return 400000000;
+	default:
+		BUG();
+	}
+}
+
+static unsigned long rt3352_periph_recalc_rate(struct clk_hw *hw,
+					       unsigned long parent_rate)
+{
+	return 40000000;
+}
+
+static unsigned long rt3352_bus_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	return parent_rate / 3;
+}
+
+static unsigned long rt305x_xtal_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	return 40000000;
+}
+
+static unsigned long rt305x_cpu_recalc_rate(struct clk_hw *hw,
+					    unsigned long xtal_clk)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 t;
+
+	regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG, &t);
+	t = (t >> RT305X_SYSCFG_CPUCLK_SHIFT) & RT305X_SYSCFG_CPUCLK_MASK;
+
+	switch (t) {
+	case RT305X_SYSCFG_CPUCLK_LOW:
+		return 320000000;
+	case RT305X_SYSCFG_CPUCLK_HIGH:
+		return 384000000;
+	default:
+		BUG();
+	}
+}
+
+static unsigned long rt3883_cpu_recalc_rate(struct clk_hw *hw,
+					    unsigned long xtal_clk)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 t;
+
+	regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG, &t);
+	t = (t >> RT3883_SYSCFG0_CPUCLK_SHIFT) & RT3883_SYSCFG0_CPUCLK_MASK;
+
+	switch (t) {
+	case RT3883_SYSCFG0_CPUCLK_250:
+		return 250000000;
+	case RT3883_SYSCFG0_CPUCLK_384:
+		return 384000000;
+	case RT3883_SYSCFG0_CPUCLK_480:
+		return 480000000;
+	case RT3883_SYSCFG0_CPUCLK_500:
+		return 500000000;
+	default:
+		BUG();
+	}
+}
+
+static unsigned long rt3883_bus_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 ddr2;
+	u32 t;
+
+	regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG, &t);
+	ddr2 = t & RT3883_SYSCFG0_DRAM_TYPE_DDR2;
+
+	switch (parent_rate) {
+	case 250000000:
+		return (ddr2) ? 125000000 : 83000000;
+	case 384000000:
+		return (ddr2) ? 128000000 : 96000000;
+	case 480000000:
+		return (ddr2) ? 160000000 : 120000000;
+	case 500000000:
+		return (ddr2) ? 166000000 : 125000000;
+	default:
+		BUG();
+	}
+}
+
+static unsigned long rt2880_cpu_recalc_rate(struct clk_hw *hw,
+					    unsigned long xtal_clk)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 t;
+
+	regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG, &t);
+	t = (t >> RT2880_CONFIG_CPUCLK_SHIFT) & RT2880_CONFIG_CPUCLK_MASK;
+
+	switch (t) {
+	case RT2880_CONFIG_CPUCLK_250:
+		return 250000000;
+	case RT2880_CONFIG_CPUCLK_266:
+		return 266000000;
+	case RT2880_CONFIG_CPUCLK_280:
+		return 280000000;
+	case RT2880_CONFIG_CPUCLK_300:
+		return 300000000;
+	default:
+		BUG();
+	}
+}
+
+static unsigned long rt2880_bus_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	return parent_rate / 2;
+}
+
+static u32 mt7620_calc_rate(u32 ref_rate, u32 mul, u32 div)
+{
+	u64 t;
+
+	t = ref_rate;
+	t *= mul;
+	do_div(t, div);
+
+	return t;
+}
+
+static unsigned long mt7620_pll_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	static const u32 clk_divider[] = { 2, 3, 4, 8 };
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	unsigned long cpu_pll;
+	u32 t;
+	u32 mul;
+	u32 div;
+
+	regmap_read(sysc, SYSC_REG_CPLL_CONFIG0, &t);
+	if (t & CPLL_CFG0_BYPASS_REF_CLK) {
+		cpu_pll = parent_rate;
+	} else if ((t & CPLL_CFG0_SW_CFG) == 0) {
+		cpu_pll = 600000000;
+	} else {
+		mul = (t >> CPLL_CFG0_PLL_MULT_RATIO_SHIFT) &
+			CPLL_CFG0_PLL_MULT_RATIO_MASK;
+		mul += 24;
+		if (t & CPLL_CFG0_LC_CURFCK)
+			mul *= 2;
+
+		div = (t >> CPLL_CFG0_PLL_DIV_RATIO_SHIFT) &
+			CPLL_CFG0_PLL_DIV_RATIO_MASK;
+
+		WARN_ON(div >= ARRAY_SIZE(clk_divider));
+
+		cpu_pll = mt7620_calc_rate(parent_rate, mul, clk_divider[div]);
+	}
+
+	regmap_read(sysc, SYSC_REG_CPLL_CONFIG1, &t);
+	if (t & CPLL_CFG1_CPU_AUX1)
+		return parent_rate;
+
+	if (t & CPLL_CFG1_CPU_AUX0)
+		return 480000000;
+
+	return cpu_pll;
+}
+
+static unsigned long mt7620_cpu_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 t;
+	u32 mul;
+	u32 div;
+
+	regmap_read(sysc, SYSC_REG_CPU_SYS_CLKCFG, &t);
+	mul = t & CPU_SYS_CLKCFG_CPU_FFRAC_MASK;
+	div = (t >> CPU_SYS_CLKCFG_CPU_FDIV_SHIFT) &
+		CPU_SYS_CLKCFG_CPU_FDIV_MASK;
+
+	return mt7620_calc_rate(parent_rate, mul, div);
+}
+
+static unsigned long mt7620_bus_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	static const u32 ocp_dividers[16] = {
+		[CPU_SYS_CLKCFG_OCP_RATIO_2] = 2,
+		[CPU_SYS_CLKCFG_OCP_RATIO_3] = 3,
+		[CPU_SYS_CLKCFG_OCP_RATIO_4] = 4,
+		[CPU_SYS_CLKCFG_OCP_RATIO_5] = 5,
+		[CPU_SYS_CLKCFG_OCP_RATIO_10] = 10,
+	};
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 t;
+	u32 ocp_ratio;
+	u32 div;
+
+	if (IS_ENABLED(CONFIG_USB)) {
+		/*
+		* When the CPU goes into sleep mode, the BUS
+		* clock will be too low for USB to function properly.
+		* Adjust the busses fractional divider to fix this
+		*/
+		regmap_read(sysc, SYSC_REG_CPU_SYS_CLKCFG, &t);
+		t &= ~(CLKCFG_FDIV_MASK | CLKCFG_FFRAC_MASK);
+		t |= CLKCFG_FDIV_USB_VAL | CLKCFG_FFRAC_USB_VAL;
+		regmap_write(sysc, SYSC_REG_CPU_SYS_CLKCFG, t);
+	}
+
+	regmap_read(sysc, SYSC_REG_CPU_SYS_CLKCFG, &t);
+	ocp_ratio = (t >> CPU_SYS_CLKCFG_OCP_RATIO_SHIFT) &
+		CPU_SYS_CLKCFG_OCP_RATIO_MASK;
+
+	if (WARN_ON(ocp_ratio >= ARRAY_SIZE(ocp_dividers)))
+		return parent_rate;
+
+	div = ocp_dividers[ocp_ratio];
+	if (WARN(!div, "invalid divider for OCP ratio %u", ocp_ratio))
+		return parent_rate;
+
+	return parent_rate / div;
+}
+
+static unsigned long mt7620_periph_recalc_rate(struct clk_hw *hw,
+					       unsigned long parent_rate)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 t;
+
+	regmap_read(sysc, SYSC_REG_CLKCFG0, &t);
+	if (t & CLKCFG0_PERI_CLK_SEL)
+		return parent_rate;
+
+	return 40000000;
+}
+
+static unsigned long mt76x8_xtal_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	struct mtmips_clk *clk = to_mtmips_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 t;
+
+	regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG, &t);
+	if (t & MT7620_XTAL_FREQ_SEL)
+		return 40000000;
+
+	return 20000000;
+}
+
+static unsigned long mt76x8_cpu_recalc_rate(struct clk_hw *hw,
+					    unsigned long xtal_clk)
+{
+	if (xtal_clk == 40000000)
+		return 580000000;
+
+	return 575000000;
+}
+
+static unsigned long mt76x8_pcmi2s_recalc_rate(struct clk_hw *hw,
+					       unsigned long xtal_clk)
+{
+	return 480000000;
+}
+
+#define CLK_BASE(_name, _parent, _recalc) {				\
+	.init = &(struct clk_init_data) {				\
+		.name = _name,						\
+		.ops = &(const struct clk_ops) {			\
+			.recalc_rate = _recalc,				\
+		},							\
+		.parent_data = &(const struct clk_parent_data) {	\
+			.name = _parent,				\
+			.fw_name = _parent				\
+		},							\
+		.num_parents = _parent ? 1 : 0				\
+	},								\
+}
+
+static struct mtmips_clk rt2880_clks_base[] = {
+	{ CLK_BASE("xtal", NULL, rt305x_xtal_recalc_rate) },
+	{ CLK_BASE("cpu", "xtal", rt2880_cpu_recalc_rate) },
+	{ CLK_BASE("bus", "cpu", rt2880_bus_recalc_rate) }
+};
+
+static struct mtmips_clk rt305x_clks_base[] = {
+	{ CLK_BASE("xtal", NULL, rt305x_xtal_recalc_rate) },
+	{ CLK_BASE("cpu", "xtal", rt305x_cpu_recalc_rate) },
+	{ CLK_BASE("bus", "cpu", rt3352_bus_recalc_rate) }
+};
+
+static struct mtmips_clk rt3352_clks_base[] = {
+	{ CLK_BASE("xtal", NULL, rt5350_xtal_recalc_rate) },
+	{ CLK_BASE("cpu", "xtal", rt3352_cpu_recalc_rate) },
+	{ CLK_BASE("periph", "xtal", rt3352_periph_recalc_rate) },
+	{ CLK_BASE("bus", "cpu", rt3352_bus_recalc_rate) }
+};
+
+static struct mtmips_clk rt3883_clks_base[] = {
+	{ CLK_BASE("xtal", NULL, rt305x_xtal_recalc_rate) },
+	{ CLK_BASE("cpu", "xtal", rt3883_cpu_recalc_rate) },
+	{ CLK_BASE("bus", "cpu", rt3883_bus_recalc_rate) }
+};
+
+static struct mtmips_clk rt5350_clks_base[] = {
+	{ CLK_BASE("xtal", NULL, rt5350_xtal_recalc_rate) },
+	{ CLK_BASE("cpu", "xtal", rt5350_cpu_recalc_rate) },
+	{ CLK_BASE("periph", "xtal", rt3352_periph_recalc_rate) },
+	{ CLK_BASE("bus", "cpu", rt5350_bus_recalc_rate) }
+};
+
+static struct mtmips_clk mt7620_clks_base[] = {
+	{ CLK_BASE("xtal", NULL, mt76x8_xtal_recalc_rate) },
+	{ CLK_BASE("pll", "xtal", mt7620_pll_recalc_rate) },
+	{ CLK_BASE("cpu", "pll", mt7620_cpu_recalc_rate) },
+	{ CLK_BASE("periph", "xtal", mt7620_periph_recalc_rate) },
+	{ CLK_BASE("bus", "cpu", mt7620_bus_recalc_rate) }
+};
+
+static struct mtmips_clk mt76x8_clks_base[] = {
+	{ CLK_BASE("xtal", NULL, mt76x8_xtal_recalc_rate) },
+	{ CLK_BASE("cpu", "xtal", mt76x8_cpu_recalc_rate) },
+	{ CLK_BASE("periph", "xtal", rt3352_periph_recalc_rate) },
+	{ CLK_BASE("pcmi2s", "xtal", mt76x8_pcmi2s_recalc_rate) },
+	{ CLK_BASE("bus", "cpu", rt3352_bus_recalc_rate) }
+};
+
+static int mtmips_register_clocks(struct device_node *np,
+				  struct clk_hw_onecell_data *clk_data,
+				  struct mtmips_clk_priv *priv)
+{
+	struct clk_hw **hws = clk_data->hws;
+	struct mtmips_clk *sclk;
+	int ret, i;
+
+	for (i = 0; i < priv->data->num_clk_base; i++) {
+		sclk = &priv->data->clk_base[i];
+		sclk->priv = priv;
+		ret = of_clk_hw_register(np, &sclk->hw);
+		if (ret) {
+			pr_err("Couldn't register top clock %i\n", i);
+			goto err_clk_unreg;
+		}
+
+		hws[i] = &sclk->hw;
+	}
+
+	return 0;
+
+err_clk_unreg:
+	while (--i >= 0) {
+		sclk = &priv->data->clk_base[i];
+		clk_hw_unregister(&sclk->hw);
+	}
+	return ret;
+}
+
+static const struct mtmips_clk_data rt2880_clk_data = {
+	.clk_base = rt2880_clks_base,
+	.num_clk_base = ARRAY_SIZE(rt2880_clks_base),
+	.clk_periph = rt2880_pherip_clks,
+	.num_clk_periph = ARRAY_SIZE(rt2880_pherip_clks),
+};
+
+static const struct mtmips_clk_data rt3050_clk_data = {
+	.clk_base = rt305x_clks_base,
+	.num_clk_base = ARRAY_SIZE(rt305x_clks_base),
+	.clk_periph = rt305x_pherip_clks,
+	.num_clk_periph = ARRAY_SIZE(rt305x_pherip_clks),
+};
+
+static const struct mtmips_clk_data rt3052_clk_data = {
+	.clk_base = rt305x_clks_base,
+	.num_clk_base = ARRAY_SIZE(rt305x_clks_base),
+	.clk_periph = rt305x_pherip_clks,
+	.num_clk_periph = ARRAY_SIZE(rt305x_pherip_clks),
+};
+
+static const struct mtmips_clk_data rt3352_clk_data = {
+	.clk_base = rt3352_clks_base,
+	.num_clk_base = ARRAY_SIZE(rt3352_clks_base),
+	.clk_periph = rt5350_pherip_clks,
+	.num_clk_periph = ARRAY_SIZE(rt5350_pherip_clks),
+};
+
+static const struct mtmips_clk_data rt3883_clk_data = {
+	.clk_base = rt3883_clks_base,
+	.num_clk_base = ARRAY_SIZE(rt3883_clks_base),
+	.clk_periph = rt5350_pherip_clks,
+	.num_clk_periph = ARRAY_SIZE(rt5350_pherip_clks),
+};
+
+static const struct mtmips_clk_data rt5350_clk_data = {
+	.clk_base = rt5350_clks_base,
+	.num_clk_base = ARRAY_SIZE(rt5350_clks_base),
+	.clk_periph = rt5350_pherip_clks,
+	.num_clk_periph = ARRAY_SIZE(rt5350_pherip_clks),
+};
+
+static const struct mtmips_clk_data mt7620_clk_data = {
+	.clk_base = mt7620_clks_base,
+	.num_clk_base = ARRAY_SIZE(mt7620_clks_base),
+	.clk_periph = mt7620_pherip_clks,
+	.num_clk_periph = ARRAY_SIZE(mt7620_pherip_clks),
+};
+
+static const struct mtmips_clk_data mt76x8_clk_data = {
+	.clk_base = mt76x8_clks_base,
+	.num_clk_base = ARRAY_SIZE(mt76x8_clks_base),
+	.clk_periph = mt76x8_pherip_clks,
+	.num_clk_periph = ARRAY_SIZE(mt76x8_pherip_clks),
+};
+
+static const struct of_device_id mtmips_of_match[] = {
+	{
+		.compatible = "ralink,rt2880-sysc",
+		.data = &rt2880_clk_data,
+	},
+	{
+		.compatible = "ralink,rt3050-sysc",
+		.data = &rt3050_clk_data,
+	},
+	{
+		.compatible = "ralink,rt3052-sysc",
+		.data = &rt3052_clk_data,
+	},
+	{
+		.compatible = "ralink,rt3352-sysc",
+		.data = &rt3052_clk_data,
+	},
+	{
+		.compatible = "ralink,rt3883-sysc",
+		.data = &rt3352_clk_data,
+	},
+	{
+		.compatible = "ralink,rt5350-sysc",
+		.data = &rt5350_clk_data,
+	},
+	{
+		.compatible = "ralink,mt7620-sysc",
+		.data = &mt7620_clk_data,
+	},
+	{
+		.compatible = "ralink,mt7620a-sysc",
+		.data = &mt7620_clk_data,
+	},
+	{
+		.compatible = "ralink,mt7628-sysc",
+		.data = &mt76x8_clk_data,
+	},
+	{
+		.compatible = "ralink,mt7688-sysc",
+		.data = &mt76x8_clk_data,
+	},
+	{}
+};
+
+static void __init mtmips_clk_init(struct device_node *node)
+{
+	const struct of_device_id *match;
+	const struct mtmips_clk_data *data;
+	struct mtmips_clk_priv *priv;
+	struct clk_hw_onecell_data *clk_data;
+	int ret, i, count;
+
+	if (!of_find_property(node, "#clock-cells", NULL)) {
+		pr_err("No '#clock-cells' property found\n");
+		return;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return;
+
+	priv->sysc = syscon_node_to_regmap(node);
+	if (IS_ERR(priv->sysc)) {
+		pr_err("Could not get sysc syscon regmap\n");
+		goto free_clk_priv;
+	}
+
+	match = of_match_node(mtmips_of_match, node);
+	if (WARN_ON(!match))
+		return;
+
+	data = match->data;
+	priv->data = data;
+	count = priv->data->num_clk_base + priv->data->num_clk_periph;
+	clk_data = kzalloc(struct_size(clk_data, hws, count), GFP_KERNEL);
+	if (!clk_data)
+		goto free_clk_priv;
+
+	ret = mtmips_register_clocks(node, clk_data, priv);
+	if (ret) {
+		pr_err("Couldn't register top clocks\n");
+		goto free_clk_data;
+	}
+
+	ret = mtmips_register_pherip_clocks(node, clk_data, priv);
+	if (ret) {
+		pr_err("Couldn't register peripheral clocks\n");
+		goto unreg_clk_top;
+	}
+
+	clk_data->num = count;
+
+	ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
+	if (ret) {
+		pr_err("Couldn't add clk hw provider\n");
+		goto unreg_clk_periph;
+	}
+
+	return;
+
+unreg_clk_periph:
+	for (i = 0; i < priv->data->num_clk_periph; i++) {
+		struct mtmips_clk *sclk = &priv->data->clk_periph[i];
+
+		clk_hw_unregister(&sclk->hw);
+	}
+
+unreg_clk_top:
+	for (i = 0; i < priv->data->num_clk_base; i++) {
+		struct mtmips_clk *sclk = &priv->data->clk_base[i];
+
+		clk_hw_unregister(&sclk->hw);
+	}
+
+free_clk_data:
+	kfree(clk_data);
+
+free_clk_priv:
+	kfree(priv);
+}
+CLK_OF_DECLARE_DRIVER(rt2880_clk, "ralink,rt2880-sysc", mtmips_clk_init);
+CLK_OF_DECLARE_DRIVER(rt3050_clk, "ralink,rt3050-sysc", mtmips_clk_init);
+CLK_OF_DECLARE_DRIVER(rt3052_clk, "ralink,rt3052-sysc", mtmips_clk_init);
+CLK_OF_DECLARE_DRIVER(rt3352_clk, "ralink,rt3352-sysc", mtmips_clk_init);
+CLK_OF_DECLARE_DRIVER(rt3883_clk, "ralink,rt3883-sysc", mtmips_clk_init);
+CLK_OF_DECLARE_DRIVER(rt5350_clk, "ralink,rt5350-sysc", mtmips_clk_init);
+CLK_OF_DECLARE_DRIVER(mt7620_clk, "ralink,mt7620-sysc", mtmips_clk_init);
+CLK_OF_DECLARE_DRIVER(mt7620a_clk, "ralink,mt7620a-sysc", mtmips_clk_init);
+CLK_OF_DECLARE_DRIVER(mt7628_clk, "ralink,mt7628-sysc", mtmips_clk_init);
+CLK_OF_DECLARE_DRIVER(mt7688_clk, "ralink,mt7688-sysc", mtmips_clk_init);
+
+struct mtmips_rst {
+	struct reset_controller_dev rcdev;
+	struct regmap *sysc;
+};
+
+static struct mtmips_rst *to_mtmips_rst(struct reset_controller_dev *dev)
+{
+	return container_of(dev, struct mtmips_rst, rcdev);
+}
+
+static int mtmips_assert_device(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct mtmips_rst *data = to_mtmips_rst(rcdev);
+	struct regmap *sysc = data->sysc;
+
+	return regmap_update_bits(sysc, SYSC_REG_RESET_CTRL, BIT(id), BIT(id));
+}
+
+static int mtmips_deassert_device(struct reset_controller_dev *rcdev,
+				  unsigned long id)
+{
+	struct mtmips_rst *data = to_mtmips_rst(rcdev);
+	struct regmap *sysc = data->sysc;
+
+	return regmap_update_bits(sysc, SYSC_REG_RESET_CTRL, BIT(id), 0);
+}
+
+static int mtmips_reset_device(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	int ret;
+
+	ret = mtmips_assert_device(rcdev, id);
+	if (ret < 0)
+		return ret;
+
+	return mtmips_deassert_device(rcdev, id);
+}
+
+static int mtmips_rst_xlate(struct reset_controller_dev *rcdev,
+			    const struct of_phandle_args *reset_spec)
+{
+	unsigned long id = reset_spec->args[0];
+
+	if (id == 0 || id >= rcdev->nr_resets)
+		return -EINVAL;
+
+	return id;
+}
+
+static const struct reset_control_ops reset_ops = {
+	.reset = mtmips_reset_device,
+	.assert = mtmips_assert_device,
+	.deassert = mtmips_deassert_device
+};
+
+static int mtmips_reset_init(struct device *dev, struct regmap *sysc)
+{
+	struct mtmips_rst *rst_data;
+
+	rst_data = devm_kzalloc(dev, sizeof(*rst_data), GFP_KERNEL);
+	if (!rst_data)
+		return -ENOMEM;
+
+	rst_data->sysc = sysc;
+	rst_data->rcdev.ops = &reset_ops;
+	rst_data->rcdev.owner = THIS_MODULE;
+	rst_data->rcdev.nr_resets = 32;
+	rst_data->rcdev.of_reset_n_cells = 1;
+	rst_data->rcdev.of_xlate = mtmips_rst_xlate;
+	rst_data->rcdev.of_node = dev_of_node(dev);
+
+	return devm_reset_controller_register(dev, &rst_data->rcdev);
+}
+
+static int mtmips_clk_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct mtmips_clk_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->sysc = syscon_node_to_regmap(np);
+	if (IS_ERR(priv->sysc)) {
+		ret = PTR_ERR(priv->sysc);
+		dev_err(dev, "Could not get sysc syscon regmap\n");
+		return ret;
+	}
+
+	ret = mtmips_reset_init(dev, priv->sysc);
+	if (ret) {
+		dev_err(dev, "Could not init reset controller\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id mtmips_clk_of_match[] = {
+	{ .compatible = "ralink,rt2880-reset" },
+	{ .compatible = "ralink,rt2880-sysc" },
+	{ .compatible = "ralink,rt3050-sysc" },
+	{ .compatible = "ralink,rt3050-sysc" },
+	{ .compatible = "ralink,rt3352-sysc" },
+	{ .compatible = "ralink,rt3883-sysc" },
+	{ .compatible = "ralink,rt5350-sysc" },
+	{ .compatible = "ralink,mt7620a-sysc" },
+	{ .compatible = "ralink,mt7620-sysc" },
+	{ .compatible = "ralink,mt7628-sysc" },
+	{ .compatible = "ralink,mt7688-sysc" },
+	{}
+};
+
+static struct platform_driver mtmips_clk_driver = {
+	.probe = mtmips_clk_probe,
+	.driver = {
+		.name = "mtmips-clk",
+		.of_match_table = mtmips_clk_of_match,
+	},
+};
+
+static int __init mtmips_clk_reset_init(void)
+{
+	return platform_driver_register(&mtmips_clk_driver);
+}
+arch_initcall(mtmips_clk_reset_init);
-- 
2.25.1

[PATCH 03/10] mips: ralink: rt288x: remove clock related code

[Sergio Paracuellos]

A properly clock driver for ralink SoCs has been added. Hence there is no
need to have clock related code in 'arch/mips/ralink' folder anymore.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 arch/mips/include/asm/mach-ralink/rt288x.h | 10 -------
 arch/mips/ralink/rt288x.c                  | 31 ----------------------
 2 files changed, 41 deletions(-)

diff --git a/arch/mips/include/asm/mach-ralink/rt288x.h b/arch/mips/include/asm/mach-ralink/rt288x.h
index 66a999cd1d80..66d190358e3a 100644
--- a/arch/mips/include/asm/mach-ralink/rt288x.h
+++ b/arch/mips/include/asm/mach-ralink/rt288x.h
@@ -18,7 +18,6 @@
 #define SYSC_REG_CHIP_NAME1		0x04
 #define SYSC_REG_CHIP_ID		0x0c
 #define SYSC_REG_SYSTEM_CONFIG		0x10
-#define SYSC_REG_CLKCFG			0x30
 
 #define RT2880_CHIP_NAME0		0x38325452
 #define RT2880_CHIP_NAME1		0x20203038
@@ -27,15 +26,6 @@
 #define CHIP_ID_ID_SHIFT		8
 #define CHIP_ID_REV_MASK		0xff
 
-#define SYSTEM_CONFIG_CPUCLK_SHIFT	20
-#define SYSTEM_CONFIG_CPUCLK_MASK	0x3
-#define SYSTEM_CONFIG_CPUCLK_250	0x0
-#define SYSTEM_CONFIG_CPUCLK_266	0x1
-#define SYSTEM_CONFIG_CPUCLK_280	0x2
-#define SYSTEM_CONFIG_CPUCLK_300	0x3
-
-#define CLKCFG_SRAM_CS_N_WDT		BIT(9)
-
 #define RT2880_SDRAM_BASE		0x08000000
 #define RT2880_MEM_SIZE_MIN		2
 #define RT2880_MEM_SIZE_MAX		128
diff --git a/arch/mips/ralink/rt288x.c b/arch/mips/ralink/rt288x.c
index 456ba0b2599e..0c6a87452dd1 100644
--- a/arch/mips/ralink/rt288x.c
+++ b/arch/mips/ralink/rt288x.c
@@ -21,37 +21,6 @@
 
 static struct ralink_soc_info *soc_info_ptr;
 
-void __init ralink_clk_init(void)
-{
-	unsigned long cpu_rate, wmac_rate = 40000000;
-	u32 t = rt_sysc_r32(SYSC_REG_SYSTEM_CONFIG);
-	t = ((t >> SYSTEM_CONFIG_CPUCLK_SHIFT) & SYSTEM_CONFIG_CPUCLK_MASK);
-
-	switch (t) {
-	case SYSTEM_CONFIG_CPUCLK_250:
-		cpu_rate = 250000000;
-		break;
-	case SYSTEM_CONFIG_CPUCLK_266:
-		cpu_rate = 266666667;
-		break;
-	case SYSTEM_CONFIG_CPUCLK_280:
-		cpu_rate = 280000000;
-		break;
-	case SYSTEM_CONFIG_CPUCLK_300:
-		cpu_rate = 300000000;
-		break;
-	}
-
-	ralink_clk_add("cpu", cpu_rate);
-	ralink_clk_add("300100.timer", cpu_rate / 2);
-	ralink_clk_add("300120.watchdog", cpu_rate / 2);
-	ralink_clk_add("300500.uart", cpu_rate / 2);
-	ralink_clk_add("300900.i2c", cpu_rate / 2);
-	ralink_clk_add("300c00.uartlite", cpu_rate / 2);
-	ralink_clk_add("400000.ethernet", cpu_rate / 2);
-	ralink_clk_add("480000.wmac", wmac_rate);
-}
-
 void __init ralink_of_remap(void)
 {
 	rt_sysc_membase = plat_of_remap_node("ralink,rt2880-sysc");
-- 
2.25.1

[PATCH 04/10] mips: ralink: rt305x: remove clock related code

[Sergio Paracuellos]

A properly clock driver for ralink SoCs has been added. Hence there is no
need to have clock related code in 'arch/mips/ralink' folder anymore.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 arch/mips/include/asm/mach-ralink/rt305x.h | 21 ------
 arch/mips/ralink/rt305x.c                  | 78 ----------------------
 2 files changed, 99 deletions(-)

diff --git a/arch/mips/include/asm/mach-ralink/rt305x.h b/arch/mips/include/asm/mach-ralink/rt305x.h
index ef58f7bff957..4fc5c279cd75 100644
--- a/arch/mips/include/asm/mach-ralink/rt305x.h
+++ b/arch/mips/include/asm/mach-ralink/rt305x.h
@@ -67,26 +67,9 @@ static inline int soc_is_rt5350(void)
 #define CHIP_ID_ID_SHIFT		8
 #define CHIP_ID_REV_MASK		0xff
 
-#define RT305X_SYSCFG_CPUCLK_SHIFT		18
-#define RT305X_SYSCFG_CPUCLK_MASK		0x1
-#define RT305X_SYSCFG_CPUCLK_LOW		0x0
-#define RT305X_SYSCFG_CPUCLK_HIGH		0x1
-
 #define RT305X_SYSCFG_SRAM_CS0_MODE_SHIFT	2
-#define RT305X_SYSCFG_CPUCLK_MASK		0x1
 #define RT305X_SYSCFG_SRAM_CS0_MODE_WDT		0x1
 
-#define RT3352_SYSCFG0_CPUCLK_SHIFT	8
-#define RT3352_SYSCFG0_CPUCLK_MASK	0x1
-#define RT3352_SYSCFG0_CPUCLK_LOW	0x0
-#define RT3352_SYSCFG0_CPUCLK_HIGH	0x1
-
-#define RT5350_SYSCFG0_CPUCLK_SHIFT	8
-#define RT5350_SYSCFG0_CPUCLK_MASK	0x3
-#define RT5350_SYSCFG0_CPUCLK_360	0x0
-#define RT5350_SYSCFG0_CPUCLK_320	0x2
-#define RT5350_SYSCFG0_CPUCLK_300	0x3
-
 #define RT5350_SYSCFG0_DRAM_SIZE_SHIFT  12
 #define RT5350_SYSCFG0_DRAM_SIZE_MASK   7
 #define RT5350_SYSCFG0_DRAM_SIZE_2M     0
@@ -117,13 +100,9 @@ static inline int soc_is_rt5350(void)
 
 #define RT3352_SYSC_REG_SYSCFG0		0x010
 #define RT3352_SYSC_REG_SYSCFG1         0x014
-#define RT3352_SYSC_REG_CLKCFG1         0x030
 #define RT3352_SYSC_REG_RSTCTRL         0x034
 #define RT3352_SYSC_REG_USB_PS          0x05c
 
-#define RT3352_CLKCFG0_XTAL_SEL		BIT(20)
-#define RT3352_CLKCFG1_UPHY0_CLK_EN	BIT(18)
-#define RT3352_CLKCFG1_UPHY1_CLK_EN	BIT(20)
 #define RT3352_RSTCTRL_UHST		BIT(22)
 #define RT3352_RSTCTRL_UDEV		BIT(25)
 #define RT3352_SYSCFG1_USB0_HOST_MODE	BIT(10)
diff --git a/arch/mips/ralink/rt305x.c b/arch/mips/ralink/rt305x.c
index d8dcc5cc66cc..9cffe69dd11d 100644
--- a/arch/mips/ralink/rt305x.c
+++ b/arch/mips/ralink/rt305x.c
@@ -56,84 +56,6 @@ static unsigned long rt5350_get_mem_size(void)
 	return ret;
 }
 
-void __init ralink_clk_init(void)
-{
-	unsigned long cpu_rate, sys_rate, wdt_rate, uart_rate;
-	unsigned long wmac_rate = 40000000;
-
-	u32 t = rt_sysc_r32(SYSC_REG_SYSTEM_CONFIG);
-
-	if (soc_is_rt305x() || soc_is_rt3350()) {
-		t = (t >> RT305X_SYSCFG_CPUCLK_SHIFT) &
-		     RT305X_SYSCFG_CPUCLK_MASK;
-		switch (t) {
-		case RT305X_SYSCFG_CPUCLK_LOW:
-			cpu_rate = 320000000;
-			break;
-		case RT305X_SYSCFG_CPUCLK_HIGH:
-			cpu_rate = 384000000;
-			break;
-		}
-		sys_rate = uart_rate = wdt_rate = cpu_rate / 3;
-	} else if (soc_is_rt3352()) {
-		t = (t >> RT3352_SYSCFG0_CPUCLK_SHIFT) &
-		     RT3352_SYSCFG0_CPUCLK_MASK;
-		switch (t) {
-		case RT3352_SYSCFG0_CPUCLK_LOW:
-			cpu_rate = 384000000;
-			break;
-		case RT3352_SYSCFG0_CPUCLK_HIGH:
-			cpu_rate = 400000000;
-			break;
-		}
-		sys_rate = wdt_rate = cpu_rate / 3;
-		uart_rate = 40000000;
-	} else if (soc_is_rt5350()) {
-		t = (t >> RT5350_SYSCFG0_CPUCLK_SHIFT) &
-		     RT5350_SYSCFG0_CPUCLK_MASK;
-		switch (t) {
-		case RT5350_SYSCFG0_CPUCLK_360:
-			cpu_rate = 360000000;
-			sys_rate = cpu_rate / 3;
-			break;
-		case RT5350_SYSCFG0_CPUCLK_320:
-			cpu_rate = 320000000;
-			sys_rate = cpu_rate / 4;
-			break;
-		case RT5350_SYSCFG0_CPUCLK_300:
-			cpu_rate = 300000000;
-			sys_rate = cpu_rate / 3;
-			break;
-		default:
-			BUG();
-		}
-		uart_rate = 40000000;
-		wdt_rate = sys_rate;
-	} else {
-		BUG();
-	}
-
-	if (soc_is_rt3352() || soc_is_rt5350()) {
-		u32 val = rt_sysc_r32(RT3352_SYSC_REG_SYSCFG0);
-
-		if (!(val & RT3352_CLKCFG0_XTAL_SEL))
-			wmac_rate = 20000000;
-	}
-
-	ralink_clk_add("cpu", cpu_rate);
-	ralink_clk_add("sys", sys_rate);
-	ralink_clk_add("10000900.i2c", uart_rate);
-	ralink_clk_add("10000a00.i2s", uart_rate);
-	ralink_clk_add("10000b00.spi", sys_rate);
-	ralink_clk_add("10000b40.spi", sys_rate);
-	ralink_clk_add("10000100.timer", wdt_rate);
-	ralink_clk_add("10000120.watchdog", wdt_rate);
-	ralink_clk_add("10000500.uart", uart_rate);
-	ralink_clk_add("10000c00.uartlite", uart_rate);
-	ralink_clk_add("10100000.ethernet", sys_rate);
-	ralink_clk_add("10180000.wmac", wmac_rate);
-}
-
 void __init ralink_of_remap(void)
 {
 	rt_sysc_membase = plat_of_remap_node("ralink,rt3050-sysc");
-- 
2.25.1

[PATCH 05/10] mips: ralink: rt3883: remove clock related code

[Sergio Paracuellos]

A properly clock driver for ralink SoCs has been added. Hence there is no
need to have clock related code in 'arch/mips/ralink' folder anymore.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 arch/mips/include/asm/mach-ralink/rt3883.h |  8 ----
 arch/mips/ralink/rt3883.c                  | 44 ----------------------
 2 files changed, 52 deletions(-)

diff --git a/arch/mips/include/asm/mach-ralink/rt3883.h b/arch/mips/include/asm/mach-ralink/rt3883.h
index ad25d5e8d2dc..4a835b178925 100644
--- a/arch/mips/include/asm/mach-ralink/rt3883.h
+++ b/arch/mips/include/asm/mach-ralink/rt3883.h
@@ -92,14 +92,6 @@
 #define RT3883_REVID_VER_ID_SHIFT	8
 #define RT3883_REVID_ECO_ID_MASK	0x0f
 
-#define RT3883_SYSCFG0_DRAM_TYPE_DDR2	BIT(17)
-#define RT3883_SYSCFG0_CPUCLK_SHIFT	8
-#define RT3883_SYSCFG0_CPUCLK_MASK	0x3
-#define RT3883_SYSCFG0_CPUCLK_250	0x0
-#define RT3883_SYSCFG0_CPUCLK_384	0x1
-#define RT3883_SYSCFG0_CPUCLK_480	0x2
-#define RT3883_SYSCFG0_CPUCLK_500	0x3
-
 #define RT3883_SYSCFG1_USB0_HOST_MODE	BIT(10)
 #define RT3883_SYSCFG1_PCIE_RC_MODE	BIT(8)
 #define RT3883_SYSCFG1_PCI_HOST_MODE	BIT(7)
diff --git a/arch/mips/ralink/rt3883.c b/arch/mips/ralink/rt3883.c
index cca887af378f..14c56993611a 100644
--- a/arch/mips/ralink/rt3883.c
+++ b/arch/mips/ralink/rt3883.c
@@ -21,50 +21,6 @@
 
 static struct ralink_soc_info *soc_info_ptr;
 
-void __init ralink_clk_init(void)
-{
-	unsigned long cpu_rate, sys_rate;
-	u32 syscfg0;
-	u32 clksel;
-	u32 ddr2;
-
-	syscfg0 = rt_sysc_r32(RT3883_SYSC_REG_SYSCFG0);
-	clksel = ((syscfg0 >> RT3883_SYSCFG0_CPUCLK_SHIFT) &
-		RT3883_SYSCFG0_CPUCLK_MASK);
-	ddr2 = syscfg0 & RT3883_SYSCFG0_DRAM_TYPE_DDR2;
-
-	switch (clksel) {
-	case RT3883_SYSCFG0_CPUCLK_250:
-		cpu_rate = 250000000;
-		sys_rate = (ddr2) ? 125000000 : 83000000;
-		break;
-	case RT3883_SYSCFG0_CPUCLK_384:
-		cpu_rate = 384000000;
-		sys_rate = (ddr2) ? 128000000 : 96000000;
-		break;
-	case RT3883_SYSCFG0_CPUCLK_480:
-		cpu_rate = 480000000;
-		sys_rate = (ddr2) ? 160000000 : 120000000;
-		break;
-	case RT3883_SYSCFG0_CPUCLK_500:
-		cpu_rate = 500000000;
-		sys_rate = (ddr2) ? 166000000 : 125000000;
-		break;
-	}
-
-	ralink_clk_add("cpu", cpu_rate);
-	ralink_clk_add("10000100.timer", sys_rate);
-	ralink_clk_add("10000120.watchdog", sys_rate);
-	ralink_clk_add("10000500.uart", 40000000);
-	ralink_clk_add("10000900.i2c", 40000000);
-	ralink_clk_add("10000a00.i2s", 40000000);
-	ralink_clk_add("10000b00.spi", sys_rate);
-	ralink_clk_add("10000b40.spi", sys_rate);
-	ralink_clk_add("10000c00.uartlite", 40000000);
-	ralink_clk_add("10100000.ethernet", sys_rate);
-	ralink_clk_add("10180000.wmac", 40000000);
-}
-
 void __init ralink_of_remap(void)
 {
 	rt_sysc_membase = plat_of_remap_node("ralink,rt3883-sysc");
-- 
2.25.1

[PATCH 06/10] mips: ralink: mt7620: remove clock related code

[Sergio Paracuellos]

A proper clock driver for ralink SoCs has been added. Hence there is no
need to have clock related code in 'arch/mips/ralink' folder anymore.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 arch/mips/include/asm/mach-ralink/mt7620.h |  35 ----
 arch/mips/ralink/mt7620.c                  | 226 ---------------------
 2 files changed, 261 deletions(-)

diff --git a/arch/mips/include/asm/mach-ralink/mt7620.h b/arch/mips/include/asm/mach-ralink/mt7620.h
index 3e37705ea9cf..62f4f072c003 100644
--- a/arch/mips/include/asm/mach-ralink/mt7620.h
+++ b/arch/mips/include/asm/mach-ralink/mt7620.h
@@ -20,52 +20,17 @@
 #define SYSC_REG_CHIP_REV		0x0c
 #define SYSC_REG_SYSTEM_CONFIG0		0x10
 #define SYSC_REG_SYSTEM_CONFIG1		0x14
-#define SYSC_REG_CLKCFG0		0x2c
-#define SYSC_REG_CPU_SYS_CLKCFG		0x3c
-#define SYSC_REG_CPLL_CONFIG0		0x54
-#define SYSC_REG_CPLL_CONFIG1		0x58
 
 #define MT7620_CHIP_NAME0		0x3637544d
 #define MT7620_CHIP_NAME1		0x20203032
 #define MT7628_CHIP_NAME1		0x20203832
 
-#define SYSCFG0_XTAL_FREQ_SEL		BIT(6)
-
 #define CHIP_REV_PKG_MASK		0x1
 #define CHIP_REV_PKG_SHIFT		16
 #define CHIP_REV_VER_MASK		0xf
 #define CHIP_REV_VER_SHIFT		8
 #define CHIP_REV_ECO_MASK		0xf
 
-#define CLKCFG0_PERI_CLK_SEL		BIT(4)
-
-#define CPU_SYS_CLKCFG_OCP_RATIO_SHIFT	16
-#define CPU_SYS_CLKCFG_OCP_RATIO_MASK	0xf
-#define CPU_SYS_CLKCFG_OCP_RATIO_1	0	/* 1:1   (Reserved) */
-#define CPU_SYS_CLKCFG_OCP_RATIO_1_5	1	/* 1:1.5 (Reserved) */
-#define CPU_SYS_CLKCFG_OCP_RATIO_2	2	/* 1:2   */
-#define CPU_SYS_CLKCFG_OCP_RATIO_2_5	3       /* 1:2.5 (Reserved) */
-#define CPU_SYS_CLKCFG_OCP_RATIO_3	4	/* 1:3   */
-#define CPU_SYS_CLKCFG_OCP_RATIO_3_5	5	/* 1:3.5 (Reserved) */
-#define CPU_SYS_CLKCFG_OCP_RATIO_4	6	/* 1:4   */
-#define CPU_SYS_CLKCFG_OCP_RATIO_5	7	/* 1:5   */
-#define CPU_SYS_CLKCFG_OCP_RATIO_10	8	/* 1:10  */
-#define CPU_SYS_CLKCFG_CPU_FDIV_SHIFT	8
-#define CPU_SYS_CLKCFG_CPU_FDIV_MASK	0x1f
-#define CPU_SYS_CLKCFG_CPU_FFRAC_SHIFT	0
-#define CPU_SYS_CLKCFG_CPU_FFRAC_MASK	0x1f
-
-#define CPLL_CFG0_SW_CFG		BIT(31)
-#define CPLL_CFG0_PLL_MULT_RATIO_SHIFT	16
-#define CPLL_CFG0_PLL_MULT_RATIO_MASK   0x7
-#define CPLL_CFG0_LC_CURFCK		BIT(15)
-#define CPLL_CFG0_BYPASS_REF_CLK	BIT(14)
-#define CPLL_CFG0_PLL_DIV_RATIO_SHIFT	10
-#define CPLL_CFG0_PLL_DIV_RATIO_MASK	0x3
-
-#define CPLL_CFG1_CPU_AUX1		BIT(25)
-#define CPLL_CFG1_CPU_AUX0		BIT(24)
-
 #define SYSCFG0_DRAM_TYPE_MASK		0x3
 #define SYSCFG0_DRAM_TYPE_SHIFT		4
 #define SYSCFG0_DRAM_TYPE_SDRAM		0
diff --git a/arch/mips/ralink/mt7620.c b/arch/mips/ralink/mt7620.c
index 4435f50b8d24..f44915b0b0c2 100644
--- a/arch/mips/ralink/mt7620.c
+++ b/arch/mips/ralink/mt7620.c
@@ -36,12 +36,6 @@
 #define PMU1_CFG		0x8C
 #define DIG_SW_SEL		BIT(25)
 
-/* clock scaling */
-#define CLKCFG_FDIV_MASK	0x1f00
-#define CLKCFG_FDIV_USB_VAL	0x0300
-#define CLKCFG_FFRAC_MASK	0x001f
-#define CLKCFG_FFRAC_USB_VAL	0x0003
-
 /* EFUSE bits */
 #define EFUSE_MT7688		0x100000
 
@@ -53,226 +47,6 @@ static int dram_type;
 
 static struct ralink_soc_info *soc_info_ptr;
 
-static __init u32
-mt7620_calc_rate(u32 ref_rate, u32 mul, u32 div)
-{
-	u64 t;
-
-	t = ref_rate;
-	t *= mul;
-	do_div(t, div);
-
-	return t;
-}
-
-#define MHZ(x)		((x) * 1000 * 1000)
-
-static __init unsigned long
-mt7620_get_xtal_rate(void)
-{
-	u32 reg;
-
-	reg = rt_sysc_r32(SYSC_REG_SYSTEM_CONFIG0);
-	if (reg & SYSCFG0_XTAL_FREQ_SEL)
-		return MHZ(40);
-
-	return MHZ(20);
-}
-
-static __init unsigned long
-mt7620_get_periph_rate(unsigned long xtal_rate)
-{
-	u32 reg;
-
-	reg = rt_sysc_r32(SYSC_REG_CLKCFG0);
-	if (reg & CLKCFG0_PERI_CLK_SEL)
-		return xtal_rate;
-
-	return MHZ(40);
-}
-
-static const u32 mt7620_clk_divider[] __initconst = { 2, 3, 4, 8 };
-
-static __init unsigned long
-mt7620_get_cpu_pll_rate(unsigned long xtal_rate)
-{
-	u32 reg;
-	u32 mul;
-	u32 div;
-
-	reg = rt_sysc_r32(SYSC_REG_CPLL_CONFIG0);
-	if (reg & CPLL_CFG0_BYPASS_REF_CLK)
-		return xtal_rate;
-
-	if ((reg & CPLL_CFG0_SW_CFG) == 0)
-		return MHZ(600);
-
-	mul = (reg >> CPLL_CFG0_PLL_MULT_RATIO_SHIFT) &
-	      CPLL_CFG0_PLL_MULT_RATIO_MASK;
-	mul += 24;
-	if (reg & CPLL_CFG0_LC_CURFCK)
-		mul *= 2;
-
-	div = (reg >> CPLL_CFG0_PLL_DIV_RATIO_SHIFT) &
-	      CPLL_CFG0_PLL_DIV_RATIO_MASK;
-
-	WARN_ON(div >= ARRAY_SIZE(mt7620_clk_divider));
-
-	return mt7620_calc_rate(xtal_rate, mul, mt7620_clk_divider[div]);
-}
-
-static __init unsigned long
-mt7620_get_pll_rate(unsigned long xtal_rate, unsigned long cpu_pll_rate)
-{
-	u32 reg;
-
-	reg = rt_sysc_r32(SYSC_REG_CPLL_CONFIG1);
-	if (reg & CPLL_CFG1_CPU_AUX1)
-		return xtal_rate;
-
-	if (reg & CPLL_CFG1_CPU_AUX0)
-		return MHZ(480);
-
-	return cpu_pll_rate;
-}
-
-static __init unsigned long
-mt7620_get_cpu_rate(unsigned long pll_rate)
-{
-	u32 reg;
-	u32 mul;
-	u32 div;
-
-	reg = rt_sysc_r32(SYSC_REG_CPU_SYS_CLKCFG);
-
-	mul = reg & CPU_SYS_CLKCFG_CPU_FFRAC_MASK;
-	div = (reg >> CPU_SYS_CLKCFG_CPU_FDIV_SHIFT) &
-	      CPU_SYS_CLKCFG_CPU_FDIV_MASK;
-
-	return mt7620_calc_rate(pll_rate, mul, div);
-}
-
-static const u32 mt7620_ocp_dividers[16] __initconst = {
-	[CPU_SYS_CLKCFG_OCP_RATIO_2] = 2,
-	[CPU_SYS_CLKCFG_OCP_RATIO_3] = 3,
-	[CPU_SYS_CLKCFG_OCP_RATIO_4] = 4,
-	[CPU_SYS_CLKCFG_OCP_RATIO_5] = 5,
-	[CPU_SYS_CLKCFG_OCP_RATIO_10] = 10,
-};
-
-static __init unsigned long
-mt7620_get_dram_rate(unsigned long pll_rate)
-{
-	if (dram_type == SYSCFG0_DRAM_TYPE_SDRAM)
-		return pll_rate / 4;
-
-	return pll_rate / 3;
-}
-
-static __init unsigned long
-mt7620_get_sys_rate(unsigned long cpu_rate)
-{
-	u32 reg;
-	u32 ocp_ratio;
-	u32 div;
-
-	reg = rt_sysc_r32(SYSC_REG_CPU_SYS_CLKCFG);
-
-	ocp_ratio = (reg >> CPU_SYS_CLKCFG_OCP_RATIO_SHIFT) &
-		    CPU_SYS_CLKCFG_OCP_RATIO_MASK;
-
-	if (WARN_ON(ocp_ratio >= ARRAY_SIZE(mt7620_ocp_dividers)))
-		return cpu_rate;
-
-	div = mt7620_ocp_dividers[ocp_ratio];
-	if (WARN(!div, "invalid divider for OCP ratio %u", ocp_ratio))
-		return cpu_rate;
-
-	return cpu_rate / div;
-}
-
-void __init ralink_clk_init(void)
-{
-	unsigned long xtal_rate;
-	unsigned long cpu_pll_rate;
-	unsigned long pll_rate;
-	unsigned long cpu_rate;
-	unsigned long sys_rate;
-	unsigned long dram_rate;
-	unsigned long periph_rate;
-	unsigned long pcmi2s_rate;
-
-	xtal_rate = mt7620_get_xtal_rate();
-
-#define RFMT(label)	label ":%lu.%03luMHz "
-#define RINT(x)		((x) / 1000000)
-#define RFRAC(x)	(((x) / 1000) % 1000)
-
-	if (is_mt76x8()) {
-		if (xtal_rate == MHZ(40))
-			cpu_rate = MHZ(580);
-		else
-			cpu_rate = MHZ(575);
-		dram_rate = sys_rate = cpu_rate / 3;
-		periph_rate = MHZ(40);
-		pcmi2s_rate = MHZ(480);
-
-		ralink_clk_add("10000d00.uartlite", periph_rate);
-		ralink_clk_add("10000e00.uartlite", periph_rate);
-	} else {
-		cpu_pll_rate = mt7620_get_cpu_pll_rate(xtal_rate);
-		pll_rate = mt7620_get_pll_rate(xtal_rate, cpu_pll_rate);
-
-		cpu_rate = mt7620_get_cpu_rate(pll_rate);
-		dram_rate = mt7620_get_dram_rate(pll_rate);
-		sys_rate = mt7620_get_sys_rate(cpu_rate);
-		periph_rate = mt7620_get_periph_rate(xtal_rate);
-		pcmi2s_rate = periph_rate;
-
-		pr_debug(RFMT("XTAL") RFMT("CPU_PLL") RFMT("PLL"),
-			 RINT(xtal_rate), RFRAC(xtal_rate),
-			 RINT(cpu_pll_rate), RFRAC(cpu_pll_rate),
-			 RINT(pll_rate), RFRAC(pll_rate));
-
-		ralink_clk_add("10000500.uart", periph_rate);
-	}
-
-	pr_debug(RFMT("CPU") RFMT("DRAM") RFMT("SYS") RFMT("PERIPH"),
-		 RINT(cpu_rate), RFRAC(cpu_rate),
-		 RINT(dram_rate), RFRAC(dram_rate),
-		 RINT(sys_rate), RFRAC(sys_rate),
-		 RINT(periph_rate), RFRAC(periph_rate));
-#undef RFRAC
-#undef RINT
-#undef RFMT
-
-	ralink_clk_add("cpu", cpu_rate);
-	ralink_clk_add("10000100.timer", periph_rate);
-	ralink_clk_add("10000120.watchdog", periph_rate);
-	ralink_clk_add("10000900.i2c", periph_rate);
-	ralink_clk_add("10000a00.i2s", pcmi2s_rate);
-	ralink_clk_add("10000b00.spi", sys_rate);
-	ralink_clk_add("10000b40.spi", sys_rate);
-	ralink_clk_add("10000c00.uartlite", periph_rate);
-	ralink_clk_add("10000d00.uart1", periph_rate);
-	ralink_clk_add("10000e00.uart2", periph_rate);
-	ralink_clk_add("10180000.wmac", xtal_rate);
-
-	if (IS_ENABLED(CONFIG_USB) && !is_mt76x8()) {
-		/*
-		 * When the CPU goes into sleep mode, the BUS clock will be
-		 * too low for USB to function properly. Adjust the busses
-		 * fractional divider to fix this
-		 */
-		u32 val = rt_sysc_r32(SYSC_REG_CPU_SYS_CLKCFG);
-
-		val &= ~(CLKCFG_FDIV_MASK | CLKCFG_FFRAC_MASK);
-		val |= CLKCFG_FDIV_USB_VAL | CLKCFG_FFRAC_USB_VAL;
-
-		rt_sysc_w32(val, SYSC_REG_CPU_SYS_CLKCFG);
-	}
-}
-
 void __init ralink_of_remap(void)
 {
 	rt_sysc_membase = plat_of_remap_node("ralink,mt7620a-sysc");
-- 
2.25.1

[PATCH 07/10] mips: ralink: remove clock related function prototypes

[Sergio Paracuellos]

Clock related code has been removed from 'arch/mips/ralink' folder and put
into drivers space. Hence remove clock related prototypes which are not
used anymore.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 arch/mips/ralink/common.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
index 87fc16751281..fcdfc9dc6210 100644
--- a/arch/mips/ralink/common.h
+++ b/arch/mips/ralink/common.h
@@ -23,9 +23,6 @@ extern struct ralink_soc_info soc_info;
 
 extern void ralink_of_remap(void);
 
-extern void ralink_clk_init(void);
-extern void ralink_clk_add(const char *dev, unsigned long rate);
-
 extern void ralink_rst_init(void);
 
 extern void __init prom_soc_init(struct ralink_soc_info *soc_info);
-- 
2.25.1

[PATCH 08/10] mips: ralink: remove reset related code

[Sergio Paracuellos]

A proper clock driver for ralink SoCs has been added. This driver is also
a reset provider for the SoC. Hence there is no need to have reset related
code in 'arch/mips/ralink' folder anymore. The only code that remains is
the one related with mips_reboot_setup where a PCI reset is performed.
We maintain this because I cannot test old ralink board with PCI to be
sure all works if we remove also this code.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 arch/mips/ralink/common.h |  2 --
 arch/mips/ralink/of.c     |  4 ---
 arch/mips/ralink/reset.c  | 61 ---------------------------------------
 3 files changed, 67 deletions(-)

diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
index fcdfc9dc6210..b0d671442966 100644
--- a/arch/mips/ralink/common.h
+++ b/arch/mips/ralink/common.h
@@ -23,8 +23,6 @@ extern struct ralink_soc_info soc_info;
 
 extern void ralink_of_remap(void);
 
-extern void ralink_rst_init(void);
-
 extern void __init prom_soc_init(struct ralink_soc_info *soc_info);
 
 __iomem void *plat_of_remap_node(const char *node);
diff --git a/arch/mips/ralink/of.c b/arch/mips/ralink/of.c
index 4d06de77d92a..df29e6c896aa 100644
--- a/arch/mips/ralink/of.c
+++ b/arch/mips/ralink/of.c
@@ -81,10 +81,6 @@ static int __init plat_of_setup(void)
 {
 	__dt_register_buses(soc_info.compatible, "palmbus");
 
-	/* make sure that the reset controller is setup early */
-	if (ralink_soc != MT762X_SOC_MT7621AT)
-		ralink_rst_init();
-
 	return 0;
 }
 
diff --git a/arch/mips/ralink/reset.c b/arch/mips/ralink/reset.c
index 274d33078c5e..4875637ef469 100644
--- a/arch/mips/ralink/reset.c
+++ b/arch/mips/ralink/reset.c
@@ -10,7 +10,6 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/delay.h>
-#include <linux/reset-controller.h>
 
 #include <asm/reboot.h>
 
@@ -22,66 +21,6 @@
 #define RSTCTL_RESET_PCI	BIT(26)
 #define RSTCTL_RESET_SYSTEM	BIT(0)
 
-static int ralink_assert_device(struct reset_controller_dev *rcdev,
-				unsigned long id)
-{
-	u32 val;
-
-	if (id == 0)
-		return -1;
-
-	val = rt_sysc_r32(SYSC_REG_RESET_CTRL);
-	val |= BIT(id);
-	rt_sysc_w32(val, SYSC_REG_RESET_CTRL);
-
-	return 0;
-}
-
-static int ralink_deassert_device(struct reset_controller_dev *rcdev,
-				  unsigned long id)
-{
-	u32 val;
-
-	if (id == 0)
-		return -1;
-
-	val = rt_sysc_r32(SYSC_REG_RESET_CTRL);
-	val &= ~BIT(id);
-	rt_sysc_w32(val, SYSC_REG_RESET_CTRL);
-
-	return 0;
-}
-
-static int ralink_reset_device(struct reset_controller_dev *rcdev,
-			       unsigned long id)
-{
-	ralink_assert_device(rcdev, id);
-	return ralink_deassert_device(rcdev, id);
-}
-
-static const struct reset_control_ops reset_ops = {
-	.reset = ralink_reset_device,
-	.assert = ralink_assert_device,
-	.deassert = ralink_deassert_device,
-};
-
-static struct reset_controller_dev reset_dev = {
-	.ops			= &reset_ops,
-	.owner			= THIS_MODULE,
-	.nr_resets		= 32,
-	.of_reset_n_cells	= 1,
-};
-
-void ralink_rst_init(void)
-{
-	reset_dev.of_node = of_find_compatible_node(NULL, NULL,
-						"ralink,rt2880-reset");
-	if (!reset_dev.of_node)
-		pr_err("Failed to find reset controller node");
-	else
-		reset_controller_register(&reset_dev);
-}
-
 static void ralink_restart(char *command)
 {
 	if (IS_ENABLED(CONFIG_PCI)) {
-- 
2.25.1

[PATCH 09/10] mips: ralink: get cpu rate from new driver code

[Sergio Paracuellos]

At very early stage on boot, there is a need to set 'mips_hpt_frequency'.
This timer frequency is a half of the CPU frequency. To get clocks properly
set we need to call to 'of_clk_init()' and properly get cpu clock frequency
afterwards. Depending on the SoC, CPU clock index in the clock provider is
different being two for MT7620 SoC and one for the rest. Hence, adapt code
to be aligned with new clock driver.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 arch/mips/ralink/clk.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c
index 5b02bb7e0829..3d29e956f785 100644
--- a/arch/mips/ralink/clk.c
+++ b/arch/mips/ralink/clk.c
@@ -11,29 +11,41 @@
 #include <linux/clkdev.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <asm/mach-ralink/ralink_regs.h>
 
 #include <asm/time.h>
 
 #include "common.h"
 
-void ralink_clk_add(const char *dev, unsigned long rate)
+static int clk_cpu_index(void)
 {
-	struct clk *clk = clk_register_fixed_rate(NULL, dev, NULL, 0, rate);
+	if (ralink_soc == RALINK_UNKNOWN)
+		return -1;
 
-	if (!clk)
-		panic("failed to add clock");
+	if (ralink_soc == MT762X_SOC_MT7620A ||
+	    ralink_soc == MT762X_SOC_MT7620N)
+		return 2;
 
-	clkdev_create(clk, NULL, "%s", dev);
+	return 1;
 }
 
 void __init plat_time_init(void)
 {
+	struct of_phandle_args clkspec;
 	struct clk *clk;
+	int cpu_clk_idx;
 
 	ralink_of_remap();
 
-	ralink_clk_init();
-	clk = clk_get_sys("cpu", NULL);
+	cpu_clk_idx = clk_cpu_index();
+	if (cpu_clk_idx == -1)
+		panic("unable to get CPU clock index");
+
+	of_clk_init(NULL);
+	clkspec.np = of_find_node_by_name(NULL, "sysc");
+	clkspec.args_count = 1;
+	clkspec.args[0] = cpu_clk_idx;
+	clk = of_clk_get_from_provider(&clkspec);
 	if (IS_ERR(clk))
 		panic("unable to get CPU clock, err=%ld", PTR_ERR(clk));
 	pr_info("CPU Clock: %ldMHz\n", clk_get_rate(clk) / 1000000);
-- 
2.25.1

[PATCH 10/10] MAINTAINERS: add Mediatek MTMIPS Clock maintainer

[Sergio Paracuellos]

Adding myself as maintainer for Mediatek MTMIPS clock driver.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d5bc223f305..f11e8d1da326 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13137,6 +13137,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
 F:	drivers/phy/ralink/phy-mt7621-pci.c
 
+MEDIATEK MTMIPS CLOCK DRIVER
+M:	Sergio Paracuellos <sergio.paracuellos@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/clock/mtmips-clock.yaml
+F:	drivers/clk/ralink/clk-mtmips.c
+
 MEDIATEK NAND CONTROLLER DRIVER
 L:	linux-mtd@lists.infradead.org
 S:	Orphan
-- 
2.25.1

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 20/03/2023 17:18, Sergio Paracuellos wrote:
> Adds device tree binding documentation for clocks and resets in the
> Mediatek MIPS and Ralink SOCs. This covers RT2880, RT3050, RT3052, RT3350,
> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

Subject: drop second/last, redundant "device tree binding
documentation". The "dt-bindings" prefix is already stating that these
are bindings.
(BTW, that's the longest redundant component I ever saw)

> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../bindings/clock/mtmips-clock.yaml          | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/mtmips-clock.yaml b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
> new file mode 100644
> index 000000000000..c92969ce231d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml

Filename matching compatible, so vendor prefix and device name (or
family of names).

> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/mtmips-clock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MTMIPS SoCs Clock

One clock? Are you sure these describe exactly one clock?

> +
> +maintainers:
> +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> +
> +description: |
> +  MediaTek MIPS and Ralink SoCs have an XTAL from where the cpu clock is
> +  provided as well as derived clocks for the bus and the peripherals.
> +
> +  Each clock is assigned an identifier and client nodes use this identifier
> +  to specify the clock which they consume.

Drop useless or obvious pieces of description. Describe the hardware.

> +
> +  The clocks are provided inside a system controller node.

???

> +
> +  This node is also a reset provider for all the peripherals.

??? Does it mean it is not only "Clock" but also reset controller?

> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - ralink,rt2880-sysc
> +          - ralink,rt3050-sysc
> +          - ralink,rt3052-sysc
> +          - ralink,rt3352-sysc
> +          - ralink,rt3883-sysc
> +          - ralink,rt5350-sysc
> +          - ralink,mt7620-sysc
> +          - ralink,mt7620a-sysc
> +          - ralink,mt7628-sysc
> +          - ralink,mt7688-sysc
> +          - ralink,rt2880-reset

That's odd. rt2880 is sysc and reset? One device with two compatibles?

Also, order these by name.


> +      - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    description:
> +      The first cell indicates the clock number.
> +    const: 1
> +
> +  '#reset-cells':
> +    description:
> +      The first cell indicates the reset bit within the register.
> +    const: 1

Wait, only rt2880-reset is reset controller? This is confusing.

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#clock-cells'
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    sysc: sysc@0 {

Drop label.

Node names should be generic, clock-controller or reset-controller or
system-controller sometimes.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +      compatible = "ralink,rt5350-sysc", "syscon";
> +      reg = <0x0 0x100>;
> +      #clock-cells = <1>;
> +      #reset-cells = <1>;
> +    };

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 20.03.2023 19:36, Krzysztof Kozlowski wrote:
> On 20/03/2023 17:18, Sergio Paracuellos wrote:
>> Adds device tree binding documentation for clocks and resets in the
>> Mediatek MIPS and Ralink SOCs. This covers RT2880, RT3050, RT3052, RT3350,
>> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
> 
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
> 
> Subject: drop second/last, redundant "device tree binding
> documentation". The "dt-bindings" prefix is already stating that these
> are bindings.
> (BTW, that's the longest redundant component I ever saw)
> 
>>
>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
>> ---
>>   .../bindings/clock/mtmips-clock.yaml          | 68 +++++++++++++++++++
>>   1 file changed, 68 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/clock/mtmips-clock.yaml b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>> new file mode 100644
>> index 000000000000..c92969ce231d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
> 
> Filename matching compatible, so vendor prefix and device name (or
> family of names).

I influenced Sergio to use MTMIPS here as I want to designate it the 
family name for the MediaTek MIPS and Ralink SoCs. We can't change the 
compatible string as it's established from my pinctrl patch series we 
don't do that.

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 20/03/2023 17:43, Arınç ÜNAL wrote:
> On 20.03.2023 19:36, Krzysztof Kozlowski wrote:
>> On 20/03/2023 17:18, Sergio Paracuellos wrote:
>>> Adds device tree binding documentation for clocks and resets in the
>>> Mediatek MIPS and Ralink SOCs. This covers RT2880, RT3050, RT3052, RT3350,
>>> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
>>
>> Use subject prefixes matching the subsystem (which you can get for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching).
>>
>> Subject: drop second/last, redundant "device tree binding
>> documentation". The "dt-bindings" prefix is already stating that these
>> are bindings.
>> (BTW, that's the longest redundant component I ever saw)
>>
>>>
>>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
>>> ---
>>>   .../bindings/clock/mtmips-clock.yaml          | 68 +++++++++++++++++++
>>>   1 file changed, 68 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/mtmips-clock.yaml b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>> new file mode 100644
>>> index 000000000000..c92969ce231d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>
>> Filename matching compatible, so vendor prefix and device name (or
>> family of names).
> 
> I influenced Sergio to use MTMIPS here as I want to designate it the 
> family name for the MediaTek MIPS and Ralink SoCs.

I don't know how to respond to this. Is it argument for not using naming
style?

> We can't change the 
> compatible string as it's established from my pinctrl patch series we 
> don't do that.

The patch did not say it is documenting existing compatibles in the
kernel DTS. And if we are at this, ralink,rt2880-reset does not look
like single clock nor like clock controller.

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Sergio Paracuellos]

Hi Krzysztof,

On Mon, Mar 20, 2023 at 5:36 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/03/2023 17:18, Sergio Paracuellos wrote:
> > Adds device tree binding documentation for clocks and resets in the
> > Mediatek MIPS and Ralink SOCs. This covers RT2880, RT3050, RT3052, RT3350,
> > RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
>
> Subject: drop second/last, redundant "device tree binding
> documentation". The "dt-bindings" prefix is already stating that these
> are bindings.

Sure, will do. Sorry for the inconvenience.

> (BTW, that's the longest redundant component I ever saw)

I thought it was better to just list compatible strings inside one
single file than adding the same binding in multiple files.

>
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  .../bindings/clock/mtmips-clock.yaml          | 68 +++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/clock/mtmips-clock.yaml b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
> > new file mode 100644
> > index 000000000000..c92969ce231d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>
> Filename matching compatible, so vendor prefix and device name (or
> family of names).

I used mtmips here but list compatibles starting with ralink. As I
have said in the cover letter I am inspired by the last merged pinctrl
series for these SoCs.
See:
- https://lore.kernel.org/linux-gpio/e9e6ad87-2db5-9767-ff39-64a302b06185@arinc9.com/T/#t

Not all of compatible currently exist. All of these are at the end the
way we can properly match compatible-data to write a proper driver.
The current ralink dtsi files which are in tree now
are totally incomplete and not documented so we are planning to align
all of this with openWRT used files and others soon. That's the reason
we are not touching
'arch/mips/boot/dts' at all now. I don't think anybody is using any of
this but mt7621 which is properly completed and documented.

>
> > @@ -0,0 +1,68 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/mtmips-clock.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MTMIPS SoCs Clock
>
> One clock? Are you sure these describe exactly one clock?

I will change this to 'Clocks'.

>
> > +
> > +maintainers:
> > +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > +
> > +description: |
> > +  MediaTek MIPS and Ralink SoCs have an XTAL from where the cpu clock is
> > +  provided as well as derived clocks for the bus and the peripherals.
> > +
> > +  Each clock is assigned an identifier and client nodes use this identifier
> > +  to specify the clock which they consume.
>
> Drop useless or obvious pieces of description. Describe the hardware.
>
> > +
> > +  The clocks are provided inside a system controller node.

>
> ???

I meant, this node is a syscon from where both clock and reset related
registers are used. I think writing in this way was enough since it
has a pretty similar description like the one in
'mediatek,mt7621-sysc.yaml'.

>
> > +
> > +  This node is also a reset provider for all the peripherals.
>
> ??? Does it mean it is not only "Clock" but also reset controller?

Yes, this node is a clock and reset controller for all the SoC.

>
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - ralink,rt2880-sysc
> > +          - ralink,rt3050-sysc
> > +          - ralink,rt3052-sysc
> > +          - ralink,rt3352-sysc
> > +          - ralink,rt3883-sysc
> > +          - ralink,rt5350-sysc
> > +          - ralink,mt7620-sysc
> > +          - ralink,mt7620a-sysc
> > +          - ralink,mt7628-sysc'
> > +          - ralink,mt7688-sysc
> > +          - ralink,rt2880-reset
>
> That's odd. rt2880 is sysc and reset? One device with two compatibles?

This 'ralink,rt2880-reset' is for compatibility reasons. Reset related
code was inside 'arch/mips/ralink' folder reset.c file but it is moved
to this new driver, so we have maintained this reset stuff for the
reset compatibility. All of the rest are the new possible stuff for
both reset and clocks. Clock driver is instantiated in two phases. The
earlier one set up the clocks via CLK_OF_DECLARE macro. Resets are set
up as a platform driver. Is only inside this where
'ralink,rt2880-reset' is used. See patch 2 of the series for details.

>
> Also, order these by name.

All are ordered but I maintained the  'ralink,rt2880-reset' at the end.

>
>
> > +      - const: syscon
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#clock-cells':
> > +    description:
> > +      The first cell indicates the clock number.
> > +    const: 1
> > +
> > +  '#reset-cells':
> > +    description:
> > +      The first cell indicates the reset bit within the register.
> > +    const: 1
>
> Wait, only rt2880-reset is reset controller? This is confusing.

No, that is the reset compatibility one. All the rest are both clock
and reset controllers from now on.

>
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#clock-cells'
> > +  - '#reset-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    sysc: sysc@0 {
>
> Drop label.

Sure, thanks.

>
> Node names should be generic, clock-controller or reset-controller or
> system-controller sometimes.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
> > +      compatible = "ralink,rt5350-sysc", "syscon";
> > +      reg = <0x0 0x100>;
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> > +    };

Ok, so I will set this as 'syscon@' if you are ok with it.

>
> Best regards,
> Krzysztof
>

Thanks to you for the review.

Best regards,
    Sergio Paracuellos

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 20/03/2023 18:24, Sergio Paracuellos wrote:
> Hi Krzysztof,
> 
> On Mon, Mar 20, 2023 at 5:36 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 20/03/2023 17:18, Sergio Paracuellos wrote:
>>> Adds device tree binding documentation for clocks and resets in the
>>> Mediatek MIPS and Ralink SOCs. This covers RT2880, RT3050, RT3052, RT3350,
>>> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
>>
>> Use subject prefixes matching the subsystem (which you can get for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching).
>>
>> Subject: drop second/last, redundant "device tree binding
>> documentation". The "dt-bindings" prefix is already stating that these
>> are bindings.
> 
> Sure, will do. Sorry for the inconvenience.
> 
>> (BTW, that's the longest redundant component I ever saw)
> 
> I thought it was better to just list compatible strings inside one
> single file than adding the same binding in multiple files.

I don't understand how this is answers about redundant piece of subject.
Amount of files are not related to repeating pieces of subject prefix.

> 
>>
>>>
>>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
>>> ---
>>>  .../bindings/clock/mtmips-clock.yaml          | 68 +++++++++++++++++++
>>>  1 file changed, 68 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/mtmips-clock.yaml b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>> new file mode 100644
>>> index 000000000000..c92969ce231d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>
>> Filename matching compatible, so vendor prefix and device name (or
>> family of names).
> 
> I used mtmips here but list compatibles starting with ralink. As I
> have said in the cover letter I am inspired by the last merged pinctrl
> series for these SoCs.
> See:
> - https://lore.kernel.org/linux-gpio/e9e6ad87-2db5-9767-ff39-64a302b06185@arinc9.com/T/#t

21 patches, so what exactly I should see (except that I was involved in
that discussions)?

Plus nothing from this thread warrants here exception from naming style.


> 
> Not all of compatible currently exist. 

Then clearly state this.

> All of these are at the end the
> way we can properly match compatible-data to write a proper driver.
> The current ralink dtsi files which are in tree now
> are totally incomplete and not documented so we are planning to align

Nothing like this was said in commit msg, so how can we know?

> all of this with openWRT used files and others soon. That's the reason
> we are not touching
> 'arch/mips/boot/dts' at all now. I don't think anybody is using any of
> this but mt7621 which is properly completed and documented.

Anyway, none of this explains exception from naming convention - vendor,
device or family name.

> 
>>
>>> @@ -0,0 +1,68 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/mtmips-clock.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MTMIPS SoCs Clock
>>
>> One clock? Are you sure these describe exactly one clock?
> 
> I will change this to 'Clocks'.

Then clock provider, but are you sure? You included there syscon and
reset controller.

> 
>>
>>> +
>>> +maintainers:
>>> +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
>>> +
>>> +description: |
>>> +  MediaTek MIPS and Ralink SoCs have an XTAL from where the cpu clock is
>>> +  provided as well as derived clocks for the bus and the peripherals.
>>> +
>>> +  Each clock is assigned an identifier and client nodes use this identifier
>>> +  to specify the clock which they consume.
>>
>> Drop useless or obvious pieces of description. Describe the hardware.
>>
>>> +
>>> +  The clocks are provided inside a system controller node.
> 
>>
>> ???
> 
> I meant, this node is a syscon from where both clock and reset related
> registers are used. I think writing in this way was enough since it
> has a pretty similar description like the one in
> 'mediatek,mt7621-sysc.yaml'.

But what is a system controller node? Some separate device? This is
description for this device - called "Clock" or "Clocks" - and "system
controller" appears for the first time.

> 
>>
>>> +
>>> +  This node is also a reset provider for all the peripherals.
>>
>> ??? Does it mean it is not only "Clock" but also reset controller?
> 
> Yes, this node is a clock and reset controller for all the SoC.
> 
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - ralink,rt2880-sysc
>>> +          - ralink,rt3050-sysc
>>> +          - ralink,rt3052-sysc
>>> +          - ralink,rt3352-sysc
>>> +          - ralink,rt3883-sysc
>>> +          - ralink,rt5350-sysc
>>> +          - ralink,mt7620-sysc
>>> +          - ralink,mt7620a-sysc
>>> +          - ralink,mt7628-sysc'
>>> +          - ralink,mt7688-sysc
>>> +          - ralink,rt2880-reset
>>
>> That's odd. rt2880 is sysc and reset? One device with two compatibles?
> 
> This 'ralink,rt2880-reset' is for compatibility reasons. 

I don't understand why. It is used in DTS, so what this node represents
there?

> Reset related
> code was inside 'arch/mips/ralink' folder reset.c file but it is moved
> to this new driver, so we have maintained this reset stuff for the
> reset compatibility. All of the rest are the new possible stuff for
> both reset and clocks. 

We talk here about hardware, not drivers, so moving driver code around
does not help me understand the rationale behind bindings.

> Clock driver is instantiated in two phases. The
> earlier one set up the clocks via CLK_OF_DECLARE macro. Resets are set
> up as a platform driver. Is only inside this where
> 'ralink,rt2880-reset' is used. See patch 2 of the series for details.

Sure, but it is not related to bindings.

> 
>>
>> Also, order these by name.
> 
> All are ordered but I maintained the  'ralink,rt2880-reset' at the end.

No, it is not. m is before r in alphabet.

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 20.03.2023 20:36, Krzysztof Kozlowski wrote:
> On 20/03/2023 18:24, Sergio Paracuellos wrote:
>> Hi Krzysztof,
>>
>> On Mon, Mar 20, 2023 at 5:36 PM Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 20/03/2023 17:18, Sergio Paracuellos wrote:
>>>> Adds device tree binding documentation for clocks and resets in the
>>>> Mediatek MIPS and Ralink SOCs. This covers RT2880, RT3050, RT3052, RT3350,
>>>> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
>>>
>>> Use subject prefixes matching the subsystem (which you can get for
>>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>>> your patch is touching).
>>>
>>> Subject: drop second/last, redundant "device tree binding
>>> documentation". The "dt-bindings" prefix is already stating that these
>>> are bindings.
>>
>> Sure, will do. Sorry for the inconvenience.
>>
>>> (BTW, that's the longest redundant component I ever saw)
>>
>> I thought it was better to just list compatible strings inside one
>> single file than adding the same binding in multiple files.
> 
> I don't understand how this is answers about redundant piece of subject.
> Amount of files are not related to repeating pieces of subject prefix.
> 
>>
>>>
>>>>
>>>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
>>>> ---
>>>>   .../bindings/clock/mtmips-clock.yaml          | 68 +++++++++++++++++++
>>>>   1 file changed, 68 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/mtmips-clock.yaml b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>>> new file mode 100644
>>>> index 000000000000..c92969ce231d
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
>>>
>>> Filename matching compatible, so vendor prefix and device name (or
>>> family of names).
>>
>> I used mtmips here but list compatibles starting with ralink. As I
>> have said in the cover letter I am inspired by the last merged pinctrl
>> series for these SoCs.
>> See:
>> - https://lore.kernel.org/linux-gpio/e9e6ad87-2db5-9767-ff39-64a302b06185@arinc9.com/T/#t
> 
> 21 patches, so what exactly I should see (except that I was involved in
> that discussions)?
> 
> Plus nothing from this thread warrants here exception from naming style.
> 
> 
>>
>> Not all of compatible currently exist.
> 
> Then clearly state this.
> 
>> All of these are at the end the
>> way we can properly match compatible-data to write a proper driver.
>> The current ralink dtsi files which are in tree now
>> are totally incomplete and not documented so we are planning to align
> 
> Nothing like this was said in commit msg, so how can we know?
> 
>> all of this with openWRT used files and others soon. That's the reason
>> we are not touching
>> 'arch/mips/boot/dts' at all now. I don't think anybody is using any of
>> this but mt7621 which is properly completed and documented.
> 
> Anyway, none of this explains exception from naming convention - vendor,
> device or family name.

Would mediatek,mtmips-clock.yaml make sense?

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 20/03/2023 17:18, Sergio Paracuellos wrote:
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - ralink,rt2880-sysc
> +          - ralink,rt3050-sysc
> +          - ralink,rt3052-sysc
> +          - ralink,rt3352-sysc
> +          - ralink,rt3883-sysc
> +          - ralink,rt5350-sysc
> +          - ralink,mt7620-sysc
> +          - ralink,mt7620a-sysc
> +          - ralink,mt7628-sysc
> +          - ralink,mt7688-sysc

One more comment - this and maybe other compatibles - have wrong vendor
prefix. This is mediatek, not ralink.


Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 20/03/2023 18:57, Arınç ÜNAL wrote:
>>> All of these are at the end the
>>> way we can properly match compatible-data to write a proper driver.
>>> The current ralink dtsi files which are in tree now
>>> are totally incomplete and not documented so we are planning to align
>>
>> Nothing like this was said in commit msg, so how can we know?
>>
>>> all of this with openWRT used files and others soon. That's the reason
>>> we are not touching
>>> 'arch/mips/boot/dts' at all now. I don't think anybody is using any of
>>> this but mt7621 which is properly completed and documented.
>>
>> Anyway, none of this explains exception from naming convention - vendor,
>> device or family name.
> 
> Would mediatek,mtmips-clock.yaml make sense?

More, except:
1. This is not clock, but sysc.
2. mips sounds redundant. Do you have rt2xxx and mt7xxx chips which are ARM?

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 20.03.2023 21:01, Krzysztof Kozlowski wrote:
> On 20/03/2023 17:18, Sergio Paracuellos wrote:
>> +properties:
>> +  compatible:
>> +    items:
>> +      - enum:
>> +          - ralink,rt2880-sysc
>> +          - ralink,rt3050-sysc
>> +          - ralink,rt3052-sysc
>> +          - ralink,rt3352-sysc
>> +          - ralink,rt3883-sysc
>> +          - ralink,rt5350-sysc
>> +          - ralink,mt7620-sysc
>> +          - ralink,mt7620a-sysc
>> +          - ralink,mt7628-sysc
>> +          - ralink,mt7688-sysc
> 
> One more comment - this and maybe other compatibles - have wrong vendor
> prefix. This is mediatek, not ralink.

This platform was acquired from Ralink by MediaTek. I couldn't change 
some existing ralink compatible strings to mediatek as Rob explained on 
my pinctrl patch series that we don't do that. The compatible strings on 
this patch series here are new but I'd rather keep the compatible 
strings ralink to keep things consistent.

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 20.03.2023 21:02, Krzysztof Kozlowski wrote:
> On 20/03/2023 18:57, Arınç ÜNAL wrote:
>>>> All of these are at the end the
>>>> way we can properly match compatible-data to write a proper driver.
>>>> The current ralink dtsi files which are in tree now
>>>> are totally incomplete and not documented so we are planning to align
>>>
>>> Nothing like this was said in commit msg, so how can we know?
>>>
>>>> all of this with openWRT used files and others soon. That's the reason
>>>> we are not touching
>>>> 'arch/mips/boot/dts' at all now. I don't think anybody is using any of
>>>> this but mt7621 which is properly completed and documented.
>>>
>>> Anyway, none of this explains exception from naming convention - vendor,
>>> device or family name.
>>
>> Would mediatek,mtmips-clock.yaml make sense?
> 
> More, except:
> 1. This is not clock, but sysc.

Sergio, beware.

> 2. mips sounds redundant. Do you have rt2xxx and mt7xxx chips which are ARM?

All of the SoCs, RTXXXX, MT7620, MT7621, MT7628, MT7688 are MIPS. So I 
decided to call this platform MTMIPS as I've seen MediaTek use this on 
other projects like U-Boot. This is what I did on my pinctrl patch 
series as well.

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 20/03/2023 19:07, Arınç ÜNAL wrote:
> On 20.03.2023 21:01, Krzysztof Kozlowski wrote:
>> On 20/03/2023 17:18, Sergio Paracuellos wrote:
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - ralink,rt2880-sysc
>>> +          - ralink,rt3050-sysc
>>> +          - ralink,rt3052-sysc
>>> +          - ralink,rt3352-sysc
>>> +          - ralink,rt3883-sysc
>>> +          - ralink,rt5350-sysc
>>> +          - ralink,mt7620-sysc
>>> +          - ralink,mt7620a-sysc
>>> +          - ralink,mt7628-sysc
>>> +          - ralink,mt7688-sysc
>>
>> One more comment - this and maybe other compatibles - have wrong vendor
>> prefix. This is mediatek, not ralink.
> 
> This platform was acquired from Ralink by MediaTek. I couldn't change 
> some existing ralink compatible strings to mediatek as Rob explained on 
> my pinctrl patch series that we don't do that. The compatible strings on 
> this patch series here are new but I'd rather keep the compatible 
> strings ralink to keep things consistent.

The comment that you cannot change existing compatibles does not apply
to these, because these are new. However indeed some SoCs have already
compatibles with ralink, so it's fine for these. mt7620 and mt7628 are
already used with mediatek, so these should be rather corrected to new
prefix.

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 20/03/2023 19:09, Arınç ÜNAL wrote:
>>> Would mediatek,mtmips-clock.yaml make sense?
>>
>> More, except:
>> 1. This is not clock, but sysc.
> 
> Sergio, beware.

I meant, that's what I understood from what Sergio said. :)

> 
>> 2. mips sounds redundant. Do you have rt2xxx and mt7xxx chips which are ARM?
> 
> All of the SoCs, RTXXXX, MT7620, MT7621, MT7628, MT7688 are MIPS. So I 
> decided to call this platform MTMIPS as I've seen MediaTek use this on 
> other projects like U-Boot. This is what I did on my pinctrl patch 
> series as well.

Ah, but indeed there are newer Mediatek MT6xxx and MT8xxx SoCs which are
ARM, so mediatek,mtmips-sysc would work.

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 20.03.2023 21:11, Krzysztof Kozlowski wrote:
> On 20/03/2023 19:07, Arınç ÜNAL wrote:
>> On 20.03.2023 21:01, Krzysztof Kozlowski wrote:
>>> On 20/03/2023 17:18, Sergio Paracuellos wrote:
>>>> +properties:
>>>> +  compatible:
>>>> +    items:
>>>> +      - enum:
>>>> +          - ralink,rt2880-sysc
>>>> +          - ralink,rt3050-sysc
>>>> +          - ralink,rt3052-sysc
>>>> +          - ralink,rt3352-sysc
>>>> +          - ralink,rt3883-sysc
>>>> +          - ralink,rt5350-sysc
>>>> +          - ralink,mt7620-sysc
>>>> +          - ralink,mt7620a-sysc
>>>> +          - ralink,mt7628-sysc
>>>> +          - ralink,mt7688-sysc
>>>
>>> One more comment - this and maybe other compatibles - have wrong vendor
>>> prefix. This is mediatek, not ralink.
>>
>> This platform was acquired from Ralink by MediaTek. I couldn't change
>> some existing ralink compatible strings to mediatek as Rob explained on
>> my pinctrl patch series that we don't do that. The compatible strings on
>> this patch series here are new but I'd rather keep the compatible
>> strings ralink to keep things consistent.
> 
> The comment that you cannot change existing compatibles does not apply
> to these, because these are new. However indeed some SoCs have already
> compatibles with ralink, so it's fine for these. mt7620 and mt7628 are
> already used with mediatek, so these should be rather corrected to new
> prefix.

If you're talking about the pinctrl schemas for MT7620 and MT7628, it's 
just the name of the yaml files that have mediatek. The compatible 
string is still ralink so it should be kept ralink here as well.

Arınç

Re: [PATCH 07/10] mips: ralink: remove clock related function prototypes

[Stephen Boyd]

Quoting Sergio Paracuellos (2023-03-20 09:18:20)
> Clock related code has been removed from 'arch/mips/ralink' folder and put
> into drivers space. Hence remove clock related prototypes which are not
> used anymore.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  arch/mips/ralink/common.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
> index 87fc16751281..fcdfc9dc6210 100644
> --- a/arch/mips/ralink/common.h
> +++ b/arch/mips/ralink/common.h
> @@ -23,9 +23,6 @@ extern struct ralink_soc_info soc_info;
>  
>  extern void ralink_of_remap(void);
>  
> -extern void ralink_clk_init(void);

Why isn't this removed in the patch that removes the function?

> -extern void ralink_clk_add(const char *dev, unsigned long rate);
> -

Same comment.

>  extern void ralink_rst_init(void);
>

Re: [PATCH 07/10] mips: ralink: remove clock related function prototypes

[Sergio Paracuellos]

Hi Stephen,

On Mon, Mar 20, 2023 at 8:38 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2023-03-20 09:18:20)
> > Clock related code has been removed from 'arch/mips/ralink' folder and put
> > into drivers space. Hence remove clock related prototypes which are not
> > used anymore.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  arch/mips/ralink/common.h | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
> > index 87fc16751281..fcdfc9dc6210 100644
> > --- a/arch/mips/ralink/common.h
> > +++ b/arch/mips/ralink/common.h
> > @@ -23,9 +23,6 @@ extern struct ralink_soc_info soc_info;
> >
> >  extern void ralink_of_remap(void);
> >
> > -extern void ralink_clk_init(void);
>
> Why isn't this removed in the patch that removes the function?

Because the function exists for all the SoCs code and there are
several patches removing it; one per SoC, so I decided to remove this
at the end. Should I squash all patches together instead?

>
> > -extern void ralink_clk_add(const char *dev, unsigned long rate);
> > -
>
> Same comment.
>
> >  extern void ralink_rst_init(void);
> >

Best regards,
    Sergio Paracuellos

Re: [PATCH 07/10] mips: ralink: remove clock related function prototypes

[Stephen Boyd]

Quoting Sergio Paracuellos (2023-03-20 13:17:47)
> Hi Stephen,
> 
> On Mon, Mar 20, 2023 at 8:38 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Sergio Paracuellos (2023-03-20 09:18:20)
> > > Clock related code has been removed from 'arch/mips/ralink' folder and put
> > > into drivers space. Hence remove clock related prototypes which are not
> > > used anymore.
> > >
> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > ---
> > >  arch/mips/ralink/common.h | 3 ---
> > >  1 file changed, 3 deletions(-)
> > >
> > > diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
> > > index 87fc16751281..fcdfc9dc6210 100644
> > > --- a/arch/mips/ralink/common.h
> > > +++ b/arch/mips/ralink/common.h
> > > @@ -23,9 +23,6 @@ extern struct ralink_soc_info soc_info;
> > >
> > >  extern void ralink_of_remap(void);
> > >
> > > -extern void ralink_clk_init(void);
> >
> > Why isn't this removed in the patch that removes the function?
> 
> Because the function exists for all the SoCs code and there are
> several patches removing it; one per SoC, so I decided to remove this
> at the end. Should I squash all patches together instead?

No. But you should squash this with whatever patch removes the last one.

Re: [PATCH 07/10] mips: ralink: remove clock related function prototypes

[Sergio Paracuellos]

On Mon, Mar 20, 2023 at 10:21 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2023-03-20 13:17:47)
> > Hi Stephen,
> >
> > On Mon, Mar 20, 2023 at 8:38 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Sergio Paracuellos (2023-03-20 09:18:20)
> > > > Clock related code has been removed from 'arch/mips/ralink' folder and put
> > > > into drivers space. Hence remove clock related prototypes which are not
> > > > used anymore.
> > > >
> > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > > ---
> > > >  arch/mips/ralink/common.h | 3 ---
> > > >  1 file changed, 3 deletions(-)
> > > >
> > > > diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
> > > > index 87fc16751281..fcdfc9dc6210 100644
> > > > --- a/arch/mips/ralink/common.h
> > > > +++ b/arch/mips/ralink/common.h
> > > > @@ -23,9 +23,6 @@ extern struct ralink_soc_info soc_info;
> > > >
> > > >  extern void ralink_of_remap(void);
> > > >
> > > > -extern void ralink_clk_init(void);
> > >
> > > Why isn't this removed in the patch that removes the function?
> >
> > Because the function exists for all the SoCs code and there are
> > several patches removing it; one per SoC, so I decided to remove this
> > at the end. Should I squash all patches together instead?
>
> No. But you should squash this with whatever patch removes the last one.

Ah, ok. I see your point. I will squash this with the last removal, then.

Thanks,
    Sergio Paracuellos

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Sergio Paracuellos]

On Mon, Mar 20, 2023 at 6:36 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/03/2023 18:24, Sergio Paracuellos wrote:
> > Hi Krzysztof,
> >
> > On Mon, Mar 20, 2023 at 5:36 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 20/03/2023 17:18, Sergio Paracuellos wrote:
> >>> Adds device tree binding documentation for clocks and resets in the
> >>> Mediatek MIPS and Ralink SOCs. This covers RT2880, RT3050, RT3052, RT3350,
> >>> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
> >>
> >> Use subject prefixes matching the subsystem (which you can get for
> >> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> >> your patch is touching).
> >>
> >> Subject: drop second/last, redundant "device tree binding
> >> documentation". The "dt-bindings" prefix is already stating that these
> >> are bindings.
> >
> > Sure, will do. Sorry for the inconvenience.
> >
> >> (BTW, that's the longest redundant component I ever saw)
> >
> > I thought it was better to just list compatible strings inside one
> > single file than adding the same binding in multiple files.
>
> I don't understand how this is answers about redundant piece of subject.
> Amount of files are not related to repeating pieces of subject prefix.

True :).

>
> >
> >>
> >>>
> >>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> >>> ---
> >>>  .../bindings/clock/mtmips-clock.yaml          | 68 +++++++++++++++++++
> >>>  1 file changed, 68 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/clock/mtmips-clock.yaml b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
> >>> new file mode 100644
> >>> index 000000000000..c92969ce231d
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml
> >>
> >> Filename matching compatible, so vendor prefix and device name (or
> >> family of names).
> >
> > I used mtmips here but list compatibles starting with ralink. As I
> > have said in the cover letter I am inspired by the last merged pinctrl
> > series for these SoCs.
> > See:
> > - https://lore.kernel.org/linux-gpio/e9e6ad87-2db5-9767-ff39-64a302b06185@arinc9.com/T/#t
>
> 21 patches, so what exactly I should see (except that I was involved in
> that discussions)?
>
> Plus nothing from this thread warrants here exception from naming style.
>
>
> >
> > Not all of compatible currently exist.
>
> Then clearly state this.

Sure. Will do in the commit message.

>
> > All of these are at the end the
> > way we can properly match compatible-data to write a proper driver.
> > The current ralink dtsi files which are in tree now
> > are totally incomplete and not documented so we are planning to align
>
> Nothing like this was said in commit msg, so how can we know?

Will add a comment about this also.

>
> > all of this with openWRT used files and others soon. That's the reason
> > we are not touching
> > 'arch/mips/boot/dts' at all now. I don't think anybody is using any of
> > this but mt7621 which is properly completed and documented.
>
> Anyway, none of this explains exception from naming convention - vendor,
> device or family name.
>
> >
> >>
> >>> @@ -0,0 +1,68 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/clock/mtmips-clock.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: MTMIPS SoCs Clock
> >>
> >> One clock? Are you sure these describe exactly one clock?
> >
> > I will change this to 'Clocks'.
>
> Then clock provider, but are you sure? You included there syscon and
> reset controller.

You are right. This is a system controller node even though it
provides clocks and resets for the rest of the world.

>
> >
> >>
> >>> +
> >>> +maintainers:
> >>> +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> >>> +
> >>> +description: |
> >>> +  MediaTek MIPS and Ralink SoCs have an XTAL from where the cpu clock is
> >>> +  provided as well as derived clocks for the bus and the peripherals.
> >>> +
> >>> +  Each clock is assigned an identifier and client nodes use this identifier
> >>> +  to specify the clock which they consume.
> >>
> >> Drop useless or obvious pieces of description. Describe the hardware.
> >>
> >>> +
> >>> +  The clocks are provided inside a system controller node.
> >
> >>
> >> ???
> >
> > I meant, this node is a syscon from where both clock and reset related
> > registers are used. I think writing in this way was enough since it
> > has a pretty similar description like the one in
> > 'mediatek,mt7621-sysc.yaml'.
>
> But what is a system controller node? Some separate device? This is
> description for this device - called "Clock" or "Clocks" - and "system
> controller" appears for the first time.
>
> >
> >>
> >>> +
> >>> +  This node is also a reset provider for all the peripherals.
> >>
> >> ??? Does it mean it is not only "Clock" but also reset controller?
> >
> > Yes, this node is a clock and reset controller for all the SoC.
> >
> >>
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    items:
> >>> +      - enum:
> >>> +          - ralink,rt2880-sysc
> >>> +          - ralink,rt3050-sysc
> >>> +          - ralink,rt3052-sysc
> >>> +          - ralink,rt3352-sysc
> >>> +          - ralink,rt3883-sysc
> >>> +          - ralink,rt5350-sysc
> >>> +          - ralink,mt7620-sysc
> >>> +          - ralink,mt7620a-sysc
> >>> +          - ralink,mt7628-sysc'
> >>> +          - ralink,mt7688-sysc
> >>> +          - ralink,rt2880-reset
> >>
> >> That's odd. rt2880 is sysc and reset? One device with two compatibles?
> >
> > This 'ralink,rt2880-reset' is for compatibility reasons.
>
> I don't understand why. It is used in DTS, so what this node represents
> there?

True, this only has to be present in driver code. Not related to
bindings at all. Will drop it, then.

>
> > Reset related
> > code was inside 'arch/mips/ralink' folder reset.c file but it is moved
> > to this new driver, so we have maintained this reset stuff for the
> > reset compatibility. All of the rest are the new possible stuff for
> > both reset and clocks.
>
> We talk here about hardware, not drivers, so moving driver code around
> does not help me understand the rationale behind bindings.

Understood.

>
> > Clock driver is instantiated in two phases. The
> > earlier one set up the clocks via CLK_OF_DECLARE macro. Resets are set
> > up as a platform driver. Is only inside this where
> > 'ralink,rt2880-reset' is used. See patch 2 of the series for details.
>
> Sure, but it is not related to bindings.
>
> >
> >>
> >> Also, order these by name.
> >
> > All are ordered but I maintained the  'ralink,rt2880-reset' at the end.
>
> No, it is not. m is before r in alphabet.

In my mind all of these were ordered taking into account the year
since they exist :). So you are right. I will order this
alphabetically.

>
> Best regards,
> Krzysztof
>

Thanks,
    Sergio Paracuellos

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Sergio Paracuellos]

On Mon, Mar 20, 2023 at 7:15 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/03/2023 19:09, Arınç ÜNAL wrote:
> >>> Would mediatek,mtmips-clock.yaml make sense?
> >>
> >> More, except:
> >> 1. This is not clock, but sysc.
> >
> > Sergio, beware.
>
> I meant, that's what I understood from what Sergio said. :)

Yes, you understood properly. I will use 'sysc' instead.

>
> >
> >> 2. mips sounds redundant. Do you have rt2xxx and mt7xxx chips which are ARM?
> >
> > All of the SoCs, RTXXXX, MT7620, MT7621, MT7628, MT7688 are MIPS. So I
> > decided to call this platform MTMIPS as I've seen MediaTek use this on
> > other projects like U-Boot. This is what I did on my pinctrl patch
> > series as well.
>
> Ah, but indeed there are newer Mediatek MT6xxx and MT8xxx SoCs which are
> ARM, so mediatek,mtmips-sysc would work.

I can use 'mediatek,mtmips-sysc.yaml' as the name but compatibles will
start with ralink. There are already some existent compatibles for
mt762x already having ralink as prefix, so to be coherent ralink
should be maintained as prefix.

>
> Best regards,
> Krzysztof
>

Thanks,
   Sergio Paracuellos

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 21/03/2023 05:34, Sergio Paracuellos wrote:
> On Mon, Mar 20, 2023 at 7:15 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 20/03/2023 19:09, Arınç ÜNAL wrote:
>>>>> Would mediatek,mtmips-clock.yaml make sense?
>>>>
>>>> More, except:
>>>> 1. This is not clock, but sysc.
>>>
>>> Sergio, beware.
>>
>> I meant, that's what I understood from what Sergio said. :)
> 
> Yes, you understood properly. I will use 'sysc' instead.
> 
>>
>>>
>>>> 2. mips sounds redundant. Do you have rt2xxx and mt7xxx chips which are ARM?
>>>
>>> All of the SoCs, RTXXXX, MT7620, MT7621, MT7628, MT7688 are MIPS. So I
>>> decided to call this platform MTMIPS as I've seen MediaTek use this on
>>> other projects like U-Boot. This is what I did on my pinctrl patch
>>> series as well.
>>
>> Ah, but indeed there are newer Mediatek MT6xxx and MT8xxx SoCs which are
>> ARM, so mediatek,mtmips-sysc would work.
> 
> I can use 'mediatek,mtmips-sysc.yaml' as the name but compatibles will
> start with ralink. There are already some existent compatibles for
> mt762x already having ralink as prefix, so to be coherent ralink
> should be maintained as prefix.

The compatibles I mentioned start already with mediatek, so why do you
want to introduce incorrect vendor name for these?

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 20/03/2023 19:23, Arınç ÜNAL wrote:
> On 20.03.2023 21:11, Krzysztof Kozlowski wrote:
>> On 20/03/2023 19:07, Arınç ÜNAL wrote:
>>> On 20.03.2023 21:01, Krzysztof Kozlowski wrote:
>>>> On 20/03/2023 17:18, Sergio Paracuellos wrote:
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    items:
>>>>> +      - enum:
>>>>> +          - ralink,rt2880-sysc
>>>>> +          - ralink,rt3050-sysc
>>>>> +          - ralink,rt3052-sysc
>>>>> +          - ralink,rt3352-sysc
>>>>> +          - ralink,rt3883-sysc
>>>>> +          - ralink,rt5350-sysc
>>>>> +          - ralink,mt7620-sysc
>>>>> +          - ralink,mt7620a-sysc
>>>>> +          - ralink,mt7628-sysc
>>>>> +          - ralink,mt7688-sysc
>>>>
>>>> One more comment - this and maybe other compatibles - have wrong vendor
>>>> prefix. This is mediatek, not ralink.
>>>
>>> This platform was acquired from Ralink by MediaTek. I couldn't change
>>> some existing ralink compatible strings to mediatek as Rob explained on
>>> my pinctrl patch series that we don't do that. The compatible strings on
>>> this patch series here are new but I'd rather keep the compatible
>>> strings ralink to keep things consistent.
>>
>> The comment that you cannot change existing compatibles does not apply
>> to these, because these are new. However indeed some SoCs have already
>> compatibles with ralink, so it's fine for these. mt7620 and mt7628 are
>> already used with mediatek, so these should be rather corrected to new
>> prefix.
> 
> If you're talking about the pinctrl schemas for MT7620 and MT7628, it's 
> just the name of the yaml files that have mediatek. The compatible 
> string is still ralink so it should be kept ralink here as well.

No, I am talking about compatibles.

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 21.03.2023 09:32, Krzysztof Kozlowski wrote:
> On 21/03/2023 05:34, Sergio Paracuellos wrote:
>> On Mon, Mar 20, 2023 at 7:15 PM Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 20/03/2023 19:09, Arınç ÜNAL wrote:
>>>>>> Would mediatek,mtmips-clock.yaml make sense?
>>>>>
>>>>> More, except:
>>>>> 1. This is not clock, but sysc.
>>>>
>>>> Sergio, beware.
>>>
>>> I meant, that's what I understood from what Sergio said. :)
>>
>> Yes, you understood properly. I will use 'sysc' instead.
>>
>>>
>>>>
>>>>> 2. mips sounds redundant. Do you have rt2xxx and mt7xxx chips which are ARM?
>>>>
>>>> All of the SoCs, RTXXXX, MT7620, MT7621, MT7628, MT7688 are MIPS. So I
>>>> decided to call this platform MTMIPS as I've seen MediaTek use this on
>>>> other projects like U-Boot. This is what I did on my pinctrl patch
>>>> series as well.
>>>
>>> Ah, but indeed there are newer Mediatek MT6xxx and MT8xxx SoCs which are
>>> ARM, so mediatek,mtmips-sysc would work.
>>
>> I can use 'mediatek,mtmips-sysc.yaml' as the name but compatibles will
>> start with ralink. There are already some existent compatibles for
>> mt762x already having ralink as prefix, so to be coherent ralink
>> should be maintained as prefix.
> 
> The compatibles I mentioned start already with mediatek, so why do you
> want to introduce incorrect vendor name for these?

Can you point out where these compatible strings for mt7620 and mt7628 are?

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 21/03/2023 07:38, Arınç ÜNAL wrote:
>>>>
>>>> Ah, but indeed there are newer Mediatek MT6xxx and MT8xxx SoCs which are
>>>> ARM, so mediatek,mtmips-sysc would work.
>>>
>>> I can use 'mediatek,mtmips-sysc.yaml' as the name but compatibles will
>>> start with ralink. There are already some existent compatibles for
>>> mt762x already having ralink as prefix, so to be coherent ralink
>>> should be maintained as prefix.
>>
>> The compatibles I mentioned start already with mediatek, so why do you
>> want to introduce incorrect vendor name for these?
> 
> Can you point out where these compatible strings for mt7620 and mt7628 are?

git grep

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Sergio Paracuellos]

On Tue, Mar 21, 2023 at 7:43 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/03/2023 07:38, Arınç ÜNAL wrote:
> >>>>
> >>>> Ah, but indeed there are newer Mediatek MT6xxx and MT8xxx SoCs which are
> >>>> ARM, so mediatek,mtmips-sysc would work.
> >>>
> >>> I can use 'mediatek,mtmips-sysc.yaml' as the name but compatibles will
> >>> start with ralink. There are already some existent compatibles for
> >>> mt762x already having ralink as prefix, so to be coherent ralink
> >>> should be maintained as prefix.
> >>
> >> The compatibles I mentioned start already with mediatek, so why do you
> >> want to introduce incorrect vendor name for these?
> >
> > Can you point out where these compatible strings for mt7620 and mt7628 are?
>
> git grep

Not for *-sysc nodes. The only current one in use (from git grep):

arch/mips/ralink/mt7620.c:      rt_sysc_membase =
plat_of_remap_node("ralink,mt7620a-sysc");

That's the reason I also used prefix ralink for the rest.

Does it make sense to you to maintain this one as ralink,mt7620a-sysc
and add the following with mediatek prefix?

mediatek,mt7620-sysc
mediatek,mt7628-sysc
mediatek,mt7688-sysc

That would be weird IMHO.

> Best regards,
> Krzysztof

Thanks,
    Sergio Paracuellos

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 21/03/2023 07:56, Sergio Paracuellos wrote:
> On Tue, Mar 21, 2023 at 7:43 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 21/03/2023 07:38, Arınç ÜNAL wrote:
>>>>>>
>>>>>> Ah, but indeed there are newer Mediatek MT6xxx and MT8xxx SoCs which are
>>>>>> ARM, so mediatek,mtmips-sysc would work.
>>>>>
>>>>> I can use 'mediatek,mtmips-sysc.yaml' as the name but compatibles will
>>>>> start with ralink. There are already some existent compatibles for
>>>>> mt762x already having ralink as prefix, so to be coherent ralink
>>>>> should be maintained as prefix.
>>>>
>>>> The compatibles I mentioned start already with mediatek, so why do you
>>>> want to introduce incorrect vendor name for these?
>>>
>>> Can you point out where these compatible strings for mt7620 and mt7628 are?
>>
>> git grep
> 
> Not for *-sysc nodes. The only current one in use (from git grep):

We do not talk about sysc nodes at all. They do not matter.

> 
> arch/mips/ralink/mt7620.c:      rt_sysc_membase =
> plat_of_remap_node("ralink,mt7620a-sysc");
> 
> That's the reason I also used prefix ralink for the rest.
> 
> Does it make sense to you to maintain this one as ralink,mt7620a-sysc
> and add the following with mediatek prefix?
> 
> mediatek,mt7620-sysc
> mediatek,mt7628-sysc
> mediatek,mt7688-sysc
> 
> That would be weird IMHO.

What exactly would be weird? Did you read the discussion about vendor
prefix from Arinc? mt7620 is not a Ralink product, so what would be
weird is to use "ralink" vendor prefix. This was never a Ralink. However
since there are compatibles using "ralink" for non-ralink devices, we
agreed not to change them.

These though use at least in one place mediatek, so the above argument
does not apply. (and before you say "but they also use ralink and
mediatek", it does not matter - it is already inconsistent thus we can
choose whatever we want and ralink is not correct).


Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Sergio Paracuellos]

On Tue, Mar 21, 2023 at 8:20 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/03/2023 07:56, Sergio Paracuellos wrote:
> > On Tue, Mar 21, 2023 at 7:43 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 21/03/2023 07:38, Arınç ÜNAL wrote:
> >>>>>>
> >>>>>> Ah, but indeed there are newer Mediatek MT6xxx and MT8xxx SoCs which are
> >>>>>> ARM, so mediatek,mtmips-sysc would work.
> >>>>>
> >>>>> I can use 'mediatek,mtmips-sysc.yaml' as the name but compatibles will
> >>>>> start with ralink. There are already some existent compatibles for
> >>>>> mt762x already having ralink as prefix, so to be coherent ralink
> >>>>> should be maintained as prefix.
> >>>>
> >>>> The compatibles I mentioned start already with mediatek, so why do you
> >>>> want to introduce incorrect vendor name for these?
> >>>
> >>> Can you point out where these compatible strings for mt7620 and mt7628 are?
> >>
> >> git grep
> >
> > Not for *-sysc nodes. The only current one in use (from git grep):
>
> We do not talk about sysc nodes at all. They do not matter.

Ah, ok. That reason was from where all of my arguments were coming
from. But if it does not matter, I don't have problems using the
'mediatek' prefix in the new stuff.

>
> >
> > arch/mips/ralink/mt7620.c:      rt_sysc_membase =
> > plat_of_remap_node("ralink,mt7620a-sysc");
> >
> > That's the reason I also used prefix ralink for the rest.
> >
> > Does it make sense to you to maintain this one as ralink,mt7620a-sysc
> > and add the following with mediatek prefix?
> >
> > mediatek,mt7620-sysc
> > mediatek,mt7628-sysc
> > mediatek,mt7688-sysc
> >
> > That would be weird IMHO.
>
> What exactly would be weird? Did you read the discussion about vendor
> prefix from Arinc? mt7620 is not a Ralink product, so what would be
> weird is to use "ralink" vendor prefix. This was never a Ralink. However
> since there are compatibles using "ralink" for non-ralink devices, we
> agreed not to change them.
>
> These though use at least in one place mediatek, so the above argument
> does not apply. (and before you say "but they also use ralink and
> mediatek", it does not matter - it is already inconsistent thus we can
> choose whatever we want and ralink is not correct).

Ok, I see your point now. Thanks for clarification. I will maintain
'ralink,mt7620a-sysc' since it already exists and change the new stuff
to use mediatek as prefix. These are 'mediatek,mt7620-sysc',
'mediatek,mt7628-sysc' and 'mediatek,mt7688-sysc'. Doing so it will
properly match the 'mediatek,mtmips-sysc.yaml' file name.

>
>
> Best regards,
> Krzysztof
>

Thanks,
    Sergio Paracuellos

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 21.03.2023 10:19, Krzysztof Kozlowski wrote:
> On 21/03/2023 07:56, Sergio Paracuellos wrote:
>> On Tue, Mar 21, 2023 at 7:43 AM Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 21/03/2023 07:38, Arınç ÜNAL wrote:
>>>>>>>
>>>>>>> Ah, but indeed there are newer Mediatek MT6xxx and MT8xxx SoCs which are
>>>>>>> ARM, so mediatek,mtmips-sysc would work.
>>>>>>
>>>>>> I can use 'mediatek,mtmips-sysc.yaml' as the name but compatibles will
>>>>>> start with ralink. There are already some existent compatibles for
>>>>>> mt762x already having ralink as prefix, so to be coherent ralink
>>>>>> should be maintained as prefix.
>>>>>
>>>>> The compatibles I mentioned start already with mediatek, so why do you
>>>>> want to introduce incorrect vendor name for these?
>>>>
>>>> Can you point out where these compatible strings for mt7620 and mt7628 are?
>>>
>>> git grep
>>
>> Not for *-sysc nodes. The only current one in use (from git grep):
> 
> We do not talk about sysc nodes at all. They do not matter.
> 
>>
>> arch/mips/ralink/mt7620.c:      rt_sysc_membase =
>> plat_of_remap_node("ralink,mt7620a-sysc");
>>
>> That's the reason I also used prefix ralink for the rest.
>>
>> Does it make sense to you to maintain this one as ralink,mt7620a-sysc
>> and add the following with mediatek prefix?
>>
>> mediatek,mt7620-sysc
>> mediatek,mt7628-sysc
>> mediatek,mt7688-sysc
>>
>> That would be weird IMHO.
> 
> What exactly would be weird? Did you read the discussion about vendor
> prefix from Arinc? mt7620 is not a Ralink product, so what would be
> weird is to use "ralink" vendor prefix. This was never a Ralink. However
> since there are compatibles using "ralink" for non-ralink devices, we
> agreed not to change them.
> 
> These though use at least in one place mediatek, so the above argument
> does not apply. (and before you say "but they also use ralink and
> mediatek", it does not matter - it is already inconsistent thus we can
> choose whatever we want and ralink is not correct).

My argument was that your point being Ralink is now Mediatek, thus there 
is no conflict and no issues with different vendor used. It's the next 
best thing to be able to address the inconsistency, call everything of 
the MTMIPS platform ralink on the compatible strings.

If we take the calling new things mediatek route, we will never get to 
the bottom of fixing the naming inconsistency.

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 21/03/2023 08:39, Arınç ÜNAL wrote:
>>>
>>> arch/mips/ralink/mt7620.c:      rt_sysc_membase =
>>> plat_of_remap_node("ralink,mt7620a-sysc");
>>>
>>> That's the reason I also used prefix ralink for the rest.
>>>
>>> Does it make sense to you to maintain this one as ralink,mt7620a-sysc
>>> and add the following with mediatek prefix?
>>>
>>> mediatek,mt7620-sysc
>>> mediatek,mt7628-sysc
>>> mediatek,mt7688-sysc
>>>
>>> That would be weird IMHO.
>>
>> What exactly would be weird? Did you read the discussion about vendor
>> prefix from Arinc? mt7620 is not a Ralink product, so what would be
>> weird is to use "ralink" vendor prefix. This was never a Ralink. However
>> since there are compatibles using "ralink" for non-ralink devices, we
>> agreed not to change them.
>>
>> These though use at least in one place mediatek, so the above argument
>> does not apply. (and before you say "but they also use ralink and
>> mediatek", it does not matter - it is already inconsistent thus we can
>> choose whatever we want and ralink is not correct).
> 
> My argument was that your point being Ralink is now Mediatek, thus there 
> is no conflict and no issues with different vendor used. It's the next 
> best thing to be able to address the inconsistency, call everything of 
> the MTMIPS platform ralink on the compatible strings.

And how does it help consistency? The mt7620 is used also with mediatek
prefix and adding more variants of realtek does not make the
inconsistency smaller. It's still inconsistent.

> 
> If we take the calling new things mediatek route, we will never get to 
> the bottom of fixing the naming inconsistency.

All new things, so new SoCs, should be called mediatek, because there is
no ralink and mediatek is already used for them. So why some new
Mediatek SoCs are "mediatek" but some other also new SoCs are "ralink"?

You can do nothing (and no actual need) about existing inconsistency...


Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 21.03.2023 11:04, Krzysztof Kozlowski wrote:
> On 21/03/2023 08:39, Arınç ÜNAL wrote:
>>>>
>>>> arch/mips/ralink/mt7620.c:      rt_sysc_membase =
>>>> plat_of_remap_node("ralink,mt7620a-sysc");
>>>>
>>>> That's the reason I also used prefix ralink for the rest.
>>>>
>>>> Does it make sense to you to maintain this one as ralink,mt7620a-sysc
>>>> and add the following with mediatek prefix?
>>>>
>>>> mediatek,mt7620-sysc
>>>> mediatek,mt7628-sysc
>>>> mediatek,mt7688-sysc
>>>>
>>>> That would be weird IMHO.
>>>
>>> What exactly would be weird? Did you read the discussion about vendor
>>> prefix from Arinc? mt7620 is not a Ralink product, so what would be
>>> weird is to use "ralink" vendor prefix. This was never a Ralink. However
>>> since there are compatibles using "ralink" for non-ralink devices, we
>>> agreed not to change them.
>>>
>>> These though use at least in one place mediatek, so the above argument
>>> does not apply. (and before you say "but they also use ralink and
>>> mediatek", it does not matter - it is already inconsistent thus we can
>>> choose whatever we want and ralink is not correct).
>>
>> My argument was that your point being Ralink is now Mediatek, thus there
>> is no conflict and no issues with different vendor used. It's the next
>> best thing to be able to address the inconsistency, call everything of
>> the MTMIPS platform ralink on the compatible strings.
> 
> And how does it help consistency? The mt7620 is used also with mediatek
> prefix and adding more variants of realtek does not make the
> inconsistency smaller. It's still inconsistent.
> 
>>
>> If we take the calling new things mediatek route, we will never get to
>> the bottom of fixing the naming inconsistency.
> 
> All new things, so new SoCs, should be called mediatek, because there is
> no ralink and mediatek is already used for them. So why some new
> Mediatek SoCs are "mediatek" but some other also new SoCs are "ralink"?
> 
> You can do nothing (and no actual need) about existing inconsistency...

I couldn't change ralink -> mediatek because company acquisitions don't 
grant the change. I don't see any reason to prevent changing mediatek -> 
ralink without breaking the ABI on the existing schemas.

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 21/03/2023 09:24, Arınç ÜNAL wrote:
>>>
>>> If we take the calling new things mediatek route, we will never get to
>>> the bottom of fixing the naming inconsistency.
>>
>> All new things, so new SoCs, should be called mediatek, because there is
>> no ralink and mediatek is already used for them. So why some new
>> Mediatek SoCs are "mediatek" but some other also new SoCs are "ralink"?
>>
>> You can do nothing (and no actual need) about existing inconsistency...
> 
> I couldn't change ralink -> mediatek because company acquisitions don't 
> grant the change. I don't see any reason to prevent changing mediatek -> 
> ralink without breaking the ABI on the existing schemas.

You cannot change mediatek->ralink without breaking the ABI for the same
reasons.

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 21.03.2023 11:27, Krzysztof Kozlowski wrote:
> On 21/03/2023 09:24, Arınç ÜNAL wrote:
>>>>
>>>> If we take the calling new things mediatek route, we will never get to
>>>> the bottom of fixing the naming inconsistency.
>>>
>>> All new things, so new SoCs, should be called mediatek, because there is
>>> no ralink and mediatek is already used for them. So why some new
>>> Mediatek SoCs are "mediatek" but some other also new SoCs are "ralink"?
>>>
>>> You can do nothing (and no actual need) about existing inconsistency...
>>
>> I couldn't change ralink -> mediatek because company acquisitions don't
>> grant the change. I don't see any reason to prevent changing mediatek ->
>> ralink without breaking the ABI on the existing schemas.
> 
> You cannot change mediatek->ralink without breaking the ABI for the same
> reasons.

Then this is where I ask for an exception.

The current solution only complicates things more.

https://github.com/paraka/linux/pull/1/files#diff-0ae6c456898d08536ce987c32f23f2eb6f4a0f7c38bff9a61bdf3d0daa3f6549R21

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 21/03/2023 09:33, Arınç ÜNAL wrote:
> On 21.03.2023 11:27, Krzysztof Kozlowski wrote:
>> On 21/03/2023 09:24, Arınç ÜNAL wrote:
>>>>>
>>>>> If we take the calling new things mediatek route, we will never get to
>>>>> the bottom of fixing the naming inconsistency.
>>>>
>>>> All new things, so new SoCs, should be called mediatek, because there is
>>>> no ralink and mediatek is already used for them. So why some new
>>>> Mediatek SoCs are "mediatek" but some other also new SoCs are "ralink"?
>>>>
>>>> You can do nothing (and no actual need) about existing inconsistency...
>>>
>>> I couldn't change ralink -> mediatek because company acquisitions don't
>>> grant the change. I don't see any reason to prevent changing mediatek ->
>>> ralink without breaking the ABI on the existing schemas.
>>
>> You cannot change mediatek->ralink without breaking the ABI for the same
>> reasons.
> 
> Then this is where I ask for an exception.
> 
> The current solution only complicates things more.
> 
> https://github.com/paraka/linux/pull/1/files#diff-0ae6c456898d08536ce987c32f23f2eb6f4a0f7c38bff9a61bdf3d0daa3f6549R21

Sorry, I don't understand what's under this link and how some Github
repo pull helps in this discussion. I don't see there any text, which
could help.

I also do not understand why this pull proves that you can change
existing mediatek compatibles (we talk also about ARM, which is shipped
to million of devices) to ralink without breaking the ABI.

I do not see how choosing one variant for compatibles having two
variants of prefixes, complicates things. Following this argument
choosing "ralink" also complicates!



Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 21.03.2023 11:39, Krzysztof Kozlowski wrote:
> On 21/03/2023 09:33, Arınç ÜNAL wrote:
>> On 21.03.2023 11:27, Krzysztof Kozlowski wrote:
>>> On 21/03/2023 09:24, Arınç ÜNAL wrote:
>>>>>>
>>>>>> If we take the calling new things mediatek route, we will never get to
>>>>>> the bottom of fixing the naming inconsistency.
>>>>>
>>>>> All new things, so new SoCs, should be called mediatek, because there is
>>>>> no ralink and mediatek is already used for them. So why some new
>>>>> Mediatek SoCs are "mediatek" but some other also new SoCs are "ralink"?
>>>>>
>>>>> You can do nothing (and no actual need) about existing inconsistency...
>>>>
>>>> I couldn't change ralink -> mediatek because company acquisitions don't
>>>> grant the change. I don't see any reason to prevent changing mediatek ->
>>>> ralink without breaking the ABI on the existing schemas.
>>>
>>> You cannot change mediatek->ralink without breaking the ABI for the same
>>> reasons.
>>
>> Then this is where I ask for an exception.
>>
>> The current solution only complicates things more.
>>
>> https://github.com/paraka/linux/pull/1/files#diff-0ae6c456898d08536ce987c32f23f2eb6f4a0f7c38bff9a61bdf3d0daa3f6549R21
> 
> Sorry, I don't understand what's under this link and how some Github
> repo pull helps in this discussion. I don't see there any text, which
> could help.

That's Sergio's current branch, before he sends out a new version of the 
patch series. So that's the current solution, having 
mediatek,mt7620-sysc and ralink,mt7620a-sysc on the schema.

> 
> I also do not understand why this pull proves that you can change
> existing mediatek compatibles (we talk also about ARM, which is shipped
> to million of devices) to ralink without breaking the ABI.

No no, I only want to do this on schemas that concern the MTMIPS 
platform. It doesn't concern the MediaTek ARM schemas.

> 
> I do not see how choosing one variant for compatibles having two
> variants of prefixes, complicates things. Following this argument
> choosing "ralink" also complicates!

The idea is to make every compatible string of MTMIPS to have the ralink 
prefix so it's not mediatek on some schemas and ralink on others. Simpler.

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Krzysztof Kozlowski]

On 21/03/2023 09:53, Arınç ÜNAL wrote:
>>
>> I do not see how choosing one variant for compatibles having two
>> variants of prefixes, complicates things. Following this argument
>> choosing "ralink" also complicates!
> 
> The idea is to make every compatible string of MTMIPS to have the ralink 
> prefix so it's not mediatek on some schemas and ralink on others. Simpler.

Which is an ABI break, so you cannot do it.

Best regards,
Krzysztof

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 21.03.2023 12:01, Krzysztof Kozlowski wrote:
> On 21/03/2023 09:53, Arınç ÜNAL wrote:
>>>
>>> I do not see how choosing one variant for compatibles having two
>>> variants of prefixes, complicates things. Following this argument
>>> choosing "ralink" also complicates!
>>
>> The idea is to make every compatible string of MTMIPS to have the ralink
>> prefix so it's not mediatek on some schemas and ralink on others. Simpler.
> 
> Which is an ABI break, so you cannot do it.

No, both strings stay on the driver, it's the schemas that will only 
keep ralink.

Arınç

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Rob Herring]

On Tue, Mar 21, 2023 at 12:02:47PM +0300, Arınç ÜNAL wrote:
> On 21.03.2023 12:01, Krzysztof Kozlowski wrote:
> > On 21/03/2023 09:53, Arınç ÜNAL wrote:
> > > > 
> > > > I do not see how choosing one variant for compatibles having two
> > > > variants of prefixes, complicates things. Following this argument
> > > > choosing "ralink" also complicates!
> > > 
> > > The idea is to make every compatible string of MTMIPS to have the ralink
> > > prefix so it's not mediatek on some schemas and ralink on others. Simpler.
> > 
> > Which is an ABI break, so you cannot do it.
> 
> No, both strings stay on the driver, it's the schemas that will only keep
> ralink.

But you are adding one of the strings to the driver, right? Still an ABI 
break, but only if you have an old kernel and new DT. That can be 
somewhat mitigated with a stable backport of the new id, but still an 
ABI break.

Rob

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Rob Herring]

On Tue, Mar 21, 2023 at 12:02:47PM +0300, Arınç ÜNAL wrote:
> On 21.03.2023 12:01, Krzysztof Kozlowski wrote:
> > On 21/03/2023 09:53, Arınç ÜNAL wrote:
> > > > 
> > > > I do not see how choosing one variant for compatibles having two
> > > > variants of prefixes, complicates things. Following this argument
> > > > choosing "ralink" also complicates!
> > > 
> > > The idea is to make every compatible string of MTMIPS to have the ralink
> > > prefix so it's not mediatek on some schemas and ralink on others. Simpler.
> > 
> > Which is an ABI break, so you cannot do it.
> 
> No, both strings stay on the driver, it's the schemas that will only keep
> ralink.

Whatever is in the driver should be in the schema too. 'make 
dt_compatible_check' will check this. And some day, I'd like that list 
to get to 0.

Rob

Re: [PATCH 01/10] dt: bindings: clock: add mtmips SoCs clock device tree binding documentation

[Arınç ÜNAL]

On 25.03.2023 01:10, Rob Herring wrote:
> On Tue, Mar 21, 2023 at 12:02:47PM +0300, Arınç ÜNAL wrote:
>> On 21.03.2023 12:01, Krzysztof Kozlowski wrote:
>>> On 21/03/2023 09:53, Arınç ÜNAL wrote:
>>>>>
>>>>> I do not see how choosing one variant for compatibles having two
>>>>> variants of prefixes, complicates things. Following this argument
>>>>> choosing "ralink" also complicates!
>>>>
>>>> The idea is to make every compatible string of MTMIPS to have the ralink
>>>> prefix so it's not mediatek on some schemas and ralink on others. Simpler.
>>>
>>> Which is an ABI break, so you cannot do it.
>>
>> No, both strings stay on the driver, it's the schemas that will only keep
>> ralink.
> 
> But you are adding one of the strings to the driver, right? Still an ABI
> break, but only if you have an old kernel and new DT. That can be
> somewhat mitigated with a stable backport of the new id, but still an
> ABI break.

Ah, that makes sense. Yes, I'd be adding new strings to the driver.

> Whatever is in the driver should be in the schema too. 'make 
> dt_compatible_check' will check this. And some day, I'd like that list 
> to get to 0.

I'll keep this in mind for the schemas I maintain. I will add 
ralink,rt2880-pinmux as deprecated on the pinctrl schemas so it would 
disappear from 'make dt_compatible check'. I believe I'm supposed to do 
it like this?

properties:
   compatible:
     enum:
       - ralink,rt2880-pinctrl
       - ralink,rt2880-pinmux
     deprecated:
       items:
         - const: ralink,rt2880-pinmux
           deprecated: true

Arınç