Skip to contentSkip to conversation

Update the icicle kit device tree

PATCH Superseded [email protected] 7 participants 70 messages Nov 8, 2021 – Nov 30, 2021

Subsystems (12)
Device tree bindings USB GPIO RISC-V +8 more subsystems

v1 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

cover Update the icicle kit device tree

From: Conor Dooley <[email protected]>

This series updates the microchip icicle kit device tree by adding a host of peripherals, and some updates to the memory map. In addition, the device tree has been split into a third part, which contains "soft" peripherals that are in the fpga fabric.

Several of the entries are for peripherals that have not get had their drivers upstreamed, so in those cases the dt bindings are included where appropriate in order to avoid as many "DT compatible string <x> appears un-documented" errors as possible.

Depends on mpfs clock driver series [1] to provide:
dt-bindings/clock/microchip,mpfs-clock.h
and on the other changes to the icicle/mpfs device tree that are already in linux/riscv/for-next.

[1] https://lore.kernel.org/linux-clk/[email protected]/

Conor Dooley (11):
dt-bindings: soc/microchip: update sys ctrlr compat string
dt-bindings: riscv: update microchip polarfire binds
dt-bindings: i2c: add bindings for microchip mpfs i2c
dt-bindings: rng: add bindings for microchip mpfs rng
dt-bindings: rtc: add bindings for microchip mpfs rtc
dt-bindings: soc/microchip: add bindings for mpfs system services
dt-bindings: gpio: add bindings for microchip mpfs gpio
dt-bindings: spi: add bindings for microchip mpfs spi
dt-bindings: usb: add bindings for microchip mpfs musb
riscv: icicle-kit: update microchip icicle kit device tree
MAINTAINERS: update riscv/microchip entry

Ivan Griffin (2):
dt-bindings: interrupt-controller: add defines for riscv-hart
dt-bindings: interrupt-controller: add defines for mpfs-plic

 .../bindings/gpio/microchip,mpfs-gpio.yaml    | 108 ++++++
 .../bindings/i2c/microchip,mpfs-i2c.yaml      |  74 ++++
 .../microchip,polarfire-soc-mailbox.yaml      |   4 +-
 .../devicetree/bindings/riscv/microchip.yaml  |   1 +
 .../bindings/rng/microchip,mpfs-rng.yaml      |  31 ++
 .../bindings/rtc/microchip,mfps-rtc.yaml      |  61 ++++
 .../microchip,mpfs-generic-service.yaml       |  31 ++
 ...icrochip,polarfire-soc-sys-controller.yaml |   4 +-
 .../bindings/spi/microchip,mpfs-spi.yaml      |  72 ++++
 .../bindings/usb/microchip,mpfs-usb-host.yaml |  70 ++++
 MAINTAINERS                                   |   2 +
 .../dts/microchip/microchip-mpfs-fabric.dtsi  |  21 ++
 .../microchip/microchip-mpfs-icicle-kit.dts   | 159 +++++++--
 .../boot/dts/microchip/microchip-mpfs.dtsi    | 333 ++++++++++++++----
 drivers/mailbox/mailbox-mpfs.c                |   1 +
 .../microchip,mpfs-plic.h                     | 199 +++++++++++
 .../interrupt-controller/riscv-hart.h         |  19 +
 17 files changed, 1103 insertions(+), 87 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
 create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml
 create mode 100644 Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml
 create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
 create mode 100644 Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml
 create mode 100644 arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi
 create mode 100644 include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h
 create mode 100644 include/dt-bindings/interrupt-controller/riscv-hart.h
Signature · 1 line
2.33.1
author maintainer
#1 lore raw

PATCH 1/13 dt-bindings: interrupt-controller: create a header for RISC-V interrupts

From: Ivan Griffin <[email protected]>

Provide named identifiers for device tree for RISC-V interrupts.

Licensed under GPL and MIT, as this file may be useful to any OS that uses device tree.

---
 .../interrupt-controller/riscv-hart.h         | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 include/dt-bindings/interrupt-controller/riscv-hart.h
include/dt-bindings/interrupt-controller/riscv-hart.h new file +19 -0
new file mode 100644
index 000000000000..e1c32f6090ac
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (C) 2021 Microchip Technology Inc.  All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_RISCV_HART_H
+#define _DT_BINDINGS_INTERRUPT_CONTROLLER_RISCV_HART_H
+
+#define HART_INT_U_SOFT   0
+#define HART_INT_S_SOFT   1
+#define HART_INT_M_SOFT   3
+#define HART_INT_U_TIMER  4
+#define HART_INT_S_TIMER  5
+#define HART_INT_M_TIMER  7
+#define HART_INT_U_EXT    8
+#define HART_INT_S_EXT    9
+#define HART_INT_M_EXT    11
+
+#endif /* _DT_BINDINGS_INTERRUPT_CONTROLLER_RISCV_HART_H */
Signature · 1 line
2.33.1
#2 lore raw

Re: [PATCH 01/13] dt-bindings: interrupt-controller: create a header for RISC-V interrupts

Am Montag, 8. November 2021, 16:05:42 CET schrieb [email protected]:

Quoted 27 lines

From: Ivan Griffin <[email protected]>

Provide named identifiers for device tree for RISC-V interrupts.

Licensed under GPL and MIT, as this file may be useful to any OS that uses device tree.

---
 .../interrupt-controller/riscv-hart.h         | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 include/dt-bindings/interrupt-controller/riscv-hart.h
include/dt-bindings/interrupt-controller/riscv-hart.h new file +7 -0
new file mode 100644
index 000000000000..e1c32f6090ac
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (C) 2021 Microchip Technology Inc.  All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_RISCV_HART_H
+#define _DT_BINDINGS_INTERRUPT_CONTROLLER_RISCV_HART_H

include/dt-bindings/interrupt-controller/riscv-hart.h line 17

Quoted 10 lines
include/dt-bindings/interrupt-controller/riscv-hart.h new file +10 -0
+
+#define HART_INT_U_SOFT   0
+#define HART_INT_S_SOFT   1
+#define HART_INT_M_SOFT   3
+#define HART_INT_U_TIMER  4
+#define HART_INT_S_TIMER  5
+#define HART_INT_M_TIMER  7
+#define HART_INT_U_EXT    8
+#define HART_INT_S_EXT    9
+#define HART_INT_M_EXT    11

(1) From checking clic doc [0] I see an additional
12 CLIC software interrupt
defined.

(2) The doc states that the ordering is a recommendation and "not mandatory in all incarnations of the CLIC"
Is that clarified somewhere else that this more than recommended?

Thanks
Heiko

[0] https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc

Quoted 3 lines
include/dt-bindings/interrupt-controller/riscv-hart.h new file +2 -0
+
+#endif /* _DT_BINDINGS_INTERRUPT_CONTROLLER_RISCV_HART_H */
#3 lore raw

Re: [PATCH 01/13] dt-bindings: interrupt-controller: create a header for RISC-V interrupts

Quoted 35 lines · Heiko Stübner

Am Montag, 8. November 2021, 16:05:42 CET schrieb [email protected]:

Quoted 33 lines

From: Ivan Griffin <[email protected]>

Provide named identifiers for device tree for RISC-V interrupts.

Licensed under GPL and MIT, as this file may be useful to any OS that uses device tree.

---
 .../interrupt-controller/riscv-hart.h         | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 include/dt-bindings/interrupt-controller/riscv-hart.h
include/dt-bindings/interrupt-controller/riscv-hart.h new file +13 -0
new file mode 100644
index 000000000000..e1c32f6090ac
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (C) 2021 Microchip Technology Inc.  All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_RISCV_HART_H
+#define _DT_BINDINGS_INTERRUPT_CONTROLLER_RISCV_HART_H
+
+#define HART_INT_U_SOFT   0
+#define HART_INT_S_SOFT   1
+#define HART_INT_M_SOFT   3
+#define HART_INT_U_TIMER  4
+#define HART_INT_S_TIMER  5
Quoted 8 lines

include/dt-bindings/interrupt-controller/riscv-hart.h line 17

Quoted 4 lines
include/dt-bindings/interrupt-controller/riscv-hart.h new file +4 -0
+#define HART_INT_M_TIMER  7
+#define HART_INT_U_EXT    8
+#define HART_INT_S_EXT    9
+#define HART_INT_M_EXT    11

(1) From checking clic doc [0] I see an additional
12 CLIC software interrupt
defined.

Local IRQ #12 is for S-mode guest external interrupts as-per the ratified H-extension specification.

I guess CLIC spec needs to be updated.

Regards,
Anup

Quoted 18 lines

(2) The doc states that the ordering is a recommendation and "not mandatory in all incarnations of the CLIC"
Is that clarified somewhere else that this more than recommended?

Thanks
Heiko

[0] https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc

Quoted 3 lines
include/dt-bindings/interrupt-controller/riscv-hart.h new file +2 -0
+
+#endif /* _DT_BINDINGS_INTERRUPT_CONTROLLER_RISCV_HART_H */
maintainer Acked-by on v1 1/13
#4 lore raw

Re: [PATCH 01/13] dt-bindings: interrupt-controller: create a header for RISC-V interrupts

In reply to [email protected]
Quoted 7 lines · On Mon, 08 Nov 2021 15:05:42 +0000, [email protected] wrote:

From: Ivan Griffin <[email protected]>

Provide named identifiers for device tree for RISC-V interrupts.

Licensed under GPL and MIT, as this file may be useful to any OS that uses device tree.

Quoted 7 lines
---
 .../interrupt-controller/riscv-hart.h         | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 include/dt-bindings/interrupt-controller/riscv-hart.h

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author maintainer
#5 lore raw

PATCH 2/13 dt-bindings: interrupt-controller: add defines for mpfs-plic

In reply to [email protected]

From: Ivan Griffin <[email protected]>

Add device tree bindings for the Microchip Polarfire Soc interrupt controller

---
 .../microchip,mpfs-plic.h                     | 199 ++++++++++++++++++
 1 file changed, 199 insertions(+)
 create mode 100644 include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h
include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h new file +199 -0
new file mode 100644
index 000000000000..81c8cd02abd8
@@ -0,0 +1,199 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (C) 2021 Microchip Technology Inc.  All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_MICROCHIP_MPFS_PLIC_H
+#define _DT_BINDINGS_INTERRUPT_CONTROLLER_MICROCHIP_MPFS_PLIC_H
+
+#define PLIC_INT_INVALID					0
+#define PLIC_INT_L2_METADATA_CORR				1
+#define PLIC_INT_L2_METADAT_UNCORR				2
+#define PLIC_INT_L2_DATA_CORR					3
+#define PLIC_INT_L2_DATA_UNCORR					4
+#define PLIC_INT_DMA_CH0_DONE					5
+#define PLIC_INT_DMA_CH0_ERR					6
+#define PLIC_INT_DMA_CH1_DONE					7
+#define PLIC_INT_DMA_CH1_ERR					8
+#define PLIC_INT_DMA_CH2_DONE					9
+#define PLIC_INT_DMA_CH2_ERR					10
+#define PLIC_INT_DMA_CH3_DONE					11
+#define PLIC_INT_DMA_CH3_ERR					12
+
+#define PLIC_INT_GPIO0_BIT0_OR_GPIO2_BIT0			13
+#define PLIC_INT_GPIO0_BIT1_OR_GPIO2_BIT1			14
+#define PLIC_INT_GPIO0_BIT2_OR_GPIO2_BIT2			15
+#define PLIC_INT_GPIO0_BIT3_OR_GPIO2_BIT3			16
+#define PLIC_INT_GPIO0_BIT4_OR_GPIO2_BIT4			17
+#define PLIC_INT_GPIO0_BIT5_OR_GPIO2_BIT5			18
+#define PLIC_INT_GPIO0_BIT6_OR_GPIO2_BIT6			19
+#define PLIC_INT_GPIO0_BIT7_OR_GPIO2_BIT7			20
+#define PLIC_INT_GPIO0_BIT8_OR_GPIO2_BIT8			21
+#define PLIC_INT_GPIO0_BIT9_OR_GPIO2_BIT9			22
+#define PLIC_INT_GPIO0_BIT10_OR_GPIO2_BIT10			23
+#define PLIC_INT_GPIO0_BIT11_OR_GPIO2_BIT11			24
+#define PLIC_INT_GPIO0_BIT12_OR_GPIO2_BIT12			25
+#define PLIC_INT_GPIO0_BIT13_OR_GPIO2_BIT13			26
+#define PLIC_INT_GPIO1_BIT0_OR_GPIO2_BIT14			27
+#define PLIC_INT_GPIO1_BIT1_OR_GPIO2_BIT15			28
+#define PLIC_INT_GPIO1_BIT2_OR_GPIO2_BIT16			29
+#define PLIC_INT_GPIO1_BIT3_OR_GPIO2_BIT17			30
+#define PLIC_INT_GPIO1_BIT4_OR_GPIO2_BIT18			31
+#define PLIC_INT_GPIO1_BIT5_OR_GPIO2_BIT19			32
+#define PLIC_INT_GPIO1_BIT6_OR_GPIO2_BIT20			33
+#define PLIC_INT_GPIO1_BIT7_OR_GPIO2_BIT21			34
+#define PLIC_INT_GPIO1_BIT8_OR_GPIO2_BIT22			35
+#define PLIC_INT_GPIO1_BIT9_OR_GPIO2_BIT23			36
+#define PLIC_INT_GPIO1_BIT10_OR_GPIO2_BIT24			37
+#define PLIC_INT_GPIO1_BIT11_OR_GPIO2_BIT25			38
+#define PLIC_INT_GPIO1_BIT12_OR_GPIO2_BIT26			39
+#define PLIC_INT_GPIO1_BIT13_OR_GPIO2_BIT27			40
+#define PLIC_INT_GPIO1_BIT14_OR_GPIO2_BIT28			41
+#define PLIC_INT_GPIO1_BIT15_OR_GPIO2_BIT29			42
+#define PLIC_INT_GPIO1_BIT16_OR_GPIO2_BIT30			43
+#define PLIC_INT_GPIO1_BIT17_OR_GPIO2_BIT31			44
+#define PLIC_INT_GPIO1_BIT18					45
+#define PLIC_INT_GPIO1_BIT19					46
+#define PLIC_INT_GPIO1_BIT20					47
+#define PLIC_INT_GPIO1_BIT21					48
+#define PLIC_INT_GPIO1_BIT22					49
+#define PLIC_INT_GPIO1_BIT23					50
+#define PLIC_INT_GPIO0_NON_DIRECT				51
+#define PLIC_INT_GPIO1_NON_DIRECT				52
+#define PLIC_INT_GPIO2_NON_DIRECT				53
+#define PLIC_INT_SPI0						54
+#define PLIC_INT_SPI1						55
+#define PLIC_INT_CAN0						56
+#define PLIC_INT_CAN1						57
+#define PLIC_INT_I2C0_MAIN					58
+#define PLIC_INT_I2C0_ALERT					59
+#define PLIC_INT_I2C0_SUS					60
+#define PLIC_INT_I2C1_MAIN					61
+#define PLIC_INT_I2C1_ALERT					62
+#define PLIC_INT_I2C1_SUS					63
+#define PLIC_INT_MAC0_INT					64
+#define PLIC_INT_MAC0_QUEUE1					65
+#define PLIC_INT_MAC0_QUEUE2					66
+#define PLIC_INT_MAC0_QUEUE3					67
+#define PLIC_INT_MAC0_EMAC					68
+#define PLIC_INT_MAC0_MMSL					69
+#define PLIC_INT_MAC1_INT					70
+#define PLIC_INT_MAC1_QUEUE1					71
+#define PLIC_INT_MAC1_QUEUE2					72
+#define PLIC_INT_MAC1_QUEUE3					73
+#define PLIC_INT_MAC1_EMAC					74
+#define PLIC_INT_MAC1_MMSL					75
+#define PLIC_INT_DDRC_TRAIN					76
+#define PLIC_INT_SCB_INTERRUPT					77
+#define PLIC_INT_ECC_ERROR					78
+#define PLIC_INT_ECC_CORRECT					79
+#define PLIC_INT_RTC_WAKEUP					80
+#define PLIC_INT_RTC_MATCH					81
+#define PLIC_INT_TIMER1						82
+#define PLIC_INT_TIMER2						83
+#define PLIC_INT_ENVM						84
+#define PLIC_INT_QSPI						85
+#define PLIC_INT_USB_DMA					86
+#define PLIC_INT_USB_MC						87
+#define PLIC_INT_MMC_MAIN					88
+#define PLIC_INT_MMC_WAKEUP					89
+#define PLIC_INT_MMUART0					90
+#define PLIC_INT_MMUART1					91
+#define PLIC_INT_MMUART2					92
+#define PLIC_INT_MMUART3					93
+#define PLIC_INT_MMUART4					94
+#define PLIC_INT_G5C_DEVRST					95
+#define PLIC_INT_G5C_MESSAGE					96
+#define PLIC_INT_USOC_VC_INTERRUPT				97
+#define PLIC_INT_USOC_SMB_INTERRUPT				98
+#define PLIC_INT_E51_0_MAINTENACE				99
+#define PLIC_INT_WDOG0_MRVP					100
+#define PLIC_INT_WDOG1_MRVP					101
+#define PLIC_INT_WDOG2_MRVP					102
+#define PLIC_INT_WDOG3_MRVP					103
+#define PLIC_INT_WDOG4_MRVP					104
+#define PLIC_INT_WDOG0_TOUT					105
+#define PLIC_INT_WDOG1_TOUT					106
+#define PLIC_INT_WDOG2_TOUT					107
+#define PLIC_INT_WDOG3_TOUT					108
+#define PLIC_INT_WDOG4_TOUT					109
+#define PLIC_INT_G5C_MSS_SPI					110
+#define PLIC_INT_VOLT_TEMP_ALARM				111
+#define PLIC_INT_ATHENA_COMPLETE				112
+#define PLIC_INT_ATHENA_ALARM					113
+#define PLIC_INT_ATHENA_BUS_ERROR				114
+#define PLIC_INT_USOC_AXIC_US					115
+#define PLIC_INT_USOC_AXIC_DS					116
+#define PLIC_INT_SPARE						117
+
+#define PLIC_INT_FABRIC_F2H_0					118
+#define PLIC_INT_FABRIC_F2H_1					119
+#define PLIC_INT_FABRIC_F2H_2					120
+#define PLIC_INT_FABRIC_F2H_3					121
+#define PLIC_INT_FABRIC_F2H_4					122
+#define PLIC_INT_FABRIC_F2H_5					123
+#define PLIC_INT_FABRIC_F2H_6					124
+#define PLIC_INT_FABRIC_F2H_7					125
+#define PLIC_INT_FABRIC_F2H_8					126
+#define PLIC_INT_FABRIC_F2H_9					127
+#define PLIC_INT_FABRIC_F2H_10					128
+#define PLIC_INT_FABRIC_F2H_11					129
+#define PLIC_INT_FABRIC_F2H_12					130
+#define PLIC_INT_FABRIC_F2H_13					131
+#define PLIC_INT_FABRIC_F2H_14					132
+#define PLIC_INT_FABRIC_F2H_15					133
+#define PLIC_INT_FABRIC_F2H_16					134
+#define PLIC_INT_FABRIC_F2H_17					135
+#define PLIC_INT_FABRIC_F2H_18					136
+#define PLIC_INT_FABRIC_F2H_19					137
+#define PLIC_INT_FABRIC_F2H_20					138
+#define PLIC_INT_FABRIC_F2H_21					139
+#define PLIC_INT_FABRIC_F2H_22					140
+#define PLIC_INT_FABRIC_F2H_23					141
+#define PLIC_INT_FABRIC_F2H_24					142
+#define PLIC_INT_FABRIC_F2H_25					143
+#define PLIC_INT_FABRIC_F2H_26					144
+#define PLIC_INT_FABRIC_F2H_27					145
+#define PLIC_INT_FABRIC_F2H_28					146
+#define PLIC_INT_FABRIC_F2H_29					147
+#define PLIC_INT_FABRIC_F2H_30					148
+#define PLIC_INT_FABRIC_F2H_31					149
+#define PLIC_INT_FABRIC_F2H_32					150
+#define PLIC_INT_FABRIC_F2H_33					151
+#define PLIC_INT_FABRIC_F2H_34					152
+#define PLIC_INT_FABRIC_F2H_35					153
+#define PLIC_INT_FABRIC_F2H_36					154
+#define PLIC_INT_FABRIC_F2H_37					155
+#define PLIC_INT_FABRIC_F2H_38					156
+#define PLIC_INT_FABRIC_F2H_39					157
+#define PLIC_INT_FABRIC_F2H_40					158
+#define PLIC_INT_FABRIC_F2H_41					159
+#define PLIC_INT_FABRIC_F2H_42					160
+#define PLIC_INT_FABRIC_F2H_43					161
+#define PLIC_INT_FABRIC_F2H_44					162
+#define PLIC_INT_FABRIC_F2H_45					163
+#define PLIC_INT_FABRIC_F2H_46					164
+#define PLIC_INT_FABRIC_F2H_47					165
+#define PLIC_INT_FABRIC_F2H_48					166
+#define PLIC_INT_FABRIC_F2H_49					167
+#define PLIC_INT_FABRIC_F2H_50					168
+#define PLIC_INT_FABRIC_F2H_51					169
+#define PLIC_INT_FABRIC_F2H_52					170
+#define PLIC_INT_FABRIC_F2H_53					171
+#define PLIC_INT_FABRIC_F2H_54					172
+#define PLIC_INT_FABRIC_F2H_55					173
+#define PLIC_INT_FABRIC_F2H_56					174
+#define PLIC_INT_FABRIC_F2H_57					175
+#define PLIC_INT_FABRIC_F2H_58					176
+#define PLIC_INT_FABRIC_F2H_59					177
+#define PLIC_INT_FABRIC_F2H_60					178
+#define PLIC_INT_FABRIC_F2H_61					179
+#define PLIC_INT_FABRIC_F2H_62					180
+#define PLIC_INT_FABRIC_F2H_63					181
+#define PLIC_INT_BUS_ERROR_UNIT_HART_0				182
+#define PLIC_INT_BUS_ERROR_UNIT_HART_1				183
+#define PLIC_INT_BUS_ERROR_UNIT_HART_2				184
+#define PLIC_INT_BUS_ERROR_UNIT_HART_3				185
+#define PLIC_INT_BUS_ERROR_UNIT_HART_4				186
+
+#endif /* _DT_BINDINGS_INTERRUPT_CONTROLLER_MICROCHIP_MPFS_PLIC_H */
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#6 lore raw

Re: [PATCH 02/13] dt-bindings: interrupt-controller: add defines for mpfs-plic

Hi,

Am Montag, 8. November 2021, 16:05:43 CET schrieb [email protected]:

Quoted 205 lines

From: Ivan Griffin <[email protected]>

Add device tree bindings for the Microchip Polarfire Soc interrupt controller

---
 .../microchip,mpfs-plic.h                     | 199 ++++++++++++++++++
 1 file changed, 199 insertions(+)
 create mode 100644 include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h
include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h new file +187 -0
new file mode 100644
index 000000000000..81c8cd02abd8
@@ -0,0 +1,199 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (C) 2021 Microchip Technology Inc.  All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_MICROCHIP_MPFS_PLIC_H
+#define _DT_BINDINGS_INTERRUPT_CONTROLLER_MICROCHIP_MPFS_PLIC_H
+
+#define PLIC_INT_INVALID					0
+#define PLIC_INT_L2_METADATA_CORR				1
+#define PLIC_INT_L2_METADAT_UNCORR				2
+#define PLIC_INT_L2_DATA_CORR					3
+#define PLIC_INT_L2_DATA_UNCORR					4
+#define PLIC_INT_DMA_CH0_DONE					5
+#define PLIC_INT_DMA_CH0_ERR					6
+#define PLIC_INT_DMA_CH1_DONE					7
+#define PLIC_INT_DMA_CH1_ERR					8
+#define PLIC_INT_DMA_CH2_DONE					9
+#define PLIC_INT_DMA_CH2_ERR					10
+#define PLIC_INT_DMA_CH3_DONE					11
+#define PLIC_INT_DMA_CH3_ERR					12
+
+#define PLIC_INT_GPIO0_BIT0_OR_GPIO2_BIT0			13
+#define PLIC_INT_GPIO0_BIT1_OR_GPIO2_BIT1			14
+#define PLIC_INT_GPIO0_BIT2_OR_GPIO2_BIT2			15
+#define PLIC_INT_GPIO0_BIT3_OR_GPIO2_BIT3			16
+#define PLIC_INT_GPIO0_BIT4_OR_GPIO2_BIT4			17
+#define PLIC_INT_GPIO0_BIT5_OR_GPIO2_BIT5			18
+#define PLIC_INT_GPIO0_BIT6_OR_GPIO2_BIT6			19
+#define PLIC_INT_GPIO0_BIT7_OR_GPIO2_BIT7			20
+#define PLIC_INT_GPIO0_BIT8_OR_GPIO2_BIT8			21
+#define PLIC_INT_GPIO0_BIT9_OR_GPIO2_BIT9			22
+#define PLIC_INT_GPIO0_BIT10_OR_GPIO2_BIT10			23
+#define PLIC_INT_GPIO0_BIT11_OR_GPIO2_BIT11			24
+#define PLIC_INT_GPIO0_BIT12_OR_GPIO2_BIT12			25
+#define PLIC_INT_GPIO0_BIT13_OR_GPIO2_BIT13			26
+#define PLIC_INT_GPIO1_BIT0_OR_GPIO2_BIT14			27
+#define PLIC_INT_GPIO1_BIT1_OR_GPIO2_BIT15			28
+#define PLIC_INT_GPIO1_BIT2_OR_GPIO2_BIT16			29
+#define PLIC_INT_GPIO1_BIT3_OR_GPIO2_BIT17			30
+#define PLIC_INT_GPIO1_BIT4_OR_GPIO2_BIT18			31
+#define PLIC_INT_GPIO1_BIT5_OR_GPIO2_BIT19			32
+#define PLIC_INT_GPIO1_BIT6_OR_GPIO2_BIT20			33
+#define PLIC_INT_GPIO1_BIT7_OR_GPIO2_BIT21			34
+#define PLIC_INT_GPIO1_BIT8_OR_GPIO2_BIT22			35
+#define PLIC_INT_GPIO1_BIT9_OR_GPIO2_BIT23			36
+#define PLIC_INT_GPIO1_BIT10_OR_GPIO2_BIT24			37
+#define PLIC_INT_GPIO1_BIT11_OR_GPIO2_BIT25			38
+#define PLIC_INT_GPIO1_BIT12_OR_GPIO2_BIT26			39
+#define PLIC_INT_GPIO1_BIT13_OR_GPIO2_BIT27			40
+#define PLIC_INT_GPIO1_BIT14_OR_GPIO2_BIT28			41
+#define PLIC_INT_GPIO1_BIT15_OR_GPIO2_BIT29			42
+#define PLIC_INT_GPIO1_BIT16_OR_GPIO2_BIT30			43
+#define PLIC_INT_GPIO1_BIT17_OR_GPIO2_BIT31			44
+#define PLIC_INT_GPIO1_BIT18					45
+#define PLIC_INT_GPIO1_BIT19					46
+#define PLIC_INT_GPIO1_BIT20					47
+#define PLIC_INT_GPIO1_BIT21					48
+#define PLIC_INT_GPIO1_BIT22					49
+#define PLIC_INT_GPIO1_BIT23					50
+#define PLIC_INT_GPIO0_NON_DIRECT				51
+#define PLIC_INT_GPIO1_NON_DIRECT				52
+#define PLIC_INT_GPIO2_NON_DIRECT				53
+#define PLIC_INT_SPI0						54
+#define PLIC_INT_SPI1						55
+#define PLIC_INT_CAN0						56
+#define PLIC_INT_CAN1						57
+#define PLIC_INT_I2C0_MAIN					58
+#define PLIC_INT_I2C0_ALERT					59
+#define PLIC_INT_I2C0_SUS					60
+#define PLIC_INT_I2C1_MAIN					61
+#define PLIC_INT_I2C1_ALERT					62
+#define PLIC_INT_I2C1_SUS					63
+#define PLIC_INT_MAC0_INT					64
+#define PLIC_INT_MAC0_QUEUE1					65
+#define PLIC_INT_MAC0_QUEUE2					66
+#define PLIC_INT_MAC0_QUEUE3					67
+#define PLIC_INT_MAC0_EMAC					68
+#define PLIC_INT_MAC0_MMSL					69
+#define PLIC_INT_MAC1_INT					70
+#define PLIC_INT_MAC1_QUEUE1					71
+#define PLIC_INT_MAC1_QUEUE2					72
+#define PLIC_INT_MAC1_QUEUE3					73
+#define PLIC_INT_MAC1_EMAC					74
+#define PLIC_INT_MAC1_MMSL					75
+#define PLIC_INT_DDRC_TRAIN					76
+#define PLIC_INT_SCB_INTERRUPT					77
+#define PLIC_INT_ECC_ERROR					78
+#define PLIC_INT_ECC_CORRECT					79
+#define PLIC_INT_RTC_WAKEUP					80
+#define PLIC_INT_RTC_MATCH					81
+#define PLIC_INT_TIMER1						82
+#define PLIC_INT_TIMER2						83
+#define PLIC_INT_ENVM						84
+#define PLIC_INT_QSPI						85
+#define PLIC_INT_USB_DMA					86
+#define PLIC_INT_USB_MC						87
+#define PLIC_INT_MMC_MAIN					88
+#define PLIC_INT_MMC_WAKEUP					89
+#define PLIC_INT_MMUART0					90
+#define PLIC_INT_MMUART1					91
+#define PLIC_INT_MMUART2					92
+#define PLIC_INT_MMUART3					93
+#define PLIC_INT_MMUART4					94
+#define PLIC_INT_G5C_DEVRST					95
+#define PLIC_INT_G5C_MESSAGE					96
+#define PLIC_INT_USOC_VC_INTERRUPT				97
+#define PLIC_INT_USOC_SMB_INTERRUPT				98
+#define PLIC_INT_E51_0_MAINTENACE				99
+#define PLIC_INT_WDOG0_MRVP					100
+#define PLIC_INT_WDOG1_MRVP					101
+#define PLIC_INT_WDOG2_MRVP					102
+#define PLIC_INT_WDOG3_MRVP					103
+#define PLIC_INT_WDOG4_MRVP					104
+#define PLIC_INT_WDOG0_TOUT					105
+#define PLIC_INT_WDOG1_TOUT					106
+#define PLIC_INT_WDOG2_TOUT					107
+#define PLIC_INT_WDOG3_TOUT					108
+#define PLIC_INT_WDOG4_TOUT					109
+#define PLIC_INT_G5C_MSS_SPI					110
+#define PLIC_INT_VOLT_TEMP_ALARM				111
+#define PLIC_INT_ATHENA_COMPLETE				112
+#define PLIC_INT_ATHENA_ALARM					113
+#define PLIC_INT_ATHENA_BUS_ERROR				114
+#define PLIC_INT_USOC_AXIC_US					115
+#define PLIC_INT_USOC_AXIC_DS					116
+#define PLIC_INT_SPARE						117
+
+#define PLIC_INT_FABRIC_F2H_0					118
+#define PLIC_INT_FABRIC_F2H_1					119
+#define PLIC_INT_FABRIC_F2H_2					120
+#define PLIC_INT_FABRIC_F2H_3					121
+#define PLIC_INT_FABRIC_F2H_4					122
+#define PLIC_INT_FABRIC_F2H_5					123
+#define PLIC_INT_FABRIC_F2H_6					124
+#define PLIC_INT_FABRIC_F2H_7					125
+#define PLIC_INT_FABRIC_F2H_8					126
+#define PLIC_INT_FABRIC_F2H_9					127
+#define PLIC_INT_FABRIC_F2H_10					128
+#define PLIC_INT_FABRIC_F2H_11					129
+#define PLIC_INT_FABRIC_F2H_12					130
+#define PLIC_INT_FABRIC_F2H_13					131
+#define PLIC_INT_FABRIC_F2H_14					132
+#define PLIC_INT_FABRIC_F2H_15					133
+#define PLIC_INT_FABRIC_F2H_16					134
+#define PLIC_INT_FABRIC_F2H_17					135
+#define PLIC_INT_FABRIC_F2H_18					136
+#define PLIC_INT_FABRIC_F2H_19					137
+#define PLIC_INT_FABRIC_F2H_20					138
+#define PLIC_INT_FABRIC_F2H_21					139
+#define PLIC_INT_FABRIC_F2H_22					140
+#define PLIC_INT_FABRIC_F2H_23					141
+#define PLIC_INT_FABRIC_F2H_24					142
+#define PLIC_INT_FABRIC_F2H_25					143
+#define PLIC_INT_FABRIC_F2H_26					144
+#define PLIC_INT_FABRIC_F2H_27					145
+#define PLIC_INT_FABRIC_F2H_28					146
+#define PLIC_INT_FABRIC_F2H_29					147
+#define PLIC_INT_FABRIC_F2H_30					148
+#define PLIC_INT_FABRIC_F2H_31					149
+#define PLIC_INT_FABRIC_F2H_32					150
+#define PLIC_INT_FABRIC_F2H_33					151
+#define PLIC_INT_FABRIC_F2H_34					152
+#define PLIC_INT_FABRIC_F2H_35					153
+#define PLIC_INT_FABRIC_F2H_36					154
+#define PLIC_INT_FABRIC_F2H_37					155
+#define PLIC_INT_FABRIC_F2H_38					156
+#define PLIC_INT_FABRIC_F2H_39					157
+#define PLIC_INT_FABRIC_F2H_40					158
+#define PLIC_INT_FABRIC_F2H_41					159
+#define PLIC_INT_FABRIC_F2H_42					160
+#define PLIC_INT_FABRIC_F2H_43					161
+#define PLIC_INT_FABRIC_F2H_44					162
+#define PLIC_INT_FABRIC_F2H_45					163
+#define PLIC_INT_FABRIC_F2H_46					164
+#define PLIC_INT_FABRIC_F2H_47					165
+#define PLIC_INT_FABRIC_F2H_48					166
+#define PLIC_INT_FABRIC_F2H_49					167
+#define PLIC_INT_FABRIC_F2H_50					168
+#define PLIC_INT_FABRIC_F2H_51					169
+#define PLIC_INT_FABRIC_F2H_52					170
+#define PLIC_INT_FABRIC_F2H_53					171
+#define PLIC_INT_FABRIC_F2H_54					172
+#define PLIC_INT_FABRIC_F2H_55					173
+#define PLIC_INT_FABRIC_F2H_56					174
+#define PLIC_INT_FABRIC_F2H_57					175
+#define PLIC_INT_FABRIC_F2H_58					176

include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h line 197

Quoted 10 lines
include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h new file +10 -0
+#define PLIC_INT_FABRIC_F2H_59					177
+#define PLIC_INT_FABRIC_F2H_60					178
+#define PLIC_INT_FABRIC_F2H_61					179
+#define PLIC_INT_FABRIC_F2H_62					180
+#define PLIC_INT_FABRIC_F2H_63					181
+#define PLIC_INT_BUS_ERROR_UNIT_HART_0				182
+#define PLIC_INT_BUS_ERROR_UNIT_HART_1				183
+#define PLIC_INT_BUS_ERROR_UNIT_HART_2				184
+#define PLIC_INT_BUS_ERROR_UNIT_HART_3				185
+#define PLIC_INT_BUS_ERROR_UNIT_HART_4				186

Some thoughts I had while reading the patch:

(1) Are these constants really helpful?
- the PLIC is part of the soc, so most references will be handled in the core soc dtsi - and only once - and no board devicetree will likely need to handle them at all
- named constants are helpful if they make the number clearer but PLIC_INT_GPIO0_BIT2_OR_GPIO2_BIT2
or PLIC_INT_FABRIC_F2H_47 (and friends) probably do not really provide any additional value to someone reading the devicetree source

(2) If they should stay, they probably need a different prefix as they're pretty specific to the plic on the polarfire soc, so maybe MPFS_PLIC_* ?

Heiko

Quoted 3 lines
include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h new file +2 -0
+
+#endif /* _DT_BINDINGS_INTERRUPT_CONTROLLER_MICROCHIP_MPFS_PLIC_H */
maintainer
#7 lore raw

Re: [PATCH 02/13] dt-bindings: interrupt-controller: add defines for mpfs-plic

In reply to [email protected]
Quoted 11 lines · On Mon, Nov 08, 2021 at 03:05:43PM +0000, [email protected] wrote:

From: Ivan Griffin <[email protected]>

Add device tree bindings for the Microchip Polarfire Soc interrupt controller

---
 .../microchip,mpfs-plic.h                     | 199 ++++++++++++++++++
 1 file changed, 199 insertions(+)
 create mode 100644 include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h

Notice how there are not SoC interrupt defines in this directory. That's because we don't do defines for them. The 'rule' is only defines for made up numbers which are not in a reference manual.

Rob

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author maintainer
#8 lore raw

Re: [PATCH 02/13] dt-bindings: interrupt-controller: add defines for mpfs-plic

Quoted 15 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 11 lines · On Mon, Nov 08, 2021 at 03:05:43PM +0000, [email protected] wrote:

From: Ivan Griffin <[email protected]>

Add device tree bindings for the Microchip Polarfire Soc interrupt controller

---
  .../microchip,mpfs-plic.h                     | 199 ++++++++++++++++++
  1 file changed, 199 insertions(+)

create mode 100644 include/dt-bindings/interrupt-controller/microchip,mpfs-plic.h

Quoted 3 lines

Notice how there are not SoC interrupt defines in this directory. That's because we don't do defines for them. The 'rule' is only defines for made up numbers which are not in a reference manual.

Oh fair enough, I will drop these changes for V2 so. Thanks.

Quoted 3 lines

Rob

author maintainer
#9 lore raw

PATCH 3/13 dt-bindings: soc/microchip: update sys ctrlr compat string

In reply to [email protected]

From: Conor Dooley <[email protected]>

Update 'compatible' strings for system controller drivers to the approved Microchip name.

---
 .../bindings/mailbox/microchip,polarfire-soc-mailbox.yaml     | 4 +++-
 .../soc/microchip/microchip,polarfire-soc-sys-controller.yaml | 4 +++-
 drivers/mailbox/mailbox-mpfs.c                                | 1 +
 3 files changed, 7 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/mailbox/microchip,polarfire-soc-mailbox.yaml +3 -1
index bbb173ea483c..b08c8a158eea 100644
@@ -11,7 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    const: microchip,polarfire-soc-mailbox
+    enum:
+      - microchip,polarfire-soc-mailbox
+      - microchip,mpfs-mailbox
 
   reg:
     items:
Documentation/devicetree/bindings/soc/microchip/microchip,polarfire-soc-sys-controller.yaml +3 -1
index 2cd3bc6bd8d6..d6c953cd154b 100644
@@ -19,7 +19,9 @@ properties:
     maxItems: 1
 
   compatible:
-    const: microchip,polarfire-soc-sys-controller
+    enum:
+      - microchip,polarfire-soc-sys-controller
+      - microchip,mpfs-sys-controller
 
 required:
   - compatible
drivers/mailbox/mailbox-mpfs.c +1 -0
index 0d6e2231a2c7..9d5e558a6ee6 100644
@@ -233,6 +233,7 @@ static int mpfs_mbox_probe(struct platform_device *pdev)
 
 static const struct of_device_id mpfs_mbox_of_match[] = {
 	{.compatible = "microchip,polarfire-soc-mailbox", },
+	{.compatible = "microchip,mpfs-mailbox", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, mpfs_mbox_of_match);
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#10 lore raw

Re: [PATCH 03/13] dt-bindings: soc/microchip: update sys ctrlr compat string

Quoted 47 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Update 'compatible' strings for system controller drivers to the approved Microchip name.

---
 .../bindings/mailbox/microchip,polarfire-soc-mailbox.yaml     | 4 +++-
 .../soc/microchip/microchip,polarfire-soc-sys-controller.yaml | 4 +++-
 drivers/mailbox/mailbox-mpfs.c                                | 1 +
 3 files changed, 7 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/mailbox/microchip,polarfire-soc-mailbox.yaml +3 -1
index bbb173ea483c..b08c8a158eea 100644
@@ -11,7 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    const: microchip,polarfire-soc-mailbox
+    enum:
+      - microchip,polarfire-soc-mailbox
+      - microchip,mpfs-mailbox
 
   reg:
     items:
Documentation/devicetree/bindings/soc/microchip/microchip,polarfire-soc-sys-controller.yaml +3 -1
index 2cd3bc6bd8d6..d6c953cd154b 100644
@@ -19,7 +19,9 @@ properties:
     maxItems: 1
 
   compatible:
-    const: microchip,polarfire-soc-sys-controller
+    enum:
+      - microchip,polarfire-soc-sys-controller
+      - microchip,mpfs-sys-controller
 
 required:
   - compatible
drivers/mailbox/mailbox-mpfs.c +0 -0
index 0d6e2231a2c7..9d5e558a6ee6 100644

drivers/mailbox/mailbox-mpfs.c static int mpfs_mbox_probe(struct platform_device *pdev) · line 239

Quoted 7 lines
drivers/mailbox/mailbox-mpfs.c +1 -0
@@ -233,6 +233,7 @@ static int mpfs_mbox_probe(struct platform_device *pdev)
 
 static const struct of_device_id mpfs_mbox_of_match[] = {
 	{.compatible = "microchip,polarfire-soc-mailbox", },
+	{.compatible = "microchip,mpfs-mailbox", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, mpfs_mbox_of_match);

Please split the bindings from the code.

Best regards,
Krzysztof

#11 lore raw

Re: [PATCH 03/13] dt-bindings: soc/microchip: update sys ctrlr compat string

In reply to [email protected]

Hi Conor,

Quoted 6 lines · On Mon, Nov 8, 2021 at 4:06 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Update 'compatible' strings for system controller drivers to the approved Microchip name.

Thanks for your patch!

Documentation/devicetree/bindings/mailbox/microchip,polarfire-soc-mailbox.yaml maintainers: · line 16

Quoted 10 lines
Documentation/devicetree/bindings/mailbox/microchip,polarfire-soc-mailbox.yaml +3 -1
@@ -11,7 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    const: microchip,polarfire-soc-mailbox
+    enum:
+      - microchip,polarfire-soc-mailbox
+      - microchip,mpfs-mailbox

Is there any point in keeping the old compatible value?
Are there any real users? Most of the MPFS upstream DT is still in flux.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author maintainer
#12 lore raw

Re: [PATCH 03/13] dt-bindings: soc/microchip: update sys ctrlr compat string

Quoted 18 lines · Geert Uytterhoeven

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Conor,

Quoted 6 lines · On Mon, Nov 8, 2021 at 4:06 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Update 'compatible' strings for system controller drivers to the approved Microchip name.

Thanks for your patch!

Quoted 4 lines
Documentation/devicetree/bindings/mailbox/microchip,polarfire-soc-mailbox.yaml +0 -0
@@ -11,7 +11,9 @@ maintainers:
 
Quoted 9 lines

Documentation/devicetree/bindings/mailbox/microchip,polarfire-soc-mailbox.yaml maintainers: · line 16

Quoted 6 lines
Documentation/devicetree/bindings/mailbox/microchip,polarfire-soc-mailbox.yaml +3 -1
  properties:
    compatible:
-    const: microchip,polarfire-soc-mailbox
+    enum:
+      - microchip,polarfire-soc-mailbox
+      - microchip,mpfs-mailbox

Is there any point in keeping the old compatible value?
Are there any real users? Most of the MPFS upstream DT is still in flux.

yeah, true. and now is as good a time as any to just outright change it.

Quoted 12 lines

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

maintainer
#13 lore raw

Re: [PATCH 03/13] dt-bindings: soc/microchip: update sys ctrlr compat string

In reply to [email protected]
Quoted 4 lines · On Mon, Nov 08, 2021 at 03:05:44PM +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Update 'compatible' strings for system controller drivers to the approved Microchip name.

Why do I care what Microchip approved? You all picked identifiers (that's all it is) and now get to live with them.

Quoted 54 lines
---
 .../bindings/mailbox/microchip,polarfire-soc-mailbox.yaml     | 4 +++-
 .../soc/microchip/microchip,polarfire-soc-sys-controller.yaml | 4 +++-
 drivers/mailbox/mailbox-mpfs.c                                | 1 +
 3 files changed, 7 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/mailbox/microchip,polarfire-soc-mailbox.yaml +3 -1
index bbb173ea483c..b08c8a158eea 100644
@@ -11,7 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    const: microchip,polarfire-soc-mailbox
+    enum:
+      - microchip,polarfire-soc-mailbox
+      - microchip,mpfs-mailbox
 
   reg:
     items:
Documentation/devicetree/bindings/soc/microchip/microchip,polarfire-soc-sys-controller.yaml +3 -1
index 2cd3bc6bd8d6..d6c953cd154b 100644
@@ -19,7 +19,9 @@ properties:
     maxItems: 1
 
   compatible:
-    const: microchip,polarfire-soc-sys-controller
+    enum:
+      - microchip,polarfire-soc-sys-controller
+      - microchip,mpfs-sys-controller
 
 required:
   - compatible
drivers/mailbox/mailbox-mpfs.c +1 -0
index 0d6e2231a2c7..9d5e558a6ee6 100644
@@ -233,6 +233,7 @@ static int mpfs_mbox_probe(struct platform_device *pdev)
 
 static const struct of_device_id mpfs_mbox_of_match[] = {
 	{.compatible = "microchip,polarfire-soc-mailbox", },
+	{.compatible = "microchip,mpfs-mailbox", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, mpfs_mbox_of_match);
Signature · 1 line
2.33.1
author maintainer
#14 lore raw

Re: [PATCH 03/13] dt-bindings: soc/microchip: update sys ctrlr compat string

Quoted 10 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 4 lines · On Mon, Nov 08, 2021 at 03:05:44PM +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Update 'compatible' strings for system controller drivers to the approved Microchip name.

Why do I care what Microchip approved? You all picked identifiers (that's all it is) and now get to live with them.

Hi Rob,

You're speaking to the person who was perfectly happy with "polarfire-soc" when I wrote the binding last year, but I was asked to change it in order avoid having a different compatible string in the device tree for the soc and in the binding for a device that only exists in that soc.

If the v2 was a complete rename from "polarfire-soc-*" to "mpfs-*", including the file name - as opposed to an addition and I changed the commit message to the following would you be happier with the patch?

Quoted 3 lines

Change "polarfire-soc-*" compatible strings to "mpfs-*" in its system controller bindings in order to match the compatible string used in the soc binding and device tree.

Thanks,
Conor.

Quoted 55 lines
Quoted 55 lines
---
  .../bindings/mailbox/microchip,polarfire-soc-mailbox.yaml     | 4 +++-
  .../soc/microchip/microchip,polarfire-soc-sys-controller.yaml | 4 +++-
  drivers/mailbox/mailbox-mpfs.c                                | 1 +
  3 files changed, 7 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/mailbox/microchip,polarfire-soc-mailbox.yaml +3 -1
index bbb173ea483c..b08c8a158eea 100644
@@ -11,7 +11,9 @@ maintainers:
 
  properties:
    compatible:
-    const: microchip,polarfire-soc-mailbox
+    enum:
+      - microchip,polarfire-soc-mailbox
+      - microchip,mpfs-mailbox
 
    reg:
      items:
Documentation/devicetree/bindings/soc/microchip/microchip,polarfire-soc-sys-controller.yaml +3 -1
index 2cd3bc6bd8d6..d6c953cd154b 100644
@@ -19,7 +19,9 @@ properties:
      maxItems: 1
 
    compatible:
-    const: microchip,polarfire-soc-sys-controller
+    enum:
+      - microchip,polarfire-soc-sys-controller
+      - microchip,mpfs-sys-controller
 
  required:
    - compatible
drivers/mailbox/mailbox-mpfs.c +1 -1
index 0d6e2231a2c7..9d5e558a6ee6 100644
@@ -233,6 +233,7 @@ static int mpfs_mbox_probe(struct platform_device *pdev)
 
  static const struct of_device_id mpfs_mbox_of_match[] = {
       {.compatible = "microchip,polarfire-soc-mailbox", },
+     {.compatible = "microchip,mpfs-mailbox", },
       {},
  };
  MODULE_DEVICE_TABLE(of, mpfs_mbox_of_match);
--

2.33.1

author maintainer
#15 lore raw

PATCH 4/13 dt-bindings: riscv: update microchip polarfire binds

In reply to [email protected]

From: Conor Dooley <[email protected]>

Add mpfs-soc to clear undocumented binding warning

---
 Documentation/devicetree/bindings/riscv/microchip.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/riscv/microchip.yaml +1 -0
index 3f981e897126..1ff7a5224bbc 100644
@@ -21,6 +21,7 @@ properties:
       - enum:
           - microchip,mpfs-icicle-kit
       - const: microchip,mpfs
+      - const: microchip,mpfs-soc
 
 additionalProperties: true
 
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#16 lore raw

Re: [PATCH 04/13] dt-bindings: riscv: update microchip polarfire binds

Quoted 3 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add mpfs-soc to clear undocumented binding warning

What warnings? There is no such compatible used.

Quoted 12 lines
---
 Documentation/devicetree/bindings/riscv/microchip.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/riscv/microchip.yaml +0 -0
index 3f981e897126..1ff7a5224bbc 100644

Documentation/devicetree/bindings/riscv/microchip.yaml properties: · line 27

Quoted 7 lines
Documentation/devicetree/bindings/riscv/microchip.yaml +1 -0
@@ -21,6 +21,7 @@ properties:
       - enum:
           - microchip,mpfs-icicle-kit
       - const: microchip,mpfs
+      - const: microchip,mpfs-soc
 
 additionalProperties: true
 

Best regards,
Krzysztof

#17 lore raw

Re: [PATCH 04/13] dt-bindings: riscv: update microchip polarfire binds

In reply to [email protected]

Hi Conor,

Quoted 14 lines · On Mon, Nov 8, 2021 at 4:06 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add mpfs-soc to clear undocumented binding warning

---
 Documentation/devicetree/bindings/riscv/microchip.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/riscv/microchip.yaml +0 -0
index 3f981e897126..1ff7a5224bbc 100644

Documentation/devicetree/bindings/riscv/microchip.yaml properties: · line 24

Quoted 4 lines
Documentation/devicetree/bindings/riscv/microchip.yaml +1 -0
@@ -21,6 +21,7 @@ properties:
       - enum:
           - microchip,mpfs-icicle-kit
       - const: microchip,mpfs
+      - const: microchip,mpfs-soc

Doesn't the "s" in "mpfs" already stand for "soc"?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#18 lore raw

Re: [PATCH 04/13] dt-bindings: riscv: update microchip polarfire binds

Quoted 14 lines · Geert Uytterhoeven

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Conor,

Quoted 9 lines · On Mon, Nov 8, 2021 at 4:06 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add mpfs-soc to clear undocumented binding warning

---
  Documentation/devicetree/bindings/riscv/microchip.yaml | 1 +
  1 file changed, 1 insertion(+)
Quoted 11 lines

Documentation/devicetree/bindings/riscv/microchip.yaml properties: · line 24

Quoted 9 lines
Documentation/devicetree/bindings/riscv/microchip.yaml +1 -0
index 3f981e897126..1ff7a5224bbc 100644
@@ -21,6 +21,7 @@ properties:
        - enum:
            - microchip,mpfs-icicle-kit
        - const: microchip,mpfs
+      - const: microchip,mpfs-soc

Doesn't the "s" in "mpfs" already stand for "soc"?

not wrong, but using mpf-soc would be confusing since "mpf" is the part name for the non soc fpga. is it fine to just reuse "mpfs" for the dtsi overall compatible and for the soc subsection?

Quoted 12 lines

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#19 lore raw

Re: [PATCH 04/13] dt-bindings: riscv: update microchip polarfire binds

Hi Conor,

Quoted 17 lines · On Tue, Nov 9, 2021 at 1:08 PM <[email protected]> wrote:
Quoted 7 lines · Geert Uytterhoeven
Quoted 5 lines · On Mon, Nov 8, 2021 at 4:06 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add mpfs-soc to clear undocumented binding warning

Quoted 9 lines

Documentation/devicetree/bindings/riscv/microchip.yaml properties: · line 24

Quoted 7 lines
Documentation/devicetree/bindings/riscv/microchip.yaml +1 -0
@@ -21,6 +21,7 @@ properties:
        - enum:
            - microchip,mpfs-icicle-kit
        - const: microchip,mpfs
+      - const: microchip,mpfs-soc

Doesn't the "s" in "mpfs" already stand for "soc"?

Quoted 3 lines

not wrong, but using mpf-soc would be confusing since "mpf" is the part name for the non soc fpga. is it fine to just reuse "mpfs" for the dtsi overall compatible and for the soc subsection?

I really meant: what is the difference between "microchip,mpfs" and "microchip,mpfs-soc"? Can't you just use the former?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#20 lore raw

Re: [PATCH 04/13] dt-bindings: riscv: update microchip polarfire binds

Am Dienstag, 9. November 2021, 14:04:45 CET schrieb Geert Uytterhoeven:

Quoted 20 lines

Hi Conor,

Quoted 17 lines · On Tue, Nov 9, 2021 at 1:08 PM <[email protected]> wrote:
Quoted 16 lines · Geert Uytterhoeven

Documentation/devicetree/bindings/riscv/microchip.yaml properties: · line 24

Quoted 13 lines · On Mon, Nov 8, 2021 at 4:06 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add mpfs-soc to clear undocumented binding warning

Documentation/devicetree/bindings/riscv/microchip.yaml +1 -0
@@ -21,6 +21,7 @@ properties:
        - enum:
            - microchip,mpfs-icicle-kit
        - const: microchip,mpfs
+      - const: microchip,mpfs-soc

Doesn't the "s" in "mpfs" already stand for "soc"?

Quoted 6 lines
Quoted 3 lines

not wrong, but using mpf-soc would be confusing since "mpf" is the part name for the non soc fpga. is it fine to just reuse "mpfs" for the dtsi overall compatible and for the soc subsection?

I really meant: what is the difference between "microchip,mpfs" and "microchip,mpfs-soc"? Can't you just use the former?

definitly agreed :-)

Having the board named as
compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs"
sounds the most sensible.

As Conor wrote, "mpfs" is the name of the soc itself - with mpf being the fpga part, so that would follow what boards in other parts of the kernel do.

Heiko

author maintainer
#21 lore raw

PATCH 5/13 dt-bindings: i2c: add bindings for microchip mpfs i2c

In reply to [email protected]

From: Conor Dooley <[email protected]>

Add device tree bindings for the i2c controller on the Microchip PolarFire SoC.

---
 .../bindings/i2c/microchip,mpfs-i2c.yaml      | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml new file +74 -0
new file mode 100644
index 000000000000..bc4ea4498d35
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/microchip,mpfs-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS I2C Controller Device Tree Bindings
+
+maintainers:
+  - Daire McNamara <[email protected]>
+
+description: |
+  This I2C controller is found on the Microchip PolarFire SoC.
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-i2c # Microchip PolarFire SoC compatible SoCs
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    description: Phandle of the clock feeding the I2C controller.
+    minItems: 1
+
+  clock-frequency:
+    description: |
+      Desired I2C bus clock frequency in Hz. As only Standard and Fast
+      modes are supported, possible values are 100000 and 400000.
+    enum: [100000, 400000]
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/microchip,mpfs-clock.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        i2c@2010a000 {
+            compatible = "microchip,mpfs-i2c";
+            reg = <0 0x2010a000 0 0x1000>;
+            interrupts = <58>;
+            clock-frequency = <100000>;
+            clocks = <&clkcfg CLK_I2C0>;
+        };
+    };
+  - |
+    #include <dt-bindings/clock/microchip,mpfs-clock.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        i2c@2010b000 {
+            compatible = "microchip,mpfs-i2c";
+            reg = <0 0x2010b000 0 0x1000>;
+            interrupts = <61>;
+            clock-frequency = <100000>;
+            clocks = <&clkcfg CLK_I2C1>;
+        };
+    };
+...
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#22 lore raw

Re: [PATCH 05/13] dt-bindings: i2c: add bindings for microchip mpfs i2c

Quoted 38 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the i2c controller on the Microchip PolarFire SoC.

---
 .../bindings/i2c/microchip,mpfs-i2c.yaml      | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml new file +20 -0
new file mode 100644
index 000000000000..bc4ea4498d35
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/microchip,mpfs-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS I2C Controller Device Tree Bindings
+
+maintainers:
+  - Daire McNamara <[email protected]>
+
+description: |
+  This I2C controller is found on the Microchip PolarFire SoC.
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:

Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml line 30

Quoted 10 lines
Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml new file +10 -0
+      - microchip,mpfs-i2c # Microchip PolarFire SoC compatible SoCs
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    description: Phandle of the clock feeding the I2C controller.

Skip such descriptions here and in other patches - they do not introduce any meaningful information.

Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml line 31

Quoted 1 line
Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml new file +1 -0
+    minItems: 1

Define instead exact number of clocks or maxItems... but why would they be variable?

Quoted 30 lines
Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml new file +30 -0
+
+  clock-frequency:
+    description: |
+      Desired I2C bus clock frequency in Hz. As only Standard and Fast
+      modes are supported, possible values are 100000 and 400000.
+    enum: [100000, 400000]
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/microchip,mpfs-clock.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        i2c@2010a000 {
+            compatible = "microchip,mpfs-i2c";
+            reg = <0 0x2010a000 0 0x1000>;
+            interrupts = <58>;
+            clock-frequency = <100000>;
+            clocks = <&clkcfg CLK_I2C0>;
+        };
+    };
+  - |

Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml line 71

Quoted 10 lines
Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml new file +10 -0
+    #include <dt-bindings/clock/microchip,mpfs-clock.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        i2c@2010b000 {
+            compatible = "microchip,mpfs-i2c";
+            reg = <0 0x2010b000 0 0x1000>;
+            interrupts = <61>;
+            clock-frequency = <100000>;
+            clocks = <&clkcfg CLK_I2C1>;

This is the same example as above, just with changed numbers. Skip it.

Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml line 74

Quoted 4 lines
Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml new file +3 -0
+        };
+    };
+...

Best regards,
Krzysztof

maintainer
#23 lore raw

Re: [PATCH 05/13] dt-bindings: i2c: add bindings for microchip mpfs i2c

In reply to [email protected]
Quoted 5 lines · On Mon, 08 Nov 2021 15:05:46 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the i2c controller on the Microchip PolarFire SoC.

Quoted 7 lines
---
 .../bindings/i2c/microchip,mpfs-i2c.yaml      | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include <dt-bindings/clock/microchip,mpfs-clock.h>

      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

compilation terminated.
make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1552366

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

author maintainer
#24 lore raw

PATCH 6/13 dt-bindings: rng: add bindings for microchip mpfs rng

In reply to [email protected]

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

---
 .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +31 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng
+
+  syscontroller:
+    maxItems: 1
+    description: name of the system controller device node
+
+required:
+  - compatible
+  - "syscontroller"
+
+additionalProperties: false
+
+examples:
+  - |
+    hwrandom: hwrandom {
+        compatible = "microchip,polarfire-soc-rng";
+        syscontroller = <&syscontroller>;
+    };
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#25 lore raw

Re: [PATCH 06/13] dt-bindings: rng: add bindings for microchip mpfs rng

Quoted 25 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

---
 .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +8 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 18

Quoted 10 lines
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +10 -0
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng
+
+  syscontroller:
+    maxItems: 1
+    description: name of the system controller device node

There are several issues with this:
1. You need to describe the type.
2. Description is not helpful (just copying the name of property) and actually misleading because you do not put there the name of device node.
3. What is it? Looks like syscon (or sometimes called sysreg). If yes, please use existing syscon bindings.

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 22

Quoted 4 lines
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +4 -0
+
+required:
+  - compatible
+  - "syscontroller"

No need for quotes.

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 31

Quoted 10 lines
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +9 -0
+
+additionalProperties: false
+
+examples:
+  - |
+    hwrandom: hwrandom {
+        compatible = "microchip,polarfire-soc-rng";
+        syscontroller = <&syscontroller>;
+    };

Best regards,
Krzysztof

author maintainer
#26 lore raw

Re: [PATCH 06/13] dt-bindings: rng: add bindings for microchip mpfs rng

Quoted 37 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 34 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

---
  .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
  1 file changed, 31 insertions(+)

create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +17 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng
+
+  syscontroller:
+    maxItems: 1
Quoted 8 lines

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 18

Quoted 1 line
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +1 -0
+    description: name of the system controller device node

There are several issues with this:
1. You need to describe the type.
2. Description is not helpful (just copying the name of property) and actually misleading because you do not put there the name of device node.
3. What is it? Looks like syscon (or sometimes called sysreg). If yes, please use existing syscon bindings.

1 & 2 - Correct, it is bad & I'll write a better description for it. 3 - Its a system controller implemented as a mailbox. The syscontroller is the mailbox client, which the rng and generic drivers both use.

Quoted 23 lines

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 22

Quoted 5 lines
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +4 -0
+
+required:
+  - compatible
+  - "syscontroller"

No need for quotes.

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 31

Quoted 10 lines
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +9 -0
+
+additionalProperties: false
+
+examples:
+  - |
+    hwrandom: hwrandom {
+        compatible = "microchip,polarfire-soc-rng";
+        syscontroller = <&syscontroller>;
+    };

Best regards,
Krzysztof

#27 lore raw

Re: [PATCH 06/13] dt-bindings: rng: add bindings for microchip mpfs rng

Quoted 46 lines · On 09/11/2021 13:54, [email protected] wrote:
Quoted 37 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 34 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

---
  .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
  1 file changed, 31 insertions(+)

create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +17 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng
+
+  syscontroller:
+    maxItems: 1
Quoted 8 lines

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 18

Quoted 1 line
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +1 -0
+    description: name of the system controller device node

There are several issues with this:
1. You need to describe the type.
2. Description is not helpful (just copying the name of property) and actually misleading because you do not put there the name of device node.
3. What is it? Looks like syscon (or sometimes called sysreg). If yes, please use existing syscon bindings.

Quoted 3 lines

1 & 2 - Correct, it is bad & I'll write a better description for it. 3 - Its a system controller implemented as a mailbox. The syscontroller is the mailbox client, which the rng and generic drivers both use.

I understood that pointed device node is a mailbox, not this node. But here, what is it here? How do you use it here?

Best regards,
Krzysztof

author maintainer
#28 lore raw

Re: [PATCH 06/13] dt-bindings: rng: add bindings for microchip mpfs rng

Quoted 49 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 46 lines · On 09/11/2021 13:54, [email protected] wrote:
Quoted 45 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 25 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

---
   .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
   1 file changed, 31 insertions(+)

create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +8 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 18

Quoted 10 lines
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +10 -0
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng
+
+  syscontroller:
+    maxItems: 1
+    description: name of the system controller device node

There are several issues with this:
1. You need to describe the type.
2. Description is not helpful (just copying the name of property) and actually misleading because you do not put there the name of device node.
3. What is it? Looks like syscon (or sometimes called sysreg). If yes, please use existing syscon bindings.

Quoted 6 lines
Quoted 3 lines

1 & 2 - Correct, it is bad & I'll write a better description for it. 3 - Its a system controller implemented as a mailbox. The syscontroller is the mailbox client, which the rng and generic drivers both use.

I understood that pointed device node is a mailbox, not this node. But here, what is it here? How do you use it here?

The system controller is the means of access to the random number generator. The phandle to the sys controller is provided here so that the rng driver can locate the mailbox client through which it requests random numbers.

I'm not quite sure that I am answering the question you are asking though.

Quoted 4 lines

Best regards,
Krzysztof

#29 lore raw

Re: [PATCH 06/13] dt-bindings: rng: add bindings for microchip mpfs rng

Quoted 54 lines · On 09/11/2021 14:36, [email protected] wrote:
Quoted 53 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 49 lines · On 09/11/2021 13:54, [email protected] wrote:
Quoted 37 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 34 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

---
   .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
   1 file changed, 31 insertions(+)

create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +17 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng
+
+  syscontroller:
+    maxItems: 1
Quoted 8 lines

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 18

Quoted 1 line
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +1 -0
+    description: name of the system controller device node

There are several issues with this:
1. You need to describe the type.
2. Description is not helpful (just copying the name of property) and actually misleading because you do not put there the name of device node.
3. What is it? Looks like syscon (or sometimes called sysreg). If yes, please use existing syscon bindings.

1 & 2 - Correct, it is bad & I'll write a better description for it. 3 - Its a system controller implemented as a mailbox. The syscontroller is the mailbox client, which the rng and generic drivers both use.

Quoted 6 lines
Quoted 2 lines

I understood that pointed device node is a mailbox, not this node. But here, what is it here? How do you use it here?

The system controller is the means of access to the random number generator. The phandle to the sys controller is provided here so that the rng driver can locate the mailbox client through which it requests random numbers.

I am asking this to understand whether there is a generic or existing property which should be used instead.

If I understand correctly, the rng driver needs a mailbox client? If it is mailbox client, then there is a property: "mboxes". Use this one (look for existing bindings, e.g. Documentation/devicetree/bindings/firmware/arm,scmi.yaml).

Best regards,
Krzysztof

author maintainer
#30 lore raw

Re: [PATCH 06/13] dt-bindings: rng: add bindings for microchip mpfs rng

Quoted 64 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 54 lines · On 09/11/2021 14:36, [email protected] wrote:
Quoted 53 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 49 lines · On 09/11/2021 13:54, [email protected] wrote:
Quoted 37 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 34 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

---
    .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
    1 file changed, 31 insertions(+)

create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +17 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng
+
+  syscontroller:
+    maxItems: 1
Quoted 8 lines

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 18

Quoted 1 line
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +1 -0
+    description: name of the system controller device node

There are several issues with this:
1. You need to describe the type.
2. Description is not helpful (just copying the name of property) and actually misleading because you do not put there the name of device node.
3. What is it? Looks like syscon (or sometimes called sysreg). If yes, please use existing syscon bindings.

1 & 2 - Correct, it is bad & I'll write a better description for it. 3 - Its a system controller implemented as a mailbox. The syscontroller is the mailbox client, which the rng and generic drivers both use.

Quoted 6 lines
Quoted 2 lines

I understood that pointed device node is a mailbox, not this node. But here, what is it here? How do you use it here?

The system controller is the means of access to the random number generator. The phandle to the sys controller is provided here so that the rng driver can locate the mailbox client through which it requests random numbers.

Quoted 4 lines

I am asking this to understand whether there is a generic or existing property which should be used instead.

If I understand correctly, the rng driver needs a mailbox client?

Correct, it needs one. Binding for that is here:
Documentation/devicetree/bindings/soc/microchip/microchip,polarfire-soc-sys-controller.yaml

Quoted 8 lines

If it is a mailbox client, then there is a property: "mboxes". Use this one (look for existing bindings, e.g. Documentation/devicetree/bindings/firmware/arm,scmi.yaml).

Best regards,
Krzysztof

maintainer
#31 lore raw

Re: [PATCH 06/13] dt-bindings: rng: add bindings for microchip mpfs rng

Quoted 65 lines · On Wed, Nov 10, 2021 at 09:46:23AM +0000, [email protected] wrote:
Quoted 64 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 60 lines · On 09/11/2021 14:36, [email protected] wrote:
Quoted 49 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 46 lines · On 09/11/2021 13:54, [email protected] wrote:
Quoted 45 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 25 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

---
    .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
    1 file changed, 31 insertions(+)

create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +8 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 18

Quoted 10 lines
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +10 -0
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng
+
+  syscontroller:
+    maxItems: 1
+    description: name of the system controller device node

There are several issues with this:
1. You need to describe the type.
2. Description is not helpful (just copying the name of property) and actually misleading because you do not put there the name of device node.
3. What is it? Looks like syscon (or sometimes called sysreg). If yes, please use existing syscon bindings.

Quoted 6 lines
Quoted 3 lines

1 & 2 - Correct, it is bad & I'll write a better description for it. 3 - Its a system controller implemented as a mailbox. The syscontroller is the mailbox client, which the rng and generic drivers both use.

I understood that pointed device node is a mailbox, not this node. But here, what is it here? How do you use it here?

The system controller is the means of access to the random number generator. The phandle to the sys controller is provided here so that the rng driver can locate the mailbox client through which it requests random numbers.

Quoted 6 lines
Quoted 4 lines

I am asking this to understand whether there is a generic or existing property which should be used instead.

If I understand correctly, the rng driver needs a mailbox client?

Correct, it needs one. Binding for that is here:
Documentation/devicetree/bindings/soc/microchip/microchip,polarfire-soc-sys-controller.yaml

The rng node and others should be a child of the sys controller node.

Rob

#32 lore raw

Re: [PATCH 06/13] dt-bindings: rng: add bindings for microchip mpfs rng

In reply to [email protected]

Hi Conor,

Quoted 6 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

Thanks for your patch!

Quoted 15 lines
---
 .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +4 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +10 -0
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng

"microchip,mpfs-rng", for consistency with other bindings?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#33 lore raw

Re: [PATCH 06/13] dt-bindings: rng: add bindings for microchip mpfs rng

Quoted 33 lines · Geert Uytterhoeven

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Conor,

Quoted 6 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the hardware rng device accessed via the system services on the Microchip PolarFire SoC.

Thanks for your patch!

Quoted 19 lines
---
  .../bindings/rng/microchip,mpfs-rng.yaml      | 31 +++++++++++++++++++
  1 file changed, 31 insertions(+)

create mode 100644 Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +8 -0
new file mode 100644
index 000000000000..e8ecb3538a86
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/rng/microchip,mpfs-rng.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS random number generator
+
Quoted 8 lines

Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml line 14

Quoted 6 lines
Documentation/devicetree/bindings/rng/microchip,mpfs-rng.yaml new file +6 -0
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,polarfire-soc-rng

"microchip,mpfs-rng", for consistency with other bindings?

correct, dropped the wrong one while cleaning up since this doesnt match the device tree.

Quoted 12 lines

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#34 lore raw

PATCH 7/13 dt-bindings: rtc: add bindings for microchip mpfs rtc

In reply to [email protected]

From: Conor Dooley <[email protected]>

Add device tree bindings for the real time clock on the Microchip PolarFire SoC.

---
 .../bindings/rtc/microchip,mfps-rtc.yaml      | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +61 -0
new file mode 100644
index 000000000000..c82b3e7351e0
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/microchip,mfps-rtc.yaml#
+
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PolarFire Soc (MPFS) RTC Device Tree Bindings
+
+allOf:
+  - $ref: "rtc.yaml#"
+
+maintainers:
+  - Daire McNamara <[email protected]>
+  - Lewis Hanly <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-rtc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  prescaler:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: rtc
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/microchip,mpfs-clock.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        rtc@20124000 {
+            compatible = "microchip,mpfs-rtc";
+            reg = <0 0x20124000 0 0x1000>;
+            clocks = <&clkcfg CLK_RTC>;
+            clock-names = "rtc";
+            interrupts = <80>;
+        };
+    };
+...
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#35 lore raw

Re: [PATCH 07/13] dt-bindings: rtc: add bindings for microchip mpfs rtc

Quoted 19 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the real time clock on the Microchip PolarFire SoC.

---
 .../bindings/rtc/microchip,mfps-rtc.yaml      | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +1 -0
new file mode 100644
index 000000000000..c82b3e7351e0
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml line 11

Quoted 10 lines
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +10 -0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/microchip,mfps-rtc.yaml#
+
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PolarFire Soc (MPFS) RTC Device Tree Bindings
+
+allOf:
+  - $ref: "rtc.yaml#"

No need for quotes.

Quoted 8 lines
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +8 -0
+
+maintainers:
+  - Daire McNamara <[email protected]>
+  - Lewis Hanly <[email protected]>
+
+properties:
+  compatible:
+    enum:

Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml line 29

Quoted 10 lines
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +10 -0
+      - microchip,mpfs-rtc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  prescaler:
+    maxItems: 1

You need to describe the type and add description. This does not look like standard property, so it needs vendor prefix.

Quoted 13 lines
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +13 -0
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: rtc
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks

Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml line 52

Quoted 10 lines
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +10 -0
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/microchip,mpfs-clock.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;

Add a blank line before new node. The same in previous examples.

Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml line 61

Quoted 10 lines
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +9 -0
+        rtc@20124000 {
+            compatible = "microchip,mpfs-rtc";
+            reg = <0 0x20124000 0 0x1000>;
+            clocks = <&clkcfg CLK_RTC>;
+            clock-names = "rtc";
+            interrupts = <80>;
+        };
+    };
+...

Best regards,
Krzysztof

maintainer
#36 lore raw

Re: [PATCH 07/13] dt-bindings: rtc: add bindings for microchip mpfs rtc

In reply to [email protected]
Quoted 5 lines · On Mon, 08 Nov 2021 15:05:48 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the real time clock on the Microchip PolarFire SoC.

Quoted 7 lines
---
 .../bindings/rtc/microchip,mfps-rtc.yaml      | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include <dt-bindings/clock/microchip,mpfs-clock.h>

      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

compilation terminated.
make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1552374

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

#37 lore raw

Re: [PATCH 07/13] dt-bindings: rtc: add bindings for microchip mpfs rtc

In reply to [email protected]

Hi Conor,

Quoted 7 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the real time clock on the Microchip PolarFire SoC.

Thanks for your patch!

Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml line 52

Quoted 10 lines
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +6 -0
@@ -0,0 +1,61 @@
 
+examples:
+  - |
+    #include <dt-bindings/clock/microchip,mpfs-clock.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;

Examples are indeed built with #{address,size}-cells = <1>. However, there is no need to override this. Just drop the zeros in the reg property below.

Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml line 61

Quoted 9 lines
Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +9 -0
+        rtc@20124000 {
+            compatible = "microchip,mpfs-rtc";
+            reg = <0 0x20124000 0 0x1000>;
+            clocks = <&clkcfg CLK_RTC>;
+            clock-names = "rtc";
+            interrupts = <80>;
+        };
+    };
+...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#38 lore raw

PATCH 8/13 dt-bindings: soc/microchip: add bindings for mpfs system services

In reply to [email protected]

From: Conor Dooley <[email protected]>

Add device tree bindings for the services provided by the system controller directly on the Microchip PolarFire SoC.

---
 .../microchip,mpfs-generic-service.yaml       | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml
Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml new file +31 -0
new file mode 100644
index 000000000000..f89d3a74c059
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/microchip/microchip,mpfs-generic-service.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS system services
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,mpfs-generic-service
+
+  syscontroller:
+    maxItems: 1
+    description: name of the system controller device node
+
+required:
+  - compatible
+  - "syscontroller"
+
+additionalProperties: false
+
+examples:
+  - |
+    sysserv: sysserv {
+        compatible = "microchip,mpfs-generic-service";
+        syscontroller = <&syscontroller>;
+    };
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#39 lore raw

Re: [PATCH 08/13] dt-bindings: soc/microchip: add bindings for mpfs system services

Quoted 25 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the services provided by the system controller directly on the Microchip PolarFire SoC.

---
 .../microchip,mpfs-generic-service.yaml       | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml
Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml new file +8 -0
new file mode 100644
index 000000000000..f89d3a74c059
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/microchip/microchip,mpfs-generic-service.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microchip MPFS system services
+

Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml line 18

Quoted 10 lines
Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml new file +10 -0
+maintainers:
+  - Conor Dooley <[email protected]>
+
+properties:
+  compatible:
+    const: microchip,mpfs-generic-service
+
+  syscontroller:
+    maxItems: 1
+    description: name of the system controller device node

Same comment as for rng patch.

Quoted 4 lines
Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml new file +3 -0
+
+required:
+  - compatible

Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml line 31

Quoted 10 lines
Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-generic-service.yaml new file +10 -0
+  - "syscontroller"
+
+additionalProperties: false
+
+examples:
+  - |
+    sysserv: sysserv {
+        compatible = "microchip,mpfs-generic-service";
+        syscontroller = <&syscontroller>;
+    };

Best regards,
Krzysztof

author maintainer
#40 lore raw

PATCH 9/13 dt-bindings: gpio: add bindings for microchip mpfs gpio

In reply to [email protected]

From: Conor Dooley <[email protected]>

Add device tree bindings for the gpio controller on the Microchip PolarFire SoC.

---
 .../bindings/gpio/microchip,mpfs-gpio.yaml    | 108 ++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +108 -0
new file mode 100644
index 000000000000..067019ddc1f7
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/microchip,mpfs-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS GPIO Controller Device Tree Bindings
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+description: |
+  This GPIO controller is found on the Microchip PolarFire SoC.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - microchip,mpfs-gpio
+          - microsemi,ms-pf-mss-gpio
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt mapping, one per GPIO. Maximum 32 GPIOs.
+    minItems: 1
+    maxItems: 32
+
+  interrupt-controller: true
+
+  clocks:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  ngpios:
+    description:
+      The number of GPIOs available.
+    minimum: 1
+    maximum: 32
+    default: 32
+
+  gpio-controller: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#interrupt-cells"
+  - "#gpio-cells"
+  - gpio-controller
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/clock/microchip,mpfs-clock.h"
+    #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h"
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      gpio2: gpio@20122000 {
+        compatible = "microchip,mpfs-gpio";
+        reg = <0x0 0x20122000 0x0 0x1000>;
+        clocks = <&clkcfg CLK_GPIO2>;
+        interrupt-parent = <&plic>;
+        interrupts = <PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        status = "disabled";
+      };
+    };
+...
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#41 lore raw

Re: [PATCH 09/13] dt-bindings: gpio: add bindings for microchip mpfs gpio

Quoted 20 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the gpio controller on the Microchip PolarFire SoC.

---
 .../bindings/gpio/microchip,mpfs-gpio.yaml    | 108 ++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +3 -0
new file mode 100644
index 000000000000..067019ddc1f7
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---

Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml line 13

Quoted 10 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +10 -0
+$id: http://devicetree.org/schemas/gpio/microchip,mpfs-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS GPIO Controller Device Tree Bindings
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+description: |
+  This GPIO controller is found on the Microchip PolarFire SoC.

If "Microchip MPFS" means "Microchip PolarFire SoC", then this is duplicating the title. Similarly to your previous patches. Skip it then, there is no point to have descriptions which are obvious or duplicating existing information.

Quoted 82 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +82 -0
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - microchip,mpfs-gpio
+          - microsemi,ms-pf-mss-gpio
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt mapping, one per GPIO. Maximum 32 GPIOs.
+    minItems: 1
+    maxItems: 32
+
+  interrupt-controller: true
+
+  clocks:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  ngpios:
+    description:
+      The number of GPIOs available.
+    minimum: 1
+    maximum: 32
+    default: 32
+
+  gpio-controller: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#interrupt-cells"
+  - "#gpio-cells"
+  - gpio-controller
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/clock/microchip,mpfs-clock.h"
+    #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h"
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      gpio2: gpio@20122000 {
+        compatible = "microchip,mpfs-gpio";
+        reg = <0x0 0x20122000 0x0 0x1000>;
+        clocks = <&clkcfg CLK_GPIO2>;
+        interrupt-parent = <&plic>;
+        interrupts = <PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT

Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml line 105

Quoted 10 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +10 -0
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        status = "disabled";

Skip status=disabled.

Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml line 108

Quoted 4 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +3 -0
+      };
+    };
+...

Best regards,
Krzysztof

maintainer
#42 lore raw

Re: [PATCH 09/13] dt-bindings: gpio: add bindings for microchip mpfs gpio

In reply to [email protected]
Quoted 11 lines · On Mon, 08 Nov 2021 15:05:50 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the gpio controller on the Microchip PolarFire SoC.

---
 .../bindings/gpio/microchip,mpfs-gpio.yaml    | 108 ++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include "dt-bindings/clock/microchip,mpfs-clock.h"

      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

compilation terminated.
make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1552384

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

#43 lore raw

Re: [PATCH 09/13] dt-bindings: gpio: add bindings for microchip mpfs gpio

In reply to [email protected]

Hi Conor,

Quoted 6 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the gpio controller on the Microchip PolarFire SoC.

Thanks for your patch!

Quoted 13 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +10 -0
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/microchip,mpfs-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS GPIO Controller Device Tree Bindings
+
+maintainers:
+  - Conor Dooley <[email protected]>

Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml line 20

Quoted 10 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +10 -0
+
+description: |
+  This GPIO controller is found on the Microchip PolarFire SoC.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - microchip,mpfs-gpio
+          - microsemi,ms-pf-mss-gpio

What's the difference between these two?
If there is a difference, please add a comment "# <explanation>" to each entry.
If there is no difference, please drop the second one.

Quoted 25 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +25 -0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt mapping, one per GPIO. Maximum 32 GPIOs.
+    minItems: 1
+    maxItems: 32
+
+  interrupt-controller: true
+
+  clocks:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  ngpios:
+    description:
+      The number of GPIOs available.
+    minimum: 1
+    maximum: 32
+    default: 32
+

Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml line 55

Quoted 10 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +10 -0
+  gpio-controller: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#interrupt-cells"
+  - "#gpio-cells"
+  - gpio-controller
+  - clocks

Any specific reason interrupt-controller is not required?

Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml line 65

Quoted 10 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +10 -0
+
+additionalProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/clock/microchip,mpfs-clock.h"
+    #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h"
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;

Just drop these two...

Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml line 68

Quoted 3 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +3 -0
+      gpio2: gpio@20122000 {
+        compatible = "microchip,mpfs-gpio";
+        reg = <0x0 0x20122000 0x0 0x1000>;

... and the zeros here.

Quoted 27 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +27 -0
+        clocks = <&clkcfg CLK_GPIO2>;
+        interrupt-parent = <&plic>;
+        interrupts = <PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT

Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml line 105

Quoted 10 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +10 -0
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT
+          PLIC_INT_GPIO2_NON_DIRECT>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        status = "disabled";

Please drop this.

Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml line 107

Quoted 2 lines
Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml new file +2 -0
+      };
+    };

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#44 lore raw

PATCH 10/13 dt-bindings: spi: add bindings for microchip mpfs spi

In reply to [email protected]

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

---
 .../bindings/spi/microchip,mpfs-spi.yaml      | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +72 -0
new file mode 100644
index 000000000000..efed145ad029
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/microchip,mpfs-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS {Q,}SPI Controller Device Tree Bindings
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+description: |
+  This {Q,}SPI controller is found on the Microchip PolarFire SoC.
+
+allOf:
+  - $ref: "spi-controller.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-spi
+      - microsemi,ms-pf-mss-spi
+      - microchip,mpfs-qspi
+      - microsemi,ms-pf-mss-qspi
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clock-names:
+    maxItems: 1
+
+  clocks:
+    maxItems: 2
+
+  num-cs:
+    description: |
+      Number of chip selects used.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 8
+    default: 8
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/clock/microchip,mpfs-clock.h"
+    #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h"
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      spi0: spi@20108000 {
+        compatible = "microchip,mpfs-spi";
+        reg = <0x0 0x20108000 0x0 0x1000>;
+        clocks = <&clkcfg CLK_SPI0>;
+        interrupt-parent = <&plic>;
+        interrupts = <PLIC_INT_SPI0>;
+        spi-max-frequency = <25000000>;
+        num-cs = <8>;
+        status = "disabled";
+      };
+    };
+...
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#45 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

Quoted 23 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

---
 .../bindings/spi/microchip,mpfs-spi.yaml      | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +6 -0
new file mode 100644
index 000000000000..efed145ad029
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/microchip,mpfs-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+

Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml line 16

Quoted 10 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +10 -0
+title: Microchip MPFS {Q,}SPI Controller Device Tree Bindings
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+description: |
+  This {Q,}SPI controller is found on the Microchip PolarFire SoC.
+
+allOf:
+  - $ref: "spi-controller.yaml#"

No need for quotes.

Quoted 10 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +10 -0
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-spi
+      - microsemi,ms-pf-mss-spi
+      - microchip,mpfs-qspi
+      - microsemi,ms-pf-mss-qspi
+
+  reg:

Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml line 36

Quoted 10 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +10 -0
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clock-names:
+    maxItems: 1
+
+  clocks:
+    maxItems: 2

This does not match clock-names. Describe clocks instead. Are you really sure your hardware can have an optional second clock?

Quoted 27 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +26 -0
+
+  num-cs:
+    description: |
+      Number of chip selects used.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 8
+    default: 8
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/clock/microchip,mpfs-clock.h"
+    #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h"
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      spi0: spi@20108000 {
+        compatible = "microchip,mpfs-spi";

Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml line 72

Quoted 10 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +10 -0
+        reg = <0x0 0x20108000 0x0 0x1000>;
+        clocks = <&clkcfg CLK_SPI0>;
+        interrupt-parent = <&plic>;
+        interrupts = <PLIC_INT_SPI0>;
+        spi-max-frequency = <25000000>;
+        num-cs = <8>;
+        status = "disabled";
+      };
+    };
+...

Best regards,
Krzysztof

maintainer
#46 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

In reply to [email protected]
Quoted 11 lines · On Mon, 08 Nov 2021 15:05:51 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

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

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include "dt-bindings/clock/microchip,mpfs-clock.h"

      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

compilation terminated.
make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1552385

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

author maintainer
#47 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

Quoted 15 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 11 lines · On Mon, 08 Nov 2021 15:05:51 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

---
  .../bindings/spi/microchip,mpfs-spi.yaml      | 72 +++++++++++++++++++
  1 file changed, 72 insertions(+)

create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml

Quoted 8 lines

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include "dt-bindings/clock/microchip,mpfs-clock.h"

Rob,
Should I drop the header from the example or is there a way for me specify the dependent patch to pass this check?

Quoted 21 lines
       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

compilation terminated.
make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1552385

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

#48 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

Quoted 19 lines · On 09/11/2021 13:16, [email protected] wrote:
Quoted 18 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 11 lines · On Mon, 08 Nov 2021 15:05:51 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

---
  .../bindings/spi/microchip,mpfs-spi.yaml      | 72 +++++++++++++++++++
  1 file changed, 72 insertions(+)

create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

Quoted 8 lines
Quoted 5 lines

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include "dt-bindings/clock/microchip,mpfs-clock.h"

Rob,
Should I drop the header from the example or is there a way for me specify the dependent patch to pass this check?

The error has to be fixed, although not necessarily by dropping the header, but by posting it. How this can pass on your system? There is no such file added in this patchset.

Best regards,
Krzysztof

author maintainer
#49 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

Quoted 27 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 24 lines · On 09/11/2021 13:16, [email protected] wrote:
Quoted 23 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 11 lines · On Mon, 08 Nov 2021 15:05:51 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

---
   .../bindings/spi/microchip,mpfs-spi.yaml      | 72 +++++++++++++++++++
   1 file changed, 72 insertions(+)

create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include "dt-bindings/clock/microchip,mpfs-clock.h"

Quoted 7 lines
Quoted 3 lines

Rob,
Should I drop the header from the example or is there a way for me specify the dependent patch to pass this check?

The error has to be fixed, although not necessarily by dropping the header, but by posting it. How this can pass on your system? There is no such file added in this patchset.

I linked the patch adding the clock as a dependency in the cover letter [1], which is why I was wondering if there was a better way to do so that would get picked up by the checker bot.

[1]
https://lore.kernel.org/linux-clk/[email protected]/

Quoted 4 lines

Best regards,
Krzysztof

#50 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

Quoted 32 lines · On 09/11/2021 13:58, [email protected] wrote:
Quoted 31 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 27 lines · On 09/11/2021 13:16, [email protected] wrote:
Quoted 15 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 11 lines · On Mon, 08 Nov 2021 15:05:51 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

---
   .../bindings/spi/microchip,mpfs-spi.yaml      | 72 +++++++++++++++++++
   1 file changed, 72 insertions(+)

create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml

Quoted 8 lines

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include "dt-bindings/clock/microchip,mpfs-clock.h"

Rob,
Should I drop the header from the example or is there a way for me specify the dependent patch to pass this check?

Quoted 6 lines
Quoted 3 lines

The error has to be fixed, although not necessarily by dropping the header, but by posting it. How this can pass on your system? There is no such file added in this patchset.

I linked the patch adding the clock as a dependency in the cover letter [1], which is why I was wondering if there was a better way to do so that would get picked up by the checker bot.

It's not only about the bot, but dependency when applied. If you did not warn clk maintainer that clock bindings should go via Rob's tree or should be provided as a tag, the patches here cannot be applied in this cycle.

Best regards,
Krzysztof

author maintainer
#51 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

Quoted 38 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 35 lines · On 09/11/2021 13:58, [email protected] wrote:
Quoted 34 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 19 lines · On 09/11/2021 13:16, [email protected] wrote:
Quoted 18 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 11 lines · On Mon, 08 Nov 2021 15:05:51 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

---
    .../bindings/spi/microchip,mpfs-spi.yaml      | 72 +++++++++++++++++++
    1 file changed, 72 insertions(+)

create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

Quoted 8 lines
Quoted 5 lines

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include "dt-bindings/clock/microchip,mpfs-clock.h"

Rob,
Should I drop the header from the example or is there a way for me specify the dependent patch to pass this check?

The error has to be fixed, although not necessarily by dropping the header, but by posting it. How this can pass on your system? There is no such file added in this patchset.

Quoted 8 lines
Quoted 3 lines

I linked the patch adding the clock as a dependency in the cover letter [1], which is why I was wondering if there was a better way to do so that would get picked up by the checker bot.

It's not only about the bot, but dependency when applied. If you did not warn clk maintainer that clock bindings should go via Rob's tree or should be provided as a tag, the patches here cannot be applied in this cycle.

It was not my (our) intention to send the clock patches via rob's tree. And since this is my first time trying to upstream wholescale changes to a device tree I honestly didn't expect this series to get accepted in this cycle anyway.

Quoted 4 lines

Best regards,
Krzysztof

#52 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

Quoted 43 lines · On 09/11/2021 14:20, [email protected] wrote:
Quoted 42 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 38 lines · On 09/11/2021 13:58, [email protected] wrote:
Quoted 27 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 24 lines · On 09/11/2021 13:16, [email protected] wrote:
Quoted 23 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 11 lines · On Mon, 08 Nov 2021 15:05:51 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

---
    .../bindings/spi/microchip,mpfs-spi.yaml      | 72 +++++++++++++++++++
    1 file changed, 72 insertions(+)

create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include "dt-bindings/clock/microchip,mpfs-clock.h"

Quoted 7 lines
Quoted 3 lines

Rob,
Should I drop the header from the example or is there a way for me specify the dependent patch to pass this check?

The error has to be fixed, although not necessarily by dropping the header, but by posting it. How this can pass on your system? There is no such file added in this patchset.

I linked the patch adding the clock as a dependency in the cover letter [1], which is why I was wondering if there was a better way to do so that would get picked up by the checker bot.

Quoted 8 lines
Quoted 4 lines

It's not only about the bot, but dependency when applied. If you did not warn clk maintainer that clock bindings should go via Rob's tree or should be provided as a tag, the patches here cannot be applied in this cycle.

It was not my (our) intention to send the clock patches via rob's tree. And since this is my first time trying to upstream wholescale changes to a device tree I honestly didn't expect this series to get accepted in this cycle anyway.

OK :)

Assuming your new bindings pass db_binding_check with DT_CHECKER_FLAGS=-m (on top of clock patch), I propose to keep the header here.

Another idea would be to submit without the header and use raw IDs (numbers) and convert it later. I prefer the first- base on clock patches.

Best regards,
Krzysztof

#53 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

In reply to [email protected]

Hi Conor,

Quoted 6 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the {q,}spi controller on the Microchip PolarFire SoC.

Thanks for your patch!

Quoted 17 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +14 -0
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/microchip,mpfs-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS {Q,}SPI Controller Device Tree Bindings
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+description: |
+  This {Q,}SPI controller is found on the Microchip PolarFire SoC.
+

Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml line 24

Quoted 10 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +10 -0
+allOf:
+  - $ref: "spi-controller.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-spi
+      - microsemi,ms-pf-mss-spi
+      - microchip,mpfs-qspi
+      - microsemi,ms-pf-mss-qspi

Same comment as before: what are the ms-pf-mss entries?

Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml line 60

Quoted 7 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +7 -0
+examples:
+  - |
+    #include "dt-bindings/clock/microchip,mpfs-clock.h"
+    #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h"
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;

Please drop these two...

Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml line 63

Quoted 3 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +3 -0
+      spi0: spi@20108000 {
+        compatible = "microchip,mpfs-spi";
+        reg = <0x0 0x20108000 0x0 0x1000>;

... and the zeros here.

Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml line 69

Quoted 6 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +6 -0
+        clocks = <&clkcfg CLK_SPI0>;
+        interrupt-parent = <&plic>;
+        interrupts = <PLIC_INT_SPI0>;
+        spi-max-frequency = <25000000>;
+        num-cs = <8>;
+        status = "disabled";

Please drop this.

Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml line 72

Quoted 3 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +3 -0
+      };
+    };
+...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#54 lore raw

Re: [PATCH 10/13] dt-bindings: spi: add bindings for microchip mpfs spi

Quoted 4 lines · Geert Uytterhoeven

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Conor,

Quoted 11 lines

Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml line 24

Quoted 8 lines
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file +8 -0
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-spi
+      - microsemi,ms-pf-mss-spi
+      - microchip,mpfs-qspi
+      - microsemi,ms-pf-mss-qspi

Same comment as before: what are the ms-pf-mss entries?

i think i will just drop the microsemi bindings, on reflection i dont think theyre relevant to this version of the device tree.

author maintainer
#55 lore raw

PATCH 11/13 dt-bindings: usb: add bindings for microchip mpfs musb

In reply to [email protected]

From: Conor Dooley <[email protected]>

Add device tree bindings for the usb controller on the Microchip PolarFire SoC.

---
 .../bindings/usb/microchip,mpfs-usb-host.yaml | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +70 -0
new file mode 100644
index 000000000000..b867f49e7d70
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/microchip,mpfs-usb-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS USB Controller Device Tree Bindings
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+description: |
+  This USB controller is found on the Microchip PolarFire SoC.
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-usb-host
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    minItems: 2
+    items:
+      - const: dma
+      - const: mc
+
+  clocks:
+    maxItems: 1
+
+  dr_mode:
+    enum:
+      - host
+      - otg
+      - peripheral
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - dr_mode
+
+additionalProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/clock/microchip,mpfs-clock.h"
+    #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h"
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      usb: usb@20201000 {
+        compatible = "microchip,mpfs-usb-host";
+        reg = <0x0 0x20201000 0x0 0x1000>;
+        clocks = <&clkcfg CLK_USB>;
+        interrupt-parent = <&plic>;
+        interrupts = <PLIC_INT_USB_DMA PLIC_INT_USB_MC>;
+        interrupt-names = "dma","mc";
+        dr_mode = "host";
+        status = "disabled";
+      };
+    };
+
+...
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#56 lore raw

Re: [PATCH 11/13] dt-bindings: usb: add bindings for microchip mpfs musb

Quoted 34 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the usb controller on the Microchip PolarFire SoC.

---
 .../bindings/usb/microchip,mpfs-usb-host.yaml | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +17 -0
new file mode 100644
index 000000000000..b867f49e7d70
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/microchip,mpfs-usb-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS USB Controller Device Tree Bindings
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+description: |
+  This USB controller is found on the Microchip PolarFire SoC.
+
+properties:
+  compatible:
+    enum:

Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml line 27

Quoted 10 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +10 -0
+      - microchip,mpfs-usb-host
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    minItems: 2

minItems should not be needed because you define all expected items below.

Quoted 29 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +29 -0
+    items:
+      - const: dma
+      - const: mc
+
+  clocks:
+    maxItems: 1
+
+  dr_mode:
+    enum:
+      - host
+      - otg
+      - peripheral
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - dr_mode
+
+additionalProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/clock/microchip,mpfs-clock.h"
+    #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h"
+    soc {
+      #address-cells = <2>;

Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml line 66

Quoted 10 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +10 -0
+      #size-cells = <2>;
+      usb: usb@20201000 {
+        compatible = "microchip,mpfs-usb-host";
+        reg = <0x0 0x20201000 0x0 0x1000>;
+        clocks = <&clkcfg CLK_USB>;
+        interrupt-parent = <&plic>;
+        interrupts = <PLIC_INT_USB_DMA PLIC_INT_USB_MC>;
+        interrupt-names = "dma","mc";
+        dr_mode = "host";
+        status = "disabled";

Skip disabled in example.

Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml line 70

Quoted 5 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +4 -0
+      };
+    };
+
+...

Best regards,
Krzysztof

maintainer
#57 lore raw

Re: [PATCH 11/13] dt-bindings: usb: add bindings for microchip mpfs musb

In reply to [email protected]
Quoted 11 lines · On Mon, 08 Nov 2021 15:05:52 +0000, [email protected] wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the usb controller on the Microchip PolarFire SoC.

---
 .../bindings/usb/microchip,mpfs-usb-host.yaml | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml: properties:interrupt-names: 'oneOf' conditional failed, one must be fixed:
[{'const': 'dma'}, {'const': 'mc'}] is too long
[{'const': 'dma'}, {'const': 'mc'}] is too short
False schema does not allow 2
1 was expected
hint: "minItems" is only needed if less than the "items" list length
from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml: ignoring, error in schema: properties: interrupt-names
warning: no schema found in file: ./Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory
19 | #include "dt-bindings/clock/microchip,mpfs-clock.h"

      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

compilation terminated.
make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1552389

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

#58 lore raw

Re: [PATCH 11/13] dt-bindings: usb: add bindings for microchip mpfs musb

In reply to [email protected]

Hi Conor,

Quoted 6 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the usb controller on the Microchip PolarFire SoC.

Thanks for your patch!

Quoted 11 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +8 -0
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/microchip,mpfs-usb-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS USB Controller Device Tree Bindings
+

Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml line 18

Quoted 10 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +10 -0
+maintainers:
+  - Conor Dooley <[email protected]>
+
+description: |
+  This USB controller is found on the Microchip PolarFire SoC.
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-usb-host

"microchip-mpfs-usb", given the dr_mode property below indicates this controller is not limited to host mode?

Quoted 29 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +29 -0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    minItems: 2
+    items:
+      - const: dma
+      - const: mc
+
+  clocks:
+    maxItems: 1
+
+  dr_mode:
+    enum:
+      - host
+      - otg
+      - peripheral
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - dr_mode

Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml line 57

Quoted 10 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +10 -0
+
+additionalProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/clock/microchip,mpfs-clock.h"
+    #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h"
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;

Please drop these two...

Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml line 60

Quoted 3 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +3 -0
+      usb: usb@20201000 {
+        compatible = "microchip,mpfs-usb-host";
+        reg = <0x0 0x20201000 0x0 0x1000>;

... and the zeros here.

Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml line 63

Quoted 3 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +3 -0
+        clocks = <&clkcfg CLK_USB>;
+        interrupt-parent = <&plic>;
+        interrupts = <PLIC_INT_USB_DMA PLIC_INT_USB_MC>;

Please group by angular brackets:

<PLIC_INT_USB_DMA>, <PLIC_INT_USB_MC>.

Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml line 66

Quoted 3 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +3 -0
+        interrupt-names = "dma","mc";
+        dr_mode = "host";
+        status = "disabled";

Please drop this.

Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml line 70

Quoted 4 lines
Documentation/devicetree/bindings/usb/microchip,mpfs-usb-host.yaml new file +4 -0
+      };
+    };
+
+...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author maintainer
#59 lore raw

PATCH 12/13 riscv: icicle-kit: update microchip icicle kit device tree

In reply to [email protected]

From: Conor Dooley <[email protected]>

Update the device tree for the icicle kit by splitting it into a third part, which contains peripherals in the fpga fabric, add new peripherals (spi, qspi, gpio, rtc, pcie, system services, i2c), update parts of the memory map which have been changed.

---
 .../dts/microchip/microchip-mpfs-fabric.dtsi  |  21 ++
 .../microchip/microchip-mpfs-icicle-kit.dts   | 159 +++++++--
 .../boot/dts/microchip/microchip-mpfs.dtsi    | 333 ++++++++++++++----
 3 files changed, 428 insertions(+), 85 deletions(-)
 create mode 100644 arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +21 -0
new file mode 100644
index 000000000000..8fa3356494f1
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
+
+/ {
+	fpgadma: fpgadma@60020000 {
+		compatible = "microchip,mpfs-fpga-dma-uio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x0 0x60020000 0x0 0x1000>;
+		interrupt-parent = <&plic>;
+		interrupts = <PLIC_INT_FABRIC_F2H_2>;
+		status = "okay";
+	};
+
+	fpgalsram: fpga_lsram@61000000 {
+		compatible = "generic-uio";
+		reg = <0x0 0x61000000 0x0 0x0001000
+			0x14 0x00000000 0x0 0x00010000>;
+		status = "okay";
+	};
+};
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +137 -22

214 lines not on this page · raw message

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +270 -63

495 lines not on this page · raw message

Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#60 lore raw

Re: [PATCH 12/13] riscv: icicle-kit: update microchip icicle kit device tree

Quoted 6 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Update the device tree for the icicle kit by splitting it into a third part, which contains peripherals in the fpga fabric, add new peripherals (spi, qspi, gpio, rtc, pcie, system services, i2c), update parts of the memory map which have been changed.

This should be multiple commits because you mix up refactoring (split) and adding new features. The patch is really, really difficult to review. I gave up in the middle.

Quoted 20 lines
---
 .../dts/microchip/microchip-mpfs-fabric.dtsi  |  21 ++
 .../microchip/microchip-mpfs-icicle-kit.dts   | 159 +++++++--
 .../boot/dts/microchip/microchip-mpfs.dtsi    | 333 ++++++++++++++----
 3 files changed, 428 insertions(+), 85 deletions(-)
 create mode 100644 arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +5 -0
new file mode 100644
index 000000000000..8fa3356494f1
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
+
+/ {
+	fpgadma: fpgadma@60020000 {

arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi line 15

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +10 -0
+		compatible = "microchip,mpfs-fpga-dma-uio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x0 0x60020000 0x0 0x1000>;
+		interrupt-parent = <&plic>;
+		interrupts = <PLIC_INT_FABRIC_F2H_2>;
+		status = "okay";
+	};
+
+	fpgalsram: fpga_lsram@61000000 {

Node names go with hyphen, but actually you should not need it, because the name should be generic, e.g. "uio".

However there is no such compatible and checkpatch should complain about it.

Quoted 20 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +6 -0
+		compatible = "generic-uio";
+		reg = <0x0 0x61000000 0x0 0x0001000
+			0x14 0x00000000 0x0 0x00010000>;
+		status = "okay";
+	};
+};
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +1 -1
index fc1e5869df1b..4212129fcdf1 100644
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
-/* Copyright (c) 2020 Microchip Technology Inc */
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
 
 /dts-v1/;
 
@@ -13,72 +13,187 @@ / {
 	compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";
 

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 19

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +4 -5
 	aliases {
-		ethernet0 = &emac1;
-		serial0 = &serial0;
-		serial1 = &serial1;
-		serial2 = &serial2;
-		serial3 = &serial3;> +		mmuart0 = &mmuart0;
+		mmuart1 = &mmuart1;
+		mmuart2 = &mmuart2;
+		mmuart3 = &mmuart3;
+		mmuart4 = &mmuart4;

Why? Commit msg does not explain it.

Quoted 60 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +19 -9
 	};
 
 	chosen {
-		stdout-path = "serial0:115200n8";
+		stdout-path = "mmuart1:115200n8";
 	};
 
 	cpus {
 		timebase-frequency = <RTCCLK_FREQ>;
 	};
 
-	memory@80000000 {
+	ddrc_cache_lo: memory@80000000 {
 		device_type = "memory";
-		reg = <0x0 0x80000000 0x0 0x40000000>;
-		clocks = <&clkcfg 26>;
+		reg = <0x0 0x80000000 0x0 0x2e000000>;
+		clocks = <&clkcfg CLK_DDRC>;
+		status = "okay";
+	};
+
+	ddrc_cache_hi: memory@1000000000 {
+		device_type = "memory";
+		reg = <0x10 0x0 0x0 0x40000000>;
+		clocks = <&clkcfg CLK_DDRC>;
+		status = "okay";
 	};
 };
 
-&serial0 {
+&mmuart1 {
 	status = "okay";
 };
 
-&serial1 {
+&mmuart2 {
 	status = "okay";
 };
 
-&serial2 {
+&mmuart3 {
 	status = "okay";
 };
 
-&serial3 {
+&mmuart4 {
 	status = "okay";
 };
 
 &mmc {
 	status = "okay";
-
 	bus-width = <4>;
 	disable-wp;
 	cap-sd-highspeed;
+	cap-mmc-highspeed;
 	card-detect-delay = <200>;
+	mmc-ddr-1_8v;
+	mmc-hs200-1_8v;
 	sd-uhs-sdr12;

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 80

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +4 -1
 	sd-uhs-sdr25;
 	sd-uhs-sdr50;
 	sd-uhs-sdr104;
 };
 
-&emac0 {
+&spi0 {
+	status = "okay";
+	spidev@0 {
+		compatible = "spidev";

1. There is no such compatible,
2. You should have big fat warning when booting, so such DT cannot be accepted.

Quoted 11 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +11 -0
+		reg = <0>; /* CS 0 */
+		spi-max-frequency = <10000000>;
+		status = "okay";
+	};
+};
+
+&spi1 {
+	status = "okay";
+};
+
+&qspi {

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 101

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +10 -0
+	status = "okay";
+};
+
+&i2c0 {
+	status = "okay";
+};
+
+&i2c1 {
+	status = "okay";
+	pac193x: pac193x@10 {

Generic node name. Looks like compatible is not documented, so first bindings.

Quoted 29 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +29 -0
+		compatible = "microchip,pac1934";
+		reg = <0x10>;
+		samp-rate = <64>;
+		status = "okay";
+		ch0: channel0 {
+			uohms-shunt-res = <10000>;
+			rail-name = "VDDREG";
+			channel_enabled;
+		};
+		ch1: channel1 {
+			uohms-shunt-res = <10000>;
+			rail-name = "VDDA25";
+			channel_enabled;
+		};
+		ch2: channel2 {
+			uohms-shunt-res = <10000>;
+			rail-name = "VDD25";
+			channel_enabled;
+		};
+		ch3: channel3 {
+			uohms-shunt-res = <10000>;
+			rail-name = "VDDA_REG";
+			channel_enabled;
+		};
+	};
+};
+
+&mac0 {
+	status = "okay";

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 135

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +1 -5
 	phy-mode = "sgmii";
 	phy-handle = <&phy0>;
-	phy0: ethernet-phy@8 {
-		reg = <8>;
-		ti,fifo-depth = <0x01>;
-	};
 };
 
-&emac1 {
+&mac1 {

I gave up here, it's not easy to find what is effect of refactoring, what is a new node.

Best regards,
Krzysztof

author maintainer
#61 lore raw

Re: [PATCH 12/13] riscv: icicle-kit: update microchip icicle kit device tree

Quoted 39 lines · Krzysztof Kozlowski

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 6 lines · On 08/11/2021 16:05, [email protected] wrote:

From: Conor Dooley <[email protected]>

Update the device tree for the icicle kit by splitting it into a third part, which contains peripherals in the fpga fabric, add new peripherals (spi, qspi, gpio, rtc, pcie, system services, i2c), update parts of the memory map which have been changed.

This should be multiple commits because you mix up refactoring (split) and adding new features. The patch is really, really difficult to review. I gave up in the middle.

Quoted 25 lines
---
  .../dts/microchip/microchip-mpfs-fabric.dtsi  |  21 ++
  .../microchip/microchip-mpfs-icicle-kit.dts   | 159 +++++++--
  .../boot/dts/microchip/microchip-mpfs.dtsi    | 333 ++++++++++++++----
  3 files changed, 428 insertions(+), 85 deletions(-)

create mode 100644 arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi

arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +10 -0
new file mode 100644
index 000000000000..8fa3356494f1
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
+
+/ {
+     fpgadma: fpgadma@60020000 {
+             compatible = "microchip,mpfs-fpga-dma-uio";
+             #address-cells = <1>;
+             #size-cells = <0>;
+             reg = <0x0 0x60020000 0x0 0x1000>;
+             interrupt-parent = <&plic>;
Quoted 8 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi line 15

Quoted 5 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +5 -0
+             interrupts = <PLIC_INT_FABRIC_F2H_2>;
+             status = "okay";
+     };
+
+     fpgalsram: fpga_lsram@61000000 {

Node names go with hyphen, but actually you should not need it, because the name should be generic, e.g. "uio".

sure, will change it to uio.

Quoted 2 lines

However there is no such compatible and checkpatch should complain about it.

yeah, this and the pac1934 i didnt send bindings for - erroneously thought i might not have to do so. ill get a binding sorted out for both of these.

Quoted 21 lines
Quoted 21 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +6 -0
+             compatible = "generic-uio";
+             reg = <0x0 0x61000000 0x0 0x0001000
+                     0x14 0x00000000 0x0 0x00010000>;
+             status = "okay";
+     };
+};
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +1 -1
index fc1e5869df1b..4212129fcdf1 100644
@@ -1,5 +1,5 @@
  // SPDX-License-Identifier: (GPL-2.0 OR MIT)
-/* Copyright (c) 2020 Microchip Technology Inc */
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
 
  /dts-v1/;
 
@@ -13,72 +13,187 @@ / {
       compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";
 
Quoted 12 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 20

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +4 -5
       aliases {
-             ethernet0 = &emac1;
-             serial0 = &serial0;
-             serial1 = &serial1;
-             serial2 = &serial2;
-             serial3 = &serial3;> +          mmuart0 = &mmuart0;
+             mmuart1 = &mmuart1;
+             mmuart2 = &mmuart2;
+             mmuart3 = &mmuart3;
+             mmuart4 = &mmuart4;

Why? Commit msg does not explain it.

changed to match all of our documentation

Quoted 66 lines
Quoted 66 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +19 -9
       };
 
       chosen {
-             stdout-path = "serial0:115200n8";
+             stdout-path = "mmuart1:115200n8";
       };
 
       cpus {
               timebase-frequency = <RTCCLK_FREQ>;
       };
 
-     memory@80000000 {
+     ddrc_cache_lo: memory@80000000 {
               device_type = "memory";
-             reg = <0x0 0x80000000 0x0 0x40000000>;
-             clocks = <&clkcfg 26>;
+             reg = <0x0 0x80000000 0x0 0x2e000000>;
+             clocks = <&clkcfg CLK_DDRC>;
+             status = "okay";
+     };
+
+     ddrc_cache_hi: memory@1000000000 {
+             device_type = "memory";
+             reg = <0x10 0x0 0x0 0x40000000>;
+             clocks = <&clkcfg CLK_DDRC>;
+             status = "okay";
       };
  };
 
-&serial0 {
+&mmuart1 {
       status = "okay";
  };
 
-&serial1 {
+&mmuart2 {
       status = "okay";
  };
 
-&serial2 {
+&mmuart3 {
       status = "okay";
  };
 
-&serial3 {
+&mmuart4 {
       status = "okay";
  };
 
  &mmc {
       status = "okay";
-
       bus-width = <4>;
       disable-wp;
       cap-sd-highspeed;
+     cap-mmc-highspeed;
       card-detect-delay = <200>;
+     mmc-ddr-1_8v;
+     mmc-hs200-1_8v;
       sd-uhs-sdr12;
       sd-uhs-sdr25;
       sd-uhs-sdr50;
       sd-uhs-sdr104;
  };
 
Quoted 9 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 80

Quoted 5 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +4 -1
-&emac0 {
+&spi0 {
+     status = "okay";
+     spidev@0 {
+             compatible = "spidev";

1. There is no such compatible,
2. You should have big fat warning when booting, so such DT cannot be accepted.

this one was an oversight from me, that compatible has never been "spidev" on its own in our internal stuff and i mustve accidentally dropped a vendor string while making these patches.

Quoted 17 lines
Quoted 17 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +16 -0
+             reg = <0>; /* CS 0 */
+             spi-max-frequency = <10000000>;
+             status = "okay";
+     };
+};
+
+&spi1 {
+     status = "okay";
+};
+
+&qspi {
+     status = "okay";
+};
+
+&i2c0 {
+     status = "okay";
Quoted 9 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 101

Quoted 5 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +5 -0
+};
+
+&i2c1 {
+     status = "okay";
+     pac193x: pac193x@10 {

Generic node name. Looks like compatible is not documented, so first bindings.

as above

Quoted 38 lines
Quoted 38 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +29 -4
+             compatible = "microchip,pac1934";
+             reg = <0x10>;
+             samp-rate = <64>;
+             status = "okay";
+             ch0: channel0 {
+                     uohms-shunt-res = <10000>;
+                     rail-name = "VDDREG";
+                     channel_enabled;
+             };
+             ch1: channel1 {
+                     uohms-shunt-res = <10000>;
+                     rail-name = "VDDA25";
+                     channel_enabled;
+             };
+             ch2: channel2 {
+                     uohms-shunt-res = <10000>;
+                     rail-name = "VDD25";
+                     channel_enabled;
+             };
+             ch3: channel3 {
+                     uohms-shunt-res = <10000>;
+                     rail-name = "VDDA_REG";
+                     channel_enabled;
+             };
+     };
+};
+
+&mac0 {
+     status = "okay";
       phy-mode = "sgmii";
       phy-handle = <&phy0>;
-     phy0: ethernet-phy@8 {
-             reg = <8>;
-             ti,fifo-depth = <0x01>;
-     };
  };
 
Quoted 5 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 135

Quoted 2 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +1 -1
-&emac1 {
+&mac1 {

I gave up here, it's not easy to find what is effect of refactoring, what is a new node.

yeah, ill split it into several patches - probably one for the splitting, one for the new defines, one for the changes to existing nodes and one for node additions.

Quoted 4 lines

Best regards,
Krzysztof

#62 lore raw

Re: [PATCH 12/13] riscv: icicle-kit: update microchip icicle kit device tree

In reply to [email protected]

Hi Conor,

Quoted 8 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Update the device tree for the icicle kit by splitting it into a third part, which contains peripherals in the fpga fabric, add new peripherals (spi, qspi, gpio, rtc, pcie, system services, i2c), update parts of the memory map which have been changed.

Thanks for your patch!

Please split it into logical separated parts.

Quoted 11 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +8 -0
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
+
+/ {
+       fpgadma: fpgadma@60020000 {
+               compatible = "microchip,mpfs-fpga-dma-uio";
+               #address-cells = <1>;
+               #size-cells = <0>;

arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi line 18

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +10 -0
+               reg = <0x0 0x60020000 0x0 0x1000>;
+               interrupt-parent = <&plic>;
+               interrupts = <PLIC_INT_FABRIC_F2H_2>;
+               status = "okay";
+       };
+
+       fpgalsram: fpga_lsram@61000000 {
+               compatible = "generic-uio";
+               reg = <0x0 0x61000000 0x0 0x0001000
+                       0x14 0x00000000 0x0 0x00010000>;

Please group by angular brackets, to increase readability, and support automatic validation:

<0x0 0x61000000 0x0 0x0001000>, <0x14 0x00000000 0x0 0x00010000>

arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi line 20

Quoted 2 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +2 -0
+               status = "okay";
+       };

Do we really want UIO nodes in upstream DT?

Quoted 16 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +1 -0
+};
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +1 -1
index fc1e5869df1b..4212129fcdf1 100644
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
-/* Copyright (c) 2020 Microchip Technology Inc */
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
 
 /dts-v1/;
 
@@ -13,72 +13,187 @@ / {
        compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";
 
        aliases {

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 20

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +5 -5
-               ethernet0 = &emac1;
-               serial0 = &serial0;
-               serial1 = &serial1;
-               serial2 = &serial2;
-               serial3 = &serial3;
+               mmuart0 = &mmuart0;
+               mmuart1 = &mmuart1;
+               mmuart2 = &mmuart2;
+               mmuart3 = &mmuart3;
+               mmuart4 = &mmuart4;

Why? SerialN is the standard alias name.

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 80

Quoted 8 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +4 -1
        };
 
 
-&emac0 {
+&spi0 {
+       status = "okay";
+       spidev@0 {
+               compatible = "spidev";

Please don't use "spidev", but a proper compatible value describing what is really connected. If you want to use it with spidev (which is software policy, not hardware description), add that compatible value to drivers/spi/spidev.c:spidev_dt_ids[], or use driver_override in sysfs at runtime.

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 88

Quoted 8 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +8 -0
+               reg = <0>; /* CS 0 */
+               spi-max-frequency = <10000000>;
+               status = "okay";
+       };
+};
+
+&spi1 {
+       status = "okay";

No slave devices specified?

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 101

Quoted 13 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +13 -0
+};
+
+&qspi {
+       status = "okay";
+};
+
+&i2c0 {
+       status = "okay";
+};
+
+&i2c1 {
+       status = "okay";
+       pac193x: pac193x@10 {

adc@, I guess?

Quoted 50 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +49 -0
+               compatible = "microchip,pac1934";
+               reg = <0x10>;
+               samp-rate = <64>;
+               status = "okay";
+               ch0: channel0 {
+                       uohms-shunt-res = <10000>;
+                       rail-name = "VDDREG";
+                       channel_enabled;
+               };
+               ch1: channel1 {
+                       uohms-shunt-res = <10000>;
+                       rail-name = "VDDA25";
+                       channel_enabled;
+               };
+               ch2: channel2 {
+                       uohms-shunt-res = <10000>;
+                       rail-name = "VDD25";
+                       channel_enabled;
+               };
+               ch3: channel3 {
+                       uohms-shunt-res = <10000>;
+                       rail-name = "VDDA_REG";
+                       channel_enabled;
+               };
+       };
+};
 
+&gpio2 {
+       interrupts = <PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 181

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +10 -0
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT>;

Why override interrupts in the board .dts file?
Doesn't this belong in the SoC .dtsi file?
Please group using angular brackets.

Quoted 9 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +1 -1
@@ -145,49 +149,55 @@ cpu4_intc: interrupt-controller {
        soc {
                #address-cells = <2>;
                #size-cells = <2>;
-               compatible = "simple-bus";
+               compatible = "microchip,mpfs-soc", "simple-bus";

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi cpu4_intc: interrupt-controller { · line 161

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -1
                ranges;
 
-               cache-controller@2010000 {
+               cctrllr: cache-controller@2010000 {
                        compatible = "sifive,fu540-c000-ccache", "cache";
+                       reg = <0x0 0x2010000 0x0 0x1000>;
+                       interrupt-parent = <&plic>;
+                       interrupts = <PLIC_INT_L2_METADATA_CORR
+                               PLIC_INT_L2_METADAT_UNCORR
+                               PLIC_INT_L2_DATA_CORR>;

Please group using angular brackets.

Quoted 15 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +1 -5
                        cache-block-size = <64>;
                        cache-level = <2>;
                        cache-sets = <1024>;
                        cache-size = <2097152>;
                        cache-unified;
-                       interrupt-parent = <&plic>;
-                       interrupts = <1 2 3>;
-                       reg = <0x0 0x2010000 0x0 0x1000>;
                };
 
-               clint@2000000 {
+               clint: clint@2000000 {
                        compatible = "sifive,fu540-c000-clint", "sifive,clint0";
                        reg = <0x0 0x2000000 0x0 0xC000>;
-                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi cpu4_intc: interrupt-controller { · line 177

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -4
-                                               &cpu1_intc 3 &cpu1_intc 7
-                                               &cpu2_intc 3 &cpu2_intc 7
-                                               &cpu3_intc 3 &cpu3_intc 7
-                                               &cpu4_intc 3 &cpu4_intc 7>;
+                       interrupts-extended =
+                                       <&cpu0_intc HART_INT_M_SOFT &cpu0_intc HART_INT_M_TIMER
+                                        &cpu1_intc HART_INT_M_SOFT &cpu1_intc HART_INT_M_TIMER
+                                        &cpu2_intc HART_INT_M_SOFT &cpu2_intc HART_INT_M_TIMER
+                                        &cpu3_intc HART_INT_M_SOFT &cpu3_intc HART_INT_M_TIMER
+                                        &cpu4_intc HART_INT_M_SOFT &cpu4_intc HART_INT_M_TIMER>;

Please group using angular brackets.

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi cpu4_intc: interrupt-controller { · line 181

Quoted 6 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +1 -2
                };
 
                plic: interrupt-controller@c000000 {
-                       #interrupt-cells = <1>;
-                       compatible = "sifive,fu540-c000-plic", "sifive,plic-1.0.0";
+                       compatible = "sifive,plic-1.0.0";

Why drop the first one again?

Quoted 17 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -6
                        reg = <0x0 0xc000000 0x0 0x4000000>;
+                       #interrupt-cells = <1>;
                        riscv,ndev = <186>;
                        interrupt-controller;
-                       interrupts-extended = <&cpu0_intc 11
-                                       &cpu1_intc 11 &cpu1_intc 9
-                                       &cpu2_intc 11 &cpu2_intc 9
-                                       &cpu3_intc 11 &cpu3_intc 9
-                                       &cpu4_intc 11 &cpu4_intc 9>;
+                       interrupts-extended = <&cpu0_intc HART_INT_M_EXT
+                                       &cpu1_intc HART_INT_M_EXT &cpu1_intc HART_INT_S_EXT
+                                       &cpu2_intc HART_INT_M_EXT &cpu2_intc HART_INT_S_EXT
+                                       &cpu3_intc HART_INT_M_EXT &cpu3_intc HART_INT_S_EXT
+                                       &cpu4_intc HART_INT_M_EXT &cpu4_intc HART_INT_S_EXT>;
                };
 
-               dma@3000000 {

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi cpu4_intc: interrupt-controller { · line 200

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -2
-                       compatible = "sifive,fu540-c000-pdma";
+               pdma: pdma@3000000 {
+                       compatible = "microchip,mpfs-pdma-uio";
                        reg = <0x0 0x3000000 0x0 0x8000>;
                        interrupt-parent = <&plic>;
-                       interrupts = <23 24 25 26 27 28 29 30>;
+                       interrupts = <PLIC_INT_DMA_CH0_DONE PLIC_INT_DMA_CH0_ERR
+                               PLIC_INT_DMA_CH1_DONE PLIC_INT_DMA_CH1_ERR
+                               PLIC_INT_DMA_CH2_DONE PLIC_INT_DMA_CH2_ERR
+                               PLIC_INT_DMA_CH3_DONE PLIC_INT_DMA_CH3_ERR>;

Please group using angular brackets.

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 221

Quoted 12 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +1 -1
                        #dma-cells = <1>;
                };
 
@@ -205,7 +215,7 @@ clkcfg: clkcfg@20002000 {
                        clocks = <&refclk>;
                        #clock-cells = <1>;
                        clock-output-names = "cpu", "axi", "ahb", "envm",       /* 0-3   */
-                                "mac0", "mac1", "mmc", "timer",                /* 4-7   */
+                               "mac0", "mac1", "mmc", "timer",                 /* 4-7   */
                                "mmuart0", "mmuart1", "mmuart2", "mmuart3",     /* 8-11  */
                                "mmuart4", "spi0", "spi1", "i2c0",              /* 12-15 */
                                "i2c1", "can0", "can1", "usb",                  /* 16-19 */

No need for clock-output-names at all.

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -2
-               emac1: ethernet@20112000 {
+               mac0: ethernet@20110000 {
                        compatible = "cdns,macb";
-                       reg = <0x0 0x20112000 0x0 0x2000>;
+                       #address-cells = <1>;
+                       #size-cells = <0>;
+                       reg = <0x0 0x20110000 0x0 0x2000>;
+                       clocks = <&clkcfg CLK_MAC0>, <&clkcfg CLK_AHB>;
+                       clock-names = "pclk", "hclk";

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 373

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -3
                        interrupt-parent = <&plic>;
-                       interrupts = <70 71 72 73>;
-                       local-mac-address = [00 00 00 00 00 00];
-                       clocks = <&clkcfg 5>, <&clkcfg 2>;
+                       interrupts = <PLIC_INT_MAC0_INT
+                               PLIC_INT_MAC0_QUEUE1
+                               PLIC_INT_MAC0_QUEUE2
+                               PLIC_INT_MAC0_QUEUE3
+                               PLIC_INT_MAC0_EMAC
+                               PLIC_INT_MAC0_MMSL>;

Please group using angular brackets.

Quoted 1 line

+ mac-address = [56 34 12 00 FC 01];

Please drop this.

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 434

Quoted 12 lines
                        status = "disabled";
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +8 -0
+               };
 
+               rtc: rtc@20124000 {
+                       compatible = "microchip,mpfs-rtc";
                        #address-cells = <1>;
                        #size-cells = <0>;
+                       reg = <0x0 0x20124000 0x0 0x1000>;
+                       clocks = <&clkcfg CLK_RTC>;
+                       clock-names = "rtc";
+                       interrupt-parent = <&plic>;
+                       interrupts = <PLIC_INT_RTC_WAKEUP PLIC_INT_RTC_MATCH>;

Please group using angular brackets.

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 444

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +8 -0
+                       status = "disabled";
                };
 
+               usb: usb@20201000 {
+                       compatible = "microchip,mpfs-usb-host";
+                       reg = <0x0 0x20201000 0x0 0x1000>;
+                       reg-names = "mc","control";
+                       clocks = <&clkcfg CLK_USB>;
+                       interrupt-parent = <&plic>;
+                       interrupts = <PLIC_INT_USB_DMA PLIC_INT_USB_MC>;

Please group using angular brackets.

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 449

Quoted 5 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +5 -0
+                       interrupt-names = "dma","mc";
+                       dr_mode = "host";
+                       status = "disabled";
+               };
+

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#63 lore raw

Re: [PATCH 12/13] riscv: icicle-kit: update microchip icicle kit device tree

Quoted 12 lines · Geert Uytterhoeven

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Conor,

Quoted 7 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Update the device tree for the icicle kit by splitting it into a third part, which contains peripherals in the fpga fabric, add new peripherals (spi, qspi, gpio, rtc, pcie, system services, i2c), update parts of the memory map which have been changed.

Quoted 5 lines
Quoted 1 line

Thanks for your patch!

Please split it into logical separated parts.

yeah, ive split it into several patches - one for the splitting into 3, one for the new defines, one for the changes to existing nodes and one for node additions.

Quoted 26 lines
Quoted 12 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +8 -0
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
+
+/ {
+       fpgadma: fpgadma@60020000 {
+               compatible = "microchip,mpfs-fpga-dma-uio";
+               #address-cells = <1>;
+               #size-cells = <0>;

arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi line 18

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +10 -0
+               reg = <0x0 0x60020000 0x0 0x1000>;
+               interrupt-parent = <&plic>;
+               interrupts = <PLIC_INT_FABRIC_F2H_2>;
+               status = "okay";
+       };
+
+       fpgalsram: fpga_lsram@61000000 {
+               compatible = "generic-uio";
+               reg = <0x0 0x61000000 0x0 0x0001000
+                       0x14 0x00000000 0x0 0x00010000>;

Please group by angular brackets, to increase readability, and support automatic validation:

Quoted 6 lines

<0x0 0x61000000 0x0 0x0001000>, <0x14 0x00000000 0x0 0x00010000>

arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi line 20

Quoted 2 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +2 -0
+               status = "okay";
+       };

Do we really want UIO nodes in upstream DT?

As I said in the replies to another patch this is my first time doing any upstreaming of a device tree, i didnt realise that this would be a problem.

Quoted 16 lines
Quoted 16 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi new file +1 -0
+};
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +1 -1
index fc1e5869df1b..4212129fcdf1 100644
@@ -1,5 +1,5 @@
  // SPDX-License-Identifier: (GPL-2.0 OR MIT)
-/* Copyright (c) 2020 Microchip Technology Inc */
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
 
  /dts-v1/;
 
@@ -13,72 +13,187 @@ / {
         compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";
 
Quoted 13 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 20

Quoted 11 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +5 -5
         aliases {
-               ethernet0 = &emac1;
-               serial0 = &serial0;
-               serial1 = &serial1;
-               serial2 = &serial2;
-               serial3 = &serial3;
+               mmuart0 = &mmuart0;
+               mmuart1 = &mmuart1;
+               mmuart2 = &mmuart2;
+               mmuart3 = &mmuart3;
+               mmuart4 = &mmuart4;

Why? SerialN is the standard alias name.

we changed the label to mmuart to match the microchip documentation. would it make more sense to call mmuart but alias it to serial? ie serial0 = &mmuart0;

Quoted 10 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 80

Quoted 9 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +4 -1
         };
 
 
-&emac0 {
+&spi0 {
+       status = "okay";
+       spidev@0 {
+               compatible = "spidev";
Quoted 5 lines

Please don't use "spidev", but a proper compatible value describing what is really connected. If you want to use it with spidev (which is software policy, not hardware description), add that compatible value to drivers/spi/spidev.c:spidev_dt_ids[], or use driver_override in sysfs at runtime.

as i said to Krzysztof - this one was an oversight from me, that compatible has never been "spidev" on its own in our internal tree and i mustve accidentally omitted the vendor string while making these patches.

Either way, after talking some more we decided that this entry is not appropriate anyway and i will drop it.

Quoted 11 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 88

Quoted 9 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +8 -0
+               reg = <0>; /* CS 0 */
+               spi-max-frequency = <10000000>;
+               status = "okay";
+       };
+};
+
+&spi1 {
+       status = "okay";

No slave devices specified?

no, but its exposed

Quoted 16 lines
Quoted 4 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +3 -0
+};
+
+&qspi {

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 101

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +10 -0
+       status = "okay";
+};
+
+&i2c0 {
+       status = "okay";
+};
+
+&i2c1 {
+       status = "okay";
+       pac193x: pac193x@10 {

adc@, I guess?

sure

Quoted 62 lines
Quoted 51 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +49 -0
+               compatible = "microchip,pac1934";
+               reg = <0x10>;
+               samp-rate = <64>;
+               status = "okay";
+               ch0: channel0 {
+                       uohms-shunt-res = <10000>;
+                       rail-name = "VDDREG";
+                       channel_enabled;
+               };
+               ch1: channel1 {
+                       uohms-shunt-res = <10000>;
+                       rail-name = "VDDA25";
+                       channel_enabled;
+               };
+               ch2: channel2 {
+                       uohms-shunt-res = <10000>;
+                       rail-name = "VDD25";
+                       channel_enabled;
+               };
+               ch3: channel3 {
+                       uohms-shunt-res = <10000>;
+                       rail-name = "VDDA_REG";
+                       channel_enabled;
+               };
+       };
+};
 
+&gpio2 {
+       interrupts = <PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 181

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +10 -0
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT>;
Quoted 2 lines

Why override interrupts in the board .dts file?
Doesn't this belong in the SoC .dtsi file?

The interrupt setup for the gpio isnt fixed, there is an option to either connect the individual gpio interrupts to the plic *or* they can be connected to a per gpio controller common interrupt, and it is up to the driver to read a register to determine which interrupt triggered the common/NON_DIRECT interrupt. This decision is made by a write to a system register in application code, which to us didn't seem like it belonged in the soc .dtsi.

Using the common interrupt for GPIO2 is the default on the polarfire-soc, there are only 38 per gpio line interrupts available of which 14 are connected to gpio0 and 24 to gpio1.

Quoted 52 lines

Please group using angular brackets.

Quoted 9 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +1 -1
@@ -145,49 +149,55 @@ cpu4_intc: interrupt-controller {
         soc {
                 #address-cells = <2>;
                 #size-cells = <2>;
-               compatible = "simple-bus";
+               compatible = "microchip,mpfs-soc", "simple-bus";

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi cpu4_intc: interrupt-controller { · line 161

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -1
                 ranges;
 
-               cache-controller@2010000 {
+               cctrllr: cache-controller@2010000 {
                         compatible = "sifive,fu540-c000-ccache", "cache";
+                       reg = <0x0 0x2010000 0x0 0x1000>;
+                       interrupt-parent = <&plic>;
+                       interrupts = <PLIC_INT_L2_METADATA_CORR
+                               PLIC_INT_L2_METADAT_UNCORR
+                               PLIC_INT_L2_DATA_CORR>;

Please group using angular brackets.

Quoted 15 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +1 -5
                         cache-block-size = <64>;
                         cache-level = <2>;
                         cache-sets = <1024>;
                         cache-size = <2097152>;
                         cache-unified;
-                       interrupt-parent = <&plic>;
-                       interrupts = <1 2 3>;
-                       reg = <0x0 0x2010000 0x0 0x1000>;
                 };
 
-               clint@2000000 {
+               clint: clint@2000000 {
                         compatible = "sifive,fu540-c000-clint", "sifive,clint0";
                         reg = <0x0 0x2000000 0x0 0xC000>;
-                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi cpu4_intc: interrupt-controller { · line 177

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -4
-                                               &cpu1_intc 3 &cpu1_intc 7
-                                               &cpu2_intc 3 &cpu2_intc 7
-                                               &cpu3_intc 3 &cpu3_intc 7
-                                               &cpu4_intc 3 &cpu4_intc 7>;
+                       interrupts-extended =
+                                       <&cpu0_intc HART_INT_M_SOFT &cpu0_intc HART_INT_M_TIMER
+                                        &cpu1_intc HART_INT_M_SOFT &cpu1_intc HART_INT_M_TIMER
+                                        &cpu2_intc HART_INT_M_SOFT &cpu2_intc HART_INT_M_TIMER
+                                        &cpu3_intc HART_INT_M_SOFT &cpu3_intc HART_INT_M_TIMER
+                                        &cpu4_intc HART_INT_M_SOFT &cpu4_intc HART_INT_M_TIMER>;

Please group using angular brackets.

Quoted 8 lines

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi cpu4_intc: interrupt-controller { · line 181

Quoted 6 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +1 -2
                 };
 
                 plic: interrupt-controller@c000000 {
-                       #interrupt-cells = <1>;
-                       compatible = "sifive,fu540-c000-plic", "sifive,plic-1.0.0";
+                       compatible = "sifive,plic-1.0.0";

Why drop the first one again?

we felt it didnt make sense to have something that specifically references the fu540 in the device tree for this board.

Quoted 67 lines
Quoted 18 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -6
                         reg = <0x0 0xc000000 0x0 0x4000000>;
+                       #interrupt-cells = <1>;
                         riscv,ndev = <186>;
                         interrupt-controller;
-                       interrupts-extended = <&cpu0_intc 11
-                                       &cpu1_intc 11 &cpu1_intc 9
-                                       &cpu2_intc 11 &cpu2_intc 9
-                                       &cpu3_intc 11 &cpu3_intc 9
-                                       &cpu4_intc 11 &cpu4_intc 9>;
+                       interrupts-extended = <&cpu0_intc HART_INT_M_EXT
+                                       &cpu1_intc HART_INT_M_EXT &cpu1_intc HART_INT_S_EXT
+                                       &cpu2_intc HART_INT_M_EXT &cpu2_intc HART_INT_S_EXT
+                                       &cpu3_intc HART_INT_M_EXT &cpu3_intc HART_INT_S_EXT
+                                       &cpu4_intc HART_INT_M_EXT &cpu4_intc HART_INT_S_EXT>;
                 };
 
-               dma@3000000 {

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi cpu4_intc: interrupt-controller { · line 200

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -2
-                       compatible = "sifive,fu540-c000-pdma";
+               pdma: pdma@3000000 {
+                       compatible = "microchip,mpfs-pdma-uio";
                         reg = <0x0 0x3000000 0x0 0x8000>;
                         interrupt-parent = <&plic>;
-                       interrupts = <23 24 25 26 27 28 29 30>;
+                       interrupts = <PLIC_INT_DMA_CH0_DONE PLIC_INT_DMA_CH0_ERR
+                               PLIC_INT_DMA_CH1_DONE PLIC_INT_DMA_CH1_ERR
+                               PLIC_INT_DMA_CH2_DONE PLIC_INT_DMA_CH2_ERR
+                               PLIC_INT_DMA_CH3_DONE PLIC_INT_DMA_CH3_ERR>;

Please group using angular brackets.

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 221

Quoted 12 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +1 -1
                         #dma-cells = <1>;
                 };
 
@@ -205,7 +215,7 @@ clkcfg: clkcfg@20002000 {
                         clocks = <&refclk>;
                         #clock-cells = <1>;
                         clock-output-names = "cpu", "axi", "ahb", "envm",       /* 0-3   */
-                                "mac0", "mac1", "mmc", "timer",                /* 4-7   */
+                               "mac0", "mac1", "mmc", "timer",                 /* 4-7   */
                                 "mmuart0", "mmuart1", "mmuart2", "mmuart3",     /* 8-11  */
                                 "mmuart4", "spi0", "spi1", "i2c0",              /* 12-15 */
                                 "i2c1", "can0", "can1", "usb",                  /* 16-19 */

No need for clock-output-names at all.

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -2
-               emac1: ethernet@20112000 {
+               mac0: ethernet@20110000 {
                         compatible = "cdns,macb";
-                       reg = <0x0 0x20112000 0x0 0x2000>;
+                       #address-cells = <1>;
+                       #size-cells = <0>;
+                       reg = <0x0 0x20110000 0x0 0x2000>;
+                       clocks = <&clkcfg CLK_MAC0>, <&clkcfg CLK_AHB>;
+                       clock-names = "pclk", "hclk";

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 373

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +6 -3
                         interrupt-parent = <&plic>;
-                       interrupts = <70 71 72 73>;
-                       local-mac-address = [00 00 00 00 00 00];
-                       clocks = <&clkcfg 5>, <&clkcfg 2>;
+                       interrupts = <PLIC_INT_MAC0_INT
+                               PLIC_INT_MAC0_QUEUE1
+                               PLIC_INT_MAC0_QUEUE2
+                               PLIC_INT_MAC0_QUEUE3
+                               PLIC_INT_MAC0_EMAC
+                               PLIC_INT_MAC0_MMSL>;
Quoted 5 lines

Please group using angular brackets.

Quoted 1 line

+ mac-address = [56 34 12 00 FC 01];

Please drop this.

Is the problem here having mac-address instead of local-, having either at all or that we have populated it rather than just filling with 0s? We set it in u-boot anyway, so I think dropping entirely is okay.

Quoted 46 lines

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 434

Quoted 13 lines
                         status = "disabled";
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +8 -0
+               };
 
+               rtc: rtc@20124000 {
+                       compatible = "microchip,mpfs-rtc";
                         #address-cells = <1>;
                         #size-cells = <0>;
+                       reg = <0x0 0x20124000 0x0 0x1000>;
+                       clocks = <&clkcfg CLK_RTC>;
+                       clock-names = "rtc";
+                       interrupt-parent = <&plic>;
+                       interrupts = <PLIC_INT_RTC_WAKEUP PLIC_INT_RTC_MATCH>;

Please group using angular brackets.

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 444

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +8 -0
+                       status = "disabled";
                 };
 
+               usb: usb@20201000 {
+                       compatible = "microchip,mpfs-usb-host";
+                       reg = <0x0 0x20201000 0x0 0x1000>;
+                       reg-names = "mc","control";
+                       clocks = <&clkcfg CLK_USB>;
+                       interrupt-parent = <&plic>;
+                       interrupts = <PLIC_INT_USB_DMA PLIC_INT_USB_MC>;

Please group using angular brackets.

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi clkcfg: clkcfg@20002000 { · line 449

Quoted 5 lines
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +5 -0
+                       interrupt-names = "dma","mc";
+                       dr_mode = "host";
+                       status = "disabled";
+               };
+

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#64 lore raw

Re: [PATCH 12/13] riscv: icicle-kit: update microchip icicle kit device tree

Hi Conor,

Quoted 9 lines · On Wed, Nov 10, 2021 at 3:20 PM <[email protected]> wrote:
Quoted 8 lines · Geert Uytterhoeven
Quoted 7 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Update the device tree for the icicle kit by splitting it into a third part, which contains peripherals in the fpga fabric, add new peripherals (spi, qspi, gpio, rtc, pcie, system services, i2c), update parts of the memory map which have been changed.

Quoted 5 lines
Quoted 1 line
Quoted 1 line

As I said in the replies to another patch this is my first time doing any upstreaming of a device tree, i didnt realise that this would be a problem.

No problem, we're here to help you ;-)

Quoted 25 lines
Quoted 12 lines
Quoted 12 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +1 -1
@@ -1,5 +1,5 @@
  // SPDX-License-Identifier: (GPL-2.0 OR MIT)
-/* Copyright (c) 2020 Microchip Technology Inc */
+/* Copyright (c) 2020-2021 Microchip Technology Inc */
 
  /dts-v1/;
 
@@ -13,72 +13,187 @@ / {
         compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";
 
Quoted 13 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 20

Quoted 11 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +5 -5
         aliases {
-               ethernet0 = &emac1;
-               serial0 = &serial0;
-               serial1 = &serial1;
-               serial2 = &serial2;
-               serial3 = &serial3;
+               mmuart0 = &mmuart0;
+               mmuart1 = &mmuart1;
+               mmuart2 = &mmuart2;
+               mmuart3 = &mmuart3;
+               mmuart4 = &mmuart4;

Why? SerialN is the standard alias name.

Quoted 1 line

we changed the label to mmuart to match the microchip documentation.

The serialN aliases are standardized, so you cannot change them.

Quoted 2 lines

would it make more sense to call mmuart but alias it to serial? ie serial0 = &mmuart0;

You can change the labels, so that's OK.

Quoted 5 lines
Quoted 4 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 22

Quoted 2 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +2 -0
+&spi1 {
+       status = "okay";

No slave devices specified?

no, but its exposed

But without specifying slave devices first you cannot use the controller anyway? While I2C supports instantiating slaves from userspace by writing to the new_device file in sysfs, SPI doesn't have that feature.

Quoted 36 lines
Quoted 34 lines
Quoted 23 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +23 -0
+&gpio2 {
+       interrupts = <PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 181

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +10 -0
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT>;
Quoted 2 lines

Why override interrupts in the board .dts file?
Doesn't this belong in the SoC .dtsi file?

Quoted 7 lines

The interrupt setup for the gpio isnt fixed, there is an option to either connect the individual gpio interrupts to the plic *or* they can be connected to a per gpio controller common interrupt, and it is up to the driver to read a register to determine which interrupt triggered the common/NON_DIRECT interrupt. This decision is made by a write to a system register in application code, which to us didn't seem like it belonged in the soc .dtsi.

So it is software policy? Then it doesn't belong in the board DTS either.

Quoted 4 lines

Using the common interrupt for GPIO2 is the default on the polarfire-soc, there are only 38 per gpio line interrupts available of which 14 are connected to gpio0 and 24 to gpio1.

Quoted 8 lines
Quoted 6 lines

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 57

Quoted 4 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +1 -2
                 plic: interrupt-controller@c000000 {
-                       #interrupt-cells = <1>;
-                       compatible = "sifive,fu540-c000-plic", "sifive,plic-1.0.0";
+                       compatible = "sifive,plic-1.0.0";

Why drop the first one again?

we felt it didnt make sense to have something that specifically references the fu540 in the device tree for this board.

That would be a revert of commit 73d3c44115514616 ("riscv: dts:
microchip: add missing compatibles for clint and plic"), which you supplied an R-b tag for?

Is this the same plic as in the FU540 SoC? Or do we need a new microchip,mpfs-plic compatible value?

Quoted 20 lines
Quoted 20 lines
Quoted 19 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +12 -5
-               emac1: ethernet@20112000 {
+               mac0: ethernet@20110000 {
                         compatible = "cdns,macb";
-                       reg = <0x0 0x20112000 0x0 0x2000>;
+                       #address-cells = <1>;
+                       #size-cells = <0>;
+                       reg = <0x0 0x20110000 0x0 0x2000>;
+                       clocks = <&clkcfg CLK_MAC0>, <&clkcfg CLK_AHB>;
+                       clock-names = "pclk", "hclk";
                         interrupt-parent = <&plic>;
-                       interrupts = <70 71 72 73>;
-                       local-mac-address = [00 00 00 00 00 00];
-                       clocks = <&clkcfg 5>, <&clkcfg 2>;
+                       interrupts = <PLIC_INT_MAC0_INT
+                               PLIC_INT_MAC0_QUEUE1
+                               PLIC_INT_MAC0_QUEUE2
+                               PLIC_INT_MAC0_QUEUE3
+                               PLIC_INT_MAC0_EMAC
+                               PLIC_INT_MAC0_MMSL>;
Quoted 7 lines
Quoted 5 lines

Please group using angular brackets.

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 72

Quoted 1 line
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +1 -0
+                       mac-address = [56 34 12 00 FC 01];

Please drop this.

Is the problem here having mac-address instead of local-, having either at all or that we have populated it rather than just filling with 0s?

MAC addresses are supposed to be unique.

Quoted 1 line

We set it in u-boot anyway, so I think dropping entirely is okay.

Exactly.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#65 lore raw

Re: [PATCH 12/13] riscv: icicle-kit: update microchip icicle kit device tree

Quoted 13 lines · Geert Uytterhoeven

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Conor,

Quoted 8 lines
Quoted 6 lines

arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi cpu4_intc: interrupt-controller { · line 181

Quoted 4 lines
                  plic: interrupt-controller@c000000 {
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +1 -2
-                       #interrupt-cells = <1>;
-                       compatible = "sifive,fu540-c000-plic", "sifive,plic-1.0.0";
+                       compatible = "sifive,plic-1.0.0";

Why drop the first one again?

we felt it didnt make sense to have something that specifically references the fu540 in the device tree for this board.

Quoted 7 lines

That would be a revert of commit 73d3c44115514616 ("riscv: dts:
microchip: add missing compatibles for clint and plic"), which you supplied an R-b tag for?

Is this the same plic as in the FU540 SoC? Or do we need a new microchip,mpfs-plic compatible value?

yeah, ignore that. i confused this change (which is an accidental clobber) with another sifive compatible that was dropped for not being relevant. hardly makes sense to drop this one when i kept it for the clint and cache controller!

Quoted 12 lines

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#66 lore raw

Re: [PATCH 12/13] riscv: icicle-kit: update microchip icicle kit device tree

In reply to Geert Uytterhoeven
Quoted 53 lines · Geert Uytterhoeven

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Conor,

Quoted 40 lines · On Wed, Nov 10, 2021 at 3:20 PM <[email protected]> wrote:
Quoted 37 lines · Geert Uytterhoeven
Quoted 25 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

+&gpio2 {

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +22 -0
+       interrupts = <PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 181

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +10 -0
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT>;
Quoted 2 lines

Why override interrupts in the board .dts file?
Doesn't this belong in the SoC .dtsi file?

Quoted 7 lines

The interrupt setup for the gpio isnt fixed, there is an option to either connect the individual gpio interrupts to the plic *or* they can be connected to a per gpio controller common interrupt, and it is up to the driver to read a register to determine which interrupt triggered the common/NON_DIRECT interrupt. This decision is made by a write to a system register in application code, which to us didn't seem like it belonged in the soc .dtsi.

Quoted 1 line

So it is software policy? Then it doesn't belong in the board DTS either.

The write (if was to be done) would be done by the bootloader, based on the bitstream written to the FPGA, before even u-boot is started. By application I meant the bootloader (or some other bare metal application), not a program running in userspace in case that's what you interpreted. Am I incorrect in thinking that if it is set up by the bootloader that Linux can take it for granted?

Quoted 19 lines
Quoted 4 lines

Using the common interrupt for GPIO2 is the default on the polarfire-soc, there are only 38 per gpio line interrupts available of which 14 are connected to gpio0 and 24 to gpio1.

Exactly.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#67 lore raw

Re: [PATCH 12/13] riscv: icicle-kit: update microchip icicle kit device tree

Hi Conor,

Quoted 50 lines · On Mon, Nov 15, 2021 at 4:39 PM <[email protected]> wrote:
Quoted 49 lines · Geert Uytterhoeven
Quoted 47 lines · On Wed, Nov 10, 2021 at 3:20 PM <[email protected]> wrote:
Quoted 37 lines · Geert Uytterhoeven
Quoted 25 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

+&gpio2 {

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +22 -0
+       interrupts = <PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 181

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +10 -0
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT>;
Quoted 2 lines

Why override interrupts in the board .dts file?
Doesn't this belong in the SoC .dtsi file?

The interrupt setup for the gpio isnt fixed, there is an option to either connect the individual gpio interrupts to the plic *or* they can be connected to a per gpio controller common interrupt, and it is up to the driver to read a register to determine which interrupt triggered the common/NON_DIRECT interrupt. This decision is made by a write to a system register in application code, which to us didn't seem like it belonged in the soc .dtsi.

Quoted 8 lines
Quoted 1 line

So it is software policy? Then it doesn't belong in the board DTS either.

The write (if was to be done) would be done by the bootloader, based on the bitstream written to the FPGA, before even u-boot is started. By application I meant the bootloader (or some other bare metal application), not a program running in userspace in case that's what you interpreted. Am I incorrect in thinking that if it is set up by the bootloader that Linux can take it for granted?

If it is to be provided by the boot loader, the boot loader should fill in the interrupts property, just like it already does (or should do, if it doesn't) for /memory and chosen/bootargs.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#68 lore raw

Re: [PATCH 12/13] riscv: icicle-kit: update microchip icicle kit device tree

Quoted 77 lines · Geert Uytterhoeven

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Conor,

Quoted 61 lines · On Mon, Nov 15, 2021 at 4:39 PM <[email protected]> wrote:
Quoted 57 lines · Geert Uytterhoeven
Quoted 41 lines · On Wed, Nov 10, 2021 at 3:20 PM <[email protected]> wrote:
Quoted 38 lines · Geert Uytterhoeven
Quoted 25 lines · On Mon, Nov 8, 2021 at 4:07 PM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

+&gpio2 {

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +22 -0
+       interrupts = <PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT

arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts / { · line 181

Quoted 10 lines
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts +10 -0
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT
+               PLIC_INT_GPIO2_NON_DIRECT>;
Quoted 2 lines

Why override interrupts in the board .dts file?
Doesn't this belong in the SoC .dtsi file?

Quoted 13 lines

The interrupt setup for the gpio isnt fixed, there is an option to
either connect the individual gpio interrupts to the plic *or* they can
be connected to a per gpio controller common interrupt, and it is up to
the driver to read a register to determine which interrupt triggered the
common/NON_DIRECT interrupt. This decision is made by a write to a
system register in application code, which to us didn't seem like it
belonged in the soc .dtsi.

Quoted 2 lines

So it is software policy? Then it doesn't belong in the board DTS either.

Quoted 9 lines

The write (if was to be done) would be done by the bootloader, based on
the bitstream written to the FPGA, before even u-boot is started. By
application I meant the bootloader (or some other bare metal application), not a program running in userspace in case that's what you
interpreted. Am I incorrect in thinking that if it is set up by the bootloader that Linux can take it for granted?

Quoted 5 lines

If it is to be provided by the boot loader, the boot loader should fill
in the interrupts property, just like it already does (or should do, if it
doesn't) for /memory and chosen/bootargs.

Whether a given GPIO is routed via a bank where it has its own interrupt line or via a bank where it shares an interrupt line is an SoC capability. A particular routing is instantiated by a particular board (e.g. Icicle). A custom bootloader feels like complete overkill for this job.

Quoted 13 lines

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#69 lore raw

PATCH 13/13 MAINTAINERS: update riscv/microchip entry

In reply to [email protected]

From: Conor Dooley <[email protected]>

Update the riscv/microchip entry by adding the microchip dts directory and myself as maintainer

---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)
MAINTAINERS +2 -0
index ca6d6fde85cf..a0000ddd02fd 100644
@@ -16077,8 +16077,10 @@ K:	riscv
 
 RISC-V/MICROCHIP POLARFIRE SOC SUPPORT
 M:	Lewis Hanly <[email protected]>
+M:	Conor Dooley <[email protected]>
 L:	[email protected]
 S:	Supported
+F:	arch/riscv/boot/dts/microchip/
 F:	drivers/mailbox/mailbox-mpfs.c
 F:	drivers/soc/microchip/
 F:	include/soc/microchip/mpfs.h
Signature · 7 lines
2.33.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv