soc: imx: add scu power domain driver

PATCH Superseded A.s. Dong 3 participants 19 messages Oct 28, 2018 – Nov 1, 2018

Subsystems (2)

Device tree bindings Arm i.MX and Layerscape

lore

Share & tools

lore

A.s. Dong

author

2018-10-28 15:19 UTC #0 lore raw

v8 cover soc: imx: add scu power domain driver

This patch set adds the scu based power domain driver. It depends on SCU driver.

Change Log:
v7->v8:
* update to #power-domain-cells 1 binding
v6->v7:
* keep "fsl,scu-pd" as fall back compatible string
v5->v6:
* only compatible string name updated from fsl,scu-pd to fsl,imx8qxp-scu-pd accordingly
v4->v5:
* some minor improvements according to Sascha's suggestion
Note: did not use dev_* print functions due to we already have proper
prefix by redefining pr_fmt. So it seems not quite neccesary to pass in a struct device * pointer for debug purpose only in each functions.
v3->v4:
* only scu headfile path update

v2->v3:
* structure and enums name update
* api usage update

Dong Aisheng (5):
dt-bindings: imx: add scu resource id headfile
firmware: imx: remove resource id enums
dt-bindings: fsl: scu: update power domain binding
firmware: imx: add pm svc headfile
firmware: imx: add SCU power domain driver

 .../devicetree/bindings/arm/freescale/fsl,scu.txt  |  37 +-
 drivers/firmware/imx/Kconfig                       |   6 +
 drivers/firmware/imx/Makefile                      |   3 +-
 drivers/firmware/imx/scu-pd.c                      | 302 +++++++++++
 include/dt-bindings/firmware/imx/rsrc.h            | 559 +++++++++++++++++++++
 include/linux/firmware/imx/sci.h                   |   1 +
 include/linux/firmware/imx/svc/pm.h                |  85 ++++
 include/linux/firmware/imx/types.h                 | 552 --------------------
 8 files changed, 963 insertions(+), 582 deletions(-)
 create mode 100644 drivers/firmware/imx/scu-pd.c
 create mode 100644 include/dt-bindings/firmware/imx/rsrc.h
 create mode 100644 include/linux/firmware/imx/svc/pm.h

Signature · 1 line

2.7.4

A.s. Dong

author

2018-10-28 15:19 UTC #1 lore raw

PATCH v8 1/5 dt-bindings: imx: add scu resource id headfile

SCU firmware uses resource id to provide services. Every device on a SCU based system has a resource id. Exported it in device tree to allow service bindings to use it. e.g. power domain.

Cc Shawn Guo <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Rob Herring <[email protected]>

Cc Mark Rutland <[email protected]>

Cc devicetree at vger.kernel.org

Signed-off-by Dong Aisheng <[email protected]>

---
v7->v8:
* new patch

---
 include/dt-bindings/firmware/imx/rsrc.h | 559 ++++++++++++++++++++++++++++++++
 1 file changed, 559 insertions(+)
 create mode 100644 include/dt-bindings/firmware/imx/rsrc.h

include/dt-bindings/firmware/imx/rsrc.h new file +559 -0

new file mode 100644
index 0000000..4481f2d
@@ -0,0 +1,559 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017-2018 NXP
+ */
+
+#ifndef __DT_BINDINGS_RSCRC_IMX_H
+#define __DT_BINDINGS_RSCRC_IMX_H
+
+/*
+ * These defines are used to indicate a resource. Resources include peripherals
+ * and bus masters (but not memory regions). Note items from list should
+ * never be changed or removed (only added to at the end of the list).
+ */
+
+#define IMX_SC_R_A53			0
+#define IMX_SC_R_A53_0			1
+#define IMX_SC_R_A53_1			2
+#define IMX_SC_R_A53_2			3
+#define IMX_SC_R_A53_3			4
+#define IMX_SC_R_A72			5
+#define IMX_SC_R_A72_0			6
+#define IMX_SC_R_A72_1			7
+#define IMX_SC_R_A72_2			8
+#define IMX_SC_R_A72_3			9
+#define IMX_SC_R_CCI			10
+#define IMX_SC_R_DB			11
+#define IMX_SC_R_DRC_0			12
+#define IMX_SC_R_DRC_1			13
+#define IMX_SC_R_GIC_SMMU		14
+#define IMX_SC_R_IRQSTR_M4_0		15
+#define IMX_SC_R_IRQSTR_M4_1		16
+#define IMX_SC_R_SMMU			17
+#define IMX_SC_R_GIC			18
+#define IMX_SC_R_DC_0_BLIT0		19
+#define IMX_SC_R_DC_0_BLIT1		20
+#define IMX_SC_R_DC_0_BLIT2		21
+#define IMX_SC_R_DC_0_BLIT_OUT		22
+#define IMX_SC_R_DC_0_CAPTURE0		23
+#define IMX_SC_R_DC_0_CAPTURE1		24
+#define IMX_SC_R_DC_0_WARP		25
+#define IMX_SC_R_DC_0_INTEGRAL0		26
+#define IMX_SC_R_DC_0_INTEGRAL1		27
+#define IMX_SC_R_DC_0_VIDEO0		28
+#define IMX_SC_R_DC_0_VIDEO1		29
+#define IMX_SC_R_DC_0_FRAC0		30
+#define IMX_SC_R_DC_0_FRAC1		31
+#define IMX_SC_R_DC_0			32
+#define IMX_SC_R_GPU_2_PID0		33
+#define IMX_SC_R_DC_0_PLL_0		34
+#define IMX_SC_R_DC_0_PLL_1		35
+#define IMX_SC_R_DC_1_BLIT0		36
+#define IMX_SC_R_DC_1_BLIT1		37
+#define IMX_SC_R_DC_1_BLIT2		38
+#define IMX_SC_R_DC_1_BLIT_OUT		39
+#define IMX_SC_R_DC_1_CAPTURE0		40
+#define IMX_SC_R_DC_1_CAPTURE1		41
+#define IMX_SC_R_DC_1_WARP		42
+#define IMX_SC_R_DC_1_INTEGRAL0		43
+#define IMX_SC_R_DC_1_INTEGRAL1		44
+#define IMX_SC_R_DC_1_VIDEO0		45
+#define IMX_SC_R_DC_1_VIDEO1		46
+#define IMX_SC_R_DC_1_FRAC0		47
+#define IMX_SC_R_DC_1_FRAC1		48
+#define IMX_SC_R_DC_1			49
+#define IMX_SC_R_GPU_3_PID0		50
+#define IMX_SC_R_DC_1_PLL_0		51
+#define IMX_SC_R_DC_1_PLL_1		52
+#define IMX_SC_R_SPI_0			53
+#define IMX_SC_R_SPI_1			54
+#define IMX_SC_R_SPI_2			55
+#define IMX_SC_R_SPI_3			56
+#define IMX_SC_R_UART_0			57
+#define IMX_SC_R_UART_1			58
+#define IMX_SC_R_UART_2			59
+#define IMX_SC_R_UART_3			60
+#define IMX_SC_R_UART_4			61
+#define IMX_SC_R_EMVSIM_0		62
+#define IMX_SC_R_EMVSIM_1		63
+#define IMX_SC_R_DMA_0_CH0		64
+#define IMX_SC_R_DMA_0_CH1		65
+#define IMX_SC_R_DMA_0_CH2		66
+#define IMX_SC_R_DMA_0_CH3		67
+#define IMX_SC_R_DMA_0_CH4		68
+#define IMX_SC_R_DMA_0_CH5		69
+#define IMX_SC_R_DMA_0_CH6		70
+#define IMX_SC_R_DMA_0_CH7		71
+#define IMX_SC_R_DMA_0_CH8		72
+#define IMX_SC_R_DMA_0_CH9		73
+#define IMX_SC_R_DMA_0_CH10		74
+#define IMX_SC_R_DMA_0_CH11		75
+#define IMX_SC_R_DMA_0_CH12		76
+#define IMX_SC_R_DMA_0_CH13		77
+#define IMX_SC_R_DMA_0_CH14		78
+#define IMX_SC_R_DMA_0_CH15		79
+#define IMX_SC_R_DMA_0_CH16		80
+#define IMX_SC_R_DMA_0_CH17		81
+#define IMX_SC_R_DMA_0_CH18		82
+#define IMX_SC_R_DMA_0_CH19		83
+#define IMX_SC_R_DMA_0_CH20		84
+#define IMX_SC_R_DMA_0_CH21		85
+#define IMX_SC_R_DMA_0_CH22		86
+#define IMX_SC_R_DMA_0_CH23		87
+#define IMX_SC_R_DMA_0_CH24		88
+#define IMX_SC_R_DMA_0_CH25		89
+#define IMX_SC_R_DMA_0_CH26		90
+#define IMX_SC_R_DMA_0_CH27		91
+#define IMX_SC_R_DMA_0_CH28		92
+#define IMX_SC_R_DMA_0_CH29		93
+#define IMX_SC_R_DMA_0_CH30		94
+#define IMX_SC_R_DMA_0_CH31		95
+#define IMX_SC_R_I2C_0			96
+#define IMX_SC_R_I2C_1			97
+#define IMX_SC_R_I2C_2			98
+#define IMX_SC_R_I2C_3			99
+#define IMX_SC_R_I2C_4			100
+#define IMX_SC_R_ADC_0			101
+#define IMX_SC_R_ADC_1			102
+#define IMX_SC_R_FTM_0			103
+#define IMX_SC_R_FTM_1			104
+#define IMX_SC_R_CAN_0			105
+#define IMX_SC_R_CAN_1			106
+#define IMX_SC_R_CAN_2			107
+#define IMX_SC_R_DMA_1_CH0		108
+#define IMX_SC_R_DMA_1_CH1		109
+#define IMX_SC_R_DMA_1_CH2		110
+#define IMX_SC_R_DMA_1_CH3		111
+#define IMX_SC_R_DMA_1_CH4		112
+#define IMX_SC_R_DMA_1_CH5		113
+#define IMX_SC_R_DMA_1_CH6		114
+#define IMX_SC_R_DMA_1_CH7		115
+#define IMX_SC_R_DMA_1_CH8		116
+#define IMX_SC_R_DMA_1_CH9		117
+#define IMX_SC_R_DMA_1_CH10		118
+#define IMX_SC_R_DMA_1_CH11		119
+#define IMX_SC_R_DMA_1_CH12		120
+#define IMX_SC_R_DMA_1_CH13		121
+#define IMX_SC_R_DMA_1_CH14		122
+#define IMX_SC_R_DMA_1_CH15		123
+#define IMX_SC_R_DMA_1_CH16		124
+#define IMX_SC_R_DMA_1_CH17		125
+#define IMX_SC_R_DMA_1_CH18		126
+#define IMX_SC_R_DMA_1_CH19		127
+#define IMX_SC_R_DMA_1_CH20		128
+#define IMX_SC_R_DMA_1_CH21		129
+#define IMX_SC_R_DMA_1_CH22		130
+#define IMX_SC_R_DMA_1_CH23		131
+#define IMX_SC_R_DMA_1_CH24		132
+#define IMX_SC_R_DMA_1_CH25		133
+#define IMX_SC_R_DMA_1_CH26		134
+#define IMX_SC_R_DMA_1_CH27		135
+#define IMX_SC_R_DMA_1_CH28		136
+#define IMX_SC_R_DMA_1_CH29		137
+#define IMX_SC_R_DMA_1_CH30		138
+#define IMX_SC_R_DMA_1_CH31		139
+#define IMX_SC_R_UNUSED1		140
+#define IMX_SC_R_UNUSED2		141
+#define IMX_SC_R_UNUSED3		142
+#define IMX_SC_R_UNUSED4		143
+#define IMX_SC_R_GPU_0_PID0		144
+#define IMX_SC_R_GPU_0_PID1		145
+#define IMX_SC_R_GPU_0_PID2		146
+#define IMX_SC_R_GPU_0_PID3		147
+#define IMX_SC_R_GPU_1_PID0		148
+#define IMX_SC_R_GPU_1_PID1		149
+#define IMX_SC_R_GPU_1_PID2		150
+#define IMX_SC_R_GPU_1_PID3		151
+#define IMX_SC_R_PCIE_A			152
+#define IMX_SC_R_SERDES_0		153
+#define IMX_SC_R_MATCH_0		154
+#define IMX_SC_R_MATCH_1		155
+#define IMX_SC_R_MATCH_2		156
+#define IMX_SC_R_MATCH_3		157
+#define IMX_SC_R_MATCH_4		158
+#define IMX_SC_R_MATCH_5		159
+#define IMX_SC_R_MATCH_6		160
+#define IMX_SC_R_MATCH_7		161
+#define IMX_SC_R_MATCH_8		162
+#define IMX_SC_R_MATCH_9		163
+#define IMX_SC_R_MATCH_10		164
+#define IMX_SC_R_MATCH_11		165
+#define IMX_SC_R_MATCH_12		166
+#define IMX_SC_R_MATCH_13		167
+#define IMX_SC_R_MATCH_14		168
+#define IMX_SC_R_PCIE_B			169
+#define IMX_SC_R_SATA_0			170
+#define IMX_SC_R_SERDES_1		171
+#define IMX_SC_R_HSIO_GPIO		172
+#define IMX_SC_R_MATCH_15		173
+#define IMX_SC_R_MATCH_16		174
+#define IMX_SC_R_MATCH_17		175
+#define IMX_SC_R_MATCH_18		176
+#define IMX_SC_R_MATCH_19		177
+#define IMX_SC_R_MATCH_20		178
+#define IMX_SC_R_MATCH_21		179
+#define IMX_SC_R_MATCH_22		180
+#define IMX_SC_R_MATCH_23		181
+#define IMX_SC_R_MATCH_24		182
+#define IMX_SC_R_MATCH_25		183
+#define IMX_SC_R_MATCH_26		184
+#define IMX_SC_R_MATCH_27		185
+#define IMX_SC_R_MATCH_28		186
+#define IMX_SC_R_LCD_0			187
+#define IMX_SC_R_LCD_0_PWM_0		188
+#define IMX_SC_R_LCD_0_I2C_0		189
+#define IMX_SC_R_LCD_0_I2C_1		190
+#define IMX_SC_R_PWM_0			191
+#define IMX_SC_R_PWM_1			192
+#define IMX_SC_R_PWM_2			193
+#define IMX_SC_R_PWM_3			194
+#define IMX_SC_R_PWM_4			195
+#define IMX_SC_R_PWM_5			196
+#define IMX_SC_R_PWM_6			197
+#define IMX_SC_R_PWM_7			198
+#define IMX_SC_R_GPIO_0			199
+#define IMX_SC_R_GPIO_1			200
+#define IMX_SC_R_GPIO_2			201
+#define IMX_SC_R_GPIO_3			202
+#define IMX_SC_R_GPIO_4			203
+#define IMX_SC_R_GPIO_5			204
+#define IMX_SC_R_GPIO_6			205
+#define IMX_SC_R_GPIO_7			206
+#define IMX_SC_R_GPT_0			207
+#define IMX_SC_R_GPT_1			208
+#define IMX_SC_R_GPT_2			209
+#define IMX_SC_R_GPT_3			210
+#define IMX_SC_R_GPT_4			211
+#define IMX_SC_R_KPP			212
+#define IMX_SC_R_MU_0A			213
+#define IMX_SC_R_MU_1A			214
+#define IMX_SC_R_MU_2A			215
+#define IMX_SC_R_MU_3A			216
+#define IMX_SC_R_MU_4A			217
+#define IMX_SC_R_MU_5A			218
+#define IMX_SC_R_MU_6A			219
+#define IMX_SC_R_MU_7A			220
+#define IMX_SC_R_MU_8A			221
+#define IMX_SC_R_MU_9A			222
+#define IMX_SC_R_MU_10A			223
+#define IMX_SC_R_MU_11A			224
+#define IMX_SC_R_MU_12A			225
+#define IMX_SC_R_MU_13A			226
+#define IMX_SC_R_MU_5B			227
+#define IMX_SC_R_MU_6B			228
+#define IMX_SC_R_MU_7B			229
+#define IMX_SC_R_MU_8B			230
+#define IMX_SC_R_MU_9B			231
+#define IMX_SC_R_MU_10B			232
+#define IMX_SC_R_MU_11B			233
+#define IMX_SC_R_MU_12B			234
+#define IMX_SC_R_MU_13B			235
+#define IMX_SC_R_ROM_0			236
+#define IMX_SC_R_FSPI_0			237
+#define IMX_SC_R_FSPI_1			238
+#define IMX_SC_R_IEE			239
+#define IMX_SC_R_IEE_R0			240
+#define IMX_SC_R_IEE_R1			241
+#define IMX_SC_R_IEE_R2			242
+#define IMX_SC_R_IEE_R3			243
+#define IMX_SC_R_IEE_R4			244
+#define IMX_SC_R_IEE_R5			245
+#define IMX_SC_R_IEE_R6			246
+#define IMX_SC_R_IEE_R7			247
+#define IMX_SC_R_SDHC_0			248
+#define IMX_SC_R_SDHC_1			249
+#define IMX_SC_R_SDHC_2			250
+#define IMX_SC_R_ENET_0			251
+#define IMX_SC_R_ENET_1			252
+#define IMX_SC_R_MLB_0			253
+#define IMX_SC_R_DMA_2_CH0		254
+#define IMX_SC_R_DMA_2_CH1		255
+#define IMX_SC_R_DMA_2_CH2		256
+#define IMX_SC_R_DMA_2_CH3		257
+#define IMX_SC_R_DMA_2_CH4		258
+#define IMX_SC_R_USB_0			259
+#define IMX_SC_R_USB_1			260
+#define IMX_SC_R_USB_0_PHY		261
+#define IMX_SC_R_USB_2			262
+#define IMX_SC_R_USB_2_PHY		263
+#define IMX_SC_R_DTCP			264
+#define IMX_SC_R_NAND			265
+#define IMX_SC_R_LVDS_0			266
+#define IMX_SC_R_LVDS_0_PWM_0		267
+#define IMX_SC_R_LVDS_0_I2C_0		268
+#define IMX_SC_R_LVDS_0_I2C_1		269
+#define IMX_SC_R_LVDS_1			270
+#define IMX_SC_R_LVDS_1_PWM_0		271
+#define IMX_SC_R_LVDS_1_I2C_0		272
+#define IMX_SC_R_LVDS_1_I2C_1		273
+#define IMX_SC_R_LVDS_2			274
+#define IMX_SC_R_LVDS_2_PWM_0		275
+#define IMX_SC_R_LVDS_2_I2C_0		276
+#define IMX_SC_R_LVDS_2_I2C_1		277
+#define IMX_SC_R_M4_0_PID0		278
+#define IMX_SC_R_M4_0_PID1		279
+#define IMX_SC_R_M4_0_PID2		280
+#define IMX_SC_R_M4_0_PID3		281
+#define IMX_SC_R_M4_0_PID4		282
+#define IMX_SC_R_M4_0_RGPIO		283
+#define IMX_SC_R_M4_0_SEMA42		284
+#define IMX_SC_R_M4_0_TPM		285
+#define IMX_SC_R_M4_0_PIT		286
+#define IMX_SC_R_M4_0_UART		287
+#define IMX_SC_R_M4_0_I2C		288
+#define IMX_SC_R_M4_0_INTMUX		289
+#define IMX_SC_R_M4_0_SIM		290
+#define IMX_SC_R_M4_0_WDOG		291
+#define IMX_SC_R_M4_0_MU_0B		292
+#define IMX_SC_R_M4_0_MU_0A0		293
+#define IMX_SC_R_M4_0_MU_0A1		294
+#define IMX_SC_R_M4_0_MU_0A2		295
+#define IMX_SC_R_M4_0_MU_0A3		296
+#define IMX_SC_R_M4_0_MU_1A		297
+#define IMX_SC_R_M4_1_PID0		298
+#define IMX_SC_R_M4_1_PID1		299
+#define IMX_SC_R_M4_1_PID2		300
+#define IMX_SC_R_M4_1_PID3		301
+#define IMX_SC_R_M4_1_PID4		302
+#define IMX_SC_R_M4_1_RGPIO		303
+#define IMX_SC_R_M4_1_SEMA42		304
+#define IMX_SC_R_M4_1_TPM		305
+#define IMX_SC_R_M4_1_PIT		306
+#define IMX_SC_R_M4_1_UART		307
+#define IMX_SC_R_M4_1_I2C		308
+#define IMX_SC_R_M4_1_INTMUX		309
+#define IMX_SC_R_M4_1_SIM		310
+#define IMX_SC_R_M4_1_WDOG		311
+#define IMX_SC_R_M4_1_MU_0B		312
+#define IMX_SC_R_M4_1_MU_0A0		313
+#define IMX_SC_R_M4_1_MU_0A1		314
+#define IMX_SC_R_M4_1_MU_0A2		315
+#define IMX_SC_R_M4_1_MU_0A3		316
+#define IMX_SC_R_M4_1_MU_1A		317
+#define IMX_SC_R_SAI_0			318
+#define IMX_SC_R_SAI_1			319
+#define IMX_SC_R_SAI_2			320
+#define IMX_SC_R_IRQSTR_SCU2		321
+#define IMX_SC_R_IRQSTR_DSP		322
+#define IMX_SC_R_ELCDIF_PLL		323
+#define IMX_SC_R_UNUSED6		324
+#define IMX_SC_R_AUDIO_PLL_0		325
+#define IMX_SC_R_PI_0			326
+#define IMX_SC_R_PI_0_PWM_0		327
+#define IMX_SC_R_PI_0_PWM_1		328
+#define IMX_SC_R_PI_0_I2C_0		329
+#define IMX_SC_R_PI_0_PLL		330
+#define IMX_SC_R_PI_1			331
+#define IMX_SC_R_PI_1_PWM_0		332
+#define IMX_SC_R_PI_1_PWM_1		333
+#define IMX_SC_R_PI_1_I2C_0		334
+#define IMX_SC_R_PI_1_PLL		335
+#define IMX_SC_R_SC_PID0		336
+#define IMX_SC_R_SC_PID1		337
+#define IMX_SC_R_SC_PID2		338
+#define IMX_SC_R_SC_PID3		339
+#define IMX_SC_R_SC_PID4		340
+#define IMX_SC_R_SC_SEMA42		341
+#define IMX_SC_R_SC_TPM			342
+#define IMX_SC_R_SC_PIT			343
+#define IMX_SC_R_SC_UART		344
+#define IMX_SC_R_SC_I2C			345
+#define IMX_SC_R_SC_MU_0B		346
+#define IMX_SC_R_SC_MU_0A0		347
+#define IMX_SC_R_SC_MU_0A1		348
+#define IMX_SC_R_SC_MU_0A2		349
+#define IMX_SC_R_SC_MU_0A3		350
+#define IMX_SC_R_SC_MU_1A		351
+#define IMX_SC_R_SYSCNT_RD		352
+#define IMX_SC_R_SYSCNT_CMP		353
+#define IMX_SC_R_DEBUG			354
+#define IMX_SC_R_SYSTEM			355
+#define IMX_SC_R_SNVS			356
+#define IMX_SC_R_OTP			357
+#define IMX_SC_R_VPU_PID0		358
+#define IMX_SC_R_VPU_PID1		359
+#define IMX_SC_R_VPU_PID2		360
+#define IMX_SC_R_VPU_PID3		361
+#define IMX_SC_R_VPU_PID4		362
+#define IMX_SC_R_VPU_PID5		363
+#define IMX_SC_R_VPU_PID6		364
+#define IMX_SC_R_VPU_PID7		365
+#define IMX_SC_R_VPU_UART		366
+#define IMX_SC_R_VPUCORE		367
+#define IMX_SC_R_VPUCORE_0		368
+#define IMX_SC_R_VPUCORE_1		369
+#define IMX_SC_R_VPUCORE_2		370
+#define IMX_SC_R_VPUCORE_3		371
+#define IMX_SC_R_DMA_4_CH0		372
+#define IMX_SC_R_DMA_4_CH1		373
+#define IMX_SC_R_DMA_4_CH2		374
+#define IMX_SC_R_DMA_4_CH3		375
+#define IMX_SC_R_DMA_4_CH4		376
+#define IMX_SC_R_ISI_CH0		377
+#define IMX_SC_R_ISI_CH1		378
+#define IMX_SC_R_ISI_CH2		379
+#define IMX_SC_R_ISI_CH3		380
+#define IMX_SC_R_ISI_CH4		381
+#define IMX_SC_R_ISI_CH5		382
+#define IMX_SC_R_ISI_CH6		383
+#define IMX_SC_R_ISI_CH7		384
+#define IMX_SC_R_MJPEG_DEC_S0		385
+#define IMX_SC_R_MJPEG_DEC_S1		386
+#define IMX_SC_R_MJPEG_DEC_S2		387
+#define IMX_SC_R_MJPEG_DEC_S3		388
+#define IMX_SC_R_MJPEG_ENC_S0		389
+#define IMX_SC_R_MJPEG_ENC_S1		390
+#define IMX_SC_R_MJPEG_ENC_S2		391
+#define IMX_SC_R_MJPEG_ENC_S3		392
+#define IMX_SC_R_MIPI_0			393
+#define IMX_SC_R_MIPI_0_PWM_0		394
+#define IMX_SC_R_MIPI_0_I2C_0		395
+#define IMX_SC_R_MIPI_0_I2C_1		396
+#define IMX_SC_R_MIPI_1			397
+#define IMX_SC_R_MIPI_1_PWM_0		398
+#define IMX_SC_R_MIPI_1_I2C_0		399
+#define IMX_SC_R_MIPI_1_I2C_1		400
+#define IMX_SC_R_CSI_0			401
+#define IMX_SC_R_CSI_0_PWM_0		402
+#define IMX_SC_R_CSI_0_I2C_0		403
+#define IMX_SC_R_CSI_1			404
+#define IMX_SC_R_CSI_1_PWM_0		405
+#define IMX_SC_R_CSI_1_I2C_0		406
+#define IMX_SC_R_HDMI			407
+#define IMX_SC_R_HDMI_I2S		408
+#define IMX_SC_R_HDMI_I2C_0		409
+#define IMX_SC_R_HDMI_PLL_0		410
+#define IMX_SC_R_HDMI_RX		411
+#define IMX_SC_R_HDMI_RX_BYPASS		412
+#define IMX_SC_R_HDMI_RX_I2C_0		413
+#define IMX_SC_R_ASRC_0			414
+#define IMX_SC_R_ESAI_0			415
+#define IMX_SC_R_SPDIF_0		416
+#define IMX_SC_R_SPDIF_1		417
+#define IMX_SC_R_SAI_3			418
+#define IMX_SC_R_SAI_4			419
+#define IMX_SC_R_SAI_5			420
+#define IMX_SC_R_GPT_5			421
+#define IMX_SC_R_GPT_6			422
+#define IMX_SC_R_GPT_7			423
+#define IMX_SC_R_GPT_8			424
+#define IMX_SC_R_GPT_9			425
+#define IMX_SC_R_GPT_10			426
+#define IMX_SC_R_DMA_2_CH5		427
+#define IMX_SC_R_DMA_2_CH6		428
+#define IMX_SC_R_DMA_2_CH7		429
+#define IMX_SC_R_DMA_2_CH8		430
+#define IMX_SC_R_DMA_2_CH9		431
+#define IMX_SC_R_DMA_2_CH10		432
+#define IMX_SC_R_DMA_2_CH11		433
+#define IMX_SC_R_DMA_2_CH12		434
+#define IMX_SC_R_DMA_2_CH13		435
+#define IMX_SC_R_DMA_2_CH14		436
+#define IMX_SC_R_DMA_2_CH15		437
+#define IMX_SC_R_DMA_2_CH16		438
+#define IMX_SC_R_DMA_2_CH17		439
+#define IMX_SC_R_DMA_2_CH18		440
+#define IMX_SC_R_DMA_2_CH19		441
+#define IMX_SC_R_DMA_2_CH20		442
+#define IMX_SC_R_DMA_2_CH21		443
+#define IMX_SC_R_DMA_2_CH22		444
+#define IMX_SC_R_DMA_2_CH23		445
+#define IMX_SC_R_DMA_2_CH24		446
+#define IMX_SC_R_DMA_2_CH25		447
+#define IMX_SC_R_DMA_2_CH26		448
+#define IMX_SC_R_DMA_2_CH27		449
+#define IMX_SC_R_DMA_2_CH28		450
+#define IMX_SC_R_DMA_2_CH29		451
+#define IMX_SC_R_DMA_2_CH30		452
+#define IMX_SC_R_DMA_2_CH31		453
+#define IMX_SC_R_ASRC_1			454
+#define IMX_SC_R_ESAI_1			455
+#define IMX_SC_R_SAI_6			456
+#define IMX_SC_R_SAI_7			457
+#define IMX_SC_R_AMIX			458
+#define IMX_SC_R_MQS_0			459
+#define IMX_SC_R_DMA_3_CH0		460
+#define IMX_SC_R_DMA_3_CH1		461
+#define IMX_SC_R_DMA_3_CH2		462
+#define IMX_SC_R_DMA_3_CH3		463
+#define IMX_SC_R_DMA_3_CH4		464
+#define IMX_SC_R_DMA_3_CH5		465
+#define IMX_SC_R_DMA_3_CH6		466
+#define IMX_SC_R_DMA_3_CH7		467
+#define IMX_SC_R_DMA_3_CH8		468
+#define IMX_SC_R_DMA_3_CH9		469
+#define IMX_SC_R_DMA_3_CH10		470
+#define IMX_SC_R_DMA_3_CH11		471
+#define IMX_SC_R_DMA_3_CH12		472
+#define IMX_SC_R_DMA_3_CH13		473
+#define IMX_SC_R_DMA_3_CH14		474
+#define IMX_SC_R_DMA_3_CH15		475
+#define IMX_SC_R_DMA_3_CH16		476
+#define IMX_SC_R_DMA_3_CH17		477
+#define IMX_SC_R_DMA_3_CH18		478
+#define IMX_SC_R_DMA_3_CH19		479
+#define IMX_SC_R_DMA_3_CH20		480
+#define IMX_SC_R_DMA_3_CH21		481
+#define IMX_SC_R_DMA_3_CH22		482
+#define IMX_SC_R_DMA_3_CH23		483
+#define IMX_SC_R_DMA_3_CH24		484
+#define IMX_SC_R_DMA_3_CH25		485
+#define IMX_SC_R_DMA_3_CH26		486
+#define IMX_SC_R_DMA_3_CH27		487
+#define IMX_SC_R_DMA_3_CH28		488
+#define IMX_SC_R_DMA_3_CH29		489
+#define IMX_SC_R_DMA_3_CH30		490
+#define IMX_SC_R_DMA_3_CH31		491
+#define IMX_SC_R_AUDIO_PLL_1		492
+#define IMX_SC_R_AUDIO_CLK_0		493
+#define IMX_SC_R_AUDIO_CLK_1		494
+#define IMX_SC_R_MCLK_OUT_0		495
+#define IMX_SC_R_MCLK_OUT_1		496
+#define IMX_SC_R_PMIC_0			497
+#define IMX_SC_R_PMIC_1			498
+#define IMX_SC_R_SECO			499
+#define IMX_SC_R_CAAM_JR1		500
+#define IMX_SC_R_CAAM_JR2		501
+#define IMX_SC_R_CAAM_JR3		502
+#define IMX_SC_R_SECO_MU_2		503
+#define IMX_SC_R_SECO_MU_3		504
+#define IMX_SC_R_SECO_MU_4		505
+#define IMX_SC_R_HDMI_RX_PWM_0		506
+#define IMX_SC_R_A35			507
+#define IMX_SC_R_A35_0			508
+#define IMX_SC_R_A35_1			509
+#define IMX_SC_R_A35_2			510
+#define IMX_SC_R_A35_3			511
+#define IMX_SC_R_DSP			512
+#define IMX_SC_R_DSP_RAM		513
+#define IMX_SC_R_CAAM_JR1_OUT		514
+#define IMX_SC_R_CAAM_JR2_OUT		515
+#define IMX_SC_R_CAAM_JR3_OUT		516
+#define IMX_SC_R_VPU_DEC_0		517
+#define IMX_SC_R_VPU_ENC_0		518
+#define IMX_SC_R_CAAM_JR0		519
+#define IMX_SC_R_CAAM_JR0_OUT		520
+#define IMX_SC_R_PMIC_2			521
+#define IMX_SC_R_DBLOGIC		522
+#define IMX_SC_R_HDMI_PLL_1		523
+#define IMX_SC_R_BOARD_R0		524
+#define IMX_SC_R_BOARD_R1		525
+#define IMX_SC_R_BOARD_R2		526
+#define IMX_SC_R_BOARD_R3		527
+#define IMX_SC_R_BOARD_R4		528
+#define IMX_SC_R_BOARD_R5		529
+#define IMX_SC_R_BOARD_R6		530
+#define IMX_SC_R_BOARD_R7		531
+#define IMX_SC_R_MJPEG_DEC_MP		532
+#define IMX_SC_R_MJPEG_ENC_MP		533
+#define IMX_SC_R_VPU_TS_0		534
+#define IMX_SC_R_VPU_MU_0		535
+#define IMX_SC_R_VPU_MU_1		536
+#define IMX_SC_R_VPU_MU_2		537
+#define IMX_SC_R_VPU_MU_3		538
+#define IMX_SC_R_VPU_ENC_1		539
+#define IMX_SC_R_VPU			540
+#define IMX_SC_R_LAST			541
+
+#endif /* __DT_BINDINGS_RSCRC_IMX_H */

Signature · 1 line

2.7.4

Rob Herring

maintainer

2018-10-30 19:30 UTC #2 lore raw

PATCH v8 1/5 dt-bindings: imx: add scu resource id headfile

Quoted 11 lines · A.s. Dong

SCU firmware uses resource id to provide services. Every device on a SCU based system has a resource id. Exported it in device tree to allow service bindings to use it. e.g. power domain.
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Rob Herring <[email protected]>
Cc Mark Rutland <[email protected]>
Cc devicetree at vger.kernel.org
Signed-off-by Dong Aisheng <[email protected]>

Quoted 8 lines

---
v7->v8:
* new patch

---
 include/dt-bindings/firmware/imx/rsrc.h | 559 ++++++++++++++++++++++++++++++++
 1 file changed, 559 insertions(+)
 create mode 100644 include/dt-bindings/firmware/imx/rsrc.h

Reviewed-by Rob Herring <[email protected]>

A.s. Dong

author

2018-10-28 15:19 UTC #3 lore raw

PATCH v8 2/5 firmware: imx: remove resource id enums

In reply to A.s. Dong

We already export resource id in dt-bindings headfile which can also be used by drivers. So no need keep the same definitions in regular headfile anymore.

Cc Shawn Guo <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc Fabio Estevam <[email protected]>

Signed-off-by Dong Aisheng <[email protected]>

---
v7->v8:
* new patch

---
 include/linux/firmware/imx/types.h | 552 -------------------------------------
 1 file changed, 552 deletions(-)

include/linux/firmware/imx/types.h +0 ~~-552~~

index 9cbf0c4..8082110 100644
@@ -10,558 +10,6 @@
 #define _SC_TYPES_H
 
 /*
- * This type is used to indicate a resource. Resources include peripherals
- * and bus masters (but not memory regions). Note items from list should
- * never be changed or removed (only added to at the end of the list).
- */
-enum imx_sc_rsrc {
-	IMX_SC_R_A53 = 0,
-	IMX_SC_R_A53_0 = 1,
-	IMX_SC_R_A53_1 = 2,
-	IMX_SC_R_A53_2 = 3,
-	IMX_SC_R_A53_3 = 4,
-	IMX_SC_R_A72 = 5,
-	IMX_SC_R_A72_0 = 6,
-	IMX_SC_R_A72_1 = 7,
-	IMX_SC_R_A72_2 = 8,
-	IMX_SC_R_A72_3 = 9,
-	IMX_SC_R_CCI = 10,
-	IMX_SC_R_DB = 11,
-	IMX_SC_R_DRC_0 = 12,
-	IMX_SC_R_DRC_1 = 13,
-	IMX_SC_R_GIC_SMMU = 14,
-	IMX_SC_R_IRQSTR_M4_0 = 15,
-	IMX_SC_R_IRQSTR_M4_1 = 16,
-	IMX_SC_R_SMMU = 17,
-	IMX_SC_R_GIC = 18,
-	IMX_SC_R_DC_0_BLIT0 = 19,
-	IMX_SC_R_DC_0_BLIT1 = 20,
-	IMX_SC_R_DC_0_BLIT2 = 21,
-	IMX_SC_R_DC_0_BLIT_OUT = 22,
-	IMX_SC_R_DC_0_CAPTURE0 = 23,
-	IMX_SC_R_DC_0_CAPTURE1 = 24,
-	IMX_SC_R_DC_0_WARP = 25,
-	IMX_SC_R_DC_0_INTEGRAL0 = 26,
-	IMX_SC_R_DC_0_INTEGRAL1 = 27,
-	IMX_SC_R_DC_0_VIDEO0 = 28,
-	IMX_SC_R_DC_0_VIDEO1 = 29,
-	IMX_SC_R_DC_0_FRAC0 = 30,
-	IMX_SC_R_DC_0_FRAC1 = 31,
-	IMX_SC_R_DC_0 = 32,
-	IMX_SC_R_GPU_2_PID0 = 33,
-	IMX_SC_R_DC_0_PLL_0 = 34,
-	IMX_SC_R_DC_0_PLL_1 = 35,
-	IMX_SC_R_DC_1_BLIT0 = 36,
-	IMX_SC_R_DC_1_BLIT1 = 37,
-	IMX_SC_R_DC_1_BLIT2 = 38,
-	IMX_SC_R_DC_1_BLIT_OUT = 39,
-	IMX_SC_R_DC_1_CAPTURE0 = 40,
-	IMX_SC_R_DC_1_CAPTURE1 = 41,
-	IMX_SC_R_DC_1_WARP = 42,
-	IMX_SC_R_DC_1_INTEGRAL0 = 43,
-	IMX_SC_R_DC_1_INTEGRAL1 = 44,
-	IMX_SC_R_DC_1_VIDEO0 = 45,
-	IMX_SC_R_DC_1_VIDEO1 = 46,
-	IMX_SC_R_DC_1_FRAC0 = 47,
-	IMX_SC_R_DC_1_FRAC1 = 48,
-	IMX_SC_R_DC_1 = 49,
-	IMX_SC_R_GPU_3_PID0 = 50,
-	IMX_SC_R_DC_1_PLL_0 = 51,
-	IMX_SC_R_DC_1_PLL_1 = 52,
-	IMX_SC_R_SPI_0 = 53,
-	IMX_SC_R_SPI_1 = 54,
-	IMX_SC_R_SPI_2 = 55,
-	IMX_SC_R_SPI_3 = 56,
-	IMX_SC_R_UART_0 = 57,
-	IMX_SC_R_UART_1 = 58,
-	IMX_SC_R_UART_2 = 59,
-	IMX_SC_R_UART_3 = 60,
-	IMX_SC_R_UART_4 = 61,
-	IMX_SC_R_EMVSIM_0 = 62,
-	IMX_SC_R_EMVSIM_1 = 63,
-	IMX_SC_R_DMA_0_CH0 = 64,
-	IMX_SC_R_DMA_0_CH1 = 65,
-	IMX_SC_R_DMA_0_CH2 = 66,
-	IMX_SC_R_DMA_0_CH3 = 67,
-	IMX_SC_R_DMA_0_CH4 = 68,
-	IMX_SC_R_DMA_0_CH5 = 69,
-	IMX_SC_R_DMA_0_CH6 = 70,
-	IMX_SC_R_DMA_0_CH7 = 71,
-	IMX_SC_R_DMA_0_CH8 = 72,
-	IMX_SC_R_DMA_0_CH9 = 73,
-	IMX_SC_R_DMA_0_CH10 = 74,
-	IMX_SC_R_DMA_0_CH11 = 75,
-	IMX_SC_R_DMA_0_CH12 = 76,
-	IMX_SC_R_DMA_0_CH13 = 77,
-	IMX_SC_R_DMA_0_CH14 = 78,
-	IMX_SC_R_DMA_0_CH15 = 79,
-	IMX_SC_R_DMA_0_CH16 = 80,
-	IMX_SC_R_DMA_0_CH17 = 81,
-	IMX_SC_R_DMA_0_CH18 = 82,
-	IMX_SC_R_DMA_0_CH19 = 83,
-	IMX_SC_R_DMA_0_CH20 = 84,
-	IMX_SC_R_DMA_0_CH21 = 85,
-	IMX_SC_R_DMA_0_CH22 = 86,
-	IMX_SC_R_DMA_0_CH23 = 87,
-	IMX_SC_R_DMA_0_CH24 = 88,
-	IMX_SC_R_DMA_0_CH25 = 89,
-	IMX_SC_R_DMA_0_CH26 = 90,
-	IMX_SC_R_DMA_0_CH27 = 91,
-	IMX_SC_R_DMA_0_CH28 = 92,
-	IMX_SC_R_DMA_0_CH29 = 93,
-	IMX_SC_R_DMA_0_CH30 = 94,
-	IMX_SC_R_DMA_0_CH31 = 95,
-	IMX_SC_R_I2C_0 = 96,
-	IMX_SC_R_I2C_1 = 97,
-	IMX_SC_R_I2C_2 = 98,
-	IMX_SC_R_I2C_3 = 99,
-	IMX_SC_R_I2C_4 = 100,
-	IMX_SC_R_ADC_0 = 101,
-	IMX_SC_R_ADC_1 = 102,
-	IMX_SC_R_FTM_0 = 103,
-	IMX_SC_R_FTM_1 = 104,
-	IMX_SC_R_CAN_0 = 105,
-	IMX_SC_R_CAN_1 = 106,
-	IMX_SC_R_CAN_2 = 107,
-	IMX_SC_R_DMA_1_CH0 = 108,
-	IMX_SC_R_DMA_1_CH1 = 109,
-	IMX_SC_R_DMA_1_CH2 = 110,
-	IMX_SC_R_DMA_1_CH3 = 111,
-	IMX_SC_R_DMA_1_CH4 = 112,
-	IMX_SC_R_DMA_1_CH5 = 113,
-	IMX_SC_R_DMA_1_CH6 = 114,
-	IMX_SC_R_DMA_1_CH7 = 115,
-	IMX_SC_R_DMA_1_CH8 = 116,
-	IMX_SC_R_DMA_1_CH9 = 117,
-	IMX_SC_R_DMA_1_CH10 = 118,
-	IMX_SC_R_DMA_1_CH11 = 119,
-	IMX_SC_R_DMA_1_CH12 = 120,
-	IMX_SC_R_DMA_1_CH13 = 121,
-	IMX_SC_R_DMA_1_CH14 = 122,
-	IMX_SC_R_DMA_1_CH15 = 123,
-	IMX_SC_R_DMA_1_CH16 = 124,
-	IMX_SC_R_DMA_1_CH17 = 125,
-	IMX_SC_R_DMA_1_CH18 = 126,
-	IMX_SC_R_DMA_1_CH19 = 127,
-	IMX_SC_R_DMA_1_CH20 = 128,
-	IMX_SC_R_DMA_1_CH21 = 129,
-	IMX_SC_R_DMA_1_CH22 = 130,
-	IMX_SC_R_DMA_1_CH23 = 131,
-	IMX_SC_R_DMA_1_CH24 = 132,
-	IMX_SC_R_DMA_1_CH25 = 133,
-	IMX_SC_R_DMA_1_CH26 = 134,
-	IMX_SC_R_DMA_1_CH27 = 135,
-	IMX_SC_R_DMA_1_CH28 = 136,
-	IMX_SC_R_DMA_1_CH29 = 137,
-	IMX_SC_R_DMA_1_CH30 = 138,
-	IMX_SC_R_DMA_1_CH31 = 139,
-	IMX_SC_R_UNUSED1 = 140,
-	IMX_SC_R_UNUSED2 = 141,
-	IMX_SC_R_UNUSED3 = 142,
-	IMX_SC_R_UNUSED4 = 143,
-	IMX_SC_R_GPU_0_PID0 = 144,
-	IMX_SC_R_GPU_0_PID1 = 145,
-	IMX_SC_R_GPU_0_PID2 = 146,
-	IMX_SC_R_GPU_0_PID3 = 147,
-	IMX_SC_R_GPU_1_PID0 = 148,
-	IMX_SC_R_GPU_1_PID1 = 149,
-	IMX_SC_R_GPU_1_PID2 = 150,
-	IMX_SC_R_GPU_1_PID3 = 151,
-	IMX_SC_R_PCIE_A = 152,
-	IMX_SC_R_SERDES_0 = 153,
-	IMX_SC_R_MATCH_0 = 154,
-	IMX_SC_R_MATCH_1 = 155,
-	IMX_SC_R_MATCH_2 = 156,
-	IMX_SC_R_MATCH_3 = 157,
-	IMX_SC_R_MATCH_4 = 158,
-	IMX_SC_R_MATCH_5 = 159,
-	IMX_SC_R_MATCH_6 = 160,
-	IMX_SC_R_MATCH_7 = 161,
-	IMX_SC_R_MATCH_8 = 162,
-	IMX_SC_R_MATCH_9 = 163,
-	IMX_SC_R_MATCH_10 = 164,
-	IMX_SC_R_MATCH_11 = 165,
-	IMX_SC_R_MATCH_12 = 166,
-	IMX_SC_R_MATCH_13 = 167,
-	IMX_SC_R_MATCH_14 = 168,
-	IMX_SC_R_PCIE_B = 169,
-	IMX_SC_R_SATA_0 = 170,
-	IMX_SC_R_SERDES_1 = 171,
-	IMX_SC_R_HSIO_GPIO = 172,
-	IMX_SC_R_MATCH_15 = 173,
-	IMX_SC_R_MATCH_16 = 174,
-	IMX_SC_R_MATCH_17 = 175,
-	IMX_SC_R_MATCH_18 = 176,
-	IMX_SC_R_MATCH_19 = 177,
-	IMX_SC_R_MATCH_20 = 178,
-	IMX_SC_R_MATCH_21 = 179,
-	IMX_SC_R_MATCH_22 = 180,
-	IMX_SC_R_MATCH_23 = 181,
-	IMX_SC_R_MATCH_24 = 182,
-	IMX_SC_R_MATCH_25 = 183,
-	IMX_SC_R_MATCH_26 = 184,
-	IMX_SC_R_MATCH_27 = 185,
-	IMX_SC_R_MATCH_28 = 186,
-	IMX_SC_R_LCD_0 = 187,
-	IMX_SC_R_LCD_0_PWM_0 = 188,
-	IMX_SC_R_LCD_0_I2C_0 = 189,
-	IMX_SC_R_LCD_0_I2C_1 = 190,
-	IMX_SC_R_PWM_0 = 191,
-	IMX_SC_R_PWM_1 = 192,
-	IMX_SC_R_PWM_2 = 193,
-	IMX_SC_R_PWM_3 = 194,
-	IMX_SC_R_PWM_4 = 195,
-	IMX_SC_R_PWM_5 = 196,
-	IMX_SC_R_PWM_6 = 197,
-	IMX_SC_R_PWM_7 = 198,
-	IMX_SC_R_GPIO_0 = 199,
-	IMX_SC_R_GPIO_1 = 200,
-	IMX_SC_R_GPIO_2 = 201,
-	IMX_SC_R_GPIO_3 = 202,
-	IMX_SC_R_GPIO_4 = 203,
-	IMX_SC_R_GPIO_5 = 204,
-	IMX_SC_R_GPIO_6 = 205,
-	IMX_SC_R_GPIO_7 = 206,
-	IMX_SC_R_GPT_0 = 207,
-	IMX_SC_R_GPT_1 = 208,
-	IMX_SC_R_GPT_2 = 209,
-	IMX_SC_R_GPT_3 = 210,
-	IMX_SC_R_GPT_4 = 211,
-	IMX_SC_R_KPP = 212,
-	IMX_SC_R_MU_0A = 213,
-	IMX_SC_R_MU_1A = 214,
-	IMX_SC_R_MU_2A = 215,
-	IMX_SC_R_MU_3A = 216,
-	IMX_SC_R_MU_4A = 217,
-	IMX_SC_R_MU_5A = 218,
-	IMX_SC_R_MU_6A = 219,
-	IMX_SC_R_MU_7A = 220,
-	IMX_SC_R_MU_8A = 221,
-	IMX_SC_R_MU_9A = 222,
-	IMX_SC_R_MU_10A = 223,
-	IMX_SC_R_MU_11A = 224,
-	IMX_SC_R_MU_12A = 225,
-	IMX_SC_R_MU_13A = 226,
-	IMX_SC_R_MU_5B = 227,
-	IMX_SC_R_MU_6B = 228,
-	IMX_SC_R_MU_7B = 229,
-	IMX_SC_R_MU_8B = 230,
-	IMX_SC_R_MU_9B = 231,
-	IMX_SC_R_MU_10B = 232,
-	IMX_SC_R_MU_11B = 233,
-	IMX_SC_R_MU_12B = 234,
-	IMX_SC_R_MU_13B = 235,
-	IMX_SC_R_ROM_0 = 236,
-	IMX_SC_R_FSPI_0 = 237,
-	IMX_SC_R_FSPI_1 = 238,
-	IMX_SC_R_IEE = 239,
-	IMX_SC_R_IEE_R0 = 240,
-	IMX_SC_R_IEE_R1 = 241,
-	IMX_SC_R_IEE_R2 = 242,
-	IMX_SC_R_IEE_R3 = 243,
-	IMX_SC_R_IEE_R4 = 244,
-	IMX_SC_R_IEE_R5 = 245,
-	IMX_SC_R_IEE_R6 = 246,
-	IMX_SC_R_IEE_R7 = 247,
-	IMX_SC_R_SDHC_0 = 248,
-	IMX_SC_R_SDHC_1 = 249,
-	IMX_SC_R_SDHC_2 = 250,
-	IMX_SC_R_ENET_0 = 251,
-	IMX_SC_R_ENET_1 = 252,
-	IMX_SC_R_MLB_0 = 253,
-	IMX_SC_R_DMA_2_CH0 = 254,
-	IMX_SC_R_DMA_2_CH1 = 255,
-	IMX_SC_R_DMA_2_CH2 = 256,
-	IMX_SC_R_DMA_2_CH3 = 257,
-	IMX_SC_R_DMA_2_CH4 = 258,
-	IMX_SC_R_USB_0 = 259,
-	IMX_SC_R_USB_1 = 260,
-	IMX_SC_R_USB_0_PHY = 261,
-	IMX_SC_R_USB_2 = 262,
-	IMX_SC_R_USB_2_PHY = 263,
-	IMX_SC_R_DTCP = 264,
-	IMX_SC_R_NAND = 265,
-	IMX_SC_R_LVDS_0 = 266,
-	IMX_SC_R_LVDS_0_PWM_0 = 267,
-	IMX_SC_R_LVDS_0_I2C_0 = 268,
-	IMX_SC_R_LVDS_0_I2C_1 = 269,
-	IMX_SC_R_LVDS_1 = 270,
-	IMX_SC_R_LVDS_1_PWM_0 = 271,
-	IMX_SC_R_LVDS_1_I2C_0 = 272,
-	IMX_SC_R_LVDS_1_I2C_1 = 273,
-	IMX_SC_R_LVDS_2 = 274,
-	IMX_SC_R_LVDS_2_PWM_0 = 275,
-	IMX_SC_R_LVDS_2_I2C_0 = 276,
-	IMX_SC_R_LVDS_2_I2C_1 = 277,
-	IMX_SC_R_M4_0_PID0 = 278,
-	IMX_SC_R_M4_0_PID1 = 279,
-	IMX_SC_R_M4_0_PID2 = 280,
-	IMX_SC_R_M4_0_PID3 = 281,
-	IMX_SC_R_M4_0_PID4 = 282,
-	IMX_SC_R_M4_0_RGPIO = 283,
-	IMX_SC_R_M4_0_SEMA42 = 284,
-	IMX_SC_R_M4_0_TPM = 285,
-	IMX_SC_R_M4_0_PIT = 286,
-	IMX_SC_R_M4_0_UART = 287,
-	IMX_SC_R_M4_0_I2C = 288,
-	IMX_SC_R_M4_0_INTMUX = 289,
-	IMX_SC_R_M4_0_SIM = 290,
-	IMX_SC_R_M4_0_WDOG = 291,
-	IMX_SC_R_M4_0_MU_0B = 292,
-	IMX_SC_R_M4_0_MU_0A0 = 293,
-	IMX_SC_R_M4_0_MU_0A1 = 294,
-	IMX_SC_R_M4_0_MU_0A2 = 295,
-	IMX_SC_R_M4_0_MU_0A3 = 296,
-	IMX_SC_R_M4_0_MU_1A = 297,
-	IMX_SC_R_M4_1_PID0 = 298,
-	IMX_SC_R_M4_1_PID1 = 299,
-	IMX_SC_R_M4_1_PID2 = 300,
-	IMX_SC_R_M4_1_PID3 = 301,
-	IMX_SC_R_M4_1_PID4 = 302,
-	IMX_SC_R_M4_1_RGPIO = 303,
-	IMX_SC_R_M4_1_SEMA42 = 304,
-	IMX_SC_R_M4_1_TPM = 305,
-	IMX_SC_R_M4_1_PIT = 306,
-	IMX_SC_R_M4_1_UART = 307,
-	IMX_SC_R_M4_1_I2C = 308,
-	IMX_SC_R_M4_1_INTMUX = 309,
-	IMX_SC_R_M4_1_SIM = 310,
-	IMX_SC_R_M4_1_WDOG = 311,
-	IMX_SC_R_M4_1_MU_0B = 312,
-	IMX_SC_R_M4_1_MU_0A0 = 313,
-	IMX_SC_R_M4_1_MU_0A1 = 314,
-	IMX_SC_R_M4_1_MU_0A2 = 315,
-	IMX_SC_R_M4_1_MU_0A3 = 316,
-	IMX_SC_R_M4_1_MU_1A = 317,
-	IMX_SC_R_SAI_0 = 318,
-	IMX_SC_R_SAI_1 = 319,
-	IMX_SC_R_SAI_2 = 320,
-	IMX_SC_R_IRQSTR_SCU2 = 321,
-	IMX_SC_R_IRQSTR_DSP = 322,
-	IMX_SC_R_UNUSED5 = 323,
-	IMX_SC_R_UNUSED6 = 324,
-	IMX_SC_R_AUDIO_PLL_0 = 325,
-	IMX_SC_R_PI_0 = 326,
-	IMX_SC_R_PI_0_PWM_0 = 327,
-	IMX_SC_R_PI_0_PWM_1 = 328,
-	IMX_SC_R_PI_0_I2C_0 = 329,
-	IMX_SC_R_PI_0_PLL = 330,
-	IMX_SC_R_PI_1 = 331,
-	IMX_SC_R_PI_1_PWM_0 = 332,
-	IMX_SC_R_PI_1_PWM_1 = 333,
-	IMX_SC_R_PI_1_I2C_0 = 334,
-	IMX_SC_R_PI_1_PLL = 335,
-	IMX_SC_R_SC_PID0 = 336,
-	IMX_SC_R_SC_PID1 = 337,
-	IMX_SC_R_SC_PID2 = 338,
-	IMX_SC_R_SC_PID3 = 339,
-	IMX_SC_R_SC_PID4 = 340,
-	IMX_SC_R_SC_SEMA42 = 341,
-	IMX_SC_R_SC_TPM = 342,
-	IMX_SC_R_SC_PIT = 343,
-	IMX_SC_R_SC_UART = 344,
-	IMX_SC_R_SC_I2C = 345,
-	IMX_SC_R_SC_MU_0B = 346,
-	IMX_SC_R_SC_MU_0A0 = 347,
-	IMX_SC_R_SC_MU_0A1 = 348,
-	IMX_SC_R_SC_MU_0A2 = 349,
-	IMX_SC_R_SC_MU_0A3 = 350,
-	IMX_SC_R_SC_MU_1A = 351,
-	IMX_SC_R_SYSCNT_RD = 352,
-	IMX_SC_R_SYSCNT_CMP = 353,
-	IMX_SC_R_DEBUG = 354,
-	IMX_SC_R_SYSTEM = 355,
-	IMX_SC_R_SNVS = 356,
-	IMX_SC_R_OTP = 357,
-	IMX_SC_R_VPU_PID0 = 358,
-	IMX_SC_R_VPU_PID1 = 359,
-	IMX_SC_R_VPU_PID2 = 360,
-	IMX_SC_R_VPU_PID3 = 361,
-	IMX_SC_R_VPU_PID4 = 362,
-	IMX_SC_R_VPU_PID5 = 363,
-	IMX_SC_R_VPU_PID6 = 364,
-	IMX_SC_R_VPU_PID7 = 365,
-	IMX_SC_R_VPU_UART = 366,
-	IMX_SC_R_VPUCORE = 367,
-	IMX_SC_R_VPUCORE_0 = 368,
-	IMX_SC_R_VPUCORE_1 = 369,
-	IMX_SC_R_VPUCORE_2 = 370,
-	IMX_SC_R_VPUCORE_3 = 371,
-	IMX_SC_R_DMA_4_CH0 = 372,
-	IMX_SC_R_DMA_4_CH1 = 373,
-	IMX_SC_R_DMA_4_CH2 = 374,
-	IMX_SC_R_DMA_4_CH3 = 375,
-	IMX_SC_R_DMA_4_CH4 = 376,
-	IMX_SC_R_ISI_CH0 = 377,
-	IMX_SC_R_ISI_CH1 = 378,
-	IMX_SC_R_ISI_CH2 = 379,
-	IMX_SC_R_ISI_CH3 = 380,
-	IMX_SC_R_ISI_CH4 = 381,
-	IMX_SC_R_ISI_CH5 = 382,
-	IMX_SC_R_ISI_CH6 = 383,
-	IMX_SC_R_ISI_CH7 = 384,
-	IMX_SC_R_MJPEG_DEC_S0 = 385,
-	IMX_SC_R_MJPEG_DEC_S1 = 386,
-	IMX_SC_R_MJPEG_DEC_S2 = 387,
-	IMX_SC_R_MJPEG_DEC_S3 = 388,
-	IMX_SC_R_MJPEG_ENC_S0 = 389,
-	IMX_SC_R_MJPEG_ENC_S1 = 390,
-	IMX_SC_R_MJPEG_ENC_S2 = 391,
-	IMX_SC_R_MJPEG_ENC_S3 = 392,
-	IMX_SC_R_MIPI_0 = 393,
-	IMX_SC_R_MIPI_0_PWM_0 = 394,
-	IMX_SC_R_MIPI_0_I2C_0 = 395,
-	IMX_SC_R_MIPI_0_I2C_1 = 396,
-	IMX_SC_R_MIPI_1 = 397,
-	IMX_SC_R_MIPI_1_PWM_0 = 398,
-	IMX_SC_R_MIPI_1_I2C_0 = 399,
-	IMX_SC_R_MIPI_1_I2C_1 = 400,
-	IMX_SC_R_CSI_0 = 401,
-	IMX_SC_R_CSI_0_PWM_0 = 402,
-	IMX_SC_R_CSI_0_I2C_0 = 403,
-	IMX_SC_R_CSI_1 = 404,
-	IMX_SC_R_CSI_1_PWM_0 = 405,
-	IMX_SC_R_CSI_1_I2C_0 = 406,
-	IMX_SC_R_HDMI = 407,
-	IMX_SC_R_HDMI_I2S = 408,
-	IMX_SC_R_HDMI_I2C_0 = 409,
-	IMX_SC_R_HDMI_PLL_0 = 410,
-	IMX_SC_R_HDMI_RX = 411,
-	IMX_SC_R_HDMI_RX_BYPASS = 412,
-	IMX_SC_R_HDMI_RX_I2C_0 = 413,
-	IMX_SC_R_ASRC_0 = 414,
-	IMX_SC_R_ESAI_0 = 415,
-	IMX_SC_R_SPDIF_0 = 416,
-	IMX_SC_R_SPDIF_1 = 417,
-	IMX_SC_R_SAI_3 = 418,
-	IMX_SC_R_SAI_4 = 419,
-	IMX_SC_R_SAI_5 = 420,
-	IMX_SC_R_GPT_5 = 421,
-	IMX_SC_R_GPT_6 = 422,
-	IMX_SC_R_GPT_7 = 423,
-	IMX_SC_R_GPT_8 = 424,
-	IMX_SC_R_GPT_9 = 425,
-	IMX_SC_R_GPT_10 = 426,
-	IMX_SC_R_DMA_2_CH5 = 427,
-	IMX_SC_R_DMA_2_CH6 = 428,
-	IMX_SC_R_DMA_2_CH7 = 429,
-	IMX_SC_R_DMA_2_CH8 = 430,
-	IMX_SC_R_DMA_2_CH9 = 431,
-	IMX_SC_R_DMA_2_CH10 = 432,
-	IMX_SC_R_DMA_2_CH11 = 433,
-	IMX_SC_R_DMA_2_CH12 = 434,
-	IMX_SC_R_DMA_2_CH13 = 435,
-	IMX_SC_R_DMA_2_CH14 = 436,
-	IMX_SC_R_DMA_2_CH15 = 437,
-	IMX_SC_R_DMA_2_CH16 = 438,
-	IMX_SC_R_DMA_2_CH17 = 439,
-	IMX_SC_R_DMA_2_CH18 = 440,
-	IMX_SC_R_DMA_2_CH19 = 441,
-	IMX_SC_R_DMA_2_CH20 = 442,
-	IMX_SC_R_DMA_2_CH21 = 443,
-	IMX_SC_R_DMA_2_CH22 = 444,
-	IMX_SC_R_DMA_2_CH23 = 445,
-	IMX_SC_R_DMA_2_CH24 = 446,
-	IMX_SC_R_DMA_2_CH25 = 447,
-	IMX_SC_R_DMA_2_CH26 = 448,
-	IMX_SC_R_DMA_2_CH27 = 449,
-	IMX_SC_R_DMA_2_CH28 = 450,
-	IMX_SC_R_DMA_2_CH29 = 451,
-	IMX_SC_R_DMA_2_CH30 = 452,
-	IMX_SC_R_DMA_2_CH31 = 453,
-	IMX_SC_R_ASRC_1 = 454,
-	IMX_SC_R_ESAI_1 = 455,
-	IMX_SC_R_SAI_6 = 456,
-	IMX_SC_R_SAI_7 = 457,
-	IMX_SC_R_AMIX = 458,
-	IMX_SC_R_MQS_0 = 459,
-	IMX_SC_R_DMA_3_CH0 = 460,
-	IMX_SC_R_DMA_3_CH1 = 461,
-	IMX_SC_R_DMA_3_CH2 = 462,
-	IMX_SC_R_DMA_3_CH3 = 463,
-	IMX_SC_R_DMA_3_CH4 = 464,
-	IMX_SC_R_DMA_3_CH5 = 465,
-	IMX_SC_R_DMA_3_CH6 = 466,
-	IMX_SC_R_DMA_3_CH7 = 467,
-	IMX_SC_R_DMA_3_CH8 = 468,
-	IMX_SC_R_DMA_3_CH9 = 469,
-	IMX_SC_R_DMA_3_CH10 = 470,
-	IMX_SC_R_DMA_3_CH11 = 471,
-	IMX_SC_R_DMA_3_CH12 = 472,
-	IMX_SC_R_DMA_3_CH13 = 473,
-	IMX_SC_R_DMA_3_CH14 = 474,
-	IMX_SC_R_DMA_3_CH15 = 475,
-	IMX_SC_R_DMA_3_CH16 = 476,
-	IMX_SC_R_DMA_3_CH17 = 477,
-	IMX_SC_R_DMA_3_CH18 = 478,
-	IMX_SC_R_DMA_3_CH19 = 479,
-	IMX_SC_R_DMA_3_CH20 = 480,
-	IMX_SC_R_DMA_3_CH21 = 481,
-	IMX_SC_R_DMA_3_CH22 = 482,
-	IMX_SC_R_DMA_3_CH23 = 483,
-	IMX_SC_R_DMA_3_CH24 = 484,
-	IMX_SC_R_DMA_3_CH25 = 485,
-	IMX_SC_R_DMA_3_CH26 = 486,
-	IMX_SC_R_DMA_3_CH27 = 487,
-	IMX_SC_R_DMA_3_CH28 = 488,
-	IMX_SC_R_DMA_3_CH29 = 489,
-	IMX_SC_R_DMA_3_CH30 = 490,
-	IMX_SC_R_DMA_3_CH31 = 491,
-	IMX_SC_R_AUDIO_PLL_1 = 492,
-	IMX_SC_R_AUDIO_CLK_0 = 493,
-	IMX_SC_R_AUDIO_CLK_1 = 494,
-	IMX_SC_R_MCLK_OUT_0 = 495,
-	IMX_SC_R_MCLK_OUT_1 = 496,
-	IMX_SC_R_PMIC_0 = 497,
-	IMX_SC_R_PMIC_1 = 498,
-	IMX_SC_R_SECO = 499,
-	IMX_SC_R_CAAM_JR1 = 500,
-	IMX_SC_R_CAAM_JR2 = 501,
-	IMX_SC_R_CAAM_JR3 = 502,
-	IMX_SC_R_SECO_MU_2 = 503,
-	IMX_SC_R_SECO_MU_3 = 504,
-	IMX_SC_R_SECO_MU_4 = 505,
-	IMX_SC_R_HDMI_RX_PWM_0 = 506,
-	IMX_SC_R_A35 = 507,
-	IMX_SC_R_A35_0 = 508,
-	IMX_SC_R_A35_1 = 509,
-	IMX_SC_R_A35_2 = 510,
-	IMX_SC_R_A35_3 = 511,
-	IMX_SC_R_DSP = 512,
-	IMX_SC_R_DSP_RAM = 513,
-	IMX_SC_R_CAAM_JR1_OUT = 514,
-	IMX_SC_R_CAAM_JR2_OUT = 515,
-	IMX_SC_R_CAAM_JR3_OUT = 516,
-	IMX_SC_R_VPU_DEC_0 = 517,
-	IMX_SC_R_VPU_ENC_0 = 518,
-	IMX_SC_R_CAAM_JR0 = 519,
-	IMX_SC_R_CAAM_JR0_OUT = 520,
-	IMX_SC_R_PMIC_2 = 521,
-	IMX_SC_R_DBLOGIC = 522,
-	IMX_SC_R_HDMI_PLL_1 = 523,
-	IMX_SC_R_BOARD_R0 = 524,
-	IMX_SC_R_BOARD_R1 = 525,
-	IMX_SC_R_BOARD_R2 = 526,
-	IMX_SC_R_BOARD_R3 = 527,
-	IMX_SC_R_BOARD_R4 = 528,
-	IMX_SC_R_BOARD_R5 = 529,
-	IMX_SC_R_BOARD_R6 = 530,
-	IMX_SC_R_BOARD_R7 = 531,
-	IMX_SC_R_MJPEG_DEC_MP = 532,
-	IMX_SC_R_MJPEG_ENC_MP = 533,
-	IMX_SC_R_VPU_TS_0 = 534,
-	IMX_SC_R_VPU_MU_0 = 535,
-	IMX_SC_R_VPU_MU_1 = 536,
-	IMX_SC_R_VPU_MU_2 = 537,
-	IMX_SC_R_VPU_MU_3 = 538,
-	IMX_SC_R_VPU_ENC_1 = 539,
-	IMX_SC_R_VPU = 540,
-	IMX_SC_R_LAST
-};
-
-/* NOTE - please add by replacing some of the UNUSED from above! */
-
-/*
  * This type is used to indicate a control.
  */
 enum imx_sc_ctrl {

Signature · 1 line

2.7.4

A.s. Dong

author

2018-10-28 15:19 UTC #4 lore raw

PATCH v8 3/5 dt-bindings: fsl: scu: update power domain binding

In reply to A.s. Dong

Update the power domain binding with #power-domain-cells 1 format. The first cell can be a global SCU power domain and the 2nd cell the device ID. With this approach, we may remove all the sub power domain nodes from device tree which can relief the device tree a lot.

Cc Shawn Guo <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc Rob Herring <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc linux-pm at vger.kernel.org

Cc devicetree at vger.kernel.org

Signed-off-by Dong Aisheng <[email protected]>

---
ChangeLog:
v7->v8:
* new patch

---
 .../devicetree/bindings/arm/freescale/fsl,scu.txt  | 37 +++++-----------------
 1 file changed, 8 insertions(+), 29 deletions(-)

Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt +8 ~~-29~~

index 46d0af1..c20f38e 100644
@@ -58,19 +58,11 @@ This binding for the SCU power domain providers uses the generic power
 domain binding[2].
 
 Required properties:
-- compatible:		Should be "fsl,scu-pd".
-- #address-cells:	Should be 1.
-- #size-cells:		Should be 0.
-
-Required properties for power domain sub nodes:
-- #power-domain-cells:	Must be 0.
-
-Optional Properties:
-- reg:			Resource ID of this power domain.
-			No exist means uncontrollable by user.
+- compatible:		Should be "fsl,imx8qxp-scu-pd".
+- #power-domain-cells:	Must be 1. Contains the Resource ID used by
+			SCU commands.
 			See detailed Resource ID list from:
-			include/dt-bindings/power/imx-rsrc.h
-- power-domains:	phandle pointing to the parent power domain.
+			include/dt-bindings/firmware/imx/rsrc.h
 
 Clock bindings based on SCU Message Protocol
 ------------------------------------------------------------

@@ -152,22 +144,9 @@ firmware {
 			...
 		};
 
-		imx8qx-pm {
-			compatible = "fsl,scu-pd";
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			pd_dma: dma-power-domain {
-				#power-domain-cells = <0>;
-
-				pd_dma_lpuart0: dma-lpuart0 at 57 {
-					reg = <SC_R_UART_0>;
-					#power-domain-cells = <0>;
-					power-domains = <&pd_dma>;
-				};
-				...
-			};
-			...
+		pd: imx8qx-pd {
+			compatible = "fsl,imx8qxp-scu-pd";
+			#power-domain-cells = <1>;
 		};
 	};
 };

@@ -179,5 +158,5 @@ serial at 5a060000 {
 	clocks = <&clk IMX8QXP_UART0_CLK>,
 		 <&clk IMX8QXP_UART0_IPG_CLK>;
 	clock-names = "per", "ipg";
-	power-domains = <&pd_dma_lpuart0>;
+	power-domains = <&pd IMX_SC_R_UART_0>;
 };

Signature · 1 line

2.7.4

Rob Herring

maintainer

2018-10-30 19:30 UTC #5 lore raw

PATCH v8 3/5 dt-bindings: fsl: scu: update power domain binding

Quoted 13 lines · A.s. Dong

Update the power domain binding with #power-domain-cells 1 format. The first cell can be a global SCU power domain and the 2nd cell the device ID. With this approach, we may remove all the sub power domain nodes from device tree which can relief the device tree a lot.
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Rob Herring <[email protected]>
Cc Ulf Hansson <[email protected]>
Cc linux-pm at vger.kernel.org
Cc devicetree at vger.kernel.org
Suggested-by Rob Herring <[email protected]>
Signed-off-by Dong Aisheng <[email protected]>

Quoted 8 lines

---
ChangeLog:
v7->v8:
* new patch

---
 .../devicetree/bindings/arm/freescale/fsl,scu.txt  | 37 +++++-----------------
 1 file changed, 8 insertions(+), 29 deletions(-)

Reviewed-by Rob Herring <[email protected]>

A.s. Dong

author

2018-10-28 15:19 UTC #6 lore raw

PATCH v8 4/5 firmware: imx: add pm svc headfile

In reply to A.s. Dong

Add SCU PM SVC related protocol definitions which will be used by a number of PM functions like Power Domain, Clock, Reset and etc. The detailed implementation of each function will put in the individual function drivers.

Cc Shawn Guo <[email protected]>

Cc Sascha Hauer <[email protected]>

Signed-off-by Dong Aisheng <[email protected]>

---
v4->v8:
* no changes
v3->v4:
* update firmware headfile patch from include/soc/imx to include/linux/firmware/imx
v2->v3:
* name updated with IMX_SC prefix
v1->v2:
* new introduced

---
 include/linux/firmware/imx/sci.h    |  1 +
 include/linux/firmware/imx/svc/pm.h | 85 +++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 include/linux/firmware/imx/svc/pm.h

include/linux/firmware/imx/sci.h +1 -0

index 29ada60..ebc5509 100644
@@ -14,4 +14,5 @@
 #include <linux/firmware/imx/types.h>
 
 #include <linux/firmware/imx/svc/misc.h>
+#include <linux/firmware/imx/svc/pm.h>
 #endif /* _SC_SCI_H */

include/linux/firmware/imx/svc/pm.h new file +85 -0

85 lines not on this page · raw message

Signature · 1 line

2.7.4

A.s. Dong

author

2018-10-28 15:19 UTC #7 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

In reply to A.s. Dong

Some i.MX SoCs contain a system controller that is responsible for controlling the state of the IPs that are present. Communication between the host processor running an OS and the system controller happens through a SCU protocol. This patch adds SCU protocol based power domains drivers.

Cc Ulf Hansson <[email protected]>

Cc Rob Herring <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc linux-pm at vger.kernel.org

Signed-off-by Dong Aisheng <[email protected]>

---
ChangeLog:
v7->v8:
* update to #power-domain-cells 1 binding
v6->v7:
* still using generic "fsl,scu-pd" to driver binding incase the future SoC is compatible
* remove unnecessary cast
v5->v6:
* only compatible string name updated from fsl,scu-pd to fsl,imx8qxp-scu-pd accordingly
v4->v5:
* minor improvements according to Sascha's suggestions
v3->v4:
* update firmware headfile patch from include/soc/imx to include/linux/firmware/imx
v2->v3:
* name of structures/enums updated with imx_sc prefix
v1->v2:
* move into drivers/firmware/imx
* Implement sc_pm_set_resource_power_mode() API in the driver instead of call it via SCU API according to Sascha's suggestion

---
 drivers/firmware/imx/Kconfig  |   6 +
 drivers/firmware/imx/Makefile |   3 +-
 drivers/firmware/imx/scu-pd.c | 302 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 310 insertions(+), 1 deletion(-)
 create mode 100644 drivers/firmware/imx/scu-pd.c

drivers/firmware/imx/Kconfig +6 -0

index b170c28..6a7a7c2 100644
@@ -9,3 +9,9 @@ config IMX_SCU
 
 	  This driver manages the IPC interface between host CPU and the
 	  SCU firmware running on M4.
+
+config IMX_SCU_PD
+	bool "IMX SCU Power Domain driver"
+	depends on IMX_SCU
+	help
+	  The System Controller Firmware (SCFW) based power domain driver.

drivers/firmware/imx/Makefile +2 -1

index 0ac04df..1b2e15b 100644
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_IMX_SCU)	+= imx-scu.o misc.o
+obj-$(CONFIG_IMX_SCU)		+= imx-scu.o misc.o
+obj-$(CONFIG_IMX_SCU_PD)	+= scu-pd.o

drivers/firmware/imx/scu-pd.c new file +302 -0

302 lines not on this page · raw message

Signature · 1 line

2.7.4

Ulf Hansson

2018-10-29 11:43 UTC #8 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

[...]

}

Quoted 5 lines

drivers/firmware/imx/scu-pd.c new file +5 -0

+
+static struct imx_sc_pm_domain *
+imx_scu_add_pm_domain(struct device *dev, int idx,
+                     const struct imx_sc_pd_range *pd_ranges)
+{

drivers/firmware/imx/scu-pd.c line 198

Quoted 10 lines

drivers/firmware/imx/scu-pd.c new file +10 -0

+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;

So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better option.

What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally by the FW, no?

In any case, it looks like what you should do is to implement the ->attach|detach_dev() callback for the genpd and use the regular of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the "resource id" and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing these kind of things).

Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above ->power_on|off() callbacks.

Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier version.

Quoted 18 lines

drivers/firmware/imx/scu-pd.c new file +18 -0

+
+       if (pd_ranges->postfix)
+               snprintf(sc_pd->name, sizeof(sc_pd->name),
+                        "%s%i", pd_ranges->name, idx);
+       else
+               snprintf(sc_pd->name, sizeof(sc_pd->name),
+                        "%s", pd_ranges->name);
+
+       sc_pd->pd.name = sc_pd->name;
+
+       if (sc_pd->rsrc >= IMX_SC_R_LAST) {
+               dev_warn(dev, "invalid pd %s rsrc id %d found",
+                        sc_pd->name, sc_pd->rsrc);
+
+               devm_kfree(dev, sc_pd);
+               return NULL;
+       }
+

drivers/firmware/imx/scu-pd.c line 226

Quoted 10 lines

drivers/firmware/imx/scu-pd.c new file +10 -0

+       ret = pm_genpd_init(&sc_pd->pd, NULL, true);
+       if (ret) {
+               dev_warn(dev, "failed to init pd %s rsrc id %d",
+                        sc_pd->name, sc_pd->rsrc);
+               devm_kfree(dev, sc_pd);
+               return NULL;
+       }
+
+       return sc_pd;
+}

[...]

Kind regards
Uffe

A.s. Dong

author

2018-10-30 13:20 UTC #9 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

[...]

Quoted 19 lines

}

Quoted 4 lines

drivers/firmware/imx/scu-pd.c new file +4 -0

+
+static struct imx_sc_pm_domain *
+imx_scu_add_pm_domain(struct device *dev, int idx,
+                     const struct imx_sc_pd_range *pd_ranges) {

drivers/firmware/imx/scu-pd.c line 198

Quoted 10 lines

drivers/firmware/imx/scu-pd.c new file +10 -0

+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;

So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better option.

Quoted 3 lines

What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally by the FW, no?

Yes, you're right.

Quoted 8 lines

In any case, it looks like what you should do is to implement the ->attach|detach_dev() callback for the genpd and use the regular of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the "resource id" and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing these kind of things).

Quoted 7 lines

Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above ->power_on|off() callbacks.
Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier version.

Appreciated for such detailed explanation. lt's a good suggestion and I really like to switch to it. However, after digged a bit more, i found a few blocking issues:
1. The .attach_dev() of power domain infrastructure still does not support multi domains case.

It looks like there's no way for .attach_dev() to understand which sub 'power domain' the device is attaching for one global power domain provider like ti_sci as you suggested. Secondly, the struct device *dev passed into is a virtual PD device. It does not help for parsing the real device resource id.

So framework probably needs some extension to support multi power domain cases.

2. It also breaks most of current drivers as the driver probe sequence behavior changed If removing .power_on() and .power_off() callback and use .start() and .stop() instead.

genpd_dev_pm_attach will only power up the domain and attach device, but will not call .start() which relies on device runtime pm. That means the device power is still not up before running driver probe function. For SCU enabled platforms, all device drivers accessing registers/clock without power domain enabled will trigger a HW access error. That means we need fix most of drivers probe sequence with proper runtime pm. So I'm a bit wondering whether we should still keep the exist way.

In summary, we probably may not be able to try ti_sci way before fixing above two issues.

Do you think I should wait for them to be fixed or if we could use the current way at first? I might be a little intend to the second way as we now have a lot upstreaming work pending on this. But please let me know if you have a different idea.

Regards
Dong Aisheng

Quoted 33 lines

Quoted 18 lines

drivers/firmware/imx/scu-pd.c new file +18 -0

+
+       if (pd_ranges->postfix)
+               snprintf(sc_pd->name, sizeof(sc_pd->name),
+                        "%s%i", pd_ranges->name, idx);
+       else
+               snprintf(sc_pd->name, sizeof(sc_pd->name),
+                        "%s", pd_ranges->name);
+
+       sc_pd->pd.name = sc_pd->name;
+
+       if (sc_pd->rsrc >= IMX_SC_R_LAST) {
+               dev_warn(dev, "invalid pd %s rsrc id %d found",
+                        sc_pd->name, sc_pd->rsrc);
+
+               devm_kfree(dev, sc_pd);
+               return NULL;
+       }
+

drivers/firmware/imx/scu-pd.c line 226

Quoted 10 lines

drivers/firmware/imx/scu-pd.c new file +10 -0

+       ret = pm_genpd_init(&sc_pd->pd, NULL, true);
+       if (ret) {
+               dev_warn(dev, "failed to init pd %s rsrc id %d",
+                        sc_pd->name, sc_pd->rsrc);
+               devm_kfree(dev, sc_pd);
+               return NULL;
+       }
+
+       return sc_pd;
+}

[...]

Kind regards
Uffe

Ulf Hansson

2018-10-30 15:59 UTC #10 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

Quoted 47 lines · A.s. Dong

[...]
Quoted 19 lines
}
Quoted 4 lines
drivers/firmware/imx/scu-pd.c new file +4 -0
+
+static struct imx_sc_pm_domain *
+imx_scu_add_pm_domain(struct device *dev, int idx,
+                     const struct imx_sc_pd_range *pd_ranges) {
drivers/firmware/imx/scu-pd.c line 198
Quoted 10 lines
drivers/firmware/imx/scu-pd.c new file +10 -0
+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;
So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better option.
Quoted 3 lines
What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally by the FW, no?
Yes, you're right.
Quoted 8 lines
In any case, it looks like what you should do is to implement the ->attach|detach_dev() callback for the genpd and use the regular of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the "resource id" and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing these kind of things).
Quoted 7 lines
Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above ->power_on|off() callbacks.
Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier version.
Appreciated for such detailed explanation. lt's a good suggestion and I really like to switch to it. However, after digged a bit more, i found a few blocking issues:
1. The .attach_dev() of power domain infrastructure still does not support multi domains case.

Quoted 6 lines

It looks like there's no way for .attach_dev() to understand which sub 'power domain' the device is attaching for one global power domain provider like ti_sci as you suggested. Secondly, the struct device *dev passed into is a virtual PD device. It does not help for parsing the real device resource id.
So framework probably needs some extension to support multi power domain cases.

Right, you are correct.

Let me think about this and see what I can come up with - unless you already have something you want to propose?

Quoted 10 lines

2. It also breaks most of current drivers as the driver probe sequence behavior changed If removing .power_on() and .power_off() callback and use .start() and .stop() instead.
genpd_dev_pm_attach will only power up the domain and attach device, but will not call .start() which relies on device runtime pm. That means the device power is still not up before running driver probe function. For SCU enabled platforms, all device drivers accessing registers/clock without power domain enabled will trigger a HW access error. That means we need fix most of drivers probe sequence with proper runtime pm. So I'm a bit wondering whether we should still keep the exist way.

I see what you are trying to say, but I am not sure why you consider this as they would break? How did they work in the first place?

Anyway, the problem you are talking about have so far been addressed by making buses/drivers to call pm_runtime_enable() and pm_runtime_get_sync() - before trying to probe their devices.

Do you have an idea of the amount of drivers that needs to fixed, in regards to this?

I guess an option could be to, that via the ->attach_dev() callback perform the same operations as also being done during ->start(). Of course, you may need to keep a flag about this being done, so that operations that need to be balanced with get/put or the like is managed correctly, the next time ->start|stop() becomes called.

Quoted 6 lines

In summary, we probably may not be able to try ti_sci way before fixing above two issues.
Do you think I should wait for them to be fixed or if we could use the current way at first? I might be a little intend to the second way as we now have a lot upstreaming work pending on this. But please let me know if you have a different idea.

As this is also about deploying a DTB with a correctly modeled HW, I think we need to fix the above issues.

I am happy to help, in one way or the other.

[...]

Kind regards
Uffe

A.s. Dong

author

2018-10-31 01:45 UTC #11 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

Quoted 3 lines

-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 12:00 AM

[...]

Quoted 60 lines · A.s. Dong

Quoted 21 lines
[...]
Quoted 19 lines
}
Quoted 4 lines
drivers/firmware/imx/scu-pd.c new file +4 -0
+
+static struct imx_sc_pm_domain *
+imx_scu_add_pm_domain(struct device *dev, int idx,
+                     const struct imx_sc_pd_range *pd_ranges) {
drivers/firmware/imx/scu-pd.c line 198
Quoted 10 lines
drivers/firmware/imx/scu-pd.c new file +10 -0
+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;
So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better option.
Quoted 2 lines
Quoted 2 lines
What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally
by the FW, no?
Quoted 19 lines
Quoted 1 line
Yes, you're right.
Quoted 15 lines
In any case, it looks like what you should do is to implement the ->attach|detach_dev() callback for the genpd and use the regular of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the "resource id" and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing these kind of things).
Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above
->power_on|off() callbacks.
Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier
version.
Quoted 4 lines
Quoted 1 line
Appreciated for such detailed explanation. lt's a good suggestion and I really like to switch to it. However, after digged a bit more, i found a few
blocking issues:
Quoted 1 line
1. The .attach_dev() of power domain infrastructure still does not support
multi domains case.
Quoted 2 lines
It looks like there's no way for .attach_dev() to understand which sub 'power
domain'
Quoted 1 line
the device is attaching for one global power domain provider like ti_sci as
you suggested.
Quoted 3 lines
Secondly, the struct device *dev passed into is a virtual PD device. It does not help for parsing the real device resource id.

Quoted 8 lines

Quoted 1 line
So framework probably needs some extension to support multi power
domain cases.
Right, you are correct.
Let me think about this and see what I can come up with - unless you already have something you want to propose?

I have thought we may need add two more params (real dev and pd index) for .attach_dev().detach_dev() and .start()/.stop() for multi PDs case. But I'm not sure if they are proper.

Quoted 11 lines

Quoted 3 lines
2. It also breaks most of current drivers as the driver probe sequence behavior changed If removing .power_on() and .power_off() callback and
use .start() and .stop() instead.
Quoted 7 lines
genpd_dev_pm_attach will only power up the domain and attach device, but will not call .start() which relies on device runtime pm. That means the device power is still not up before running driver probe function. For SCU enabled platforms, all device drivers accessing registers/clock without power domain enabled will trigger a HW access error. That means we need fix most of drivers probe sequence with proper

Quoted 5 lines

runtime pm. So I'm a bit wondering whether we should still keep the exist way.
I see what you are trying to say, but I am not sure why you consider this as they would break? How did they work in the first place?

Most drivers does not require power domains before. E.g. GPIO, I2C, PWM, MMC. Or even we need power domain(PUs/PCIE), they're enabled in .power_on()/.power_off() which has been already up before probe.

Quoted 4 lines

Anyway, the problem you are talking about have so far been addressed by making buses/drivers to call pm_runtime_enable() and pm_runtime_get_sync() - before trying to probe their devices.

Yes, called them early before accessing clock and hw registers.

Quoted 3 lines

Do you have an idea of the amount of drivers that needs to fixed, in regards to this?

So far all enabled devices I tested need fixes. GPIO/I2C/FEC/MMC and etc. Suppose other need the same fix as we have not met this situation before.

Quoted 6 lines

I guess an option could be to, that via the ->attach_dev() callback perform the same operations as also being done during ->start(). Of course, you may need to keep a flag about this being done, so that operations that need to be balanced with get/put or the like is managed correctly, the next time ->start|stop() becomes called.

That might be a way to try. The follow seems may introduce a bit complexities and driver needs to maintainer the runtime status along with framework in order to keep the balance?

Quoted 4 lines

Quoted 2 lines
In summary, we probably may not be able to try ti_sci way before fixing
above two issues.
Quoted 1 line

Quoted 9 lines

Quoted 1 line
Do you think I should wait for them to be fixed or if we could use the current
way at first?
Quoted 2 lines
I might be a little intend to the second way as we now have a lot upstreaming work pending on this. But please let me know if you have a
different idea.
As this is also about deploying a DTB with a correctly modeled HW, I think we need to fix the above issues.

DTB are the same with #power-domain-cells 1 (resource ID) in either case. So DTB don't need change and driver could be optimized.

Quoted 2 lines

I am happy to help, in one way or the other.

Thanks.

Regards
Dong Aisheng

Quoted 4 lines

[...]
Kind regards
Uffe

Ulf Hansson

2018-10-31 15:55 UTC #12 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

Quoted 69 lines · A.s. Dong

Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 12:00 AM
[...]
Quoted 65 lines · A.s. Dong
Quoted 21 lines
[...]
Quoted 19 lines
}
Quoted 4 lines
drivers/firmware/imx/scu-pd.c new file +4 -0
+
+static struct imx_sc_pm_domain *
+imx_scu_add_pm_domain(struct device *dev, int idx,
+                     const struct imx_sc_pd_range *pd_ranges) {
drivers/firmware/imx/scu-pd.c line 198
Quoted 10 lines
drivers/firmware/imx/scu-pd.c new file +10 -0
+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;
So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better option.
Quoted 2 lines
Quoted 2 lines
What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally
by the FW, no?
Quoted 19 lines
Quoted 1 line
Yes, you're right.
Quoted 15 lines
In any case, it looks like what you should do is to implement the ->attach|detach_dev() callback for the genpd and use the regular of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the "resource id" and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing these kind of things).
Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above
->power_on|off() callbacks.
Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier
version.
Quoted 4 lines
Quoted 1 line
Appreciated for such detailed explanation. lt's a good suggestion and I really like to switch to it. However, after digged a bit more, i found a few
blocking issues:
Quoted 1 line
1. The .attach_dev() of power domain infrastructure still does not support
multi domains case.
Quoted 2 lines
It looks like there's no way for .attach_dev() to understand which sub 'power
domain'
Quoted 1 line
the device is attaching for one global power domain provider like ti_sci as
you suggested.
Quoted 4 lines
Secondly, the struct device *dev passed into is a virtual PD device. It does not help for parsing the real device resource id.
So framework probably needs some extension to support multi power
domain cases.
Right, you are correct.

Quoted 7 lines

Quoted 3 lines
Let me think about this and see what I can come up with - unless you already have something you want to propose?
I have thought we may need add two more params (real dev and pd index) for .attach_dev().detach_dev() and .start()/.stop() for multi PDs case. But I'm not sure if they are proper.

Right.

I don't think we need to change the definition of the callbacks. However, from within the callbacks (or at least the ->attach_dev()) we need access to the data you suggest above.

To make that data available, genpd needs to store it in the per device struct generic_pm_domain_data, but of course before invoking the ->attach_dev() callback.

Quoted 28 lines

Quoted 13 lines
Quoted 5 lines
2. It also breaks most of current drivers as the driver probe sequence behavior changed If removing .power_on() and .power_off() callback and
use .start() and .stop() instead.
Quoted 7 lines
genpd_dev_pm_attach will only power up the domain and attach device, but will not call .start() which relies on device runtime pm. That means the device power is still not up before running driver probe function. For SCU enabled platforms, all device drivers accessing registers/clock without power domain enabled will trigger a HW access error. That means we need fix most of drivers probe sequence with proper
Quoted 5 lines
runtime pm. So I'm a bit wondering whether we should still keep the exist way.
I see what you are trying to say, but I am not sure why you consider this as they would break? How did they work in the first place?
Most drivers does not require power domains before. E.g. GPIO, I2C, PWM, MMC. Or even we need power domain(PUs/PCIE), they're enabled in .power_on()/.power_off() which has been already up before probe.
Quoted 4 lines
Anyway, the problem you are talking about have so far been addressed by making buses/drivers to call pm_runtime_enable() and pm_runtime_get_sync() - before trying to probe their devices.

Quoted 8 lines

Yes, called them early before accessing clock and hw registers.
Quoted 3 lines
Do you have an idea of the amount of drivers that needs to fixed, in regards to this?
So far all enabled devices I tested need fixes. GPIO/I2C/FEC/MMC and etc. Suppose other need the same fix as we have not met this situation before.

Okay, I see.

Quoted 11 lines

Quoted 7 lines
I guess an option could be to, that via the ->attach_dev() callback perform the same operations as also being done during ->start(). Of course, you may need to keep a flag about this being done, so that operations that need to be balanced with get/put or the like is managed correctly, the next time ->start|stop() becomes called.
That might be a way to try. The follow seems may introduce a bit complexities and driver needs to maintainer the runtime status along with framework in order to keep the balance?

Yeah, it's likely not going to be very elegant, but should be rather self contained in the PM domain driver and could serve as a way forward - while working on fixing the drivers long term.

Quoted 9 lines

Quoted 9 lines
Quoted 3 lines
In summary, we probably may not be able to try ti_sci way before fixing
above two issues.
Quoted 2 lines
Do you think I should wait for them to be fixed or if we could use the current
way at first?
Quoted 2 lines
I might be a little intend to the second way as we now have a lot upstreaming work pending on this. But please let me know if you have a

Quoted 8 lines

Quoted 5 lines
different idea.
As this is also about deploying a DTB with a correctly modeled HW, I think we need to fix the above issues.
DTB are the same with #power-domain-cells 1 (resource ID) in either case. So DTB don't need change and driver could be optimized.

You are correct.

Although, how does $subject patch solves the case where a device may have multiple PM domains attached to it? Or is left as a future improvement?

Kind regards
Uffe

A.s. Dong

author

2018-10-31 17:24 UTC #13 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

Quoted 3 lines

-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 11:56 PM

[...]

Quoted 87 lines · A.s. Dong

Quoted 75 lines
Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 12:00 AM
[...]
Quoted 71 lines · A.s. Dong
Quoted 21 lines
[...]
Quoted 19 lines
}
Quoted 4 lines
drivers/firmware/imx/scu-pd.c new file +4 -0
+
+static struct imx_sc_pm_domain * imx_scu_add_pm_domain(struct
+device *dev, int idx,
+                     const struct imx_sc_pd_range *pd_ranges) {
drivers/firmware/imx/scu-pd.c line 198
Quoted 10 lines
drivers/firmware/imx/scu-pd.c new file +10 -0
+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;
So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better option.
Quoted 3 lines
Quoted 3 lines
What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally
by the FW, no?
Quoted 20 lines
Quoted 1 line
Yes, you're right.
Quoted 16 lines
In any case, it looks like what you should do is to implement the ->attach|detach_dev() callback for the genpd and use the regular of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the "resource id" and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing these kind of things).
Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above
->power_on|off() callbacks.
Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier
version.
Quoted 5 lines
Quoted 1 line
Appreciated for such detailed explanation. lt's a good suggestion and I really like to switch to it. However, after digged a bit more, i found a few
blocking issues:
Quoted 2 lines
1. The .attach_dev() of power domain infrastructure still does not support
multi domains case.
Quoted 3 lines
It looks like there's no way for .attach_dev() to understand which sub 'power
domain'
Quoted 2 lines
the device is attaching for one global power domain provider like ti_sci as
you suggested.
Quoted 4 lines
Secondly, the struct device *dev passed into is a virtual PD device. It does not help for parsing the real device resource id.
So framework probably needs some extension to support multi power
domain cases.
Right, you are correct.
Quoted 8 lines
Quoted 3 lines
Let me think about this and see what I can come up with - unless you already have something you want to propose?
I have thought we may need add two more params (real dev and pd index) for
.attach_dev().detach_dev() and .start()/.stop() for multi PDs case. But I'm not sure if they are proper.
Right.

Quoted 8 lines

I don't think we need to change the definition of the callbacks. However, from within the callbacks (or at least the ->attach_dev()) we need access to the data you suggest above.
To make that data available, genpd needs to store it in the per device struct generic_pm_domain_data, but of course before invoking the ->attach_dev() callback.

Yes, that's an option.
Currently gpd_data is allocated in genpd_add_device() which looks like to be designed to attach device to the specified power domain passed in, so it's unware of whether it's multi domains or not. Then we may still need fine tune those functions in different abstract layers.

Quoted 78 lines

Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
2. It also breaks most of current drivers as the driver probe sequence behavior changed If removing .power_on() and .power_off() callback and
use .start() and .stop() instead.
Quoted 1 line
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
genpd_dev_pm_attach will only power up the domain and attach device, but will not call .start() which relies on device runtime pm. That means the device power is still not up before running driver probe function. For SCU enabled platforms, all device drivers accessing registers/clock without power domain enabled will trigger a HW access error. That means we need fix most of drivers probe sequence with proper
runtime pm. So I'm a bit wondering whether we should still keep the exist
way.
Quoted 6 lines
Quoted 4 lines
I see what you are trying to say, but I am not sure why you consider this as they would break? How did they work in the first place?
Most drivers does not require power domains before. E.g. GPIO, I2C, PWM,
MMC.
Quoted 8 lines
Or even we need power domain(PUs/PCIE), they're enabled in .power_on()/.power_off() which has been already up before probe.
Quoted 4 lines
Anyway, the problem you are talking about have so far been addressed by making buses/drivers to call pm_runtime_enable() and pm_runtime_get_sync() - before trying to probe their devices.
Quoted 8 lines
Yes, called them early before accessing clock and hw registers.
Quoted 3 lines
Do you have an idea of the amount of drivers that needs to fixed, in regards to this?
So far all enabled devices I tested need fixes. GPIO/I2C/FEC/MMC and etc. Suppose other need the same fix as we have not met this situation before.
Okay, I see.
Quoted 9 lines
Quoted 8 lines
I guess an option could be to, that via the ->attach_dev() callback perform the same operations as also being done during ->start(). Of course, you may need to keep a flag about this being done, so that operations that need to be balanced with get/put or the like is managed correctly, the next time
->start|stop() becomes called.
Quoted 3 lines
That might be a way to try. The follow seems may introduce a bit complexities and driver needs to maintainer the runtime status along with framework in order to keep the balance?
Yeah, it's likely not going to be very elegant, but should be rather self contained in the PM domain driver and could serve as a way forward - while working on fixing the drivers long term.
Quoted 18 lines
Quoted 17 lines
Quoted 4 lines
In summary, we probably may not be able to try ti_sci way before fixing
above two issues.
Quoted 3 lines
Do you think I should wait for them to be fixed or if we could use the current
way at first?
Quoted 3 lines
I might be a little intend to the second way as we now have a lot upstreaming work pending on this. But please let me know if you have a
different idea.
As this is also about deploying a DTB with a correctly modeled HW, I think we need to fix the above issues.

Quoted 8 lines

Quoted 2 lines
DTB are the same with #power-domain-cells 1 (resource ID) in either case. So DTB don't need change and driver could be optimized.
You are correct.
Although, how does $subject patch solves the case where a device may have multiple PM domains attached to it? Or is left as a future improvement?

IIRC this patch does not have such issue for multi PM domains as we have individual power domains for each resources. Devices will be attached to each individual domains in __genpd_dev_pm_attach rather than the single one global virtual SCU domain. Furthermore we're using domains .power_on()/ power_off() callback which means the power will be up before the probe. (no the issue like when using .attach_dev()/.detach_dev() and run .start()/stop() via runtime pm).

Actually I'm a bit wondering that if abstracting SCU domains into one global virtual domain is certainly better than individual domains. For SCU based SoCs, the power domain service is provided by SCU firmware, users don't have to care too much about the HW internals. And according to SCU definition, each devices is associated with a power domain, devices should treat it as a separate domain and have to enable it before accessing HW. So one single global domain looks like not exactly as SCU firmware defines. Not quite sure, but a bit feel like that....

Anyway, give us more time to fine tune it in the future might be a choice IMHO.

Thanks

Regards
Dong Aisheng

Quoted 2 lines

Kind regards
Uffe

Ulf Hansson

2018-10-31 21:48 UTC #14 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

Quoted 100 lines · A.s. Dong

Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 11:56 PM
[...]
Quoted 87 lines · A.s. Dong
Quoted 75 lines
Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 12:00 AM
[...]
Quoted 71 lines · A.s. Dong
Quoted 21 lines
[...]
Quoted 19 lines
}
Quoted 4 lines
(quoted diff) +4 -0
+
+static struct imx_sc_pm_domain * imx_scu_add_pm_domain(struct
+device *dev, int idx,
+                     const struct imx_sc_pd_range *pd_ranges) {
Review
Quoted 10 lines
(quoted diff) +10 -0
+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;
So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better option.
Quoted 3 lines
Quoted 3 lines
What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally
by the FW, no?
Quoted 20 lines
Quoted 1 line
Yes, you're right.
Quoted 16 lines
In any case, it looks like what you should do is to implement the ->attach|detach_dev() callback for the genpd and use the regular of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the "resource id" and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing these kind of things).
Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above
->power_on|off() callbacks.
Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier
version.
Quoted 5 lines
Quoted 1 line
Appreciated for such detailed explanation. lt's a good suggestion and I really like to switch to it. However, after digged a bit more, i found a few
blocking issues:
Quoted 2 lines
1. The .attach_dev() of power domain infrastructure still does not support
multi domains case.
Quoted 3 lines
It looks like there's no way for .attach_dev() to understand which sub 'power
domain'
Quoted 2 lines
the device is attaching for one global power domain provider like ti_sci as
you suggested.
Quoted 4 lines
Secondly, the struct device *dev passed into is a virtual PD device. It does not help for parsing the real device resource id.
So framework probably needs some extension to support multi power
domain cases.
Right, you are correct.
Quoted 8 lines
Quoted 3 lines
Let me think about this and see what I can come up with - unless you already have something you want to propose?
I have thought we may need add two more params (real dev and pd index) for
.attach_dev().detach_dev() and .start()/.stop() for multi PDs case. But I'm not sure if they are proper.
Right.
Quoted 8 lines
I don't think we need to change the definition of the callbacks. However, from within the callbacks (or at least the ->attach_dev()) we need access to the data you suggest above.
To make that data available, genpd needs to store it in the per device struct generic_pm_domain_data, but of course before invoking the ->attach_dev() callback.

Quoted 5 lines

Yes, that's an option.
Currently gpd_data is allocated in genpd_add_device() which looks like to be designed to attach device to the specified power domain passed in, so it's unware of whether it's multi domains or not. Then we may still need fine tune those functions in different abstract layers.

Yes, something like that. I don't think it should be that hard, I can have a stab at it of you like?

Quoted 88 lines

Quoted 79 lines
Quoted 9 lines
Quoted 9 lines
Quoted 7 lines
2. It also breaks most of current drivers as the driver probe sequence behavior changed If removing .power_on() and .power_off() callback and
use .start() and .stop() instead.
Quoted 1 line
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
genpd_dev_pm_attach will only power up the domain and attach device, but will not call .start() which relies on device runtime pm. That means the device power is still not up before running driver probe function. For SCU enabled platforms, all device drivers accessing registers/clock without power domain enabled will trigger a HW access error. That means we need fix most of drivers probe sequence with proper
runtime pm. So I'm a bit wondering whether we should still keep the exist
way.
Quoted 6 lines
Quoted 4 lines
I see what you are trying to say, but I am not sure why you consider this as they would break? How did they work in the first place?
Most drivers does not require power domains before. E.g. GPIO, I2C, PWM,
MMC.
Quoted 8 lines
Or even we need power domain(PUs/PCIE), they're enabled in .power_on()/.power_off() which has been already up before probe.
Quoted 4 lines
Anyway, the problem you are talking about have so far been addressed by making buses/drivers to call pm_runtime_enable() and pm_runtime_get_sync() - before trying to probe their devices.
Quoted 8 lines
Yes, called them early before accessing clock and hw registers.
Quoted 3 lines
Do you have an idea of the amount of drivers that needs to fixed, in regards to this?
So far all enabled devices I tested need fixes. GPIO/I2C/FEC/MMC and etc. Suppose other need the same fix as we have not met this situation before.
Okay, I see.
Quoted 9 lines
Quoted 8 lines
I guess an option could be to, that via the ->attach_dev() callback perform the same operations as also being done during ->start(). Of course, you may need to keep a flag about this being done, so that operations that need to be balanced with get/put or the like is managed correctly, the next time
->start|stop() becomes called.
Quoted 3 lines
That might be a way to try. The follow seems may introduce a bit complexities and driver needs to maintainer the runtime status along with framework in order to keep the balance?
Yeah, it's likely not going to be very elegant, but should be rather self contained in the PM domain driver and could serve as a way forward - while working on fixing the drivers long term.
Quoted 18 lines
Quoted 17 lines
Quoted 4 lines
In summary, we probably may not be able to try ti_sci way before fixing
above two issues.
Quoted 3 lines
Do you think I should wait for them to be fixed or if we could use the current
way at first?
Quoted 3 lines
I might be a little intend to the second way as we now have a lot upstreaming work pending on this. But please let me know if you have a
different idea.
As this is also about deploying a DTB with a correctly modeled HW, I think we need to fix the above issues.
Quoted 8 lines
Quoted 2 lines
DTB are the same with #power-domain-cells 1 (resource ID) in either case. So DTB don't need change and driver could be optimized.
You are correct.
Although, how does $subject patch solves the case where a device may have multiple PM domains attached to it? Or is left as a future improvement?

Quoted 7 lines

IIRC this patch does not have such issue for multi PM domains as we have individual power domains for each resources. Devices will be attached to each individual domains in __genpd_dev_pm_attach rather than the single one global virtual SCU domain. Furthermore we're using domains .power_on()/ power_off() callback which means the power will be up before the probe. (no the issue like when using .attach_dev()/.detach_dev() and run .start()/stop() via runtime pm).

I don't follow your reasoning here, sorry. You stated earlier, that there was a need to support multiple PM domains per device.

How can that problem be resolved by using one genpd per device? What am I missing?

Quoted 8 lines

Actually I'm a bit wondering that if abstracting SCU domains into one global virtual domain is certainly better than individual domains. For SCU based SoCs, the power domain service is provided by SCU firmware, users don't have to care too much about the HW internals. And according to SCU definition, each devices is associated with a power domain, devices should treat it as a separate domain and have to enable it before accessing HW. So one single global domain looks like not exactly as SCU firmware defines. Not quite sure, but a bit feel like that....

I don't agree with you here, sorry. As also pointed out by Rob earlier, in regards to the DT bindings. Simply put, I doubt the SoC deploys separate power rails/islands for each of the available devices. Instead, it seems like the SCU interface, allows its clients to controls "power" to each of the available *devices* - and not power domains.

Anyway, we are debating how to implement this in software and not in DT. :-)

That said, if you strongly feel that the short cut, about registering one genpd per device and thus suffer from the overhead it gives, is the best approach to start with, feel free to do that.

Kind regards
Uffe

A.s. Dong

author

2018-11-01 01:28 UTC #15 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

Quoted 3 lines

-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Thursday, November 1, 2018 5:49 AM

[...]

Quoted 109 lines · A.s. Dong

Quoted 10 lines
Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 11:56 PM
[...]
Quoted 6 lines · A.s. Dong
Quoted 5 lines
Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 12:00 AM
[...]
Quoted 1 line
On 30 October 2018 at 14:20, A.s. Dong <[email protected]>
Quoted 7 lines · wrote:
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
[...]
Quoted 5 lines
}
Review
Quoted 4 lines
(quoted diff) +4 -0
+
+static struct imx_sc_pm_domain *
+imx_scu_add_pm_domain(struct device *dev, int idx,
+                     const struct imx_sc_pd_range *pd_ranges)
{
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 14 lines
Quoted 14 lines
Review
Quoted 10 lines
(quoted diff) +10 -0
+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;
So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better option.
Quoted 3 lines
Quoted 3 lines
What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally
by the FW, no?
Quoted 4 lines
Quoted 1 line
Yes, you're right.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
In any case, it looks like what you should do is to implement the
->attach|detach_dev() callback for the genpd and use the ->attach|regular
of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the
"resource id"
Quoted 58 lines
Quoted 57 lines
Quoted 37 lines
Quoted 37 lines
Quoted 11 lines
Quoted 11 lines
and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing these kind of things).
Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above
->power_on|off() callbacks.
Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier
version.
Quoted 5 lines
Quoted 1 line
Appreciated for such detailed explanation. lt's a good suggestion and I really like to switch to it. However, after digged a bit more, i found a few
blocking issues:
Quoted 2 lines
1. The .attach_dev() of power domain infrastructure still does not support
multi domains case.
Quoted 3 lines
It looks like there's no way for .attach_dev() to understand which sub 'power
domain'
Quoted 2 lines
the device is attaching for one global power domain provider like ti_sci as
you suggested.
Quoted 5 lines
Secondly, the struct device *dev passed into is a virtual PD device. It does not help for parsing the real device resource id.
So framework probably needs some extension to support multi power
domain cases.
Right, you are correct.
Quoted 8 lines
Quoted 3 lines
Let me think about this and see what I can come up with - unless you already have something you want to propose?
I have thought we may need add two more params (real dev and pd index) for
.attach_dev().detach_dev() and .start()/.stop() for multi PDs case. But I'm not sure if they are proper.
Right.
I don't think we need to change the definition of the callbacks. However, from within the callbacks (or at least the ->attach_dev()) we need access to the data you suggest above.
To make that data available, genpd needs to store it in the per device struct generic_pm_domain_data, but of course before invoking the
->attach_dev() callback.

Quoted 9 lines

Quoted 5 lines
Yes, that's an option.
Currently gpd_data is allocated in genpd_add_device() which looks like to be designed to attach device to the specified power domain passed in, so it's unware of whether it's multi domains or not. Then we may still need fine tune those functions in different abstract layers.
Yes, something like that. I don't think it should be that hard, I can have a stab at it of you like?

Yes, that's fine to me. I just don't have too much time to handle it right now. I can help do some basic function test later. May not be stress as we still have no devices using multi domains supported In kernel.
So I can construct simple case to test.

Quoted 100 lines

Quoted 81 lines
Quoted 81 lines
Quoted 16 lines
Quoted 16 lines
Quoted 7 lines
2. It also breaks most of current drivers as the driver probe sequence behavior changed If removing .power_on() and .power_off() callback and
use .start() and .stop() instead.
Quoted 8 lines
genpd_dev_pm_attach will only power up the domain and attach device, but will not call .start() which relies on device runtime pm. That means the device power is still not up before running driver probe function. For SCU enabled platforms, all device drivers accessing registers/clock without power domain enabled will trigger a HW access error. That means we need fix most of drivers probe sequence with proper
Quoted 2 lines
Quoted 2 lines
runtime pm. So I'm a bit wondering whether we should still keep the exist
way.
Quoted 7 lines
Quoted 4 lines
I see what you are trying to say, but I am not sure why you consider this as they would break? How did they work in the first place?
Most drivers does not require power domains before. E.g. GPIO, I2C, PWM,
MMC.
Quoted 8 lines
Or even we need power domain(PUs/PCIE), they're enabled in .power_on()/.power_off() which has been already up before probe.
Quoted 4 lines
Anyway, the problem you are talking about have so far been addressed by making buses/drivers to call pm_runtime_enable() and pm_runtime_get_sync() - before trying to probe their devices.
Quoted 8 lines
Yes, called them early before accessing clock and hw registers.
Quoted 3 lines
Do you have an idea of the amount of drivers that needs to fixed, in regards to this?
So far all enabled devices I tested need fixes. GPIO/I2C/FEC/MMC and etc. Suppose other need the same fix as we have not met this situation before.
Okay, I see.
Quoted 9 lines
Quoted 8 lines
I guess an option could be to, that via the ->attach_dev() callback perform the same operations as also being done during ->start(). Of course, you may need to keep a flag about this being done, so that operations that need to be balanced with get/put or the like is managed correctly, the next time ->start|stop() becomes called.
Quoted 3 lines
That might be a way to try. The follow seems may introduce a bit complexities and driver needs to maintainer the runtime status along with framework in order to keep the balance?
Yeah, it's likely not going to be very elegant, but should be rather self contained in the PM domain driver and could serve as a way forward - while working on fixing the drivers long term.
Quoted 18 lines
Quoted 17 lines
Quoted 4 lines
In summary, we probably may not be able to try ti_sci way before fixing
above two issues.
Quoted 3 lines
Do you think I should wait for them to be fixed or if we could use the current
way at first?
Quoted 3 lines
I might be a little intend to the second way as we now have a lot upstreaming work pending on this. But please let me know if you have a
different idea.
As this is also about deploying a DTB with a correctly modeled HW, I think we need to fix the above issues.
Quoted 7 lines
Quoted 7 lines
Quoted 2 lines
DTB are the same with #power-domain-cells 1 (resource ID) in either case. So DTB don't need change and driver could be optimized.
You are correct.
Although, how does $subject patch solves the case where a device may have multiple PM domains attached to it? Or is left as a future
improvement?
Quoted 10 lines
Quoted 1 line
IIRC this patch does not have such issue for multi PM domains as we have individual power domains for each resources. Devices will be attached to each individual domains in __genpd_dev_pm_attach rather than the single one global virtual SCU domain. Furthermore we're using domains .power_on()/
power_off() callback which means the power will be up before the probe. (no the issue like when using .attach_dev()/.detach_dev() and run .start()/stop() via runtime pm).

Quoted 5 lines

I don't follow your reasoning here, sorry. You stated earlier, that there was a need to support multiple PM domains per device.
How can that problem be resolved by using one genpd per device? What am I missing?

Sorry I may be not clear before.
We still support using multiple power domains.
For example:

Mipi_csi0 {
	...
	Power_domains = <&PD SC_R_IPI_CH0>,
				   <&PD SC_R_MIPI_CSI0>;
	Power_domains_names = "ss", "csi";
};

The difference is that each SC resource has individual power domains in driver And __genpd_dev_pm_attach will bind the virtual devices to the correct individual domain and do properly power_on/off of that domains.
So it does not have the issue that .attach_dev() has, also does not have issue that power domain is still not up before running probe.
(The detailed using is just like Tegra xHCI you pointed to me before)

Quoted 9 lines

Quoted 9 lines
Actually I'm a bit wondering that if abstracting SCU domains into one global virtual domain is certainly better than individual domains. For SCU based SoCs, the power domain service is provided by SCU firmware, users don't have to care too much about the HW internals. And according to SCU definition, each devices is associated with a power domain, devices should treat it as a separate domain and have to enable it before accessing HW. So one single global domain looks like not

Quoted 8 lines

exactly as SCU firmware defines. Not quite sure, but a bit feel like that....
I don't agree with you here, sorry. As also pointed out by Rob earlier, in regards to the DT bindings. Simply put, I doubt the SoC deploys separate power rails/islands for each of the available devices. Instead, it seems like the SCU interface, allows its clients to controls "power" to each of the available *devices* - and not power domains.

Yes, that's the trick part. For me, I feel like devices are unware of HW internals but using SCU power domains service. So devices think it's seaparte domains may not be strictly wrong.
Anyway, I'm also agree with the later way (signal global power domains with .attach_dev()/.start() to power on device). Just because we met those two Issues blocking us to do it right now.

Quoted 6 lines

Anyway, we are debating how to implement this in software and not in DT. :-)
That said, if you strongly feel that the short cut, about registering one genpd per device and thus suffer from the overhead it gives, is the best approach to start with, feel free to do that.

I just feel like that starting with current way could give us more time to fine tune it later without blocking other drivers upstreaming. And maybe we can do better work once we have real multi domains users in kernel.

Anyway, I can do either, so please me know if you still think we should strictly start with signal global power domain way.

Regards
Dong Aisheng

Quoted 2 lines

Kind regards
Uffe

Ulf Hansson

2018-11-01 09:51 UTC #16 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

Quoted 119 lines · A.s. Dong

Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Thursday, November 1, 2018 5:49 AM
[...]
Quoted 115 lines · A.s. Dong
Quoted 10 lines
Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 11:56 PM
[...]
Quoted 6 lines · A.s. Dong
Quoted 5 lines
Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 12:00 AM
[...]
Quoted 1 line
On 30 October 2018 at 14:20, A.s. Dong <[email protected]>
Quoted 7 lines · wrote:
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
[...]
Quoted 5 lines
}
Review
Quoted 4 lines
(quoted diff) +4 -0
+
+static struct imx_sc_pm_domain *
+imx_scu_add_pm_domain(struct device *dev, int idx,
+                     const struct imx_sc_pd_range *pd_ranges)
{
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 14 lines
Quoted 14 lines
Review
Quoted 10 lines
(quoted diff) +10 -0
+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;
So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better option.
Quoted 3 lines
Quoted 3 lines
What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally
by the FW, no?
Quoted 4 lines
Quoted 1 line
Yes, you're right.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
In any case, it looks like what you should do is to implement the
->attach|detach_dev() callback for the genpd and use the ->attach|regular
of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the
"resource id"
Quoted 63 lines
Quoted 48 lines
Quoted 37 lines
Quoted 37 lines
Quoted 11 lines
Quoted 11 lines
and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing these kind of things).
Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above
->power_on|off() callbacks.
Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier
version.
Quoted 5 lines
Quoted 1 line
Appreciated for such detailed explanation. lt's a good suggestion and I really like to switch to it. However, after digged a bit more, i found a few
blocking issues:
Quoted 2 lines
1. The .attach_dev() of power domain infrastructure still does not support
multi domains case.
Quoted 3 lines
It looks like there's no way for .attach_dev() to understand which sub 'power
domain'
Quoted 2 lines
the device is attaching for one global power domain provider like ti_sci as
you suggested.
Quoted 5 lines
Secondly, the struct device *dev passed into is a virtual PD device. It does not help for parsing the real device resource id.
So framework probably needs some extension to support multi power
domain cases.
Right, you are correct.
Quoted 8 lines
Quoted 3 lines
Let me think about this and see what I can come up with - unless you already have something you want to propose?
I have thought we may need add two more params (real dev and pd index) for
.attach_dev().detach_dev() and .start()/.stop() for multi PDs case. But I'm not sure if they are proper.
Right.
Quoted 9 lines
I don't think we need to change the definition of the callbacks. However, from within the callbacks (or at least the ->attach_dev()) we need access to the data you suggest above.
To make that data available, genpd needs to store it in the per device struct generic_pm_domain_data, but of course before invoking the
->attach_dev() callback.
Yes, that's an option.
Currently gpd_data is allocated in genpd_add_device() which looks like to be designed to attach device to the specified power domain passed in, so it's unware of whether it's multi domains or not. Then we may still need fine tune those functions in different abstract layers.

Quoted 8 lines

Quoted 3 lines
Yes, something like that. I don't think it should be that hard, I can have a stab at it of you like?
Yes, that's fine to me. I just don't have too much time to handle it right now. I can help do some basic function test later. May not be stress as we still have no devices using multi domains supported In kernel.
So I can construct simple case to test.

Okay, I am adding it to my TODO list for genpd.

Quoted 117 lines

Quoted 101 lines
Quoted 82 lines
Quoted 82 lines
Quoted 17 lines
Quoted 17 lines
Quoted 8 lines
2. It also breaks most of current drivers as the driver probe sequence behavior changed If removing .power_on() and .power_off() callback and
use .start() and .stop() instead.
Quoted 8 lines
genpd_dev_pm_attach will only power up the domain and attach device, but will not call .start() which relies on device runtime pm. That means the device power is still not up before running driver probe function. For SCU enabled platforms, all device drivers accessing registers/clock without power domain enabled will trigger a HW access error. That means we need fix most of drivers probe sequence with proper
Quoted 2 lines
Quoted 2 lines
runtime pm. So I'm a bit wondering whether we should still keep the exist
way.
Quoted 7 lines
Quoted 4 lines
I see what you are trying to say, but I am not sure why you consider this as they would break? How did they work in the first place?
Most drivers does not require power domains before. E.g. GPIO, I2C, PWM,
MMC.
Quoted 8 lines
Or even we need power domain(PUs/PCIE), they're enabled in .power_on()/.power_off() which has been already up before probe.
Quoted 4 lines
Anyway, the problem you are talking about have so far been addressed by making buses/drivers to call pm_runtime_enable() and pm_runtime_get_sync() - before trying to probe their devices.
Quoted 8 lines
Yes, called them early before accessing clock and hw registers.
Quoted 3 lines
Do you have an idea of the amount of drivers that needs to fixed, in regards to this?
So far all enabled devices I tested need fixes. GPIO/I2C/FEC/MMC and etc. Suppose other need the same fix as we have not met this situation before.
Okay, I see.
Quoted 9 lines
Quoted 8 lines
I guess an option could be to, that via the ->attach_dev() callback perform the same operations as also being done during ->start(). Of course, you may need to keep a flag about this being done, so that operations that need to be balanced with get/put or the like is managed correctly, the next time ->start|stop() becomes called.
Quoted 3 lines
That might be a way to try. The follow seems may introduce a bit complexities and driver needs to maintainer the runtime status along with framework in order to keep the balance?
Yeah, it's likely not going to be very elegant, but should be rather self contained in the PM domain driver and could serve as a way forward - while working on fixing the drivers long term.
Quoted 18 lines
Quoted 17 lines
Quoted 4 lines
In summary, we probably may not be able to try ti_sci way before fixing
above two issues.
Quoted 3 lines
Do you think I should wait for them to be fixed or if we could use the current
way at first?
Quoted 3 lines
I might be a little intend to the second way as we now have a lot upstreaming work pending on this. But please let me know if you have a
different idea.
As this is also about deploying a DTB with a correctly modeled HW, I think we need to fix the above issues.
Quoted 7 lines
Quoted 7 lines
Quoted 2 lines
DTB are the same with #power-domain-cells 1 (resource ID) in either case. So DTB don't need change and driver could be optimized.
You are correct.
Although, how does $subject patch solves the case where a device may have multiple PM domains attached to it? Or is left as a future
improvement?
Quoted 10 lines
Quoted 1 line
IIRC this patch does not have such issue for multi PM domains as we have individual power domains for each resources. Devices will be attached to each individual domains in __genpd_dev_pm_attach rather than the single one global virtual SCU domain. Furthermore we're using domains .power_on()/
power_off() callback which means the power will be up before the probe. (no the issue like when using .attach_dev()/.detach_dev() and run .start()/stop() via runtime pm).
Quoted 5 lines
I don't follow your reasoning here, sorry. You stated earlier, that there was a need to support multiple PM domains per device.
How can that problem be resolved by using one genpd per device? What am I missing?
Sorry I may be not clear before.
We still support using multiple power domains.
For example:
Mipi_csi0 {
        ...
        Power_domains = <&PD SC_R_IPI_CH0>,
                                   <&PD SC_R_MIPI_CSI0>;
        Power_domains_names = "ss", "csi";
};

Quoted 6 lines

The difference is that each SC resource has individual power domains in driver And __genpd_dev_pm_attach will bind the virtual devices to the correct individual domain and do properly power_on/off of that domains.
So it does not have the issue that .attach_dev() has, also does not have issue that power domain is still not up before running probe.
(The detailed using is just like Tegra xHCI you pointed to me before)

Aha, I see. Thanks for clarifying.

Quoted 33 lines

Quoted 10 lines
Quoted 10 lines
Actually I'm a bit wondering that if abstracting SCU domains into one global virtual domain is certainly better than individual domains. For SCU based SoCs, the power domain service is provided by SCU firmware, users don't have to care too much about the HW internals. And according to SCU definition, each devices is associated with a power domain, devices should treat it as a separate domain and have to enable it before accessing HW. So one single global domain looks like not
Quoted 8 lines
exactly as SCU firmware defines. Not quite sure, but a bit feel like that....
I don't agree with you here, sorry. As also pointed out by Rob earlier, in regards to the DT bindings. Simply put, I doubt the SoC deploys separate power rails/islands for each of the available devices. Instead, it seems like the SCU interface, allows its clients to controls "power" to each of the available *devices* - and not power domains.
Yes, that's the trick part. For me, I feel like devices are unware of HW internals but using SCU power domains service. So devices think it's seaparte domains may not be strictly wrong.
Anyway, I'm also agree with the later way (signal global power domains with .attach_dev()/.start() to power on device). Just because we met those two Issues blocking us to do it right now.
Quoted 6 lines
Anyway, we are debating how to implement this in software and not in DT. :-)
That said, if you strongly feel that the short cut, about registering one genpd per device and thus suffer from the overhead it gives, is the best approach to start with, feel free to do that.

Quoted 6 lines

I just feel like that starting with current way could give us more time to fine tune it later without blocking other drivers upstreaming. And maybe we can do better work once we have real multi domains users in kernel.
Anyway, I can do either, so please me know if you still think we should strictly start with signal global power domain way.

As stated, feel free to pick whatever option that makes sense to you.

However, if you decide to start with the approach taken in $subject patch, it would be nice if the new drivers/firmware/imx/scu-pd.c file, could have some comments in the top, about what things are needed to convert to the "single global domain", just so we don't forget about it.

Kind regards
Uffe

A.s. Dong

author

2018-11-01 14:02 UTC #17 lore raw

PATCH v8 5/5 firmware: imx: add SCU power domain driver

Quoted 3 lines

-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Thursday, November 1, 2018 5:52 PM

[...]

Quoted 132 lines · A.s. Dong

Quoted 11 lines
Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Thursday, November 1, 2018 5:49 AM
[...]
Quoted 7 lines · A.s. Dong
Quoted 5 lines
Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 11:56 PM
[...]
Quoted 1 line
On 31 October 2018 at 02:45, A.s. Dong <[email protected]>
Quoted 15 lines · wrote:
Quoted 15 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
-----Original Message-----
From: Ulf Hansson [mailto:ulf.hansson at linaro.org]
Sent: Wednesday, October 31, 2018 12:00 AM
[...]
Quoted 1 line
On 30 October 2018 at 14:20, A.s. Dong <[email protected]>
Quoted 8 lines · wrote:
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
[...]
Quoted 6 lines
}
Review
Quoted 5 lines
(quoted diff) +5 -0
+
+static struct imx_sc_pm_domain *
+imx_scu_add_pm_domain(struct device *dev, int idx,
+                     const struct imx_sc_pd_range
+*pd_ranges)
{
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Review
Quoted 4 lines
(quoted diff) +4 -0
+       struct imx_sc_pm_domain *sc_pd;
+       int ret;
+
+       sc_pd = devm_kzalloc(dev, sizeof(*sc_pd),
GFP_KERNEL);
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+       if (!sc_pd)
+               return ERR_PTR(-ENOMEM);
+
+       sc_pd->rsrc = pd_ranges->rsrc + idx;
+       sc_pd->pd.power_off = imx_sc_pd_power_off;
+       sc_pd->pd.power_on = imx_sc_pd_power_on;
So, this means you are going to register one genpd per device (one for each uart, pwm etc), but there is actually a better
option.
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
Quoted 4 lines
Quoted 4 lines
What you seems to be needing is a way to "power on/off" devices, rather than the PM domain. Right? The PM domain, is managed internally
by the FW, no?
Quoted 12 lines
Quoted 1 line
Yes, you're right.
Quoted 8 lines
In any case, it looks like what you should do is to implement the
->attach|detach_dev() callback for the genpd and use the ->attach|regular
of_genpd_add_provider_simple(). From within the ->attach_dev(), you should get the OF node for the device that is being attached and then parse the power-domain cell containing the
Quoted 3 lines
Quoted 3 lines
"resource id"
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
and store that in the per device struct generic_pm_domain_data (we have void pointer there for storing
these kind of things).
Quoted 56 lines
Quoted 56 lines
Quoted 56 lines
Quoted 55 lines
Quoted 35 lines
Quoted 35 lines
Quoted 9 lines
Quoted 9 lines
Additionally, you need to implement the ->stop() and ->start() callbacks of genpd, which is where you "power on/off" devices, rather than using the above ->power_on|off() callbacks.
Please have a look a drivers/soc/ti/ti_sci_pm_domains.c. Apologize, I should have pointed you towards that reference already in the earlier
version.
Quoted 5 lines
Quoted 1 line
Appreciated for such detailed explanation. lt's a good suggestion and I really like to switch to it. However, after digged a bit more, i found a few
blocking issues:
Quoted 2 lines
1. The .attach_dev() of power domain infrastructure still does not support
multi domains case.
Quoted 3 lines
It looks like there's no way for .attach_dev() to understand which sub 'power
domain'
Quoted 2 lines
the device is attaching for one global power domain provider like ti_sci as
you suggested.
Quoted 5 lines
Secondly, the struct device *dev passed into is a virtual PD device. It does not help for parsing the real device resource id.
So framework probably needs some extension to support multi power
domain cases.
Right, you are correct.
Quoted 8 lines
Quoted 3 lines
Let me think about this and see what I can come up with - unless you already have something you want to propose?
I have thought we may need add two more params (real dev and pd index) for
.attach_dev().detach_dev() and .start()/.stop() for multi PDs case. But I'm not sure if they are proper.
Right.
I don't think we need to change the definition of the callbacks. However, from within the callbacks (or at least the ->attach_dev()) we need access to the data you suggest above.
To make that data available, genpd needs to store it in the per device struct generic_pm_domain_data, but of course before invoking the
->attach_dev() callback.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Yes, that's an option.
Currently gpd_data is allocated in genpd_add_device() which looks like to be designed to attach device to the specified power domain passed in, so it's unware of whether it's multi domains or not. Then we may still need fine tune those functions in different abstract
layers.
Quoted 5 lines
Quoted 4 lines
Yes, something like that. I don't think it should be that hard, I can have a stab at it of you like?

Quoted 7 lines

Quoted 4 lines
Yes, that's fine to me. I just don't have too much time to handle it right now. I can help do some basic function test later. May not be stress as we still have no devices using multi domains supported In kernel.
So I can construct simple case to test.
Okay, I am adding it to my TODO list for genpd.

Great.

Quoted 179 lines

Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 8 lines
2. It also breaks most of current drivers as the driver probe sequence behavior changed If removing .power_on() and .power_off() callback and
use .start() and .stop() instead.
Quoted 9 lines
genpd_dev_pm_attach will only power up the domain and attach device, but will not call .start() which relies on device runtime pm. That means the device power is still not up before running driver probe function. For SCU enabled platforms, all device drivers accessing registers/clock without power domain enabled will trigger a HW access error. That means we need fix most of drivers probe sequence with proper
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines
runtime pm. So I'm a bit wondering whether we should still keep the exist
way.
Quoted 3 lines
Quoted 3 lines
I see what you are trying to say, but I am not sure why you consider this as they would break? How did they work in the first
place?
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 4 lines
Quoted 1 line
Most drivers does not require power domains before. E.g. GPIO, I2C, PWM,
MMC.
Quoted 9 lines
Or even we need power domain(PUs/PCIE), they're enabled in .power_on()/.power_off() which has been already up before probe.
Quoted 5 lines
Anyway, the problem you are talking about have so far been addressed by making buses/drivers to call pm_runtime_enable() and
pm_runtime_get_sync() - before trying to probe their devices.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Yes, called them early before accessing clock and hw registers.
Quoted 3 lines
Do you have an idea of the amount of drivers that needs to fixed, in regards to this?
So far all enabled devices I tested need fixes. GPIO/I2C/FEC/MMC and
etc.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Suppose other need the same fix as we have not met this situation
before.
Quoted 33 lines
Quoted 33 lines
Quoted 33 lines
Quoted 33 lines
Okay, I see.
Quoted 10 lines
Quoted 9 lines
I guess an option could be to, that via the ->attach_dev() callback perform the same operations as also being done during ->start(). Of course, you may need to keep a flag about this ->being
done, so that operations that need to be balanced with get/put or the like is managed correctly, the next time ->start|stop() becomes called.
Quoted 3 lines
That might be a way to try. The follow seems may introduce a bit complexities and driver needs to maintainer the runtime status along with framework in order to keep the balance?
Yeah, it's likely not going to be very elegant, but should be rather self contained in the PM domain driver and could serve as a way forward - while working on fixing the drivers long term.
Quoted 12 lines
Quoted 12 lines
Quoted 4 lines
In summary, we probably may not be able to try ti_sci way before fixing
above two issues.
Quoted 3 lines
Do you think I should wait for them to be fixed or if we could use the current
way at first?
Quoted 3 lines
I might be a little intend to the second way as we now have a lot upstreaming work pending on this. But please let me know if you have a
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
different idea.
As this is also about deploying a DTB with a correctly modeled HW, I think we need to fix the above issues.
DTB are the same with #power-domain-cells 1 (resource ID) in either
case.
Quoted 34 lines
Quoted 19 lines
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
So DTB don't need change and driver could be optimized.
You are correct.
Although, how does $subject patch solves the case where a device may have multiple PM domains attached to it? Or is left as a future
improvement?
Quoted 10 lines
Quoted 1 line
IIRC this patch does not have such issue for multi PM domains as we have individual power domains for each resources. Devices will be attached to each individual domains in __genpd_dev_pm_attach rather than the single one global virtual SCU domain. Furthermore we're using domains .power_on()/
power_off() callback which means the power will be up before the probe. (no the issue like when using .attach_dev()/.detach_dev() and run .start()/stop() via runtime pm).
Quoted 5 lines
I don't follow your reasoning here, sorry. You stated earlier, that there was a need to support multiple PM domains per device.
How can that problem be resolved by using one genpd per device? What am I missing?
Sorry I may be not clear before.
We still support using multiple power domains.
For example:
Mipi_csi0 {
        ...
        Power_domains = <&PD SC_R_IPI_CH0>,
                                   <&PD SC_R_MIPI_CSI0>;
        Power_domains_names = "ss", "csi"; };
Quoted 3 lines
The difference is that each SC resource has individual power domains in driver And __genpd_dev_pm_attach will bind the virtual devices to the correct individual domain and do properly power_on/off of that
domains.
Quoted 3 lines
So it does not have the issue that .attach_dev() has, also does not have issue that power domain is still not up before running probe. (The detailed using is just like Tegra xHCI you pointed to me before)
Aha, I see. Thanks for clarifying.
Quoted 32 lines
Quoted 11 lines
Quoted 11 lines
Actually I'm a bit wondering that if abstracting SCU domains into one global virtual domain is certainly better than individual domains. For SCU based SoCs, the power domain service is provided by SCU firmware, users don't have to care too much about the HW internals. And according to SCU definition, each devices is associated with a power domain, devices should treat it as a separate domain and have to enable it before accessing HW. So one single global domain looks like not
Quoted 9 lines
exactly as SCU firmware defines. Not quite sure, but a bit feel like that....
I don't agree with you here, sorry. As also pointed out by Rob earlier, in regards to the DT bindings. Simply put, I doubt the SoC deploys separate power rails/islands for each of the available devices. Instead, it seems like the SCU interface, allows its clients to controls "power" to each of the available
*devices* - and not power domains.
Yes, that's the trick part. For me, I feel like devices are unware of HW internals but using SCU power domains service. So devices think it's seaparte domains may not be strictly wrong.
Anyway, I'm also agree with the later way (signal global power domains with
.attach_dev()/.start() to power on device). Just because we met those two Issues blocking us to do it right now.
Quoted 3 lines
Anyway, we are debating how to implement this in software and not in DT. :-)
Quoted 8 lines
Quoted 4 lines
That said, if you strongly feel that the short cut, about registering one genpd per device and thus suffer from the overhead it gives, is the best approach to start with, feel free to do that.
I just feel like that starting with current way could give us more time to fine tune it later without blocking other drivers upstreaming. And maybe we can do better work once we have real multi domains users in
kernel.
Quoted 3 lines
Anyway, I can do either, so please me know if you still think we should strictly start with signal global power domain way.

Quoted 7 lines

As stated, feel free to pick whatever option that makes sense to you.
However, if you decide to start with the approach taken in $subject patch, it would be nice if the new drivers/firmware/imx/scu-pd.c file, could have some comments in the top, about what things are needed to convert to the "single global domain", just so we don't forget about it.

Sounds good to me.
Thanks for the suggestion.
I will do it.

Regards
Dong Aisheng

Quoted 2 lines

Kind regards
Uffe

Ulf Hansson

2018-10-29 11:14 UTC #18 lore raw

v8 cover soc: imx: add scu power domain driver

In reply to A.s. Dong

Quoted 45 lines · A.s. Dong

This patch set adds the scu based power domain driver. It depends on SCU driver.
Change Log:
v7->v8:
* update to #power-domain-cells 1 binding
v6->v7:
* keep "fsl,scu-pd" as fall back compatible string
v5->v6:
* only compatible string name updated from fsl,scu-pd to fsl,imx8qxp-scu-pd accordingly
v4->v5:
* some minor improvements according to Sascha's suggestion
Note: did not use dev_* print functions due to we already have proper
prefix by redefining pr_fmt. So it seems not quite neccesary to pass in a struct device * pointer for debug purpose only in each functions.
v3->v4:
* only scu headfile path update
v2->v3:
* structure and enums name update
* api usage update
Dong Aisheng (5):
dt-bindings: imx: add scu resource id headfile
firmware: imx: remove resource id enums
dt-bindings: fsl: scu: update power domain binding
firmware: imx: add pm svc headfile
firmware: imx: add SCU power domain driver
 .../devicetree/bindings/arm/freescale/fsl,scu.txt  |  37 +-
 drivers/firmware/imx/Kconfig                       |   6 +
 drivers/firmware/imx/Makefile                      |   3 +-
 drivers/firmware/imx/scu-pd.c                      | 302 +++++++++++
 include/dt-bindings/firmware/imx/rsrc.h            | 559 +++++++++++++++++++++
 include/linux/firmware/imx/sci.h                   |   1 +
 include/linux/firmware/imx/svc/pm.h                |  85 ++++
 include/linux/firmware/imx/types.h                 | 552 --------------------
 8 files changed, 963 insertions(+), 582 deletions(-)
 create mode 100644 drivers/firmware/imx/scu-pd.c
 create mode 100644 include/dt-bindings/firmware/imx/rsrc.h
 create mode 100644 include/linux/firmware/imx/svc/pm.h

Quoted 3 lines

--
2.7.4

For patch 1 -> 4 (comments on patch5 is on its way).

Reviewed-by Ulf Hansson <[email protected]>

Kind regards
Uffe