[PATCH v2 00/16] Microchip OTPC driver on SAM9X60 exposing UIDxR as additional nvmem device

[PATCH v2 00/16] Microchip OTPC driver on SAM9X60 exposing UIDxR as additional nvmem device

[Alexander Dahl]

Hei hei,

on a custom sam9x60 based board we want to access a unique ID of the
SoC.  Microchip sam-ba has a command 'readuniqueid' which returns the
content of the OTPC Product UID x Register in that case.

(On different boards with a SAMA5D2 sam-ba and we use the Serial Number
x Register for that purpose.  In the linux kernel those are exposed
through the atmel soc driver.  Those registers are not present in the
SAM9X60 series, but only for SAMA5D2/SAMA5D4 AFAIK.)

There is a driver for the OTPC of the SAMA7G5 and after comparing
register layouts it seems that one is almost identical to the one used
by SAM9X60.  Currently that driver has no support for the UIDx
registers, but I suppose it would be the right place to implement it,
because the registers are within the OTPC register address offsets.

When developing and testing it became clear the OTPC needs explicit
enabling of certain clocks in the otpc driver.  So the patch series
starts with some rework of clock bindings and clock drivers, along with
referencing that in SoC dtsi files.

After two patches with some unrelated fixups for the otpc driver, there
are more patches dealing with sam9x60 support for the driver and the
necessary clock enablement.

The last patch adds an additional nvmem device for the UIDx registers,
fullfilling the initial goal of mine.

I tested the series only on SAM9X60, not on SAMA7G5 or the other SoCs
affected, because I only have sam9x60 hardware for testing.  If someone
could test on SAM9X7, SAMA7G5, and SAMA7D65, that would be highly
appreciated.

Last question: Should the UID be added to the device entropy pool with
add_device_randomness() as done in the SAMA5D2 sfr driver?

For detailed patch changelog see each patch, general changelog below.

Greets
Alex

(series based on v6.14-rc2)

v2:
- Removed patch adding bad OPTC conditions warnings on probe
- Removed patch adding more register definitions (the one register
  definition required was moved to the last patch adding the nvmem for
  UID registers)
- Added multiple new patches handling the dt-bindings issues
- Extend the possibility to enable the main rc oscillator to all at91
  SoCs with an OTPC
- Added patches to reference and enable the OTPC peripheral clock on all
  capable SoCs
- Reordered patches
- Reworded commit messages
- Squashed patches with dts changes for sam9x60
- Fixed bot warnings

v1:
- Link: https://lore.kernel.org/all/20240821105943.230281-1-ada@thorsis.com/T/#u

(I sent an RFC patch on this topic earlier this year, you'll find the
link below as a reference to the discussion.  The patch itself was
trivial and not meant for applying as is anyways, so I decided to not
write a full changelog from RFC to v1.)

RFC:
- Link: https://lore.kernel.org/all/20240412140802.1571935-1-ada@thorsis.com/T/#u

Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-clk@vger.kernel.org

Alexander Dahl (16):
  dt-bindings: clock: at91: Split up per SoC partially
  ARM: dts: microchip: Use new PMC bindings
  clk: at91: Use new PMC bindings
  dt-bindings: clock: at91: Allow referencing main rc oscillator in DT
  clk: at91: Allow enabling main_rc_osc through DT
  clk: at91: Add peripheral id for OTPC
  dt-bindings: nvmem: microchip-otpc: Add compatible for SAM9X60
  dt-bindings: nvmem: microchip-otpc: Add required clocks
  nvmem: microchip-otpc: Avoid reading a write-only register
  nvmem: microchip-otpc: Fix swapped 'sleep' and 'timeout' parameters
  nvmem: microchip-otpc: Add SAM9X60 support
  ARM: dts: microchip: sama7g5: Add OTPC clocks
  ARM: dts: microchip: sam9x60: Add OTPC node
  ARM: dts: microchip: sam9x60_curiosity: Enable OTP Controller
  nvmem: microchip-otpc: Enable necessary clocks
  nvmem: microchip-otpc: Expose UID registers as 2nd nvmem device

 .../nvmem/microchip,sama7g5-otpc.yaml         | 32 ++++++++++-
 .../dts/microchip/at91-sam9x60_curiosity.dts  |  4 ++
 arch/arm/boot/dts/microchip/sam9x60.dtsi      | 25 ++++++---
 arch/arm/boot/dts/microchip/sam9x7.dtsi       | 11 ++--
 arch/arm/boot/dts/microchip/sama7d65.dtsi     |  5 +-
 arch/arm/boot/dts/microchip/sama7g5.dtsi      | 25 +++++----
 drivers/clk/at91/sam9x60.c                    | 16 +++---
 drivers/clk/at91/sam9x7.c                     | 24 +++++----
 drivers/clk/at91/sama7d65.c                   | 44 +++++++--------
 drivers/clk/at91/sama7g5.c                    | 30 ++++++-----
 drivers/nvmem/microchip-otpc.c                | 54 ++++++++++++++++---
 .../dt-bindings/clock/microchip,sam9x60-pmc.h | 22 ++++++++
 .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 28 ++++++++++
 .../clock/microchip,sama7d65-pmc.h            | 35 ++++++++++++
 .../dt-bindings/clock/microchip,sama7g5-pmc.h | 27 ++++++++++
 15 files changed, 296 insertions(+), 86 deletions(-)
 create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
 create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
 create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
 create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h


base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
-- 
2.39.5

[PATCH v2 01/16] dt-bindings: clock: at91: Split up per SoC partially

[Alexander Dahl]

Before adding even more new indexes creating more holes in the
clk at91 drivers pmc_data->chws arrays, split this up.

This is a partial split up only for SoCs affected by upcoming changes
and by that PMC_MAIN + x hack, others could follow by the same scheme.

Binding splitup was proposed for several reasons:

1) keep the driver code simple, readable, and efficient
2) avoid accidental array index duplication
3) avoid memory waste by creating more and more unused array members.

Old values are kept to not break dts, and to maintain dt ABI.

Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - new patch, not present in v1

 .../dt-bindings/clock/microchip,sam9x60-pmc.h | 19 +++++++++++
 .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 25 +++++++++++++++
 .../clock/microchip,sama7d65-pmc.h            | 32 +++++++++++++++++++
 .../dt-bindings/clock/microchip,sama7g5-pmc.h | 24 ++++++++++++++
 4 files changed, 100 insertions(+)
 create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
 create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
 create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
 create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h

diff --git a/include/dt-bindings/clock/microchip,sam9x60-pmc.h b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
new file mode 100644
index 0000000000000..e01e867e8c4da
--- /dev/null
+++ b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * The constants defined in this header are being used in dts and in
+ * at91 sam9x60 clock driver.
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H
+#define _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H
+
+#include <dt-bindings/clock/at91.h>
+
+/* old from before bindings splitup */
+#define SAM9X60_PMC_MCK		PMC_MCK		/* 1 */
+#define SAM9X60_PMC_UTMI	PMC_UTMI	/* 2 */
+#define SAM9X60_PMC_MAIN	PMC_MAIN	/* 3 */
+
+#define SAM9X60_PMC_PLLACK	PMC_PLLACK	/* 7 */
+
+#endif
diff --git a/include/dt-bindings/clock/microchip,sam9x7-pmc.h b/include/dt-bindings/clock/microchip,sam9x7-pmc.h
new file mode 100644
index 0000000000000..2df1ff97a5b18
--- /dev/null
+++ b/include/dt-bindings/clock/microchip,sam9x7-pmc.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * The constants defined in this header are being used in dts and in
+ * at91 sam9x7 clock driver.
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X7_PMC_H
+#define _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X7_PMC_H
+
+#include <dt-bindings/clock/at91.h>
+
+/* old from before bindings splitup */
+#define SAM9X7_PMC_MCK		PMC_MCK		/* 1 */
+#define SAM9X7_PMC_UTMI		PMC_UTMI	/* 2 */
+#define SAM9X7_PMC_MAIN		PMC_MAIN	/* 3 */
+
+#define SAM9X7_PMC_PLLACK	PMC_PLLACK	/* 7 */
+
+#define SAM9X7_PMC_AUDIOPMCPLL	PMC_AUDIOPMCPLL	/* 9 */
+#define SAM9X7_PMC_AUDIOIOPLL	PMC_AUDIOIOPLL	/* 10 */
+
+#define SAM9X7_PMC_PLLADIV2	PMC_PLLADIV2	/* 14 */
+#define SAM9X7_PMC_LVDSPLL	PMC_LVDSPLL	/* 15 */
+
+#endif
diff --git a/include/dt-bindings/clock/microchip,sama7d65-pmc.h b/include/dt-bindings/clock/microchip,sama7d65-pmc.h
new file mode 100644
index 0000000000000..f5be643be9b36
--- /dev/null
+++ b/include/dt-bindings/clock/microchip,sama7d65-pmc.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * The constants defined in this header are being used in dts and in
+ * at91 sama7d65 clock driver.
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7D65_PMC_H
+#define _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7D65_PMC_H
+
+#include <dt-bindings/clock/at91.h>
+
+/* old from before bindings splitup */
+#define SAMA7D65_PMC_MCK0		PMC_MCK		/* 1 */
+#define SAMA7D65_PMC_UTMI		PMC_UTMI	/* 2 */
+#define SAMA7D65_PMC_MAIN		PMC_MAIN	/* 3 */
+#define SAMA7D65_PMC_CPUPLL		PMC_CPUPLL	/* 4 */
+#define SAMA7D65_PMC_SYSPLL		PMC_SYSPLL	/* 5 */
+
+#define SAMA7D65_PMC_BAUDPLL		PMC_BAUDPLL	/* 8 */
+#define SAMA7D65_PMC_AUDIOPMCPLL	PMC_AUDIOPMCPLL	/* 9 */
+#define SAMA7D65_PMC_AUDIOIOPLL		PMC_AUDIOIOPLL	/* 10 */
+#define SAMA7D65_PMC_ETHPLL		PMC_ETHPLL	/* 11 */
+
+#define SAMA7D65_PMC_MCK1		PMC_MCK1	/* 13 */
+
+#define SAMA7D65_PMC_LVDSPLL		PMC_LVDSPLL	/* 15 */
+#define SAMA7D65_PMC_MCK3		PMC_MCK3	/* 16 */
+#define SAMA7D65_PMC_MCK5		PMC_MCK5	/* 17 */
+
+#define SAMA7D65_PMC_INDEX_MAX		25
+
+#endif
diff --git a/include/dt-bindings/clock/microchip,sama7g5-pmc.h b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
new file mode 100644
index 0000000000000..ad69ccdf9dc78
--- /dev/null
+++ b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * The constants defined in this header are being used in dts and in
+ * at91 sama7g5 clock driver.
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
+#define _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
+
+#include <dt-bindings/clock/at91.h>
+
+/* old from before bindings splitup */
+#define SAMA7G5_PMC_MCK0	PMC_MCK		/* 1 */
+#define SAMA7G5_PMC_UTMI	PMC_UTMI	/* 2 */
+#define SAMA7G5_PMC_MAIN	PMC_MAIN	/* 3 */
+#define SAMA7G5_PMC_CPUPLL	PMC_CPUPLL	/* 4 */
+#define SAMA7G5_PMC_SYSPLL	PMC_SYSPLL	/* 5 */
+
+#define SAMA7G5_PMC_AUDIOPMCPLL	PMC_AUDIOPMCPLL	/* 9 */
+#define SAMA7G5_PMC_AUDIOIOPLL	PMC_AUDIOIOPLL	/* 10 */
+
+#define SAMA7G5_PMC_MCK1	PMC_MCK1	/* 13 */
+
+#endif
-- 
2.39.5

[PATCH v2 02/16] ARM: dts: microchip: Use new PMC bindings

[Alexander Dahl]

The bindings were split up per SoC before adding new members for
missing clocks.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - new patch, not present in v1

 arch/arm/boot/dts/microchip/sam9x60.dtsi  | 15 ++++++++-------
 arch/arm/boot/dts/microchip/sam9x7.dtsi   | 11 ++++++-----
 arch/arm/boot/dts/microchip/sama7d65.dtsi |  5 +++--
 arch/arm/boot/dts/microchip/sama7g5.dtsi  | 23 ++++++++++++-----------
 4 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi
index b8b2c1ddf3f1e..1724b79967a17 100644
--- a/arch/arm/boot/dts/microchip/sam9x60.dtsi
+++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi
@@ -12,6 +12,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/clock/at91.h>
+#include <dt-bindings/clock/microchip,sam9x60-pmc.h>
 #include <dt-bindings/mfd/at91-usart.h>
 #include <dt-bindings/mfd/atmel-flexcom.h>
 
@@ -81,9 +82,9 @@ usb0: gadget@500000 {
 			reg = <0x00500000 0x100000
 				0xf803c000 0x400>;
 			interrupts = <23 IRQ_TYPE_LEVEL_HIGH 2>;
-			clocks = <&pmc PMC_TYPE_PERIPHERAL 23>, <&pmc PMC_TYPE_CORE PMC_UTMI>;
+			clocks = <&pmc PMC_TYPE_PERIPHERAL 23>, <&pmc PMC_TYPE_CORE SAM9X60_PMC_UTMI>;
 			clock-names = "pclk", "hclk";
-			assigned-clocks = <&pmc PMC_TYPE_CORE PMC_UTMI>;
+			assigned-clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_UTMI>;
 			assigned-clock-rates = <480000000>;
 			status = "disabled";
 		};
@@ -101,9 +102,9 @@ usb2: ehci@700000 {
 			compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
 			reg = <0x00700000 0x100000>;
 			interrupts = <22 IRQ_TYPE_LEVEL_HIGH 2>;
-			clocks = <&pmc PMC_TYPE_CORE PMC_UTMI>, <&pmc PMC_TYPE_PERIPHERAL 22>;
+			clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_UTMI>, <&pmc PMC_TYPE_PERIPHERAL 22>;
 			clock-names = "usb_clk", "ehci_clk";
-			assigned-clocks = <&pmc PMC_TYPE_CORE PMC_UTMI>;
+			assigned-clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_UTMI>;
 			assigned-clock-rates = <480000000>;
 			status = "disabled";
 		};
@@ -121,7 +122,7 @@ ebi: ebi@10000000 {
 				  0x3 0x0 0x40000000 0x10000000
 				  0x4 0x0 0x50000000 0x10000000
 				  0x5 0x0 0x60000000 0x10000000>;
-			clocks = <&pmc PMC_TYPE_CORE PMC_MCK>;
+			clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_MCK>;
 			status = "disabled";
 
 			nand_controller: nand-controller {
@@ -1063,7 +1064,7 @@ hlcdc: hlcdc@f8038000 {
 				clocks = <&pmc PMC_TYPE_PERIPHERAL 25>, <&pmc PMC_TYPE_GCK 25>, <&clk32k 1>;
 				clock-names = "periph_clk","sys_clk", "slow_clk";
 				assigned-clocks = <&pmc PMC_TYPE_GCK 25>;
-				assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_MCK>;
+				assigned-clock-parents = <&pmc PMC_TYPE_CORE SAM9X60_PMC_MCK>;
 				status = "disabled";
 
 				hlcdc-display-controller {
@@ -1369,7 +1370,7 @@ pit: timer@fffffe40 {
 				compatible = "atmel,at91sam9260-pit";
 				reg = <0xfffffe40 0x10>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
-				clocks = <&pmc PMC_TYPE_CORE PMC_MCK>;
+				clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_MCK>;
 			};
 
 			clk32k: clock-controller@fffffe50 {
diff --git a/arch/arm/boot/dts/microchip/sam9x7.dtsi b/arch/arm/boot/dts/microchip/sam9x7.dtsi
index b217a908f5253..458cfb81f8bcf 100644
--- a/arch/arm/boot/dts/microchip/sam9x7.dtsi
+++ b/arch/arm/boot/dts/microchip/sam9x7.dtsi
@@ -8,6 +8,7 @@
  */
 
 #include <dt-bindings/clock/at91.h>
+#include <dt-bindings/clock/microchip,sam9x7-pmc.h>
 #include <dt-bindings/dma/at91.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -477,9 +478,9 @@ can0: can@f8000000 {
 			interrupt-names = "int0", "int1";
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 29>, <&pmc PMC_TYPE_GCK 29>;
 			clock-names = "hclk", "cclk";
-			assigned-clocks = <&pmc PMC_TYPE_CORE PMC_UTMI>, <&pmc PMC_TYPE_GCK 29>;
+			assigned-clocks = <&pmc PMC_TYPE_CORE SAM9X7_PMC_UTMI>, <&pmc PMC_TYPE_GCK 29>;
 			assigned-clock-rates = <480000000>, <40000000>;
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_UTMI>, <&pmc PMC_TYPE_CORE PMC_UTMI>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAM9X7_PMC_UTMI>, <&pmc PMC_TYPE_CORE SAM9X7_PMC_UTMI>;
 			bosch,mram-cfg = <0x3400 0 0 64 0 0 32 32>;
 			status = "disabled";
 		};
@@ -493,9 +494,9 @@ can1: can@f8004000 {
 			interrupt-names = "int0", "int1";
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 30>, <&pmc PMC_TYPE_GCK 30>;
 			clock-names = "hclk", "cclk";
-			assigned-clocks = <&pmc PMC_TYPE_CORE PMC_UTMI>, <&pmc PMC_TYPE_GCK 30>;
+			assigned-clocks = <&pmc PMC_TYPE_CORE SAM9X7_PMC_UTMI>, <&pmc PMC_TYPE_GCK 30>;
 			assigned-clock-rates = <480000000>, <40000000>;
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_UTMI>, <&pmc PMC_TYPE_CORE PMC_UTMI>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAM9X7_PMC_UTMI>, <&pmc PMC_TYPE_CORE SAM9X7_PMC_UTMI>;
 			bosch,mram-cfg = <0x7800 0 0 64 0 0 32 32>;
 			status = "disabled";
 		};
@@ -1100,7 +1101,7 @@ pmecc: ecc-engine@ffffe000 {
 		mpddrc: mpddrc@ffffe800 {
 			compatible = "microchip,sam9x7-ddramc", "atmel,sama5d3-ddramc";
 			reg = <0xffffe800 0x200>;
-			clocks = <&pmc PMC_TYPE_SYSTEM 2>, <&pmc PMC_TYPE_CORE PMC_MCK>;
+			clocks = <&pmc PMC_TYPE_SYSTEM 2>, <&pmc PMC_TYPE_CORE SAM9X7_PMC_MCK>;
 			clock-names = "ddrck", "mpddr";
 		};
 
diff --git a/arch/arm/boot/dts/microchip/sama7d65.dtsi b/arch/arm/boot/dts/microchip/sama7d65.dtsi
index 854b30d15dcd4..111a6a6ef0e00 100644
--- a/arch/arm/boot/dts/microchip/sama7d65.dtsi
+++ b/arch/arm/boot/dts/microchip/sama7d65.dtsi
@@ -9,6 +9,7 @@
  */
 
 #include <dt-bindings/clock/at91.h>
+#include <dt-bindings/clock/microchip,sama7d65-pmc.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
@@ -29,7 +30,7 @@ cpu0: cpu@0 {
 			compatible = "arm,cortex-a7";
 			reg = <0x0>;
 			device_type = "cpu";
-			clocks = <&pmc PMC_TYPE_CORE PMC_CPUPLL>;
+			clocks = <&pmc PMC_TYPE_CORE SAMA7D65_PMC_CPUPLL>;
 			clock-names = "cpu";
 		};
 	};
@@ -91,7 +92,7 @@ sdmmc1: mmc@e1208000 {
 			clock-names = "hclock", "multclk";
 			assigned-clocks = <&pmc PMC_TYPE_GCK 76>;
 			assigned-clock-rates = <200000000>;
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_MCK1>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7D65_PMC_MCK1>;
 			status = "disabled";
 		};
 
diff --git a/arch/arm/boot/dts/microchip/sama7g5.dtsi b/arch/arm/boot/dts/microchip/sama7g5.dtsi
index 17bcdcf0cf4a0..f68c2eb8edd54 100644
--- a/arch/arm/boot/dts/microchip/sama7g5.dtsi
+++ b/arch/arm/boot/dts/microchip/sama7g5.dtsi
@@ -13,6 +13,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/at91.h>
+#include <dt-bindings/clock/microchip,sama7g5-pmc.h>
 #include <dt-bindings/dma/at91.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/mfd/at91-usart.h>
@@ -34,7 +35,7 @@ cpu0: cpu@0 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a7";
 			reg = <0x0>;
-			clocks = <&pmc PMC_TYPE_CORE PMC_CPUPLL>;
+			clocks = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_CPUPLL>;
 			clock-names = "cpu";
 			operating-points-v2 = <&cpu_opp_table>;
 			#cooling-cells = <2>; /* min followed by max */
@@ -189,7 +190,7 @@ ebi: ebi@40000000 {
 				  0x1 0x0 0x48000000 0x8000000
 				  0x2 0x0 0x50000000 0x8000000
 				  0x3 0x0 0x58000000 0x8000000>;
-			clocks = <&pmc PMC_TYPE_CORE PMC_MCK1>;
+			clocks = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_MCK1>;
 			status = "disabled";
 
 			nand_controller: nand-controller {
@@ -372,7 +373,7 @@ can0: can@e0828000 {
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 61>, <&pmc PMC_TYPE_GCK 61>;
 			clock-names = "hclk", "cclk";
 			assigned-clocks = <&pmc PMC_TYPE_GCK 61>;
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_SYSPLL>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_SYSPLL>;
 			assigned-clock-rates = <40000000>;
 			bosch,mram-cfg = <0x3400 0 0 64 0 0 32 32>;
 			status = "disabled";
@@ -388,7 +389,7 @@ can1: can@e082c000 {
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 62>, <&pmc PMC_TYPE_GCK 62>;
 			clock-names = "hclk", "cclk";
 			assigned-clocks = <&pmc PMC_TYPE_GCK 62>;
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_SYSPLL>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_SYSPLL>;
 			assigned-clock-rates = <40000000>;
 			bosch,mram-cfg = <0x7800 0 0 64 0 0 32 32>;
 			status = "disabled";
@@ -404,7 +405,7 @@ can2: can@e0830000 {
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 63>, <&pmc PMC_TYPE_GCK 63>;
 			clock-names = "hclk", "cclk";
 			assigned-clocks = <&pmc PMC_TYPE_GCK 63>;
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_SYSPLL>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_SYSPLL>;
 			assigned-clock-rates = <40000000>;
 			bosch,mram-cfg = <0xbc00 0 0 64 0 0 32 32>;
 			status = "disabled";
@@ -420,7 +421,7 @@ can3: can@e0834000 {
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 64>, <&pmc PMC_TYPE_GCK 64>;
 			clock-names = "hclk", "cclk";
 			assigned-clocks = <&pmc PMC_TYPE_GCK 64>;
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_SYSPLL>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_SYSPLL>;
 			assigned-clock-rates = <40000000>;
 			bosch,mram-cfg = <0x0 0 0 64 0 0 32 32>;
 			status = "disabled";
@@ -436,7 +437,7 @@ can4: can@e0838000 {
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 65>, <&pmc PMC_TYPE_GCK 65>;
 			clock-names = "hclk", "cclk";
 			assigned-clocks = <&pmc PMC_TYPE_GCK 65>;
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_SYSPLL>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_SYSPLL>;
 			assigned-clock-rates = <40000000>;
 			bosch,mram-cfg = <0x4400 0 0 64 0 0 32 32>;
 			status = "disabled";
@@ -452,7 +453,7 @@ can5: can@e083c000 {
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 66>, <&pmc PMC_TYPE_GCK 66>;
 			clock-names = "hclk", "cclk";
 			assigned-clocks = <&pmc PMC_TYPE_GCK 66>;
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_SYSPLL>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_SYSPLL>;
 			assigned-clock-rates = <40000000>;
 			bosch,mram-cfg = <0x8800 0 0 64 0 0 32 32>;
 			status = "disabled";
@@ -483,7 +484,7 @@ sdmmc0: mmc@e1204000 {
 			interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 80>, <&pmc PMC_TYPE_GCK 80>;
 			clock-names = "hclock", "multclk";
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_SYSPLL>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_SYSPLL>;
 			assigned-clocks = <&pmc PMC_TYPE_GCK 80>;
 			assigned-clock-rates = <200000000>;
 			microchip,sdcal-inverted;
@@ -496,7 +497,7 @@ sdmmc1: mmc@e1208000 {
 			interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 81>, <&pmc PMC_TYPE_GCK 81>;
 			clock-names = "hclock", "multclk";
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_SYSPLL>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_SYSPLL>;
 			assigned-clocks = <&pmc PMC_TYPE_GCK 81>;
 			assigned-clock-rates = <200000000>;
 			microchip,sdcal-inverted;
@@ -509,7 +510,7 @@ sdmmc2: mmc@e120c000 {
 			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 82>, <&pmc PMC_TYPE_GCK 82>;
 			clock-names = "hclock", "multclk";
-			assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_SYSPLL>;
+			assigned-clock-parents = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_SYSPLL>;
 			assigned-clocks = <&pmc PMC_TYPE_GCK 82>;
 			assigned-clock-rates = <200000000>;
 			microchip,sdcal-inverted;
-- 
2.39.5

[PATCH v2 03/16] clk: at91: Use new PMC bindings

[Alexander Dahl]

The bindings were split up per SoC before adding new array members for
missing clocks.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - new patch, not present in v1

 drivers/clk/at91/sam9x60.c  | 14 ++++++-------
 drivers/clk/at91/sam9x7.c   | 22 +++++++++----------
 drivers/clk/at91/sama7d65.c | 42 ++++++++++++++++++-------------------
 drivers/clk/at91/sama7g5.c  | 28 ++++++++++++-------------
 4 files changed, 52 insertions(+), 54 deletions(-)

diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
index db6db9e2073eb..11fe2d05fa9bb 100644
--- a/drivers/clk/at91/sam9x60.c
+++ b/drivers/clk/at91/sam9x60.c
@@ -3,7 +3,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/slab.h>
 
-#include <dt-bindings/clock/at91.h>
+#include <dt-bindings/clock/microchip,sam9x60-pmc.h>
 
 #include "pmc.h"
 
@@ -214,7 +214,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
 	if (IS_ERR(regmap))
 		return;
 
-	sam9x60_pmc = pmc_data_allocate(PMC_PLLACK + 1,
+	sam9x60_pmc = pmc_data_allocate(SAM9X60_PMC_PLLACK + 1,
 					nck(sam9x60_systemck),
 					nck(sam9x60_periphck),
 					nck(sam9x60_gck), 8);
@@ -237,10 +237,10 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sam9x60_pmc->chws[PMC_MAIN] = hw;
+	sam9x60_pmc->chws[SAM9X60_PMC_MAIN] = hw;
 
 	hw = sam9x60_clk_register_frac_pll(regmap, &pmc_pll_lock, "pllack_fracck",
-					   "mainck", sam9x60_pmc->chws[PMC_MAIN],
+					   "mainck", sam9x60_pmc->chws[SAM9X60_PMC_MAIN],
 					   0, &plla_characteristics,
 					   &pll_frac_layout,
 					   /*
@@ -263,7 +263,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sam9x60_pmc->chws[PMC_PLLACK] = hw;
+	sam9x60_pmc->chws[SAM9X60_PMC_PLLACK] = hw;
 
 	hw = sam9x60_clk_register_frac_pll(regmap, &pmc_pll_lock, "upllck_fracck",
 					   "main_osc", main_osc_hw, 1,
@@ -281,7 +281,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sam9x60_pmc->chws[PMC_UTMI] = hw;
+	sam9x60_pmc->chws[SAM9X60_PMC_UTMI] = hw;
 
 	parent_names[0] = md_slck_name;
 	parent_names[1] = "mainck";
@@ -299,7 +299,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sam9x60_pmc->chws[PMC_MCK] = hw;
+	sam9x60_pmc->chws[SAM9X60_PMC_MCK] = hw;
 
 	parent_names[0] = "pllack_divck";
 	parent_names[1] = "upllck_divck";
diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
index cbb8b220f16bc..c3c12e0523c4b 100644
--- a/drivers/clk/at91/sam9x7.c
+++ b/drivers/clk/at91/sam9x7.c
@@ -12,7 +12,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/slab.h>
 
-#include <dt-bindings/clock/at91.h>
+#include <dt-bindings/clock/microchip,sam9x7-pmc.h>
 
 #include "pmc.h"
 
@@ -220,7 +220,7 @@ static const struct {
 			.t = PLL_TYPE_DIV,
 			/* This feeds CPU. It should not be disabled */
 			.f = CLK_IS_CRITICAL | CLK_SET_RATE_GATE,
-			.eid = PMC_PLLACK,
+			.eid = SAM9X7_PMC_PLLACK,
 			.c = &plla_characteristics,
 		},
 	},
@@ -242,7 +242,7 @@ static const struct {
 			.t = PLL_TYPE_DIV,
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
-			.eid = PMC_UTMI,
+			.eid = SAM9X7_PMC_UTMI,
 			.c = &upll_characteristics,
 		},
 	},
@@ -264,7 +264,7 @@ static const struct {
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
 			.c = &audiopll_characteristics,
-			.eid = PMC_AUDIOPMCPLL,
+			.eid = SAM9X7_PMC_AUDIOPMCPLL,
 			.t = PLL_TYPE_DIV,
 		},
 
@@ -275,7 +275,7 @@ static const struct {
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
 			.c = &audiopll_characteristics,
-			.eid = PMC_AUDIOIOPLL,
+			.eid = SAM9X7_PMC_AUDIOIOPLL,
 			.t = PLL_TYPE_DIV,
 		},
 	},
@@ -297,7 +297,7 @@ static const struct {
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
 			.c = &lvdspll_characteristics,
-			.eid = PMC_LVDSPLL,
+			.eid = SAM9X7_PMC_LVDSPLL,
 			.t = PLL_TYPE_DIV,
 		},
 	},
@@ -313,7 +313,7 @@ static const struct {
 			 */
 			.f = CLK_IS_CRITICAL | CLK_SET_RATE_GATE,
 			.c = &plladiv2_characteristics,
-			.eid = PMC_PLLADIV2,
+			.eid = SAM9X7_PMC_PLLADIV2,
 			.t = PLL_TYPE_DIV,
 		},
 	},
@@ -741,7 +741,7 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
 	if (IS_ERR(regmap))
 		return;
 
-	sam9x7_pmc = pmc_data_allocate(PMC_LVDSPLL + 1,
+	sam9x7_pmc = pmc_data_allocate(SAM9X7_PMC_LVDSPLL + 1,
 				       nck(sam9x7_systemck),
 				       nck(sam9x7_periphck),
 				       nck(sam9x7_gck), 8);
@@ -770,7 +770,7 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sam9x7_pmc->chws[PMC_MAIN] = hw;
+	sam9x7_pmc->chws[SAM9X7_PMC_MAIN] = hw;
 
 	for (i = 0; i < PLL_ID_MAX; i++) {
 		for (j = 0; j < 3; j++) {
@@ -782,7 +782,7 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
 			switch (sam9x7_plls[i][j].t) {
 			case PLL_TYPE_FRAC:
 				if (!strcmp(sam9x7_plls[i][j].p, "mainck"))
-					parent_hw = sam9x7_pmc->chws[PMC_MAIN];
+					parent_hw = sam9x7_pmc->chws[SAM9X7_PMC_MAIN];
 				else if (!strcmp(sam9x7_plls[i][j].p, "main_osc"))
 					parent_hw = main_osc_hw;
 				else
@@ -838,7 +838,7 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sam9x7_pmc->chws[PMC_MCK] = hw;
+	sam9x7_pmc->chws[SAM9X7_PMC_MCK] = hw;
 
 	parent_names[0] = "plla_divpmcck";
 	parent_names[1] = "upll_divpmcck";
diff --git a/drivers/clk/at91/sama7d65.c b/drivers/clk/at91/sama7d65.c
index a5d40df8b2f27..024c5abee25ff 100644
--- a/drivers/clk/at91/sama7d65.c
+++ b/drivers/clk/at91/sama7d65.c
@@ -11,7 +11,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/slab.h>
 
-#include <dt-bindings/clock/at91.h>
+#include <dt-bindings/clock/microchip,sama7d65-pmc.h>
 
 #include "pmc.h"
 
@@ -19,8 +19,6 @@ static DEFINE_SPINLOCK(pmc_pll_lock);
 static DEFINE_SPINLOCK(pmc_mck0_lock);
 static DEFINE_SPINLOCK(pmc_mckX_lock);
 
-#define PMC_INDEX_MAX	25
-
 /*
  * PLL clocks identifiers
  * @PLL_ID_CPU:		CPU PLL identifier
@@ -221,7 +219,7 @@ static struct sama7d65_pll {
 			.t = PLL_TYPE_DIV,
 			/* This feeds CPU. It should not be disabled. */
 			.f = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
-			.eid = PMC_CPUPLL,
+			.eid = SAMA7D65_PMC_CPUPLL,
 			/*
 			 * Safe div=15 should be safe even for switching b/w 1GHz and
 			 * 90MHz (frac pll might go up to 1.2GHz).
@@ -256,7 +254,7 @@ static struct sama7d65_pll {
 			 * Therefore it should not be disabled.
 			 */
 			.f = CLK_IS_CRITICAL | CLK_SET_RATE_GATE,
-			.eid = PMC_SYSPLL,
+			.eid = SAMA7D65_PMC_SYSPLL,
 		},
 	},
 
@@ -324,7 +322,7 @@ static struct sama7d65_pll {
 			.t = PLL_TYPE_DIV,
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
-			.eid = PMC_BAUDPLL,
+			.eid = SAMA7D65_PMC_BAUDPLL,
 		},
 	},
 
@@ -346,7 +344,7 @@ static struct sama7d65_pll {
 			.t = PLL_TYPE_DIV,
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
-			.eid = PMC_AUDIOPMCPLL,
+			.eid = SAMA7D65_PMC_AUDIOPMCPLL,
 		},
 
 		[PLL_COMPID_DIV1] = {
@@ -357,7 +355,7 @@ static struct sama7d65_pll {
 			.t = PLL_TYPE_DIV,
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
-			.eid = PMC_AUDIOIOPLL,
+			.eid = SAMA7D65_PMC_AUDIOIOPLL,
 		},
 	},
 
@@ -379,7 +377,7 @@ static struct sama7d65_pll {
 			.t = PLL_TYPE_DIV,
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
-			.eid = PMC_ETHPLL,
+			.eid = SAMA7D65_PMC_ETHPLL,
 		},
 	},
 
@@ -401,7 +399,7 @@ static struct sama7d65_pll {
 			.t = PLL_TYPE_DIV,
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
-			.eid = PMC_LVDSPLL,
+			.eid = SAMA7D65_PMC_LVDSPLL,
 		},
 	},
 
@@ -423,7 +421,7 @@ static struct sama7d65_pll {
 			.t = PLL_TYPE_DIV,
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
-			.eid = PMC_UTMI,
+			.eid = SAMA7D65_PMC_UTMI,
 		},
 	},
 };
@@ -466,7 +464,7 @@ static struct {
 	  .ep_mux_table = { 5, },
 	  .ep_count = 1,
 	  .ep_chg_id = INT_MIN,
-	  .eid = PMC_MCK1,
+	  .eid = SAMA7D65_PMC_MCK1,
 	  .c = 1, },
 
 	{ .n = "mck2",
@@ -483,7 +481,7 @@ static struct {
 	  .ep_mux_table = { 5, 6, },
 	  .ep_count = 2,
 	  .ep_chg_id = INT_MIN,
-	  .eid = PMC_MCK3,
+	  .eid = SAMA7D65_PMC_MCK3,
 	  .c = 1, },
 
 	{ .n = "mck4",
@@ -500,7 +498,7 @@ static struct {
 	  .ep_mux_table = { 5, },
 	  .ep_count = 1,
 	  .ep_chg_id = INT_MIN,
-	  .eid = PMC_MCK5,
+	  .eid = SAMA7D65_PMC_MCK5,
 	  .c = 1, },
 
 	{ .n = "mck6",
@@ -1116,7 +1114,7 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
 	if (IS_ERR(regmap))
 		return;
 
-	sama7d65_pmc = pmc_data_allocate(PMC_INDEX_MAX,
+	sama7d65_pmc = pmc_data_allocate(SAMA7D65_PMC_INDEX_MAX,
 					 nck(sama7d65_systemck),
 					 nck(sama7d65_periphck),
 					 nck(sama7d65_gck), 8);
@@ -1149,7 +1147,7 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sama7d65_pmc->chws[PMC_MAIN] = hw;
+	sama7d65_pmc->chws[SAMA7D65_PMC_MAIN] = hw;
 
 	for (i = 0; i < PLL_ID_MAX; i++) {
 		for (j = 0; j < PLL_COMPID_MAX; j++) {
@@ -1162,7 +1160,7 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
 			case PLL_TYPE_FRAC:
 				switch (sama7d65_plls[i][j].p) {
 				case SAMA7D65_PLL_PARENT_MAINCK:
-					parent_hw = sama7d65_pmc->chws[PMC_MAIN];
+					parent_hw = sama7d65_pmc->chws[SAMA7D65_PMC_MAIN];
 					break;
 				case SAMA7D65_PLL_PARENT_MAIN_XTAL:
 					parent_hw = main_xtal_hw;
@@ -1211,12 +1209,12 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sama7d65_pmc->chws[PMC_MCK] = hw;
+	sama7d65_pmc->chws[SAMA7D65_PMC_MCK0] = hw;
 	sama7d65_mckx[PCK_PARENT_HW_MCK0].hw = hw;
 
 	parent_hws[0] = md_slck_hw;
 	parent_hws[1] = td_slck_hw;
-	parent_hws[2] = sama7d65_pmc->chws[PMC_MAIN];
+	parent_hws[2] = sama7d65_pmc->chws[SAMA7D65_PMC_MAIN];
 	for (i = PCK_PARENT_HW_MCK1; i < ARRAY_SIZE(sama7d65_mckx); i++) {
 		u8 num_parents = 3 + sama7d65_mckx[i].ep_count;
 		struct clk_hw *tmp_parent_hws[8];
@@ -1265,7 +1263,7 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
 
 	parent_hws[0] = md_slck_hw;
 	parent_hws[1] = td_slck_hw;
-	parent_hws[2] = sama7d65_pmc->chws[PMC_MAIN];
+	parent_hws[2] = sama7d65_pmc->chws[SAMA7D65_PMC_MAIN];
 	parent_hws[3] = sama7d65_plls[PLL_ID_SYS][PLL_COMPID_DIV0].hw;
 	parent_hws[4] = sama7d65_plls[PLL_ID_DDR][PLL_COMPID_DIV0].hw;
 	parent_hws[5] = sama7d65_plls[PLL_ID_GPU][PLL_COMPID_DIV0].hw;
@@ -1316,8 +1314,8 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
 
 	parent_hws[0] = md_slck_hw;
 	parent_hws[1] = td_slck_hw;
-	parent_hws[2] = sama7d65_pmc->chws[PMC_MAIN];
-	parent_hws[3] = sama7d65_pmc->chws[PMC_MCK1];
+	parent_hws[2] = sama7d65_pmc->chws[SAMA7D65_PMC_MAIN];
+	parent_hws[3] = sama7d65_pmc->chws[SAMA7D65_PMC_MCK1];
 	for (i = 0; i < ARRAY_SIZE(sama7d65_gck); i++) {
 		u8 num_parents = 4 + sama7d65_gck[i].pp_count;
 		struct clk_hw *tmp_parent_hws[8];
diff --git a/drivers/clk/at91/sama7g5.c b/drivers/clk/at91/sama7g5.c
index 8385badc1c706..7dfeec8f2232b 100644
--- a/drivers/clk/at91/sama7g5.c
+++ b/drivers/clk/at91/sama7g5.c
@@ -12,7 +12,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/slab.h>
 
-#include <dt-bindings/clock/at91.h>
+#include <dt-bindings/clock/microchip,sama7g5-pmc.h>
 
 #include "pmc.h"
 
@@ -181,7 +181,7 @@ static struct sama7g5_pll {
 			.t = PLL_TYPE_DIV,
 			/* This feeds CPU. It should not be disabled. */
 			.f = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
-			.eid = PMC_CPUPLL,
+			.eid = SAMA7G5_PMC_CPUPLL,
 			/*
 			 * Safe div=15 should be safe even for switching b/w 1GHz and
 			 * 90MHz (frac pll might go up to 1.2GHz).
@@ -216,7 +216,7 @@ static struct sama7g5_pll {
 			 * Therefore it should not be disabled.
 			 */
 			.f = CLK_IS_CRITICAL | CLK_SET_RATE_GATE,
-			.eid = PMC_SYSPLL,
+			.eid = SAMA7G5_PMC_SYSPLL,
 		},
 	},
 
@@ -304,7 +304,7 @@ static struct sama7g5_pll {
 			.t = PLL_TYPE_DIV,
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
-			.eid = PMC_AUDIOPMCPLL,
+			.eid = SAMA7G5_PMC_AUDIOPMCPLL,
 		},
 
 		[PLL_COMPID_DIV1] = {
@@ -315,7 +315,7 @@ static struct sama7g5_pll {
 			.t = PLL_TYPE_DIV,
 			.f = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE |
 			     CLK_SET_RATE_PARENT,
-			.eid = PMC_AUDIOIOPLL,
+			.eid = SAMA7G5_PMC_AUDIOIOPLL,
 		},
 	},
 
@@ -379,7 +379,7 @@ static struct {
 	  .ep_mux_table = { 5, },
 	  .ep_count = 1,
 	  .ep_chg_id = INT_MIN,
-	  .eid = PMC_MCK1,
+	  .eid = SAMA7G5_PMC_MCK1,
 	  .c = 1, },
 
 	{ .n = "mck2",
@@ -995,7 +995,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 	if (IS_ERR(regmap))
 		return;
 
-	sama7g5_pmc = pmc_data_allocate(PMC_MCK1 + 1,
+	sama7g5_pmc = pmc_data_allocate(SAMA7G5_PMC_MCK1 + 1,
 					nck(sama7g5_systemck),
 					nck(sama7g5_periphck),
 					nck(sama7g5_gck), 8);
@@ -1028,7 +1028,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sama7g5_pmc->chws[PMC_MAIN] = hw;
+	sama7g5_pmc->chws[SAMA7G5_PMC_MAIN] = hw;
 
 	for (i = 0; i < PLL_ID_MAX; i++) {
 		for (j = 0; j < PLL_COMPID_MAX; j++) {
@@ -1041,7 +1041,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 			case PLL_TYPE_FRAC:
 				switch (sama7g5_plls[i][j].p) {
 				case SAMA7G5_PLL_PARENT_MAINCK:
-					parent_hw = sama7g5_pmc->chws[PMC_MAIN];
+					parent_hw = sama7g5_pmc->chws[SAMA7G5_PMC_MAIN];
 					break;
 				case SAMA7G5_PLL_PARENT_MAIN_XTAL:
 					parent_hw = main_xtal_hw;
@@ -1090,11 +1090,11 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sama7g5_mckx[PCK_PARENT_HW_MCK0].hw = sama7g5_pmc->chws[PMC_MCK] = hw;
+	sama7g5_mckx[PCK_PARENT_HW_MCK0].hw = sama7g5_pmc->chws[SAMA7G5_PMC_MCK0] = hw;
 
 	parent_hws[0] = md_slck_hw;
 	parent_hws[1] = td_slck_hw;
-	parent_hws[2] = sama7g5_pmc->chws[PMC_MAIN];
+	parent_hws[2] = sama7g5_pmc->chws[SAMA7G5_PMC_MAIN];
 	for (i = PCK_PARENT_HW_MCK1; i < ARRAY_SIZE(sama7g5_mckx); i++) {
 		u8 num_parents = 3 + sama7g5_mckx[i].ep_count;
 		struct clk_hw *tmp_parent_hws[8];
@@ -1136,11 +1136,11 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 	if (IS_ERR(hw))
 		goto err_free;
 
-	sama7g5_pmc->chws[PMC_UTMI] = hw;
+	sama7g5_pmc->chws[SAMA7G5_PMC_UTMI] = hw;
 
 	parent_hws[0] = md_slck_hw;
 	parent_hws[1] = td_slck_hw;
-	parent_hws[2] = sama7g5_pmc->chws[PMC_MAIN];
+	parent_hws[2] = sama7g5_pmc->chws[SAMA7G5_PMC_MAIN];
 	parent_hws[3] = sama7g5_plls[PLL_ID_SYS][PLL_COMPID_DIV0].hw;
 	parent_hws[4] = sama7g5_plls[PLL_ID_DDR][PLL_COMPID_DIV0].hw;
 	parent_hws[5] = sama7g5_plls[PLL_ID_IMG][PLL_COMPID_DIV0].hw;
@@ -1190,7 +1190,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 
 	parent_hws[0] = md_slck_hw;
 	parent_hws[1] = td_slck_hw;
-	parent_hws[2] = sama7g5_pmc->chws[PMC_MAIN];
+	parent_hws[2] = sama7g5_pmc->chws[SAMA7G5_PMC_MAIN];
 	for (i = 0; i < ARRAY_SIZE(sama7g5_gck); i++) {
 		u8 num_parents = 3 + sama7g5_gck[i].pp_count;
 		struct clk_hw *tmp_parent_hws[8];
-- 
2.39.5

[PATCH v2 04/16] dt-bindings: clock: at91: Allow referencing main rc oscillator in DT

[Alexander Dahl]

The main rc oscillator will be needed for the OTPC to work properly.

The new index introduced here was not used on the four affected SoC
clock drivers before, but for sama5d2 only (PMC_I2S1_MUX).

Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - new patch, not present in v1

 include/dt-bindings/clock/microchip,sam9x60-pmc.h  | 3 +++
 include/dt-bindings/clock/microchip,sam9x7-pmc.h   | 3 +++
 include/dt-bindings/clock/microchip,sama7d65-pmc.h | 3 +++
 include/dt-bindings/clock/microchip,sama7g5-pmc.h  | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/include/dt-bindings/clock/microchip,sam9x60-pmc.h b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
index e01e867e8c4da..dcd3c74f75b54 100644
--- a/include/dt-bindings/clock/microchip,sam9x60-pmc.h
+++ b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
@@ -16,4 +16,7 @@
 
 #define SAM9X60_PMC_PLLACK	PMC_PLLACK	/* 7 */
 
+/* new from after bindings splitup */
+#define SAM9X60_PMC_MAIN_RC	6
+
 #endif
diff --git a/include/dt-bindings/clock/microchip,sam9x7-pmc.h b/include/dt-bindings/clock/microchip,sam9x7-pmc.h
index 2df1ff97a5b18..6f17d6553b33c 100644
--- a/include/dt-bindings/clock/microchip,sam9x7-pmc.h
+++ b/include/dt-bindings/clock/microchip,sam9x7-pmc.h
@@ -22,4 +22,7 @@
 #define SAM9X7_PMC_PLLADIV2	PMC_PLLADIV2	/* 14 */
 #define SAM9X7_PMC_LVDSPLL	PMC_LVDSPLL	/* 15 */
 
+/* new from after bindings splitup */
+#define SAM9X7_PMC_MAIN_RC	6
+
 #endif
diff --git a/include/dt-bindings/clock/microchip,sama7d65-pmc.h b/include/dt-bindings/clock/microchip,sama7d65-pmc.h
index f5be643be9b36..5c8e52299c110 100644
--- a/include/dt-bindings/clock/microchip,sama7d65-pmc.h
+++ b/include/dt-bindings/clock/microchip,sama7d65-pmc.h
@@ -29,4 +29,7 @@
 
 #define SAMA7D65_PMC_INDEX_MAX		25
 
+/* new from after bindings splitup */
+#define SAMA7D65_PMC_MAIN_RC		6
+
 #endif
diff --git a/include/dt-bindings/clock/microchip,sama7g5-pmc.h b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
index ad69ccdf9dc78..7bcd2634da37e 100644
--- a/include/dt-bindings/clock/microchip,sama7g5-pmc.h
+++ b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
@@ -21,4 +21,7 @@
 
 #define SAMA7G5_PMC_MCK1	PMC_MCK1	/* 13 */
 
+/* new from after bindings splitup */
+#define SAMA7G5_PMC_MAIN_RC	6
+
 #endif
-- 
2.39.5

[PATCH v2 05/16] clk: at91: Allow enabling main_rc_osc through DT

[Alexander Dahl]

SAM9X60 Datasheet (DS60001579G) Section "23.4 Product Dependencies"
says:

    "The OTPC is clocked through the Power Management Controller (PMC).
    The user must power on the main RC oscillator and enable the
    peripheral clock of the OTPC prior to reading or writing the OTP
    memory."

The code for enabling/disabling that clock is already present, it was
just not possible to hook into DT anymore, after at91 clk devicetree
binding rework back in 2018 for kernel v4.19.

Do it for all controllers with an OTPC controller, where the main rc
oscillator is required for proper operation.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - split out dt-bindings changes into separate patch
    - extend to drivers for other SoCs providing the OTPC

 drivers/clk/at91/sam9x60.c  | 1 +
 drivers/clk/at91/sam9x7.c   | 1 +
 drivers/clk/at91/sama7d65.c | 1 +
 drivers/clk/at91/sama7g5.c  | 1 +
 4 files changed, 4 insertions(+)

diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
index 11fe2d05fa9bb..58a5b6c4473da 100644
--- a/drivers/clk/at91/sam9x60.c
+++ b/drivers/clk/at91/sam9x60.c
@@ -225,6 +225,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np)
 					   50000000);
 	if (IS_ERR(hw))
 		goto err_free;
+	sam9x60_pmc->chws[SAM9X60_PMC_MAIN_RC] = hw;
 
 	hw = at91_clk_register_main_osc(regmap, "main_osc", mainxtal_name, NULL, 0);
 	if (IS_ERR(hw))
diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
index c3c12e0523c4b..8a2955d1f67c6 100644
--- a/drivers/clk/at91/sam9x7.c
+++ b/drivers/clk/at91/sam9x7.c
@@ -758,6 +758,7 @@ static void __init sam9x7_pmc_setup(struct device_node *np)
 					   50000000);
 	if (IS_ERR(hw))
 		goto err_free;
+	sam9x7_pmc->chws[SAM9X7_PMC_MAIN_RC] = hw;
 
 	hw = at91_clk_register_main_osc(regmap, "main_osc", mainxtal_name, NULL, 0);
 	if (IS_ERR(hw))
diff --git a/drivers/clk/at91/sama7d65.c b/drivers/clk/at91/sama7d65.c
index 024c5abee25ff..eaddb154c4381 100644
--- a/drivers/clk/at91/sama7d65.c
+++ b/drivers/clk/at91/sama7d65.c
@@ -1131,6 +1131,7 @@ static void __init sama7d65_pmc_setup(struct device_node *np)
 						   50000000);
 	if (IS_ERR(main_rc_hw))
 		goto err_free;
+	sama7d65_pmc->chws[SAMA7D65_PMC_MAIN_RC] = hw;
 
 	bypass = of_property_read_bool(np, "atmel,osc-bypass");
 
diff --git a/drivers/clk/at91/sama7g5.c b/drivers/clk/at91/sama7g5.c
index 7dfeec8f2232b..e6d5739371a76 100644
--- a/drivers/clk/at91/sama7g5.c
+++ b/drivers/clk/at91/sama7g5.c
@@ -1012,6 +1012,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 						   50000000);
 	if (IS_ERR(main_rc_hw))
 		goto err_free;
+	sama7g5_pmc->chws[SAMA7G5_PMC_MAIN_RC] = hw;
 
 	bypass = of_property_read_bool(np, "atmel,osc-bypass");
 
-- 
2.39.5

[PATCH v2 06/16] clk: at91: Add peripheral id for OTPC

[Alexander Dahl]

That peripheral clock is required for proper OTPC function.

Link: https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@microchip.com/T/#u
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - new patch in series, was not present in v1

 drivers/clk/at91/sam9x60.c  | 1 +
 drivers/clk/at91/sam9x7.c   | 1 +
 drivers/clk/at91/sama7d65.c | 1 +
 drivers/clk/at91/sama7g5.c  | 1 +
 4 files changed, 4 insertions(+)

diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
index 58a5b6c4473da..ce0f73125e87c 100644
--- a/drivers/clk/at91/sam9x60.c
+++ b/drivers/clk/at91/sam9x60.c
@@ -144,6 +144,7 @@ static const struct {
 	{ .n = "isi_clk",    .id = 43, },
 	{ .n = "pioD_clk",   .id = 44, },
 	{ .n = "tcb1_clk",   .id = 45, },
+	{ .n = "otpc_clk",   .id = 46, },
 	{ .n = "dbgu_clk",   .id = 47, },
 	/*
 	 * mpddr_clk feeds DDR controller and is enabled by bootloader thus we
diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
index 8a2955d1f67c6..7278b9d15d0cf 100644
--- a/drivers/clk/at91/sam9x7.c
+++ b/drivers/clk/at91/sam9x7.c
@@ -402,6 +402,7 @@ static const struct {
 	{ .n = "isi_clk",	.id = 43, },
 	{ .n = "pioD_clk",	.id = 44, },
 	{ .n = "tcb1_clk",	.id = 45, },
+	{ .n = "otpc_clk",	.id = 46, },
 	{ .n = "dbgu_clk",	.id = 47, },
 	/*
 	 * mpddr_clk feeds DDR controller and is enabled by bootloader thus we
diff --git a/drivers/clk/at91/sama7d65.c b/drivers/clk/at91/sama7d65.c
index eaddb154c4381..19613e587d5b9 100644
--- a/drivers/clk/at91/sama7d65.c
+++ b/drivers/clk/at91/sama7d65.c
@@ -637,6 +637,7 @@ static struct {
 	{ .n = "mcan2_clk",	.p = PCK_PARENT_HW_MCK5, .id = 60, .r = { .max = 200000000, }, },
 	{ .n = "mcan3_clk",	.p = PCK_PARENT_HW_MCK5, .id = 61, .r = { .max = 200000000, }, },
 	{ .n = "mcan4_clk",	.p = PCK_PARENT_HW_MCK5, .id = 62, .r = { .max = 200000000, }, },
+	{ .n = "otpc_clk",	.p = PCK_PARENT_HW_MCK0, .id = 63, },
 	{ .n = "pdmc0_clk",	.p = PCK_PARENT_HW_MCK9, .id = 64, .r = { .max = 200000000, }, },
 	{ .n = "pdmc1_clk",	.p = PCK_PARENT_HW_MCK9, .id = 65, .r = { .max = 200000000, }, },
 	{ .n = "pit64b0_clk",	.p = PCK_PARENT_HW_MCK7, .id = 66, },
diff --git a/drivers/clk/at91/sama7g5.c b/drivers/clk/at91/sama7g5.c
index e6d5739371a76..5147d8f34a3be 100644
--- a/drivers/clk/at91/sama7g5.c
+++ b/drivers/clk/at91/sama7g5.c
@@ -502,6 +502,7 @@ static struct {
 	{ .n = "mcan3_clk",	.p = PCK_PARENT_HW_MCK1, .id = 64, .r = { .max = 200000000, }, },
 	{ .n = "mcan4_clk",	.p = PCK_PARENT_HW_MCK1, .id = 65, .r = { .max = 200000000, }, },
 	{ .n = "mcan5_clk",	.p = PCK_PARENT_HW_MCK1, .id = 66, .r = { .max = 200000000, }, },
+	{ .n = "otpc_clk",	.p = PCK_PARENT_HW_MCK0, .id = 67, },
 	{ .n = "pdmc0_clk",	.p = PCK_PARENT_HW_MCK1, .id = 68, .r = { .max = 200000000, }, },
 	{ .n = "pdmc1_clk",	.p = PCK_PARENT_HW_MCK1, .id = 69, .r = { .max = 200000000, }, },
 	{ .n = "pit64b0_clk",	.p = PCK_PARENT_HW_MCK1, .id = 70, },
-- 
2.39.5

[PATCH v2 07/16] dt-bindings: nvmem: microchip-otpc: Add compatible for SAM9X60

[Alexander Dahl]

The SAM9X60 SoC family has a similar, but slightly different OTPC to the
SAMA7G5 family.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - Fix dt_binding_check warnings

 .../devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml     | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
index cc25f2927682e..9a7aaf64eef32 100644
--- a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
+++ b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
@@ -21,7 +21,9 @@ allOf:
 properties:
   compatible:
     items:
-      - const: microchip,sama7g5-otpc
+      - enum:
+          - microchip,sam9x60-otpc
+          - microchip,sama7g5-otpc
       - const: syscon
 
   reg:
-- 
2.39.5

[PATCH v2 08/16] dt-bindings: nvmem: microchip-otpc: Add required clocks

[Alexander Dahl]

The OTPC requires both the peripheral clock through PMC and the main RC
oscillator.  Seemed to work without explicitly enabling those clocks on
sama7g5 before, but did not on sam9x60.

Older datasheets were not clear and explicit about this, but recent are,
e.g. SAMA7G5 series datasheet (DS60001765B),
section 30.4.1 Power Management:

> The OTPC is clocked through the Power Management Controller (PMC).
> The user must power on the main RC oscillator and enable the
> peripheral clock of the OTPC prior to reading or writing the OTP
> memory.

Link: https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@microchip.com/T/#u
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - new patch, not present in v1

 .../nvmem/microchip,sama7g5-otpc.yaml         | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
index 9a7aaf64eef32..1fa40610888f3 100644
--- a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
+++ b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
@@ -29,6 +29,16 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    items:
+      - description: main rc oscillator
+      - description: otpc peripheral clock
+
+  clock-names:
+    items:
+      - const: main_rc_osc
+      - const: otpc_clk
+
 required:
   - compatible
   - reg
@@ -37,6 +47,8 @@ unevaluatedProperties: false
 
 examples:
   - |
+    #include <dt-bindings/clock/at91.h>
+    #include <dt-bindings/clock/microchip,sama7g5-pmc.h>
     #include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>
 
     otpc: efuse@e8c00000 {
@@ -44,10 +56,26 @@ examples:
         reg = <0xe8c00000 0xec>;
         #address-cells = <1>;
         #size-cells = <1>;
+        clocks = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 67>;
+        clock-names = "main_rc_osc", "otpc_clk";
 
         temperature_calib: calib@1 {
             reg = <OTP_PKT(1) 76>;
         };
     };
 
+  - |
+    #include <dt-bindings/clock/at91.h>
+    #include <dt-bindings/clock/microchip,sam9x60-pmc.h>
+    #include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>
+
+    efuse@eff00000 {
+        compatible = "microchip,sam9x60-otpc", "syscon";
+        reg = <0xeff00000 0xec>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+        clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 46>;
+        clock-names = "main_rc_osc", "otpc_clk";
+    };
+
 ...
-- 
2.39.5

[PATCH v2 09/16] nvmem: microchip-otpc: Avoid reading a write-only register

[Alexander Dahl]

The OTPC Control Register (OTPC_CR) has just write-only members.
Reading from that register leads to a warning in OTPC Write Protection
Status Register (OTPC_WPSR) in field Software Error Type (SWETYP) of
type READ_WO (A write-only register has been read (warning).)

Just create the register write content from scratch is sufficient here.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
Fixes: 98830350d3fc ("nvmem: microchip-otpc: add support")
---

Notes:
    v2:
    - Add Fixes tag
    - Remove temporary variable usage
    - Reword misleading subject (s/writing/reading/)

 drivers/nvmem/microchip-otpc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
index df979e8549fdb..e2851c63cc0b4 100644
--- a/drivers/nvmem/microchip-otpc.c
+++ b/drivers/nvmem/microchip-otpc.c
@@ -82,9 +82,7 @@ static int mchp_otpc_prepare_read(struct mchp_otpc *otpc,
 	writel_relaxed(tmp, otpc->base + MCHP_OTPC_MR);
 
 	/* Set read. */
-	tmp = readl_relaxed(otpc->base + MCHP_OTPC_CR);
-	tmp |= MCHP_OTPC_CR_READ;
-	writel_relaxed(tmp, otpc->base + MCHP_OTPC_CR);
+	writel_relaxed(MCHP_OTPC_CR_READ, otpc->base + MCHP_OTPC_CR);
 
 	/* Wait for packet to be transferred into temporary buffers. */
 	return read_poll_timeout(readl_relaxed, tmp, !(tmp & MCHP_OTPC_SR_READ),
-- 
2.39.5

Re: [PATCH v2 08/16] dt-bindings: nvmem: microchip-otpc: Add required clocks

[Krzysztof Kozlowski]

On 10/02/2025 17:44, Alexander Dahl wrote:
> The OTPC requires both the peripheral clock through PMC and the main RC
> oscillator.  Seemed to work without explicitly enabling those clocks on
> sama7g5 before, but did not on sam9x60.
> 
> Older datasheets were not clear and explicit about this, but recent are,
> e.g. SAMA7G5 series datasheet (DS60001765B),
> section 30.4.1 Power Management:
> 
>> The OTPC is clocked through the Power Management Controller (PMC).
>> The user must power on the main RC oscillator and enable the
>> peripheral clock of the OTPC prior to reading or writing the OTP
>> memory.
> 
> Link: https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@microchip.com/T/#u
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
> 
> Notes:
>     v2:
>     - new patch, not present in v1
> 
>  .../nvmem/microchip,sama7g5-otpc.yaml         | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
> index 9a7aaf64eef32..1fa40610888f3 100644
> --- a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
> @@ -29,6 +29,16 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  clocks:
> +    items:
> +      - description: main rc oscillator
> +      - description: otpc peripheral clock
> +
> +  clock-names:
> +    items:
> +      - const: main_rc_osc

osc

> +      - const: otpc_clk

otpc or bus or whatever logically this is

> +
>  required:
>    - compatible
>    - reg
> @@ -37,6 +47,8 @@ unevaluatedProperties: false
>  
>  examples:
>    - |
> +    #include <dt-bindings/clock/at91.h>
> +    #include <dt-bindings/clock/microchip,sama7g5-pmc.h>
>      #include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>
>  
>      otpc: efuse@e8c00000 {
> @@ -44,10 +56,26 @@ examples:
>          reg = <0xe8c00000 0xec>;
>          #address-cells = <1>;
>          #size-cells = <1>;
> +        clocks = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 67>;
> +        clock-names = "main_rc_osc", "otpc_clk";
>  
>          temperature_calib: calib@1 {
>              reg = <OTP_PKT(1) 76>;
>          };
>      };
>  
> +  - |
> +    #include <dt-bindings/clock/at91.h>
> +    #include <dt-bindings/clock/microchip,sam9x60-pmc.h>
> +    #include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>
> +
> +    efuse@eff00000 {
> +        compatible = "microchip,sam9x60-otpc", "syscon";
> +        reg = <0xeff00000 0xec>;

No need for new example with difference in what exactly? Even compatible
was not added here...

> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 46>;
> +        clock-names = "main_rc_osc", "otpc_clk";
> +    };
> +
>  ...


Best regards,
Krzysztof

Re: [PATCH v2 01/16] dt-bindings: clock: at91: Split up per SoC partially

[Krzysztof Kozlowski]

On 10/02/2025 17:44, Alexander Dahl wrote:
> Before adding even more new indexes creating more holes in the
> clk at91 drivers pmc_data->chws arrays, split this up.
> 
> This is a partial split up only for SoCs affected by upcoming changes
> and by that PMC_MAIN + x hack, others could follow by the same scheme.
> 
> Binding splitup was proposed for several reasons:
> 
> 1) keep the driver code simple, readable, and efficient
> 2) avoid accidental array index duplication
> 3) avoid memory waste by creating more and more unused array members.
> 
> Old values are kept to not break dts, and to maintain dt ABI.
> 
> Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
> 
> Notes:
>     v2:
>     - new patch, not present in v1
> 
>  .../dt-bindings/clock/microchip,sam9x60-pmc.h | 19 +++++++++++
>  .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 25 +++++++++++++++
>  .../clock/microchip,sama7d65-pmc.h            | 32 +++++++++++++++++++
>  .../dt-bindings/clock/microchip,sama7g5-pmc.h | 24 ++++++++++++++
>  4 files changed, 100 insertions(+)
>  create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
>  create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
>  create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
>  create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h
> 
> diff --git a/include/dt-bindings/clock/microchip,sam9x60-pmc.h b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> new file mode 100644
> index 0000000000000..e01e867e8c4da
> --- /dev/null
> +++ b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * The constants defined in this header are being used in dts and in
> + * at91 sam9x60 clock driver.
> + */
> +
> +#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H
> +#define _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H
> +
> +#include <dt-bindings/clock/at91.h>
> +
> +/* old from before bindings splitup */
> +#define SAM9X60_PMC_MCK		PMC_MCK		/* 1 */
> +#define SAM9X60_PMC_UTMI	PMC_UTMI	/* 2 */
> +#define SAM9X60_PMC_MAIN	PMC_MAIN	/* 3 */
> +
> +#define SAM9X60_PMC_PLLACK	PMC_PLLACK	/* 7 */

IIUC, you want to have bindings per SoC, so why not adding proper
constants here instead of including entire old binding header? The
binding header should be entirely abandoned later.


Best regards,
Krzysztof

Re: [PATCH v2 04/16] dt-bindings: clock: at91: Allow referencing main rc oscillator in DT

[Krzysztof Kozlowski]

On 10/02/2025 17:44, Alexander Dahl wrote:
> The main rc oscillator will be needed for the OTPC to work properly.
> 
> The new index introduced here was not used on the four affected SoC
> clock drivers before, but for sama5d2 only (PMC_I2S1_MUX).
> 
> Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
> 
> Notes:
>     v2:
>     - new patch, not present in v1
> 
>  include/dt-bindings/clock/microchip,sam9x60-pmc.h  | 3 +++
>  include/dt-bindings/clock/microchip,sam9x7-pmc.h   | 3 +++
>  include/dt-bindings/clock/microchip,sama7d65-pmc.h | 3 +++
>  include/dt-bindings/clock/microchip,sama7g5-pmc.h  | 3 +++
>  4 files changed, 12 insertions(+)
> 
> diff --git a/include/dt-bindings/clock/microchip,sam9x60-pmc.h b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> index e01e867e8c4da..dcd3c74f75b54 100644
> --- a/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> +++ b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> @@ -16,4 +16,7 @@
>  
>  #define SAM9X60_PMC_PLLACK	PMC_PLLACK	/* 7 */
>  
> +/* new from after bindings splitup */
> +#define SAM9X60_PMC_MAIN_RC	6

This is confusing me, because:
1. You still have holes in IDs
2. This should be placed in proper order by ID
3. Why not using 4 - the next available empty ID?

Best regards,
Krzysztof

Re: [PATCH v2 09/16] nvmem: microchip-otpc: Avoid reading a write-only register

[Greg Kroah-Hartman]

On Mon, Feb 10, 2025 at 05:50:47PM +0100, Alexander Dahl wrote:
> The OTPC Control Register (OTPC_CR) has just write-only members.
> Reading from that register leads to a warning in OTPC Write Protection
> Status Register (OTPC_WPSR) in field Software Error Type (SWETYP) of
> type READ_WO (A write-only register has been read (warning).)
> 
> Just create the register write content from scratch is sufficient here.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> Fixes: 98830350d3fc ("nvmem: microchip-otpc: add support")
> ---
> 
> Notes:
>     v2:
>     - Add Fixes tag
>     - Remove temporary variable usage
>     - Reword misleading subject (s/writing/reading/)
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

[PATCH v2 10/16] nvmem: microchip-otpc: Fix swapped 'sleep' and 'timeout' parameters

[Alexander Dahl]

Makes no sense to have a timeout shorter than the sleep time, it would
run into timeout right after the first sleep already.
While at it, use a more specific macro instead of the generic one, which
does exactly the same, but needs less parameters.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
Fixes: 98830350d3fc ("nvmem: microchip-otpc: add support")
---

Notes:
    v2:
    - Add Fixes tag

 drivers/nvmem/microchip-otpc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
index e2851c63cc0b4..bf7e5167152cb 100644
--- a/drivers/nvmem/microchip-otpc.c
+++ b/drivers/nvmem/microchip-otpc.c
@@ -85,8 +85,8 @@ static int mchp_otpc_prepare_read(struct mchp_otpc *otpc,
 	writel_relaxed(MCHP_OTPC_CR_READ, otpc->base + MCHP_OTPC_CR);
 
 	/* Wait for packet to be transferred into temporary buffers. */
-	return read_poll_timeout(readl_relaxed, tmp, !(tmp & MCHP_OTPC_SR_READ),
-				 10000, 2000, false, otpc->base + MCHP_OTPC_SR);
+	return readl_relaxed_poll_timeout(otpc->base + MCHP_OTPC_SR, tmp,
+					  !(tmp & MCHP_OTPC_SR_READ), 2000, 10000);
 }
 
 /*
-- 
2.39.5

[PATCH v2 11/16] nvmem: microchip-otpc: Add SAM9X60 support

[Alexander Dahl]

Register layout is almost identical to SAMA7G5 OTPC.  SAMA7G5 has some
additional bits in common registers, and some additional registers all
related to custom packages in secure world.  None of these are currently
used by the driver.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - Reword commit message (additional information about SoC differences)

 drivers/nvmem/microchip-otpc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
index bf7e5167152cb..d39f2d57e5f5e 100644
--- a/drivers/nvmem/microchip-otpc.c
+++ b/drivers/nvmem/microchip-otpc.c
@@ -269,6 +269,7 @@ static int mchp_otpc_probe(struct platform_device *pdev)
 
 static const struct of_device_id __maybe_unused mchp_otpc_ids[] = {
 	{ .compatible = "microchip,sama7g5-otpc", },
+	{ .compatible = "microchip,sam9x60-otpc", },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, mchp_otpc_ids);
-- 
2.39.5

[PATCH v2 12/16] ARM: dts: microchip: sama7g5: Add OTPC clocks

[Alexander Dahl]

These clocks should be enabled, datasheet says:

> The OTPC is clocked through the Power Management Controller (PMC).
> The user must power on the main RC oscillator and enable the
> peripheral clock of the OTPC prior to reading or writing the OTP
> memory.

Earlier discussions suggest, MCK0 must be enabled, too.  MCK0 is parent
of peripheral otpc_clk, so this is done implicitly.

Link: https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@microchip.com/T/#u
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - new patch, not present in v1

 arch/arm/boot/dts/microchip/sama7g5.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/microchip/sama7g5.dtsi b/arch/arm/boot/dts/microchip/sama7g5.dtsi
index f68c2eb8edd54..b33204688b90c 100644
--- a/arch/arm/boot/dts/microchip/sama7g5.dtsi
+++ b/arch/arm/boot/dts/microchip/sama7g5.dtsi
@@ -1023,6 +1023,8 @@ otpc: efuse@e8c00000 {
 			reg = <0xe8c00000 0x100>;
 			#address-cells = <1>;
 			#size-cells = <1>;
+			clocks = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 67>;
+			clock-names = "main_rc_osc", "otpc_clk";
 
 			temperature_calib: calib@1 {
 				reg = <OTP_PKT(1) 76>;
-- 
2.39.5

[PATCH v2 13/16] ARM: dts: microchip: sam9x60: Add OTPC node

[Alexander Dahl]

The One-Time Programmable (OTP) Memory Controller (OTPC) is the secure
interface between the system and the OTP memory.  It also features the
Unique Product ID (UID) registers containing a unique serial number.

See datasheet (DS60001579G) sections "7. Memories" and "23. OTP Memory
Controller (OTPC)" for reference.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - squashed with patch adding the clock properties

 arch/arm/boot/dts/microchip/sam9x60.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi
index 1724b79967a17..af859f0b83a0f 100644
--- a/arch/arm/boot/dts/microchip/sam9x60.dtsi
+++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi
@@ -15,6 +15,7 @@
 #include <dt-bindings/clock/microchip,sam9x60-pmc.h>
 #include <dt-bindings/mfd/at91-usart.h>
 #include <dt-bindings/mfd/atmel-flexcom.h>
+#include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>
 
 / {
 	#address-cells = <1>;
@@ -157,6 +158,15 @@ sdmmc1: sdio-host@90000000 {
 			status = "disabled";
 		};
 
+		otpc: efuse@eff00000 {
+			compatible = "microchip,sam9x60-otpc", "syscon";
+			reg = <0xeff00000 0xec>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 46>;
+			clock-names = "main_rc_osc", "otpc_clk";
+		};
+
 		apb {
 			compatible = "simple-bus";
 			#address-cells = <1>;
-- 
2.39.5

[PATCH v2 14/16] ARM: dts: microchip: sam9x60_curiosity: Enable OTP Controller

[Alexander Dahl]

Allows to access the OTP memory now, and Product UID later.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - same as in v1, no changes

 arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts
index b9ffd9e5faacc..c110a8e87568a 100644
--- a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts
+++ b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts
@@ -252,6 +252,10 @@ ethernet-phy@0 {
 	};
 };
 
+&otpc {
+	status = "okay";
+};
+
 &pinctrl {
 	adc {
 		pinctrl_adc_default: adc-default {
-- 
2.39.5

[PATCH v2 15/16] nvmem: microchip-otpc: Enable necessary clocks

[Alexander Dahl]

Without enabling the main rc clock, initializing the packet list leads
to a read timeout on the first packet, at least on sam9x60.

According to SAM9X60 datasheet (DS60001579G) section "23.4 Product
Dependencies" the clock must be enabled for reading and writing.

Tested on sam9x60-curiosity board.

Link: https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@microchip.com/T/#u
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - Rewrite to enable _all_ clocks defined in dts

 drivers/nvmem/microchip-otpc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
index d39f2d57e5f5e..2c524c163b7e2 100644
--- a/drivers/nvmem/microchip-otpc.c
+++ b/drivers/nvmem/microchip-otpc.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/clk.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/nvmem-provider.h>
@@ -241,6 +242,7 @@ static struct nvmem_config mchp_nvmem_config = {
 static int mchp_otpc_probe(struct platform_device *pdev)
 {
 	struct nvmem_device *nvmem;
+	struct clk_bulk_data *clks;
 	struct mchp_otpc *otpc;
 	u32 size;
 	int ret;
@@ -253,6 +255,11 @@ static int mchp_otpc_probe(struct platform_device *pdev)
 	if (IS_ERR(otpc->base))
 		return PTR_ERR(otpc->base);
 
+	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &clks);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Error getting clocks!\n");
+
 	otpc->dev = &pdev->dev;
 	ret = mchp_otpc_init_packets_list(otpc, &size);
 	if (ret)
-- 
2.39.5

[PATCH v2 16/16] nvmem: microchip-otpc: Expose UID registers as 2nd nvmem device

[Alexander Dahl]

For SAM9X60 the Product UID x Register containing the Unique Product ID
is part of the OTPC registers.  We have everything at hand here to just
create a trivial nvmem device for those.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - Use dev_err_probe() for error reporting (thanks Claudiu)
    - Move required register definition over here from removed patch

 drivers/nvmem/microchip-otpc.c | 38 +++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
index 2c524c163b7e2..8353a117769a8 100644
--- a/drivers/nvmem/microchip-otpc.c
+++ b/drivers/nvmem/microchip-otpc.c
@@ -25,10 +25,14 @@
 #define MCHP_OTPC_HR			(0x20)
 #define MCHP_OTPC_HR_SIZE		GENMASK(15, 8)
 #define MCHP_OTPC_DR			(0x24)
+#define MCHP_OTPC_UID0R			(0x60)
 
 #define MCHP_OTPC_NAME			"mchp-otpc"
 #define MCHP_OTPC_SIZE			(11 * 1024)
 
+#define MCHP_OTPC_UID_NAME		"mchp-uid"
+#define MCHP_OTPC_UID_SIZE		16
+
 /**
  * struct mchp_otpc - OTPC private data structure
  * @base: base address
@@ -230,6 +234,16 @@ static int mchp_otpc_init_packets_list(struct mchp_otpc *otpc, u32 *size)
 	return 0;
 }
 
+static int mchp_otpc_uid_read(void *priv, unsigned int offset,
+			      void *val, size_t bytes)
+{
+	struct mchp_otpc *otpc = priv;
+
+	memcpy_fromio(val, otpc->base + MCHP_OTPC_UID0R + offset, bytes);
+
+	return 0;
+}
+
 static struct nvmem_config mchp_nvmem_config = {
 	.name = MCHP_OTPC_NAME,
 	.type = NVMEM_TYPE_OTP,
@@ -239,6 +253,15 @@ static struct nvmem_config mchp_nvmem_config = {
 	.reg_read = mchp_otpc_read,
 };
 
+static struct nvmem_config mchp_otpc_uid_nvmem_config = {
+	.name = MCHP_OTPC_UID_NAME,
+	.read_only = true,
+	.word_size = 4,
+	.stride = 4,
+	.size = MCHP_OTPC_UID_SIZE,
+	.reg_read = mchp_otpc_uid_read,
+};
+
 static int mchp_otpc_probe(struct platform_device *pdev)
 {
 	struct nvmem_device *nvmem;
@@ -270,8 +293,21 @@ static int mchp_otpc_probe(struct platform_device *pdev)
 	mchp_nvmem_config.size = size;
 	mchp_nvmem_config.priv = otpc;
 	nvmem = devm_nvmem_register(&pdev->dev, &mchp_nvmem_config);
+	if (IS_ERR(nvmem)) {
+		return dev_err_probe(&pdev->dev, PTR_ERR(nvmem),
+				     "Error registering OTP as nvmem device\n");
+	}
 
-	return PTR_ERR_OR_ZERO(nvmem);
+	mchp_otpc_uid_nvmem_config.dev = otpc->dev;
+	mchp_otpc_uid_nvmem_config.priv = otpc;
+
+	nvmem = devm_nvmem_register(&pdev->dev, &mchp_otpc_uid_nvmem_config);
+	if (IS_ERR(nvmem)) {
+		return dev_err_probe(&pdev->dev, PTR_ERR(nvmem),
+				     "Error registering UIDxR as nvmem device\n");
+	}
+
+	return 0;
 }
 
 static const struct of_device_id __maybe_unused mchp_otpc_ids[] = {
-- 
2.39.5

Re: [PATCH v2 08/16] dt-bindings: nvmem: microchip-otpc: Add required clocks

[Alexander Dahl]

Hello Krzysztof,

Am Mon, Feb 10, 2025 at 05:59:52PM +0100 schrieb Krzysztof Kozlowski:
> On 10/02/2025 17:44, Alexander Dahl wrote:
> > The OTPC requires both the peripheral clock through PMC and the main RC
> > oscillator.  Seemed to work without explicitly enabling those clocks on
> > sama7g5 before, but did not on sam9x60.
> > 
> > Older datasheets were not clear and explicit about this, but recent are,
> > e.g. SAMA7G5 series datasheet (DS60001765B),
> > section 30.4.1 Power Management:
> > 
> >> The OTPC is clocked through the Power Management Controller (PMC).
> >> The user must power on the main RC oscillator and enable the
> >> peripheral clock of the OTPC prior to reading or writing the OTP
> >> memory.
> > 
> > Link: https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@microchip.com/T/#u
> > Signed-off-by: Alexander Dahl <ada@thorsis.com>
> > ---
> > 
> > Notes:
> >     v2:
> >     - new patch, not present in v1
> > 
> >  .../nvmem/microchip,sama7g5-otpc.yaml         | 28 +++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
> > index 9a7aaf64eef32..1fa40610888f3 100644
> > --- a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
> > +++ b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
> > @@ -29,6 +29,16 @@ properties:
> >    reg:
> >      maxItems: 1
> >  
> > +  clocks:
> > +    items:
> > +      - description: main rc oscillator
> > +      - description: otpc peripheral clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: main_rc_osc
> 
> osc

On at91 SoCs main oscillator and main RC oscillator are two different
things, and those are different clocks in Linux as well.  This clock
is named "main_rc_osc" in the clock driver.  In
drivers/clk/at91/sam9x60.c this clock is added like this:

    hw = at91_clk_register_main_rc_osc(regmap, "main_rc_osc", 12000000, 50000000);

The datasheet makes it explicit, it's exactly the main rc oscillator
clock required for the OTPC to work, no other clock.

So why name this "osc" only then?  This is confusing at best.

> 
> > +      - const: otpc_clk
> 
> otpc or bus or whatever logically this is

Okay the "_clk" suffix is redundant.  Since the peripheral clock for
the OTPC is required here, I would go with "otpc" only then.

> 
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -37,6 +47,8 @@ unevaluatedProperties: false
> >  
> >  examples:
> >    - |
> > +    #include <dt-bindings/clock/at91.h>
> > +    #include <dt-bindings/clock/microchip,sama7g5-pmc.h>
> >      #include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>
> >  
> >      otpc: efuse@e8c00000 {
> > @@ -44,10 +56,26 @@ examples:
> >          reg = <0xe8c00000 0xec>;
> >          #address-cells = <1>;
> >          #size-cells = <1>;
> > +        clocks = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 67>;
> > +        clock-names = "main_rc_osc", "otpc_clk";
> >  
> >          temperature_calib: calib@1 {
> >              reg = <OTP_PKT(1) 76>;
> >          };
> >      };
> >  
> > +  - |
> > +    #include <dt-bindings/clock/at91.h>
> > +    #include <dt-bindings/clock/microchip,sam9x60-pmc.h>
> > +    #include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>
> > +
> > +    efuse@eff00000 {
> > +        compatible = "microchip,sam9x60-otpc", "syscon";
> > +        reg = <0xeff00000 0xec>;
> 
> No need for new example with difference in what exactly? Even compatible
> was not added here...

Different compatible, different clocks, no sub nodes, different
peripheral clock id …  From a human doc readers I'd like another
example, but fine, we can drop it if it adds too much redundancy.

Greets
Alex

> 
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 46>;
> > +        clock-names = "main_rc_osc", "otpc_clk";
> > +    };
> > +
> >  ...
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 01/16] dt-bindings: clock: at91: Split up per SoC partially

[Alexander Dahl]

Hello everyone,

Am Mon, Feb 10, 2025 at 06:05:31PM +0100 schrieb Krzysztof Kozlowski:
> On 10/02/2025 17:44, Alexander Dahl wrote:
> > Before adding even more new indexes creating more holes in the
> > clk at91 drivers pmc_data->chws arrays, split this up.
> > 
> > This is a partial split up only for SoCs affected by upcoming changes
> > and by that PMC_MAIN + x hack, others could follow by the same scheme.
> > 
> > Binding splitup was proposed for several reasons:
> > 
> > 1) keep the driver code simple, readable, and efficient
> > 2) avoid accidental array index duplication
> > 3) avoid memory waste by creating more and more unused array members.
> > 
> > Old values are kept to not break dts, and to maintain dt ABI.
> > 
> > Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
> > Signed-off-by: Alexander Dahl <ada@thorsis.com>
> > ---
> > 
> > Notes:
> >     v2:
> >     - new patch, not present in v1
> > 
> >  .../dt-bindings/clock/microchip,sam9x60-pmc.h | 19 +++++++++++
> >  .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 25 +++++++++++++++
> >  .../clock/microchip,sama7d65-pmc.h            | 32 +++++++++++++++++++
> >  .../dt-bindings/clock/microchip,sama7g5-pmc.h | 24 ++++++++++++++
> >  4 files changed, 100 insertions(+)
> >  create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
> >  create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
> >  create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
> >  create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h
> > 
> > diff --git a/include/dt-bindings/clock/microchip,sam9x60-pmc.h b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> > new file mode 100644
> > index 0000000000000..e01e867e8c4da
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> > +/*
> > + * The constants defined in this header are being used in dts and in
> > + * at91 sam9x60 clock driver.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H
> > +#define _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H
> > +
> > +#include <dt-bindings/clock/at91.h>
> > +
> > +/* old from before bindings splitup */
> > +#define SAM9X60_PMC_MCK		PMC_MCK		/* 1 */
> > +#define SAM9X60_PMC_UTMI	PMC_UTMI	/* 2 */
> > +#define SAM9X60_PMC_MAIN	PMC_MAIN	/* 3 */
> > +
> > +#define SAM9X60_PMC_PLLACK	PMC_PLLACK	/* 7 */
> 
> IIUC, you want to have bindings per SoC, so why not adding proper
> constants here instead of including entire old binding header? The
> binding header should be entirely abandoned later.

Which binding header should be abandoned entirely?

The bindings per SoC idea was proposed in series v1 feedback.  I'm
neither a binding nor a clock expeert.  As far as I understood it's
important to keep the exact same values as before to not change any
ABI.  The non SoC specific values are still used on older SoCs of the
at91 family, so this is why I used the old constants for now.

These PMC indexes are not the only definitions in
dt-bindings/clock/at91.h however, there are more which are not SoC
specific.

I'd like some thoughts from the Microchip maintainers here,
what's their idea on how to proceed with the at91 clock stuff?

This works for me, but the current state is more or less still an idea
as base for discussion.  Please don't make it overly complicated, this
is not the primary focus of my work.

Thanks and greets
Alex

> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 04/16] dt-bindings: clock: at91: Allow referencing main rc oscillator in DT

[Alexander Dahl]

Hello Krzysztof,

Am Mon, Feb 10, 2025 at 06:07:10PM +0100 schrieb Krzysztof Kozlowski:
> On 10/02/2025 17:44, Alexander Dahl wrote:
> > The main rc oscillator will be needed for the OTPC to work properly.
> > 
> > The new index introduced here was not used on the four affected SoC
> > clock drivers before, but for sama5d2 only (PMC_I2S1_MUX).
> > 
> > Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
> > Signed-off-by: Alexander Dahl <ada@thorsis.com>
> > ---
> > 
> > Notes:
> >     v2:
> >     - new patch, not present in v1
> > 
> >  include/dt-bindings/clock/microchip,sam9x60-pmc.h  | 3 +++
> >  include/dt-bindings/clock/microchip,sam9x7-pmc.h   | 3 +++
> >  include/dt-bindings/clock/microchip,sama7d65-pmc.h | 3 +++
> >  include/dt-bindings/clock/microchip,sama7g5-pmc.h  | 3 +++
> >  4 files changed, 12 insertions(+)
> > 
> > diff --git a/include/dt-bindings/clock/microchip,sam9x60-pmc.h b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> > index e01e867e8c4da..dcd3c74f75b54 100644
> > --- a/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> > +++ b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
> > @@ -16,4 +16,7 @@
> >  
> >  #define SAM9X60_PMC_PLLACK	PMC_PLLACK	/* 7 */
> >  
> > +/* new from after bindings splitup */
> > +#define SAM9X60_PMC_MAIN_RC	6
> 
> This is confusing me, because:
> 1. You still have holes in IDs

Yes, I was told to maintain the old values for interface stability in
series v1 feedback.

> 2. This should be placed in proper order by ID

Okay, no problem.

> 3. Why not using 4 - the next available empty ID?

The MAIN_RC clock is used on four out of thirteen (?) SoC variants
which all used the same IDs before.  6 is the first ID which is free
on all of sam9x60, sam9x7, sama7g5, and sama7d65.  The last two
already use 4 for a different clock.

The whole splitup is to avoid even more and/or bigger holes, but is it
important where the existent holes are filled?

Technically if the next available empty ID should be used it would be
4 for sam9x60 and sam9x7, 2 for sama7d65, and 6 for sama7g5.  I
thought it would be nice to use the same value instead to make
somewhat compatible to the old approach.

Greets
Alex

Re: [PATCH v2 01/16] dt-bindings: clock: at91: Split up per SoC partially

[Krzysztof Kozlowski]

On 11/02/2025 08:16, Alexander Dahl wrote:
> Hello everyone,
> 
> Am Mon, Feb 10, 2025 at 06:05:31PM +0100 schrieb Krzysztof Kozlowski:
>> On 10/02/2025 17:44, Alexander Dahl wrote:
>>> Before adding even more new indexes creating more holes in the
>>> clk at91 drivers pmc_data->chws arrays, split this up.
>>>
>>> This is a partial split up only for SoCs affected by upcoming changes
>>> and by that PMC_MAIN + x hack, others could follow by the same scheme.
>>>
>>> Binding splitup was proposed for several reasons:
>>>
>>> 1) keep the driver code simple, readable, and efficient
>>> 2) avoid accidental array index duplication
>>> 3) avoid memory waste by creating more and more unused array members.
>>>
>>> Old values are kept to not break dts, and to maintain dt ABI.
>>>
>>> Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
>>> ---
>>>
>>> Notes:
>>>     v2:
>>>     - new patch, not present in v1
>>>
>>>  .../dt-bindings/clock/microchip,sam9x60-pmc.h | 19 +++++++++++
>>>  .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 25 +++++++++++++++
>>>  .../clock/microchip,sama7d65-pmc.h            | 32 +++++++++++++++++++
>>>  .../dt-bindings/clock/microchip,sama7g5-pmc.h | 24 ++++++++++++++
>>>  4 files changed, 100 insertions(+)
>>>  create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
>>>  create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
>>>  create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
>>>  create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h
>>>
>>> diff --git a/include/dt-bindings/clock/microchip,sam9x60-pmc.h b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
>>> new file mode 100644
>>> index 0000000000000..e01e867e8c4da
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
>>> @@ -0,0 +1,19 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>>> +/*
>>> + * The constants defined in this header are being used in dts and in
>>> + * at91 sam9x60 clock driver.
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H
>>> +#define _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H
>>> +
>>> +#include <dt-bindings/clock/at91.h>
>>> +
>>> +/* old from before bindings splitup */
>>> +#define SAM9X60_PMC_MCK		PMC_MCK		/* 1 */
>>> +#define SAM9X60_PMC_UTMI	PMC_UTMI	/* 2 */
>>> +#define SAM9X60_PMC_MAIN	PMC_MAIN	/* 3 */
>>> +
>>> +#define SAM9X60_PMC_PLLACK	PMC_PLLACK	/* 7 */
>>
>> IIUC, you want to have bindings per SoC, so why not adding proper
>> constants here instead of including entire old binding header? The
>> binding header should be entirely abandoned later.
> 
> Which binding header should be abandoned entirely?

The one you claim to split. I assume it is the same  one included here.

> 
> The bindings per SoC idea was proposed in series v1 feedback.  I'm
> neither a binding nor a clock expeert.  As far as I understood it's
> important to keep the exact same values as before to not change any

Yes, I did not propose to change any IDs.

> ABI.  The non SoC specific values are still used on older SoCs of the
> at91 family, so this is why I used the old constants for now.
> 
> These PMC indexes are not the only definitions in
> dt-bindings/clock/at91.h however, there are more which are not SoC
> specific.
> 
> I'd like some thoughts from the Microchip maintainers here,
> what's their idea on how to proceed with the at91 clock stuff?
> 
> This works for me, but the current state is more or less still an idea
> as base for discussion.  Please don't make it overly complicated, this
> is not the primary focus of my work.

You made this binding more complicated than it should be - instead of
simple list of clocks you include some other file and split between old
and new.

Best regards,
Krzysztof

Re: [PATCH v2 08/16] dt-bindings: nvmem: microchip-otpc: Add required clocks

[Krzysztof Kozlowski]

On 11/02/2025 08:05, Alexander Dahl wrote:
> Hello Krzysztof,
> 
> Am Mon, Feb 10, 2025 at 05:59:52PM +0100 schrieb Krzysztof Kozlowski:
>> On 10/02/2025 17:44, Alexander Dahl wrote:
>>> The OTPC requires both the peripheral clock through PMC and the main RC
>>> oscillator.  Seemed to work without explicitly enabling those clocks on
>>> sama7g5 before, but did not on sam9x60.
>>>
>>> Older datasheets were not clear and explicit about this, but recent are,
>>> e.g. SAMA7G5 series datasheet (DS60001765B),
>>> section 30.4.1 Power Management:
>>>
>>>> The OTPC is clocked through the Power Management Controller (PMC).
>>>> The user must power on the main RC oscillator and enable the
>>>> peripheral clock of the OTPC prior to reading or writing the OTP
>>>> memory.
>>>
>>> Link: https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@microchip.com/T/#u
>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
>>> ---
>>>
>>> Notes:
>>>     v2:
>>>     - new patch, not present in v1
>>>
>>>  .../nvmem/microchip,sama7g5-otpc.yaml         | 28 +++++++++++++++++++
>>>  1 file changed, 28 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
>>> index 9a7aaf64eef32..1fa40610888f3 100644
>>> --- a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
>>> +++ b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
>>> @@ -29,6 +29,16 @@ properties:
>>>    reg:
>>>      maxItems: 1
>>>  
>>> +  clocks:
>>> +    items:
>>> +      - description: main rc oscillator
>>> +      - description: otpc peripheral clock
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: main_rc_osc
>>
>> osc
> 
> On at91 SoCs main oscillator and main RC oscillator are two different
> things, and those are different clocks in Linux as well.  This clock
> is named "main_rc_osc" in the clock driver.  In

It does not matter, you could have 5 oscillators. This device takes one.
What is this clock from the point of view of device?

> drivers/clk/at91/sam9x60.c this clock is added like this:
> 
>     hw = at91_clk_register_main_rc_osc(regmap, "main_rc_osc", 12000000, 50000000);
> 
> The datasheet makes it explicit, it's exactly the main rc oscillator
> clock required for the OTPC to work, no other clock.

You speak about source clock but this has to be the clock here, in the
device context.

> 
> So why name this "osc" only then?  This is confusing at best.
> 
>>
>>> +      - const: otpc_clk
>>
>> otpc or bus or whatever logically this is
> 
> Okay the "_clk" suffix is redundant.  Since the peripheral clock for
> the OTPC is required here, I would go with "otpc" only then.
> 
>>
>>> +
>>>  required:
>>>    - compatible
>>>    - reg
>>> @@ -37,6 +47,8 @@ unevaluatedProperties: false
>>>  
>>>  examples:
>>>    - |
>>> +    #include <dt-bindings/clock/at91.h>
>>> +    #include <dt-bindings/clock/microchip,sama7g5-pmc.h>
>>>      #include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>
>>>  
>>>      otpc: efuse@e8c00000 {
>>> @@ -44,10 +56,26 @@ examples:
>>>          reg = <0xe8c00000 0xec>;
>>>          #address-cells = <1>;
>>>          #size-cells = <1>;
>>> +        clocks = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 67>;
>>> +        clock-names = "main_rc_osc", "otpc_clk";
>>>  
>>>          temperature_calib: calib@1 {
>>>              reg = <OTP_PKT(1) 76>;
>>>          };
>>>      };
>>>  
>>> +  - |
>>> +    #include <dt-bindings/clock/at91.h>
>>> +    #include <dt-bindings/clock/microchip,sam9x60-pmc.h>
>>> +    #include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>
>>> +
>>> +    efuse@eff00000 {
>>> +        compatible = "microchip,sam9x60-otpc", "syscon";
>>> +        reg = <0xeff00000 0xec>;
>>
>> No need for new example with difference in what exactly? Even compatible
>> was not added here...
> 
> Different compatible, different clocks, no sub nodes, different
> peripheral clock id …  From a human doc readers I'd like another

Clocks are the same. Their actual value does not matter.



Best regards,
Krzysztof

Re: [PATCH v2 04/16] dt-bindings: clock: at91: Allow referencing main rc oscillator in DT

[Krzysztof Kozlowski]

On 11/02/2025 08:26, Alexander Dahl wrote:
> Hello Krzysztof,
> 
> Am Mon, Feb 10, 2025 at 06:07:10PM +0100 schrieb Krzysztof Kozlowski:
>> On 10/02/2025 17:44, Alexander Dahl wrote:
>>> The main rc oscillator will be needed for the OTPC to work properly.
>>>
>>> The new index introduced here was not used on the four affected SoC
>>> clock drivers before, but for sama5d2 only (PMC_I2S1_MUX).
>>>
>>> Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
>>> ---
>>>
>>> Notes:
>>>     v2:
>>>     - new patch, not present in v1
>>>
>>>  include/dt-bindings/clock/microchip,sam9x60-pmc.h  | 3 +++
>>>  include/dt-bindings/clock/microchip,sam9x7-pmc.h   | 3 +++
>>>  include/dt-bindings/clock/microchip,sama7d65-pmc.h | 3 +++
>>>  include/dt-bindings/clock/microchip,sama7g5-pmc.h  | 3 +++
>>>  4 files changed, 12 insertions(+)
>>>
>>> diff --git a/include/dt-bindings/clock/microchip,sam9x60-pmc.h b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
>>> index e01e867e8c4da..dcd3c74f75b54 100644
>>> --- a/include/dt-bindings/clock/microchip,sam9x60-pmc.h
>>> +++ b/include/dt-bindings/clock/microchip,sam9x60-pmc.h
>>> @@ -16,4 +16,7 @@
>>>  
>>>  #define SAM9X60_PMC_PLLACK	PMC_PLLACK	/* 7 */
>>>  
>>> +/* new from after bindings splitup */
>>> +#define SAM9X60_PMC_MAIN_RC	6
>>
>> This is confusing me, because:
>> 1. You still have holes in IDs
> 
> Yes, I was told to maintain the old values for interface stability in
> series v1 feedback.
> 
>> 2. This should be placed in proper order by ID
> 
> Okay, no problem.
> 
>> 3. Why not using 4 - the next available empty ID?
> 
> The MAIN_RC clock is used on four out of thirteen (?) SoC variants
> which all used the same IDs before.  6 is the first ID which is free
> on all of sam9x60, sam9x7, sama7g5, and sama7d65.  The last two
> already use 4 for a different clock.

So driver for this device already uses something for 4?

> 
> The whole splitup is to avoid even more and/or bigger holes, but is it
> important where the existent holes are filled?
> 
> Technically if the next available empty ID should be used it would be
> 4 for sam9x60 and sam9x7, 2 for sama7d65, and 6 for sama7g5.  I
> thought it would be nice to use the same value instead to make
> somewhat compatible to the old approach.


Best regards,
Krzysztof

Re: [PATCH v2 07/16] dt-bindings: nvmem: microchip-otpc: Add compatible for SAM9X60

[Krzysztof Kozlowski]

On Mon, Feb 10, 2025 at 05:44:57PM +0100, Alexander Dahl wrote:
> The SAM9X60 SoC family has a similar, but slightly different OTPC to the
> SAMA7G5 family.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 15/16] nvmem: microchip-otpc: Enable necessary clocks

[Claudiu Beznea]

Hi, Alexander,

On 11.02.2025 08:53, Alexander Dahl wrote:
> Without enabling the main rc clock, initializing the packet list leads
> to a read timeout on the first packet, at least on sam9x60.
> 
> According to SAM9X60 datasheet (DS60001579G) section "23.4 Product
> Dependencies" the clock must be enabled for reading and writing.
> 
> Tested on sam9x60-curiosity board.
> 
> Link: https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@microchip.com/T/#u
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
> 
> Notes:
>     v2:
>     - Rewrite to enable _all_ clocks defined in dts
> 
>  drivers/nvmem/microchip-otpc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
> index d39f2d57e5f5e..2c524c163b7e2 100644
> --- a/drivers/nvmem/microchip-otpc.c
> +++ b/drivers/nvmem/microchip-otpc.c
> @@ -8,6 +8,7 @@
>   */
>  
>  #include <linux/bitfield.h>
> +#include <linux/clk.h>
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
>  #include <linux/nvmem-provider.h>
> @@ -241,6 +242,7 @@ static struct nvmem_config mchp_nvmem_config = {
>  static int mchp_otpc_probe(struct platform_device *pdev)
>  {
>  	struct nvmem_device *nvmem;
> +	struct clk_bulk_data *clks;
>  	struct mchp_otpc *otpc;
>  	u32 size;
>  	int ret;
> @@ -253,6 +255,11 @@ static int mchp_otpc_probe(struct platform_device *pdev)
>  	if (IS_ERR(otpc->base))
>  		return PTR_ERR(otpc->base);
>  
> +	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &clks);
> +	if (ret < 0)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "Error getting clocks!\n");

This fits on a single line.

> +
>  	otpc->dev = &pdev->dev;
>  	ret = mchp_otpc_init_packets_list(otpc, &size);
>  	if (ret)

General remark: please organize your patches as follows:
- dt-bindings patches
- driver patches
- device tree binding patches

Applying this to your series, will result the following order:
- dt bindings for clocks
- driver changes for clocks
- dt-bindings for nvmeme
- driver changes for nvmem
- device tree for clocks
- device tree for nvmem
Thank you,
Claudiu

Re: [PATCH v2 13/16] ARM: dts: microchip: sam9x60: Add OTPC node

[Claudiu Beznea]

On 11.02.2025 08:53, Alexander Dahl wrote:
> The One-Time Programmable (OTP) Memory Controller (OTPC) is the secure
> interface between the system and the OTP memory.  It also features the
> Unique Product ID (UID) registers containing a unique serial number.
> 
> See datasheet (DS60001579G) sections "7. Memories" and "23. OTP Memory
> Controller (OTPC)" for reference.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
> 
> Notes:
>     v2:
>     - squashed with patch adding the clock properties
> 
>  arch/arm/boot/dts/microchip/sam9x60.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> index 1724b79967a17..af859f0b83a0f 100644
> --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi
> +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> @@ -15,6 +15,7 @@
>  #include <dt-bindings/clock/microchip,sam9x60-pmc.h>
>  #include <dt-bindings/mfd/at91-usart.h>
>  #include <dt-bindings/mfd/atmel-flexcom.h>
> +#include <dt-bindings/nvmem/microchip,sama7g5-otpc.h>

This is not needed, atm.

>  
>  / {
>  	#address-cells = <1>;
> @@ -157,6 +158,15 @@ sdmmc1: sdio-host@90000000 {
>  			status = "disabled";
>  		};
>  
> +		otpc: efuse@eff00000 {
> +			compatible = "microchip,sam9x60-otpc", "syscon";
> +			reg = <0xeff00000 0xec>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			clocks = <&pmc PMC_TYPE_CORE SAM9X60_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 46>;
> +			clock-names = "main_rc_osc", "otpc_clk";
> +		};
> +
>  		apb {
>  			compatible = "simple-bus";
>  			#address-cells = <1>;

Re: [PATCH v2 10/16] nvmem: microchip-otpc: Fix swapped 'sleep' and 'timeout' parameters

[Claudiu Beznea]

On 11.02.2025 08:52, Alexander Dahl wrote:
> Makes no sense to have a timeout shorter than the sleep time, it would
> run into timeout right after the first sleep already.
> While at it, use a more specific macro instead of the generic one, which
> does exactly the same, but needs less parameters.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> Fixes: 98830350d3fc ("nvmem: microchip-otpc: add support")

Fixes tag goes above you SoB tag.

> ---
> 
> Notes:
>     v2:
>     - Add Fixes tag
> 
>  drivers/nvmem/microchip-otpc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
> index e2851c63cc0b4..bf7e5167152cb 100644
> --- a/drivers/nvmem/microchip-otpc.c
> +++ b/drivers/nvmem/microchip-otpc.c
> @@ -85,8 +85,8 @@ static int mchp_otpc_prepare_read(struct mchp_otpc *otpc,
>  	writel_relaxed(MCHP_OTPC_CR_READ, otpc->base + MCHP_OTPC_CR);
>  
>  	/* Wait for packet to be transferred into temporary buffers. */
> -	return read_poll_timeout(readl_relaxed, tmp, !(tmp & MCHP_OTPC_SR_READ),
> -				 10000, 2000, false, otpc->base + MCHP_OTPC_SR);
> +	return readl_relaxed_poll_timeout(otpc->base + MCHP_OTPC_SR, tmp,
> +					  !(tmp & MCHP_OTPC_SR_READ), 2000, 10000);
>  }
>  
>  /*

Re: [PATCH v2 09/16] nvmem: microchip-otpc: Avoid reading a write-only register

[Claudiu Beznea]

On 10.02.2025 18:50, Alexander Dahl wrote:
> The OTPC Control Register (OTPC_CR) has just write-only members.
> Reading from that register leads to a warning in OTPC Write Protection
> Status Register (OTPC_WPSR) in field Software Error Type (SWETYP) of
> type READ_WO (A write-only register has been read (warning).)
> 
> Just create the register write content from scratch is sufficient here.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> Fixes: 98830350d3fc ("nvmem: microchip-otpc: add support")

Fixes tag goes above your SoB.

> ---
> 
> Notes:
>     v2:
>     - Add Fixes tag
>     - Remove temporary variable usage
>     - Reword misleading subject (s/writing/reading/)
> 
>  drivers/nvmem/microchip-otpc.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
> index df979e8549fdb..e2851c63cc0b4 100644
> --- a/drivers/nvmem/microchip-otpc.c
> +++ b/drivers/nvmem/microchip-otpc.c
> @@ -82,9 +82,7 @@ static int mchp_otpc_prepare_read(struct mchp_otpc *otpc,
>  	writel_relaxed(tmp, otpc->base + MCHP_OTPC_MR);
>  
>  	/* Set read. */
> -	tmp = readl_relaxed(otpc->base + MCHP_OTPC_CR);
> -	tmp |= MCHP_OTPC_CR_READ;
> -	writel_relaxed(tmp, otpc->base + MCHP_OTPC_CR);
> +	writel_relaxed(MCHP_OTPC_CR_READ, otpc->base + MCHP_OTPC_CR);
>  
>  	/* Wait for packet to be transferred into temporary buffers. */
>  	return read_poll_timeout(readl_relaxed, tmp, !(tmp & MCHP_OTPC_SR_READ),

Re: [PATCH v2 01/16] dt-bindings: clock: at91: Split up per SoC partially

[Claudiu Beznea]

Hi, Alexander,

On 10.02.2025 18:44, Alexander Dahl wrote:
> Before adding even more new indexes creating more holes in the
> clk at91 drivers pmc_data->chws arrays, split this up.
> 
> This is a partial split up only for SoCs affected by upcoming changes
> and by that PMC_MAIN + x hack, others could follow by the same scheme.
> 
> Binding splitup was proposed for several reasons:
> 
> 1) keep the driver code simple, readable, and efficient
> 2) avoid accidental array index duplication
> 3) avoid memory waste by creating more and more unused array members.
> 
> Old values are kept to not break dts, and to maintain dt ABI.
> 
> Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
> 
> Notes:
>     v2:
>     - new patch, not present in v1
> 
>  .../dt-bindings/clock/microchip,sam9x60-pmc.h | 19 +++++++++++
>  .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 25 +++++++++++++++
>  .../clock/microchip,sama7d65-pmc.h            | 32 +++++++++++++++++++
>  .../dt-bindings/clock/microchip,sama7g5-pmc.h | 24 ++++++++++++++
>  4 files changed, 100 insertions(+)
>  create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
>  create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
>  create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
>  create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h
> 

[ ...]

> diff --git a/include/dt-bindings/clock/microchip,sama7g5-pmc.h b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
> new file mode 100644
> index 0000000000000..ad69ccdf9dc78
> --- /dev/null
> +++ b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * The constants defined in this header are being used in dts and in
> + * at91 sama7g5 clock driver.
> + */
> +
> +#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
> +#define _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
> +
> +#include <dt-bindings/clock/at91.h>
> +
> +/* old from before bindings splitup */
> +#define SAMA7G5_PMC_MCK0	PMC_MCK		/* 1 */
> +#define SAMA7G5_PMC_UTMI	PMC_UTMI	/* 2 */
> +#define SAMA7G5_PMC_MAIN	PMC_MAIN	/* 3 */
> +#define SAMA7G5_PMC_CPUPLL	PMC_CPUPLL	/* 4 */
> +#define SAMA7G5_PMC_SYSPLL	PMC_SYSPLL	/* 5 */
> +
> +#define SAMA7G5_PMC_AUDIOPMCPLL	PMC_AUDIOPMCPLL	/* 9 */
> +#define SAMA7G5_PMC_AUDIOIOPLL	PMC_AUDIOIOPLL	/* 10 */
> +
> +#define SAMA7G5_PMC_MCK1	PMC_MCK1	/* 13 */
> +
> +#endif

I would have expected this to be something like:

#ifndef __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__
#define __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__

/* Core clocks. */
#define SAMA7G5_MCK0			1
#define SAMA7G5_UTMI			2
#define SAMA7G5_MAIN			3
#define SAMA7G5_CPUPLL			4
#define SAMA7G5_SYSPLL			5
#define SAMA7G5_DDRPLL			6
#define SAMA7G5_IMGPLL			7
#define SAMA7G5_BAUDPLL			8

// ...

#define SAMA7G5_MCK1			13

#endif /* __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__ */

Same for the other affected SoCs.

The content of include/dt-bindings/clock/at91.h would be limited eventually
only to the PMC clock types.

The other "#define PMC_*" defines will eventually go to SoC specific
bindings. "#define AT91_PMC_*" seems to not belong here anyway and these
would in the end removed, as well.

Thank you,
Claudiu

Re: [PATCH v2 01/16] dt-bindings: clock: at91: Split up per SoC partially

[Alexander Dahl]

Hello Claudiu,

Am Mon, Feb 17, 2025 at 11:11:44AM +0200 schrieb Claudiu Beznea:
> Hi, Alexander,
> 
> On 10.02.2025 18:44, Alexander Dahl wrote:
> > Before adding even more new indexes creating more holes in the
> > clk at91 drivers pmc_data->chws arrays, split this up.
> > 
> > This is a partial split up only for SoCs affected by upcoming changes
> > and by that PMC_MAIN + x hack, others could follow by the same scheme.
> > 
> > Binding splitup was proposed for several reasons:
> > 
> > 1) keep the driver code simple, readable, and efficient
> > 2) avoid accidental array index duplication
> > 3) avoid memory waste by creating more and more unused array members.
> > 
> > Old values are kept to not break dts, and to maintain dt ABI.
> > 
> > Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
> > Signed-off-by: Alexander Dahl <ada@thorsis.com>
> > ---
> > 
> > Notes:
> >     v2:
> >     - new patch, not present in v1
> > 
> >  .../dt-bindings/clock/microchip,sam9x60-pmc.h | 19 +++++++++++
> >  .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 25 +++++++++++++++
> >  .../clock/microchip,sama7d65-pmc.h            | 32 +++++++++++++++++++
> >  .../dt-bindings/clock/microchip,sama7g5-pmc.h | 24 ++++++++++++++
> >  4 files changed, 100 insertions(+)
> >  create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
> >  create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
> >  create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
> >  create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h
> > 
> 
> [ ...]
> 
> > diff --git a/include/dt-bindings/clock/microchip,sama7g5-pmc.h b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
> > new file mode 100644
> > index 0000000000000..ad69ccdf9dc78
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> > +/*
> > + * The constants defined in this header are being used in dts and in
> > + * at91 sama7g5 clock driver.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
> > +#define _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
> > +
> > +#include <dt-bindings/clock/at91.h>
> > +
> > +/* old from before bindings splitup */
> > +#define SAMA7G5_PMC_MCK0	PMC_MCK		/* 1 */
> > +#define SAMA7G5_PMC_UTMI	PMC_UTMI	/* 2 */
> > +#define SAMA7G5_PMC_MAIN	PMC_MAIN	/* 3 */
> > +#define SAMA7G5_PMC_CPUPLL	PMC_CPUPLL	/* 4 */
> > +#define SAMA7G5_PMC_SYSPLL	PMC_SYSPLL	/* 5 */
> > +
> > +#define SAMA7G5_PMC_AUDIOPMCPLL	PMC_AUDIOPMCPLL	/* 9 */
> > +#define SAMA7G5_PMC_AUDIOIOPLL	PMC_AUDIOIOPLL	/* 10 */
> > +
> > +#define SAMA7G5_PMC_MCK1	PMC_MCK1	/* 13 */
> > +
> > +#endif
> 
> I would have expected this to be something like:
> 
> #ifndef __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__
> #define __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__
> 
> /* Core clocks. */
> #define SAMA7G5_MCK0			1
> #define SAMA7G5_UTMI			2
> #define SAMA7G5_MAIN			3
> #define SAMA7G5_CPUPLL			4
> #define SAMA7G5_SYSPLL			5
> #define SAMA7G5_DDRPLL			6
> #define SAMA7G5_IMGPLL			7
> #define SAMA7G5_BAUDPLL			8

Okay no reference to the old header, but numbers.  Got that.

I'm not sure where you got the 7 and 8 from here, according to my
analysis, sama7g5 does not use those.

> 
> // ...
> 
> #define SAMA7G5_MCK1			13
> 
> #endif /* __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__ */
> 
> Same for the other affected SoCs.
> 
> The content of include/dt-bindings/clock/at91.h would be limited eventually
> only to the PMC clock types.

What does this mean?  The clocks split out are no PMC clocks?  Then
the old PMC_MAIN etc. definitions were named wrong?  All or only some
of them?  Or is this different between older and newer SoC variants of
the at91 family?

From a quick glance in the SAM9X60 datasheet for example the clock
generator provides MD_SLCK, TD_SLCK, MAINCK, and PLL clocks, while the
PMC provides MCK, USB clocks, GCLK, PCK, and the peripheral clocks.

The chws array in drivers/clk/at91/sam9x60.c however gets main_rc_osc
(from clock generator), mainck (clock generator), pllack (clock
generator), upllck (clock generator, UTMI), but also mck (from PMC).

This creates the impression things are mixed up here.  I find all this
quite confusing to be honest.

> The other "#define PMC_*" defines will eventually go to SoC specific
> bindings. "#define AT91_PMC_*" seems to not belong here anyway and these
> would in the end removed, as well.

Okay, you seem to have an idea how this should look like in the long
run.  Are there any plans at Microchip or at91 clock maintainer side
to clean this up in the near future?

I would like to rather put my small changes for otpc on top of a clean
tree, instead of trying to clean up clock drivers and bindings for a
whole family of SoCs and boards, where I can test only one of them.
O:-)

Greets
Alex

Re: [PATCH v2 01/16] dt-bindings: clock: at91: Split up per SoC partially

[Claudiu Beznea]

Hi, Alexander,

On 17.02.2025 11:47, Alexander Dahl wrote:
> Hello Claudiu,
> 
> Am Mon, Feb 17, 2025 at 11:11:44AM +0200 schrieb Claudiu Beznea:
>> Hi, Alexander,
>>
>> On 10.02.2025 18:44, Alexander Dahl wrote:
>>> Before adding even more new indexes creating more holes in the
>>> clk at91 drivers pmc_data->chws arrays, split this up.
>>>
>>> This is a partial split up only for SoCs affected by upcoming changes
>>> and by that PMC_MAIN + x hack, others could follow by the same scheme.
>>>
>>> Binding splitup was proposed for several reasons:
>>>
>>> 1) keep the driver code simple, readable, and efficient
>>> 2) avoid accidental array index duplication
>>> 3) avoid memory waste by creating more and more unused array members.
>>>
>>> Old values are kept to not break dts, and to maintain dt ABI.
>>>
>>> Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
>>> ---
>>>
>>> Notes:
>>>     v2:
>>>     - new patch, not present in v1
>>>
>>>  .../dt-bindings/clock/microchip,sam9x60-pmc.h | 19 +++++++++++
>>>  .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 25 +++++++++++++++
>>>  .../clock/microchip,sama7d65-pmc.h            | 32 +++++++++++++++++++
>>>  .../dt-bindings/clock/microchip,sama7g5-pmc.h | 24 ++++++++++++++
>>>  4 files changed, 100 insertions(+)
>>>  create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
>>>  create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
>>>  create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
>>>  create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h
>>>
>>
>> [ ...]
>>
>>> diff --git a/include/dt-bindings/clock/microchip,sama7g5-pmc.h b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
>>> new file mode 100644
>>> index 0000000000000..ad69ccdf9dc78
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
>>> @@ -0,0 +1,24 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>>> +/*
>>> + * The constants defined in this header are being used in dts and in
>>> + * at91 sama7g5 clock driver.
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
>>> +#define _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
>>> +
>>> +#include <dt-bindings/clock/at91.h>
>>> +
>>> +/* old from before bindings splitup */
>>> +#define SAMA7G5_PMC_MCK0	PMC_MCK		/* 1 */
>>> +#define SAMA7G5_PMC_UTMI	PMC_UTMI	/* 2 */
>>> +#define SAMA7G5_PMC_MAIN	PMC_MAIN	/* 3 */
>>> +#define SAMA7G5_PMC_CPUPLL	PMC_CPUPLL	/* 4 */
>>> +#define SAMA7G5_PMC_SYSPLL	PMC_SYSPLL	/* 5 */
>>> +
>>> +#define SAMA7G5_PMC_AUDIOPMCPLL	PMC_AUDIOPMCPLL	/* 9 */
>>> +#define SAMA7G5_PMC_AUDIOIOPLL	PMC_AUDIOIOPLL	/* 10 */
>>> +
>>> +#define SAMA7G5_PMC_MCK1	PMC_MCK1	/* 13 */
>>> +
>>> +#endif
>>
>> I would have expected this to be something like:
>>
>> #ifndef __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__
>> #define __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__
>>
>> /* Core clocks. */
>> #define SAMA7G5_MCK0			1
>> #define SAMA7G5_UTMI			2
>> #define SAMA7G5_MAIN			3
>> #define SAMA7G5_CPUPLL			4
>> #define SAMA7G5_SYSPLL			5
>> #define SAMA7G5_DDRPLL			6
>> #define SAMA7G5_IMGPLL			7
>> #define SAMA7G5_BAUDPLL			8
> 
> Okay no reference to the old header, but numbers.  Got that.
> 
> I'm not sure where you got the 7 and 8 from here, according to my
> analysis, sama7g5 does not use those.

From include/dt-bindings/clock/at91.sh

#define PMC_IMGPLL              (PMC_MAIN + 4)

#define PMC_BAUDPLL             (PMC_MAIN + 5)


> 
>>
>> // ...
>>
>> #define SAMA7G5_MCK1			13
>>
>> #endif /* __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__ */
>>
>> Same for the other affected SoCs.
>>
>> The content of include/dt-bindings/clock/at91.h would be limited eventually
>> only to the PMC clock types.
> 
> What does this mean?  The clocks split out are no PMC clocks?  

Still PMC clocks. Keeping the types in separate header allows keeping the
code PMC code common for all SoCs. Then the newly added headers will be
used only in the SoC DTes and SoC clock driver (e.g. in your case
drivers/clk/at91/sam9x60.c)

> Then
> the old PMC_MAIN etc. definitions were named wrong?  All or only some
> of them?  Or is this different between older and newer SoC variants of
> the at91 family?
> 
> From a quick glance in the SAM9X60 datasheet for example the clock
> generator provides MD_SLCK, TD_SLCK, MAINCK, and PLL clocks, while the
> PMC provides MCK, USB clocks, GCLK, PCK, and the peripheral clocks.

drivers splits this into:
- core clocks
- peripheral clocks
- generic clocks
- system clocks
- programmable

It's how the code sees it, just a logical split.

Thank you,
Claudiu

> 
> The chws array in drivers/clk/at91/sam9x60.c however gets main_rc_osc
> (from clock generator), mainck (clock generator), pllack (clock
> generator), upllck (clock generator, UTMI), but also mck (from PMC).
> 
> This creates the impression things are mixed up here.  I find all this
> quite confusing to be honest.
> 
>> The other "#define PMC_*" defines will eventually go to SoC specific
>> bindings. "#define AT91_PMC_*" seems to not belong here anyway and these
>> would in the end removed, as well.
> 
> Okay, you seem to have an idea how this should look like in the long
> run.  Are there any plans at Microchip or at91 clock maintainer side
> to clean this up in the near future?
> 
> I would like to rather put my small changes for otpc on top of a clean
> tree, instead of trying to clean up clock drivers and bindings for a
> whole family of SoCs and boards, where I can test only one of them.
> O:-)
> 
> Greets
> Alex
> 

Re: [PATCH v2 01/16] dt-bindings: clock: at91: Split up per SoC partially

[Alexander Dahl]

Hello Claudiu,

Am Wed, Feb 19, 2025 at 10:51:40AM +0200 schrieb Claudiu Beznea:
> Hi, Alexander,
> 
> On 17.02.2025 11:47, Alexander Dahl wrote:
> > Hello Claudiu,
> > 
> > Am Mon, Feb 17, 2025 at 11:11:44AM +0200 schrieb Claudiu Beznea:
> >> Hi, Alexander,
> >>
> >> On 10.02.2025 18:44, Alexander Dahl wrote:
> >>> Before adding even more new indexes creating more holes in the
> >>> clk at91 drivers pmc_data->chws arrays, split this up.
> >>>
> >>> This is a partial split up only for SoCs affected by upcoming changes
> >>> and by that PMC_MAIN + x hack, others could follow by the same scheme.
> >>>
> >>> Binding splitup was proposed for several reasons:
> >>>
> >>> 1) keep the driver code simple, readable, and efficient
> >>> 2) avoid accidental array index duplication
> >>> 3) avoid memory waste by creating more and more unused array members.
> >>>
> >>> Old values are kept to not break dts, and to maintain dt ABI.
> >>>
> >>> Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@thorsis.com/T/#u
> >>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> >>> ---
> >>>
> >>> Notes:
> >>>     v2:
> >>>     - new patch, not present in v1
> >>>
> >>>  .../dt-bindings/clock/microchip,sam9x60-pmc.h | 19 +++++++++++
> >>>  .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 25 +++++++++++++++
> >>>  .../clock/microchip,sama7d65-pmc.h            | 32 +++++++++++++++++++
> >>>  .../dt-bindings/clock/microchip,sama7g5-pmc.h | 24 ++++++++++++++
> >>>  4 files changed, 100 insertions(+)
> >>>  create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
> >>>  create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
> >>>  create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
> >>>  create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h
> >>>
> >>
> >> [ ...]
> >>
> >>> diff --git a/include/dt-bindings/clock/microchip,sama7g5-pmc.h b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
> >>> new file mode 100644
> >>> index 0000000000000..ad69ccdf9dc78
> >>> --- /dev/null
> >>> +++ b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
> >>> @@ -0,0 +1,24 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> >>> +/*
> >>> + * The constants defined in this header are being used in dts and in
> >>> + * at91 sama7g5 clock driver.
> >>> + */
> >>> +
> >>> +#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
> >>> +#define _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
> >>> +
> >>> +#include <dt-bindings/clock/at91.h>
> >>> +
> >>> +/* old from before bindings splitup */
> >>> +#define SAMA7G5_PMC_MCK0	PMC_MCK		/* 1 */
> >>> +#define SAMA7G5_PMC_UTMI	PMC_UTMI	/* 2 */
> >>> +#define SAMA7G5_PMC_MAIN	PMC_MAIN	/* 3 */
> >>> +#define SAMA7G5_PMC_CPUPLL	PMC_CPUPLL	/* 4 */
> >>> +#define SAMA7G5_PMC_SYSPLL	PMC_SYSPLL	/* 5 */
> >>> +
> >>> +#define SAMA7G5_PMC_AUDIOPMCPLL	PMC_AUDIOPMCPLL	/* 9 */
> >>> +#define SAMA7G5_PMC_AUDIOIOPLL	PMC_AUDIOIOPLL	/* 10 */
> >>> +
> >>> +#define SAMA7G5_PMC_MCK1	PMC_MCK1	/* 13 */
> >>> +
> >>> +#endif
> >>
> >> I would have expected this to be something like:
> >>
> >> #ifndef __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__
> >> #define __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__
> >>
> >> /* Core clocks. */
> >> #define SAMA7G5_MCK0			1
> >> #define SAMA7G5_UTMI			2
> >> #define SAMA7G5_MAIN			3
> >> #define SAMA7G5_CPUPLL			4
> >> #define SAMA7G5_SYSPLL			5
> >> #define SAMA7G5_DDRPLL			6
> >> #define SAMA7G5_IMGPLL			7
> >> #define SAMA7G5_BAUDPLL			8
> > 
> > Okay no reference to the old header, but numbers.  Got that.
> > 
> > I'm not sure where you got the 7 and 8 from here, according to my
> > analysis, sama7g5 does not use those.
> 
> From include/dt-bindings/clock/at91.sh
> 
> #define PMC_IMGPLL              (PMC_MAIN + 4)
> 
> #define PMC_BAUDPLL             (PMC_MAIN + 5)

Okay fine, but those defines are not used anywhere in the whole kernel
as of v6.14-rc3, also not by sama7g5 clock driver.  Are those used in
a different version or tree?  Should I rebase?

Or do you want the whole SAMA7G5 section moved away from
include/dt-bindings/clock/at91.sh already?  Then this would be four
steps, right?

1. introduce the new defines
2. use the new defines in driver
3. use the new defines in dt
4. remove the old defines

(Same for SAM9X7 and SAMA7D65 sections?)

Or is this out of scope for this series?

> 
> 
> > 
> >>
> >> // ...
> >>
> >> #define SAMA7G5_MCK1			13
> >>
> >> #endif /* __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__ */
> >>
> >> Same for the other affected SoCs.
> >>
> >> The content of include/dt-bindings/clock/at91.h would be limited eventually
> >> only to the PMC clock types.
> > 
> > What does this mean?  The clocks split out are no PMC clocks?  
> 
> Still PMC clocks. Keeping the types in separate header allows keeping the
> code PMC code common for all SoCs. Then the newly added headers will be
> used only in the SoC DTes and SoC clock driver (e.g. in your case
> drivers/clk/at91/sam9x60.c)

I understand this as: PMC_TYPE_CORE, PMC_TYPE_SYSTEM, etc. will stay
in include/dt-bindings/clock/at91.h as they are, but the IDs for core
clocks I started to split out will be removed eventually?

Then I would have to change my patch to slightly rename the new
defines and use the static numbers instead of referencing the old
defines, but besides that it's fine?

Correct me, if I understood that wrong.

Thanks and Greets
Alex

> 
> > Then
> > the old PMC_MAIN etc. definitions were named wrong?  All or only some
> > of them?  Or is this different between older and newer SoC variants of
> > the at91 family?
> > 
> > From a quick glance in the SAM9X60 datasheet for example the clock
> > generator provides MD_SLCK, TD_SLCK, MAINCK, and PLL clocks, while the
> > PMC provides MCK, USB clocks, GCLK, PCK, and the peripheral clocks.
> 
> drivers splits this into:
> - core clocks
> - peripheral clocks
> - generic clocks
> - system clocks
> - programmable
> 
> It's how the code sees it, just a logical split.
> 
> Thank you,
> Claudiu
> 
> > 
> > The chws array in drivers/clk/at91/sam9x60.c however gets main_rc_osc
> > (from clock generator), mainck (clock generator), pllack (clock
> > generator), upllck (clock generator, UTMI), but also mck (from PMC).
> > 
> > This creates the impression things are mixed up here.  I find all this
> > quite confusing to be honest.
> > 
> >> The other "#define PMC_*" defines will eventually go to SoC specific
> >> bindings. "#define AT91_PMC_*" seems to not belong here anyway and these
> >> would in the end removed, as well.
> > 
> > Okay, you seem to have an idea how this should look like in the long
> > run.  Are there any plans at Microchip or at91 clock maintainer side
> > to clean this up in the near future?
> > 
> > I would like to rather put my small changes for otpc on top of a clean
> > tree, instead of trying to clean up clock drivers and bindings for a
> > whole family of SoCs and boards, where I can test only one of them.
> > O:-)
> > 
> > Greets
> > Alex
> > 
>