Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 7/9] arm64: Unify exception masking at entry and exit of exception
From: Liao Chang @ 2024-04-09  1:23 UTC (permalink / raw)
  To: catalin.marinas, will, maz, oliver.upton, james.morse,
	suzuki.poulose, yuzenghui, tglx, mark.rutland, ardb, broonie,
	liaochang1, anshuman.khandual, miguel.luis, joey.gouly,
	ryan.roberts, jeremy.linton, liwei391, daniel.thompson,
	sumit.garg, kristina.martsenko, jpoimboe, ericchancf, robh, scott,
	songshuaishuai, shijie, bhe, akpm, thunder.leizhen, horms,
	rmk+kernel, takakura, dianders, swboyd, frederic, reijiw,
	akihiko.odaki, ruanjinjie
  Cc: linux-arm-kernel, linux-kernel, kvmarm
In-Reply-To: <20240409012344.3194724-1-liaochang1@huawei.com>

Currently, different exception types require specific mask. For example:

  - Interrupt handlers: Mask IRQ, FIQ, and NMI on entry.
  - Synchronous handler: Restore exception masks to pre-exception value.
  - Serror handler: Mask all interrupts and Serror on entry (strictest).
  - Debug handler: Keep all exception masked as exception taken.

This patch introduces new helper functions to unify exception masking
behavior at the entry and exit of exceptions on arm64. This approach
improves code clarity and maintainability.

Signed-off-by: Liao Chang <liaochang1@huawei.com>
---
 arch/arm64/include/asm/daifflags.h | 81 ++++++++++++++++++-------
 arch/arm64/kernel/entry-common.c   | 96 ++++++++++++++----------------
 arch/arm64/kernel/entry.S          |  2 -
 3 files changed, 105 insertions(+), 74 deletions(-)

diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
index df4c4989babd..6d391d221432 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -121,28 +121,6 @@ static inline void local_daif_restore(unsigned long flags)
 		trace_hardirqs_off();
 }
 
-/*
- * Called by synchronous exception handlers to restore the DAIF bits that were
- * modified by taking an exception.
- */
-static inline void local_daif_inherit(struct pt_regs *regs)
-{
-	unsigned long flags = regs->pstate & DAIF_MASK;
-
-	if (interrupts_enabled(regs))
-		trace_hardirqs_on();
-
-	if (system_uses_irq_prio_masking())
-		gic_write_pmr(regs->pmr_save);
-
-	/*
-	 * We can't use local_daif_restore(regs->pstate) here as
-	 * system_has_prio_mask_debugging() won't restore the I bit if it can
-	 * use the pmr instead.
-	 */
-	write_sysreg(flags, daif);
-}
-
 /*
  * For Arm64 processor support Armv8.8 or later, kernel supports three types
  * of irqflags, they used for corresponding configuration depicted as below:
@@ -381,4 +359,63 @@ static inline void local_allint_inherit(struct pt_regs *regs)
 			_allint_clear();
 	}
 }
+
+/*
+ * local_allint_disable - Disable IRQ, FIQ and NMI, with or without
+ * superpriority.
+ */
+static inline void local_allint_disable(void)
+{
+	arch_irqflags_t irqflags;
+
+	irqflags.fields.daif = DAIF_PROCCTX_NOIRQ;
+	irqflags.fields.pmr = GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET;
+	irqflags.fields.allint = 1;
+	local_allint_restore(irqflags);
+}
+
+/*
+ * local_allint_mark_enabled - When the kernel enables priority masking,
+ * interrupts cannot be handled util ICC_PMR_EL1 is set to GIC_PRIO_IRQON
+ * and PSTATE.IF is cleared. This helper function indicates that interrupts
+ * remains in a semi-masked state, requring further clearing of PSTATE.IF.
+ *
+ * Kernel will give a warning, if some function try to enable semi-masked
+ * interrupt via the arch_local_irq_enable() defined in <asm/irqflags.h>.
+ *
+ * This function is typically used before handling the Debug exception.
+ */
+static inline void local_allint_mark_enabled(void)
+{
+	if (system_uses_irq_prio_masking())
+		gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET);
+}
+
+/*
+ * local_errint_disable - Disable all types of interrupt including IRQ, FIQ,
+ * Serror and NMI, with or without superpriority.
+ */
+static inline void local_errint_disable(void)
+{
+	arch_irqflags_t irqflags;
+
+	irqflags.fields.daif = DAIF_ERRCTX;
+	irqflags.fields.pmr = GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET;
+	irqflags.fields.allint = 1;
+	local_allint_restore(irqflags);
+}
+
+/*
+ * local_errint_enable - Enable all types of interrupt including IRQ, FIQ,
+ * Serror and NMI, with or without superpriority.
+ */
+static inline void local_errint_enable(void)
+{
+	arch_irqflags_t irqflags;
+
+	irqflags.fields.daif = DAIF_PROCCTX;
+	irqflags.fields.pmr = GIC_PRIO_IRQON;
+	irqflags.fields.allint = 0;
+	local_allint_restore(irqflags);
+}
 #endif
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index b77a15955f28..99168223508b 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -168,7 +168,7 @@ static __always_inline void exit_to_user_mode_prepare(struct pt_regs *regs)
 	if (unlikely(flags & _TIF_WORK_MASK))
 		do_notify_resume(regs, flags);
 
-	local_daif_mask();
+	local_allint_mask();
 
 	lockdep_sys_exit();
 }
@@ -428,9 +428,9 @@ static void noinstr el1_abort(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	enter_from_kernel_mode(regs);
-	local_daif_inherit(regs);
+	local_allint_inherit(regs);
 	do_mem_abort(far, esr, regs);
-	local_daif_mask();
+	local_allint_mask();
 	exit_to_kernel_mode(regs);
 }
 
@@ -439,33 +439,36 @@ static void noinstr el1_pc(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	enter_from_kernel_mode(regs);
-	local_daif_inherit(regs);
+	local_allint_inherit(regs);
 	do_sp_pc_abort(far, esr, regs);
-	local_daif_mask();
+	local_allint_mask();
 	exit_to_kernel_mode(regs);
 }
 
 static void noinstr el1_undef(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_kernel_mode(regs);
-	local_daif_inherit(regs);
+	local_allint_inherit(regs);
 	do_el1_undef(regs, esr);
-	local_daif_mask();
+	local_allint_mask();
 	exit_to_kernel_mode(regs);
 }
 
 static void noinstr el1_bti(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_kernel_mode(regs);
-	local_daif_inherit(regs);
+	local_allint_inherit(regs);
 	do_el1_bti(regs, esr);
-	local_daif_mask();
+	local_allint_mask();
 	exit_to_kernel_mode(regs);
 }
 
 static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr)
 {
-	unsigned long far = read_sysreg(far_el1);
+	unsigned long far;
+
+	local_allint_mark_enabled();
+	far = read_sysreg(far_el1);
 
 	arm64_enter_el1_dbg(regs);
 	if (!cortex_a76_erratum_1463225_debug_handler(regs))
@@ -476,9 +479,9 @@ static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr)
 static void noinstr el1_fpac(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_kernel_mode(regs);
-	local_daif_inherit(regs);
+	local_allint_inherit(regs);
 	do_el1_fpac(regs, esr);
-	local_daif_mask();
+	local_allint_mask();
 	exit_to_kernel_mode(regs);
 }
 
@@ -543,7 +546,7 @@ static __always_inline void __el1_irq(struct pt_regs *regs,
 static void noinstr el1_interrupt(struct pt_regs *regs,
 				  void (*handler)(struct pt_regs *))
 {
-	write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
+	local_allint_disable();
 
 	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
 		__el1_pnmi(regs, handler);
@@ -565,7 +568,7 @@ asmlinkage void noinstr el1h_64_error_handler(struct pt_regs *regs)
 {
 	unsigned long esr = read_sysreg(esr_el1);
 
-	local_daif_restore(DAIF_ERRCTX);
+	local_errint_disable();
 	arm64_enter_nmi(regs);
 	do_serror(regs, esr);
 	arm64_exit_nmi(regs);
@@ -576,7 +579,7 @@ static void noinstr el0_da(struct pt_regs *regs, unsigned long esr)
 	unsigned long far = read_sysreg(far_el1);
 
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_mem_abort(far, esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -594,7 +597,7 @@ static void noinstr el0_ia(struct pt_regs *regs, unsigned long esr)
 		arm64_apply_bp_hardening();
 
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_mem_abort(far, esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -602,7 +605,7 @@ static void noinstr el0_ia(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_fpsimd_acc(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_fpsimd_acc(esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -610,7 +613,7 @@ static void noinstr el0_fpsimd_acc(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_sve_acc(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_sve_acc(esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -618,7 +621,7 @@ static void noinstr el0_sve_acc(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_sme_acc(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_sme_acc(esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -626,7 +629,7 @@ static void noinstr el0_sme_acc(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_fpsimd_exc(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_fpsimd_exc(esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -634,7 +637,7 @@ static void noinstr el0_fpsimd_exc(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_sys(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_el0_sys(esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -647,7 +650,7 @@ static void noinstr el0_pc(struct pt_regs *regs, unsigned long esr)
 		arm64_apply_bp_hardening();
 
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_sp_pc_abort(far, esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -655,7 +658,7 @@ static void noinstr el0_pc(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_sp(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_sp_pc_abort(regs->sp, esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -663,7 +666,7 @@ static void noinstr el0_sp(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_undef(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_el0_undef(regs, esr);
 	exit_to_user_mode(regs);
 }
@@ -671,7 +674,7 @@ static void noinstr el0_undef(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_bti(struct pt_regs *regs)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_el0_bti(regs);
 	exit_to_user_mode(regs);
 }
@@ -679,7 +682,7 @@ static void noinstr el0_bti(struct pt_regs *regs)
 static void noinstr el0_mops(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_el0_mops(regs, esr);
 	exit_to_user_mode(regs);
 }
@@ -687,7 +690,7 @@ static void noinstr el0_mops(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_inv(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	bad_el0_sync(regs, 0, esr);
 	exit_to_user_mode(regs);
 }
@@ -695,11 +698,14 @@ static void noinstr el0_inv(struct pt_regs *regs, unsigned long esr)
 static void noinstr el0_dbg(struct pt_regs *regs, unsigned long esr)
 {
 	/* Only watchpoints write FAR_EL1, otherwise its UNKNOWN */
-	unsigned long far = read_sysreg(far_el1);
+	unsigned long far;
+
+	local_allint_mark_enabled();
+	far = read_sysreg(far_el1);
 
 	enter_from_user_mode(regs);
 	do_debug_exception(far, esr, regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	exit_to_user_mode(regs);
 }
 
@@ -708,7 +714,7 @@ static void noinstr el0_svc(struct pt_regs *regs)
 	enter_from_user_mode(regs);
 	cortex_a76_erratum_1463225_svc_handler();
 	fp_user_discard();
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_el0_svc(regs);
 	exit_to_user_mode(regs);
 }
@@ -716,7 +722,7 @@ static void noinstr el0_svc(struct pt_regs *regs)
 static void noinstr el0_fpac(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_el0_fpac(regs, esr);
 	exit_to_user_mode(regs);
 }
@@ -785,7 +791,7 @@ static void noinstr el0_interrupt(struct pt_regs *regs,
 {
 	enter_from_user_mode(regs);
 
-	write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
+	local_allint_disable();
 
 	if (regs->pc & BIT(55))
 		arm64_apply_bp_hardening();
@@ -797,24 +803,14 @@ static void noinstr el0_interrupt(struct pt_regs *regs,
 	exit_to_user_mode(regs);
 }
 
-static void noinstr __el0_irq_handler_common(struct pt_regs *regs)
-{
-	el0_interrupt(regs, handle_arch_irq);
-}
-
 asmlinkage void noinstr el0t_64_irq_handler(struct pt_regs *regs)
 {
-	__el0_irq_handler_common(regs);
-}
-
-static void noinstr __el0_fiq_handler_common(struct pt_regs *regs)
-{
-	el0_interrupt(regs, handle_arch_fiq);
+	el0_interrupt(regs, handle_arch_irq);
 }
 
 asmlinkage void noinstr el0t_64_fiq_handler(struct pt_regs *regs)
 {
-	__el0_fiq_handler_common(regs);
+	el0_interrupt(regs, handle_arch_fiq);
 }
 
 static void noinstr __el0_error_handler_common(struct pt_regs *regs)
@@ -822,11 +818,11 @@ static void noinstr __el0_error_handler_common(struct pt_regs *regs)
 	unsigned long esr = read_sysreg(esr_el1);
 
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_ERRCTX);
+	local_errint_disable();
 	arm64_enter_nmi(regs);
 	do_serror(regs, esr);
 	arm64_exit_nmi(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	exit_to_user_mode(regs);
 }
 
@@ -839,7 +835,7 @@ asmlinkage void noinstr el0t_64_error_handler(struct pt_regs *regs)
 static void noinstr el0_cp15(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_el0_cp15(esr, regs);
 	exit_to_user_mode(regs);
 }
@@ -848,7 +844,7 @@ static void noinstr el0_svc_compat(struct pt_regs *regs)
 {
 	enter_from_user_mode(regs);
 	cortex_a76_erratum_1463225_svc_handler();
-	local_daif_restore(DAIF_PROCCTX);
+	local_errint_enable();
 	do_el0_svc_compat(regs);
 	exit_to_user_mode(regs);
 }
@@ -899,12 +895,12 @@ asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs)
 
 asmlinkage void noinstr el0t_32_irq_handler(struct pt_regs *regs)
 {
-	__el0_irq_handler_common(regs);
+	el0_interrupt(regs, handle_arch_irq);
 }
 
 asmlinkage void noinstr el0t_32_fiq_handler(struct pt_regs *regs)
 {
-	__el0_fiq_handler_common(regs);
+	el0_interrupt(regs, handle_arch_fiq);
 }
 
 asmlinkage void noinstr el0t_32_error_handler(struct pt_regs *regs)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 7ef0e127b149..0b311fefedc2 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -316,8 +316,6 @@ alternative_else_nop_endif
 
 	mrs_s	x20, SYS_ICC_PMR_EL1
 	str	x20, [sp, #S_PMR_SAVE]
-	mov	x20, #GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET
-	msr_s	SYS_ICC_PMR_EL1, x20
 
 .Lskip_pmr_save\@:
 #endif
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 5/9] arm64/cpufeature: Use alternatives to check enabled ARM64_HAS_NMI feature
From: Liao Chang @ 2024-04-09  1:23 UTC (permalink / raw)
  To: catalin.marinas, will, maz, oliver.upton, james.morse,
	suzuki.poulose, yuzenghui, tglx, mark.rutland, ardb, broonie,
	liaochang1, anshuman.khandual, miguel.luis, joey.gouly,
	ryan.roberts, jeremy.linton, liwei391, daniel.thompson,
	sumit.garg, kristina.martsenko, jpoimboe, ericchancf, robh, scott,
	songshuaishuai, shijie, bhe, akpm, thunder.leizhen, horms,
	rmk+kernel, takakura, dianders, swboyd, frederic, reijiw,
	akihiko.odaki, ruanjinjie
  Cc: linux-arm-kernel, linux-kernel, kvmarm
In-Reply-To: <20240409012344.3194724-1-liaochang1@huawei.com>

Due to the historical reasons, cpus_have_const_cap() is more complicated
than it needs to be. When CONFIG_ARM64_NMI=y the ARM64_HAS_NMI cpucap is
a strict boot cpu feature which is detected and patched early on the
boot cpu, which means no code depends on ARM64_HAS_NMI cpucap run in the
window between the ARM64_HAS_NMI cpucap is detected and alternative is
patched. So it would be nice to migrate caller over to
alternative_has_cap_likey().

Signed-off-by: Liao Chang <liaochang1@huawei.com>
---
 arch/arm64/include/asm/cpufeature.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index dc8b2d0d3763..4c35565ad656 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -803,7 +803,7 @@ static __always_inline bool system_uses_irq_prio_masking(void)
 static __always_inline bool system_uses_nmi(void)
 {
 	return IS_ENABLED(CONFIG_ARM64_NMI) &&
-		cpus_have_const_cap(ARM64_USES_NMI);
+		alternative_has_cap_likely(ARM64_USES_NMI);
 }
 
 static inline bool system_supports_mte(void)
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 6/9] arm64: daifflags: Add logical exception masks covering DAIF + PMR + ALLINT
From: Liao Chang @ 2024-04-09  1:23 UTC (permalink / raw)
  To: catalin.marinas, will, maz, oliver.upton, james.morse,
	suzuki.poulose, yuzenghui, tglx, mark.rutland, ardb, broonie,
	liaochang1, anshuman.khandual, miguel.luis, joey.gouly,
	ryan.roberts, jeremy.linton, liwei391, daniel.thompson,
	sumit.garg, kristina.martsenko, jpoimboe, ericchancf, robh, scott,
	songshuaishuai, shijie, bhe, akpm, thunder.leizhen, horms,
	rmk+kernel, takakura, dianders, swboyd, frederic, reijiw,
	akihiko.odaki, ruanjinjie
  Cc: linux-arm-kernel, linux-kernel, kvmarm
In-Reply-To: <20240409012344.3194724-1-liaochang1@huawei.com>

In Mark Brown's support for FEAT_NMI patchset [1], Mark Rutland suggest
to refactor the way of DAIF management via adding new "logical exception
mask" helpers that treat DAIF + PMR + ALLINT as separate elements.

A series of new exception mask helpers that has a similar interface as
the existing counterparts, which starts with "local_allint_". The usage
and behavior of new ones suppose to align with the old ones, otherwise,
some unexpected result will occurs.

[1] https://lore.kernel.org/linux-arm-kernel/Y4sH5qX5bK9xfEBp@lpieralisi/

Signed-off-by: Liao Chang <liaochang1@huawei.com>
---
 arch/arm64/include/asm/daifflags.h   | 240 +++++++++++++++++++++++++++
 arch/arm64/include/uapi/asm/ptrace.h |   1 +
 2 files changed, 241 insertions(+)

diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
index 55f57dfa8e2f..df4c4989babd 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -11,6 +11,7 @@
 #include <asm/barrier.h>
 #include <asm/cpufeature.h>
 #include <asm/ptrace.h>
+#include <asm/nmi.h>
 
 #define DAIF_PROCCTX		0
 #define DAIF_PROCCTX_NOIRQ	(PSR_I_BIT | PSR_F_BIT)
@@ -141,4 +142,243 @@ static inline void local_daif_inherit(struct pt_regs *regs)
 	 */
 	write_sysreg(flags, daif);
 }
+
+/*
+ * For Arm64 processor support Armv8.8 or later, kernel supports three types
+ * of irqflags, they used for corresponding configuration depicted as below:
+ *
+ * 1. When CONFIG_ARM64_PSEUDO_NMI and CONFIG_ARM64_NMI are not 'y', kernel
+ *    does not support handling NMI.
+ *
+ * 2. When CONFIG_ARM64_PSEUDO_NMI=y and irqchip.gicv3_pseudo_nmi=1, kernel
+ *    makes use of the CPU Interface PMR and GIC priority feature to support
+ *    handling NMI.
+ *
+ * 3. When CONFIG_ARM64_NMI=y and irqchip.gicv3_pseudo_nmi is not enabled,
+ *    kernel makes use of the FEAT_NMI extension added since Armv8.8 to
+ *    support handling NMI.
+ */
+union arch_irqflags {
+	unsigned long flags;
+	struct {
+		unsigned long pmr : 8;     // SYS_ICC_PMR_EL1
+		unsigned long daif : 10;   // PSTATE.DAIF at bits[6-9]
+		unsigned long allint : 14; // PSTATE.ALLINT at bits[13]
+	} fields;
+};
+
+typedef union arch_irqflags arch_irqflags_t;
+
+static inline void __pmr_local_allint_mask(void)
+{
+	WARN_ON(system_has_prio_mask_debugging() &&
+		(read_sysreg_s(SYS_ICC_PMR_EL1) ==
+		 (GIC_PRIO_IRQOFF | GIC_PRIO_PSR_I_SET)));
+	/*
+	 * Don't really care for a dsb here, we don't intend to enable
+	 * IRQs.
+	 */
+	gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET);
+}
+
+static inline void __nmi_local_allint_mask(void)
+{
+	_allint_set();
+}
+
+static inline void local_allint_mask(void)
+{
+	asm volatile(
+		"msr	daifset, #0xf		// local_daif_mask\n"
+		:
+		:
+		: "memory");
+
+	if (system_uses_irq_prio_masking())
+		__pmr_local_allint_mask();
+	else if (system_uses_nmi())
+		__nmi_local_allint_mask();
+
+	trace_hardirqs_off();
+}
+
+static inline arch_irqflags_t __pmr_local_allint_save_flags(void)
+{
+	arch_irqflags_t irqflags;
+
+	irqflags.fields.pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+	irqflags.fields.daif = read_sysreg(daif);
+	irqflags.fields.allint = 0;
+	/*
+	 * If IRQs are masked with PMR, reflect it in the daif of irqflags.
+	 * If NMIs and IRQs are masked with PMR, reflect it in the daif and
+	 * allint of irqflags, this avoid the need of checking PSTATE.A in
+	 * local_allint_restore() to determine if NMIs are masked.
+	 */
+	switch (irqflags.fields.pmr) {
+	case GIC_PRIO_IRQON:
+		break;
+
+	case __GIC_PRIO_IRQOFF:
+	case __GIC_PRIO_IRQOFF_NS:
+		irqflags.fields.daif |= PSR_I_BIT | PSR_F_BIT;
+		break;
+
+	case GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET:
+		irqflags.fields.allint = 1;
+		break;
+
+	default:
+		WARN_ON(1);
+	}
+
+	return irqflags;
+}
+
+static inline arch_irqflags_t __nmi_local_allint_save_flags(void)
+{
+	arch_irqflags_t irqflags;
+
+	irqflags.fields.daif = read_sysreg(daif);
+	irqflags.fields.allint = read_sysreg_s(SYS_ALLINT);
+
+	return irqflags;
+}
+
+static inline arch_irqflags_t local_allint_save_flags(void)
+{
+	arch_irqflags_t irqflags = { .flags = 0UL };
+
+	if (system_uses_irq_prio_masking())
+		return __pmr_local_allint_save_flags();
+	else if (system_uses_nmi())
+		return __nmi_local_allint_save_flags();
+
+	irqflags.fields.daif = read_sysreg(daif);
+	return irqflags;
+}
+
+static inline arch_irqflags_t local_allint_save(void)
+{
+	arch_irqflags_t irqflags;
+
+	irqflags = local_allint_save_flags();
+
+	local_allint_mask();
+
+	return irqflags;
+}
+
+static inline void gic_pmr_prio_check(void)
+{
+	WARN_ON(system_has_prio_mask_debugging() &&
+		(read_sysreg(daif) & (PSR_I_BIT | PSR_F_BIT)) !=
+		(PSR_I_BIT | PSR_F_BIT));
+}
+
+static inline void __pmr_local_allint_restore(arch_irqflags_t irqflags)
+{
+	unsigned long pmr = irqflags.fields.pmr;
+	unsigned long daif = irqflags.fields.daif;
+	unsigned long allint = irqflags.fields.allint;
+
+	gic_pmr_prio_check();
+
+	gic_write_pmr(pmr);
+
+	if (!(daif & PSR_I_BIT)) {
+		pmr_sync();
+	} else if (!allint) {
+		/*
+		 * Use arch_allint.fields.allint to indicates we can take
+		 * NMIs, instead of the old hacking style that use PSTATE.A.
+		 *
+		 * There has been concern that the write to daif
+		 * might be reordered before this write to PMR.
+		 * From the ARM ARM DDI 0487D.a, section D1.7.1
+		 * "Accessing PSTATE fields":
+		 *   Writes to the PSTATE fields have side-effects on
+		 *   various aspects of the PE operation. All of these
+		 *   side-effects are guaranteed:
+		 *     - Not to be visible to earlier instructions in
+		 *       the execution stream.
+		 *     - To be visible to later instructions in the
+		 *       execution stream
+		 *
+		 * Also, writes to PMR are self-synchronizing, so no
+		 * interrupts with a lower priority than PMR is signaled
+		 * to the PE after the write.
+		 *
+		 * So we don't need additional synchronization here.
+		 */
+		daif &= ~(PSR_I_BIT | PSR_F_BIT);
+	}
+	write_sysreg(daif, daif);
+}
+
+static inline void __nmi_local_allint_restore(arch_irqflags_t irqflags)
+{
+	if (irqflags.fields.allint)
+		_allint_set();
+	else
+		_allint_clear();
+
+	write_sysreg(irqflags.fields.daif, daif);
+}
+
+static inline int local_allint_disabled(arch_irqflags_t irqflags)
+{
+	return irqflags.fields.allint || (irqflags.fields.daif & PSR_I_BIT);
+}
+
+/*
+ * It has to conside the different kernel configure and parameters, that need
+ * to use coresspoding operations to mask interrupts properly. For example, the
+ * kernel disable PSEUDO_NMI, the kernel uses prio masking to support
+ * PSEUDO_NMI, or the kernel uses FEAT_NMI extension to support PSEUDO_NMI.
+ */
+static inline void local_allint_restore(arch_irqflags_t irqflags)
+{
+	int irq_disabled = local_allint_disabled(irqflags);
+
+	if (!irq_disabled)
+		trace_hardirqs_on();
+
+	if (system_uses_irq_prio_masking())
+		__pmr_local_allint_restore(irqflags);
+	else if (system_uses_nmi())
+		__nmi_local_allint_restore(irqflags);
+	else
+		write_sysreg(irqflags.fields.daif, daif);
+
+	if (irq_disabled)
+		trace_hardirqs_off();
+}
+
+/*
+ * Called by synchronous exception handlers to restore the DAIF bits that were
+ * modified by taking an exception.
+ */
+static inline void local_allint_inherit(struct pt_regs *regs)
+{
+	if (interrupts_enabled(regs))
+		trace_hardirqs_on();
+
+	if (system_uses_irq_prio_masking())
+		gic_write_pmr(regs->pmr_save);
+
+	/*
+	 * We can't use local_daif_restore(regs->pstate) here as
+	 * system_has_prio_mask_debugging() won't restore the I bit if it can
+	 * use the pmr instead.
+	 */
+	write_sysreg(regs->pstate & DAIF_MASK, daif);
+
+	if (system_uses_nmi()) {
+		if (regs->pstate & PSR_ALLINT_BIT)
+			_allint_set();
+		else
+			_allint_clear();
+	}
+}
 #endif
diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index 7fa2f7036aa7..8a125a1986be 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -48,6 +48,7 @@
 #define PSR_D_BIT	0x00000200
 #define PSR_BTYPE_MASK	0x00000c00
 #define PSR_SSBS_BIT	0x00001000
+#define PSR_ALLINT_BIT	0x00002000
 #define PSR_PAN_BIT	0x00400000
 #define PSR_UAO_BIT	0x00800000
 #define PSR_DIT_BIT	0x01000000
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v2] ASoC: dt-bindings: imx-audio-spdif: convert to YAML
From: Shengjiu Wang @ 2024-04-09  1:37 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Shengjiu Wang, lgirdwood, broonie, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-sound, devicetree,
	linux-kernel, shawnguo, s.hauer, kernel, festevam, imx,
	linux-arm-kernel
In-Reply-To: <69ab2bd9-401a-42ff-90fa-6600dd071722@linaro.org>

On Mon, Apr 8, 2024 at 11:06 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/04/2024 10:01, Shengjiu Wang wrote:
> >>> +
> >>> +anyOf:
> >>> +  - required:
> >>> +      - spdif-in
> >>> +  - required:
> >>> +      - spdif-out
> >>> +  - required:
> >>> +      - spdif-out
> >>> +      - spdif-in
> >>
> >> Do you need the last required block?
> >
> > Yes,  one of them or both are required.
>
> And? It's already there: that's the meaning of any. It is not oneOf...
> Before answering please test your changes and ideas. I pointed issue
> here and you responded just to close my comment. That does not make me
> happy, just wastes my time.

Maybe I didn't express clearly.

we need at least one of them (spdif-in, spdif-out) in the node.  which means
that we need to select  "spdif-in",  or "spdif-out",  or "spdif-in and
spdif-out".

So my understanding is that need to use "anyOf", if it is wrong, please let
me know.

>
> >
> >>
> >>> +
> >>> +additionalProperties: false
> >>> +
> >>> +examples:
> >>> +  - |
> >>> +    sound {
> >>
> >> That's a random change...

So  I can use "sound-spdif", right?

best regards
wang shengjiu

> >>
> >> Instead of sending two patches per day, please carefully address the
> >> feedback.
> >
> > In v1 you suggest to change it to spdif?  but spdif may conflict
> > with the fsl,spdif.yaml.   so which name I should use?
>
> I don't understand where is the conflict. That's a different binding.
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v1 2/2] of: dynamic: Fix probing of overlay devices
From: Saravana Kannan @ 2024-04-09  1:40 UTC (permalink / raw)
  To: Herve Codina, Geert Uytterhoeven
  Cc: Rob Herring, kernel-team, linux-kernel, imx, linux-arm-kernel,
	linux-i2c, devicetree, linux-spi, linux-acpi
In-Reply-To: <20240408231310.325451-3-saravanak@google.com>

On Mon, Apr 8, 2024 at 4:13 PM Saravana Kannan <saravanak@google.com> wrote:
>
> Get fw_devlink to work well with overlay devices.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  drivers/base/core.c    | 10 ++++++++++
>  drivers/of/dynamic.c   |  8 ++++++++
>  include/linux/fwnode.h |  2 ++
>  3 files changed, 20 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 5f4e03336e68..d856f9c5d601 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -237,6 +237,16 @@ static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
>                 __fw_devlink_pickup_dangling_consumers(child, new_sup);
>  }
>
> +
> +void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
> +                                                struct fwnode_handle *parent)
> +{
> +       mutex_lock(&fwnode_link_lock);
> +       __fw_devlink_pickup_dangling_consumers(child, parent);
> +       __fw_devlink_link_to_consumers(parent->dev);
> +       mutex_unlock(&fwnode_link_lock);
> +}
> +
>  static DEFINE_MUTEX(device_links_lock);
>  DEFINE_STATIC_SRCU(device_links_srcu);
>
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index 19a1a38554f2..0a936f46820e 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -237,6 +237,7 @@ static void __of_attach_node(struct device_node *np)
>  int of_attach_node(struct device_node *np)
>  {
>         struct of_reconfig_data rd;
> +       struct fwnode_handle *fwnode, *parent;
>
>         memset(&rd, 0, sizeof(rd));
>         rd.dn = np;
> @@ -246,6 +247,13 @@ int of_attach_node(struct device_node *np)
>         mutex_unlock(&of_mutex);
>
>         of_reconfig_notify(OF_RECONFIG_ATTACH_NODE, &rd);
> +       fwnode = of_fwnode_handle(np);
> +       fwnode_for_each_parent_node(fwnode, parent)
> +               if (parent->dev) {
> +                       fw_devlink_pickup_dangling_consumers(fwnode, parent);
> +                       fwnode_handle_put(parent);
> +                       break;
> +               }

I'm clearly calling this in the wrong location. Please move this logic
to __of_changeset_entry_notify() and for the case
OF_RECONFIG_ATTACH_NODE. Haven't fully thought through the DETACH
case, but it should work correctly for that case too. If not, I'll
take care of that next.

-Saravana

>
>         return 0;
>  }
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 0d79070c5a70..4b3f697a90e8 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -220,6 +220,8 @@ int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,
>                     u8 flags);
>  void fwnode_links_purge(struct fwnode_handle *fwnode);
>  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
> +void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
> +                                         struct fwnode_handle *parent);
>  bool fw_devlink_is_strict(void);
>
>  #endif
> --
> 2.44.0.478.gd926399ef9-goog
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
From: Thinh Nguyen @ 2024-04-09  1:54 UTC (permalink / raw)
  To: Anand Moon
  Cc: Thinh Nguyen, Greg Kroah-Hartman, Krzysztof Kozlowski,
	Alim Akhtar, Christophe JAILLET, Johan Hovold,
	linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20240404071350.4242-7-linux.amoon@gmail.com>

On Thu, Apr 04, 2024, Anand Moon wrote:
> Use the new PM macros for the suspend and resume functions to be
> automatically dropped by the compiler when CONFIG_PM_SLEEP are disabled,
> without having to use #ifdef guards. If CONFIG_PM_SLEEP unused,
> they will simply be discarded by the compiler.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM is
>    disabled.

The compiler discards the code, yet we still need __maybe_unused?

BR,
Thinh

> ---
>  drivers/usb/dwc3/dwc3-exynos.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH net] net: dsa: mt7530: trap link-local frames regardless of ST Port State
From: Daniel Golle @ 2024-04-09  2:08 UTC (permalink / raw)
  To: arinc.unal
  Cc: DENG Qingfang, Sean Wang, Andrew Lunn, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
	Bartel Eerdekens, mithat.guner, erkin.bozoglu, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <20240407-b4-for-net-mt7530-fix-link-local-when-stp-discarding-v1-1-b4b20ac93457@arinc9.com>

On Sun, Apr 07, 2024 at 09:11:11PM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> In Clause 5 of IEEE Std 802-2014, two sublayers of the data link layer
> (DLL) of the Open Systems Interconnection basic reference model (OSI/RM)
> are described; the medium access control (MAC) and logical link control
> (LLC) sublayers. The MAC sublayer is the one facing the physical layer.
> 
> In 8.2 of IEEE Std 802.1Q-2022, the Bridge architecture is described. A
> Bridge component comprises a MAC Relay Entity for interconnecting the Ports
> of the Bridge, at least two Ports, and higher layer entities with at least
> a Spanning Tree Protocol Entity included.
> 
> Each Bridge Port also functions as an end station and shall provide the MAC
> Service to an LLC Entity. Each instance of the MAC Service is provided to a
> distinct LLC Entity that supports protocol identification, multiplexing,
> and demultiplexing, for protocol data unit (PDU) transmission and reception
> by one or more higher layer entities.
> 
> It is described in 8.13.9 of IEEE Std 802.1Q-2022 that in a Bridge, the LLC
> Entity associated with each Bridge Port is modeled as being directly
> connected to the attached Local Area Network (LAN).
> 
> On the switch with CPU port architecture, CPU port functions as Management
> Port, and the Management Port functionality is provided by software which
> functions as an end station. Software is connected to an IEEE 802 LAN that
> is wholly contained within the system that incorporates the Bridge.
> Software provides access to the LLC Entity associated with each Bridge Port
> by the value of the source port field on the special tag on the frame
> received by software.
> 
> We call frames that carry control information to determine the active
> topology and current extent of each Virtual Local Area Network (VLAN),
> i.e., spanning tree or Shortest Path Bridging (SPB) and Multiple VLAN
> Registration Protocol Data Units (MVRPDUs), and frames from other link
> constrained protocols, such as Extensible Authentication Protocol over LAN
> (EAPOL) and Link Layer Discovery Protocol (LLDP), link-local frames. They
> are not forwarded by a Bridge. Permanently configured entries in the
> filtering database (FDB) ensure that such frames are discarded by the
> Forwarding Process. In 8.6.3 of IEEE Std 802.1Q-2022, this is described in
> detail:
> 
> Each of the reserved MAC addresses specified in Table 8-1
> (01-80-C2-00-00-[00,01,02,03,04,05,06,07,08,09,0A,0B,0C,0D,0E,0F]) shall be
> permanently configured in the FDB in C-VLAN components and ERs.
> 
> Each of the reserved MAC addresses specified in Table 8-2
> (01-80-C2-00-00-[01,02,03,04,05,06,07,08,09,0A,0E]) shall be permanently
> configured in the FDB in S-VLAN components.
> 
> Each of the reserved MAC addresses specified in Table 8-3
> (01-80-C2-00-00-[01,02,04,0E]) shall be permanently configured in the FDB
> in TPMR components.
> 
> The FDB entries for reserved MAC addresses shall specify filtering for all
> Bridge Ports and all VIDs. Management shall not provide the capability to
> modify or remove entries for reserved MAC addresses.
> 
> The addresses in Table 8-1, Table 8-2, and Table 8-3 determine the scope of
> propagation of PDUs within a Bridged Network, as follows:
> 
>   The Nearest Bridge group address (01-80-C2-00-00-0E) is an address that
>   no conformant Two-Port MAC Relay (TPMR) component, Service VLAN (S-VLAN)
>   component, Customer VLAN (C-VLAN) component, or MAC Bridge can forward.
>   PDUs transmitted using this destination address, or any other addresses
>   that appear in Table 8-1, Table 8-2, and Table 8-3
>   (01-80-C2-00-00-[00,01,02,03,04,05,06,07,08,09,0A,0B,0C,0D,0E,0F]), can
>   therefore travel no further than those stations that can be reached via a
>   single individual LAN from the originating station.
> 
>   The Nearest non-TPMR Bridge group address (01-80-C2-00-00-03), is an
>   address that no conformant S-VLAN component, C-VLAN component, or MAC
>   Bridge can forward; however, this address is relayed by a TPMR component.
>   PDUs using this destination address, or any of the other addresses that
>   appear in both Table 8-1 and Table 8-2 but not in Table 8-3
>   (01-80-C2-00-00-[00,03,05,06,07,08,09,0A,0B,0C,0D,0F]), will be relayed
>   by any TPMRs but will propagate no further than the nearest S-VLAN
>   component, C-VLAN component, or MAC Bridge.
> 
>   The Nearest Customer Bridge group address (01-80-C2-00-00-00) is an
>   address that no conformant C-VLAN component, MAC Bridge can forward;
>   however, it is relayed by TPMR components and S-VLAN components. PDUs
>   using this destination address, or any of the other addresses that appear
>   in Table 8-1 but not in either Table 8-2 or Table 8-3
>   (01-80-C2-00-00-[00,0B,0C,0D,0F]), will be relayed by TPMR components and
>   S-VLAN components but will propagate no further than the nearest C-VLAN
>   component or MAC Bridge.
> 
> Because the LLC Entity associated with each Bridge Port is provided via CPU
> port, we must not filter these frames but forward them to CPU port.
> 
> In a Bridge, the transmission Port is majorly decided by ingress and egress
> rules, FDB, and spanning tree Port State functions of the Forwarding
> Process. For link-local frames, only CPU port should be designated as
> destination port in the FDB, and the other functions of the Forwarding
> Process must not interfere with the decision of the transmission Port. We
> call this process trapping frames to CPU port.
> 
> Therefore, on the switch with CPU port architecture, link-local frames must
> be trapped to CPU port, and certain link-local frames received by a Port of
> a Bridge comprising a TPMR component or an S-VLAN component must be
> excluded from it.
> 
> A Bridge of the switch with CPU architecture cannot comprise a non-Two-Port
> MAC Relay (TPMR) component as TPMR component is only supposed to support a
> subset of the functionality of a MAC Bridge. A Bridge comprising two ports
> (Management Port doesn't count) of this architecture will either function
> as a standard MAC Bridge or a standard VLAN Bridge.
> 
> Therefore, a Bridge of this architecture can only comprise S-VLAN
> components, C-VLAN components, or MAC Bridge components. Since there's no
> TPMR component, we don't need to relay PDUs using the destination addresses
> specified on the Nearest non-TPMR section, and the proportion of the
> Nearest Customer Bridge section where they must be relayed by TPMR
> components.
> 
> One option to trap link-local frames to CPU port is to add static FDB
> entries with CPU port designated as destination port. However, because that
> Independent VLAN Learning (IVL) is being used on every VID, each entry only
> applies to a single VLAN Identifier (VID). For a Bridge comprising a MAC
> Bridge component or a C-VLAN component, there would have to be 16 times
> 4096 entries. This switch intellectual property can only hold a maximum of
> 2048 entries. Using this option, there also isn't a mechanism to prevent
> link-local frames from being discarded when the spanning tree Port State of
> the reception Port is discarding.
> 
> The remaining option is to utilise the BPC, RGAC1, RGAC2, RGAC3, and RGAC4
> registers. Whilst this applies to every VID, it doesn't contain all of the
> reserved MAC addresses without affecting the remaining Standard Group MAC
> Addresses. The REV_UN frame tag utilised using the RGAC4 register covers
> the remaining 01-80-C2-00-00-[04,05,06,07,08,09,0A,0B,0C,0D,0F] destination
> addresses. It also includes the 01-80-C2-00-00-22 to 01-80-C2-00-00-FF
> destination addresses which may be relayed by MAC Bridges or VLAN Bridges.
> The latter option provides better but not complete conformance.
> 
> This switch intellectual property also does not provide a mechanism to trap
> link-local frames with specific destination addresses to CPU port by
> Bridge, to conform to the filtering rules for the distinct Bridge
> components.
> 
> Therefore, regardless of the type of the Bridge component, link-local
> frames with these destination addresses will be trapped to CPU port:
> 
> 01-80-C2-00-00-[00,01,02,03,0E]
> 
> In a Bridge comprising a MAC Bridge component or a C-VLAN component:
> 
>   Link-local frames with these destination addresses won't be trapped to
>   CPU port which won't conform to IEEE Std 802.1Q-2022:
> 
>   01-80-C2-00-00-[04,05,06,07,08,09,0A,0B,0C,0D,0F]
> 
> In a Bridge comprising an S-VLAN component:
> 
>   Link-local frames with these destination addresses will be trapped to CPU
>   port which won't conform to IEEE Std 802.1Q-2022:
> 
>   01-80-C2-00-00-00
> 
>   Link-local frames with these destination addresses won't be trapped to
>   CPU port which won't conform to IEEE Std 802.1Q-2022:
> 
>   01-80-C2-00-00-[04,05,06,07,08,09,0A]
> 
> Currently on this switch intellectual property, if the spanning tree Port
> State of the reception Port is discarding, link-local frames will be
> discarded.
> 
> To trap link-local frames regardless of the spanning tree Port State, make
> the switch regard them as BPDU. This switch intellectual property only lets
> the frames regarded as BPDU bypass the spanning tree Port State function of
> the Forwarding Process.
> 
> With this change, the only remaining interference left is the ingress
> rules. When the reception Port has no PVID assigned on software,
> VLAN-untagged frames won't be allowed in. There doesn't seem to be a
> mechanism on the switch intellectual property to have link-local frames
> bypass this function of the Forwarding Process.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Reviewed-by: Daniel Golle <daniel@makrotopia.org>

Thank you for the excellent write up!

> ---
>  drivers/net/dsa/mt7530.c | 229 ++++++++++++++++++++++++++++++++++++++++-------
>  drivers/net/dsa/mt7530.h |   5 ++
>  2 files changed, 200 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 1035820c2377..254501f63731 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -950,20 +950,173 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
>  	mutex_unlock(&priv->reg_mutex);
>  }
>  
> -/* On page 205, section "8.6.3 Frame filtering" of the active standard, IEEE Std
> - * 802.1Q™-2022, it is stated that frames with 01:80:C2:00:00:00-0F as MAC DA
> - * must only be propagated to C-VLAN and MAC Bridge components. That means
> - * VLAN-aware and VLAN-unaware bridges. On the switch designs with CPU ports,
> - * these frames are supposed to be processed by the CPU (software). So we make
> - * the switch only forward them to the CPU port. And if received from a CPU
> - * port, forward to a single port. The software is responsible of making the
> - * switch conform to the latter by setting a single port as destination port on
> - * the special tag.
> +/* In Clause 5 of IEEE Std 802-2014, two sublayers of the data link layer (DLL)
> + * of the Open Systems Interconnection basic reference model (OSI/RM) are
> + * described; the medium access control (MAC) and logical link control (LLC)
> + * sublayers. The MAC sublayer is the one facing the physical layer.
>   *
> - * This switch intellectual property cannot conform to this part of the standard
> - * fully. Whilst the REV_UN frame tag covers the remaining :04-0D and :0F MAC
> - * DAs, it also includes :22-FF which the scope of propagation is not supposed
> - * to be restricted for these MAC DAs.
> + * In 8.2 of IEEE Std 802.1Q-2022, the Bridge architecture is described. A
> + * Bridge component comprises a MAC Relay Entity for interconnecting the Ports
> + * of the Bridge, at least two Ports, and higher layer entities with at least a
> + * Spanning Tree Protocol Entity included.
> + *
> + * Each Bridge Port also functions as an end station and shall provide the MAC
> + * Service to an LLC Entity. Each instance of the MAC Service is provided to a
> + * distinct LLC Entity that supports protocol identification, multiplexing, and
> + * demultiplexing, for protocol data unit (PDU) transmission and reception by
> + * one or more higher layer entities.
> + *
> + * It is described in 8.13.9 of IEEE Std 802.1Q-2022 that in a Bridge, the LLC
> + * Entity associated with each Bridge Port is modeled as being directly
> + * connected to the attached Local Area Network (LAN).
> + *
> + * On the switch with CPU port architecture, CPU port functions as Management
> + * Port, and the Management Port functionality is provided by software which
> + * functions as an end station. Software is connected to an IEEE 802 LAN that is
> + * wholly contained within the system that incorporates the Bridge. Software
> + * provides access to the LLC Entity associated with each Bridge Port by the
> + * value of the source port field on the special tag on the frame received by
> + * software.
> + *
> + * We call frames that carry control information to determine the active
> + * topology and current extent of each Virtual Local Area Network (VLAN), i.e.,
> + * spanning tree or Shortest Path Bridging (SPB) and Multiple VLAN Registration
> + * Protocol Data Units (MVRPDUs), and frames from other link constrained
> + * protocols, such as Extensible Authentication Protocol over LAN (EAPOL) and
> + * Link Layer Discovery Protocol (LLDP), link-local frames. They are not
> + * forwarded by a Bridge. Permanently configured entries in the filtering
> + * database (FDB) ensure that such frames are discarded by the Forwarding
> + * Process. In 8.6.3 of IEEE Std 802.1Q-2022, this is described in detail:
> + *
> + * Each of the reserved MAC addresses specified in Table 8-1
> + * (01-80-C2-00-00-[00,01,02,03,04,05,06,07,08,09,0A,0B,0C,0D,0E,0F]) shall be
> + * permanently configured in the FDB in C-VLAN components and ERs.
> + *
> + * Each of the reserved MAC addresses specified in Table 8-2
> + * (01-80-C2-00-00-[01,02,03,04,05,06,07,08,09,0A,0E]) shall be permanently
> + * configured in the FDB in S-VLAN components.
> + *
> + * Each of the reserved MAC addresses specified in Table 8-3
> + * (01-80-C2-00-00-[01,02,04,0E]) shall be permanently configured in the FDB in
> + * TPMR components.
> + *
> + * The FDB entries for reserved MAC addresses shall specify filtering for all
> + * Bridge Ports and all VIDs. Management shall not provide the capability to
> + * modify or remove entries for reserved MAC addresses.
> + *
> + * The addresses in Table 8-1, Table 8-2, and Table 8-3 determine the scope of
> + * propagation of PDUs within a Bridged Network, as follows:
> + *
> + *   The Nearest Bridge group address (01-80-C2-00-00-0E) is an address that no
> + *   conformant Two-Port MAC Relay (TPMR) component, Service VLAN (S-VLAN)
> + *   component, Customer VLAN (C-VLAN) component, or MAC Bridge can forward.
> + *   PDUs transmitted using this destination address, or any other addresses
> + *   that appear in Table 8-1, Table 8-2, and Table 8-3
> + *   (01-80-C2-00-00-[00,01,02,03,04,05,06,07,08,09,0A,0B,0C,0D,0E,0F]), can
> + *   therefore travel no further than those stations that can be reached via a
> + *   single individual LAN from the originating station.
> + *
> + *   The Nearest non-TPMR Bridge group address (01-80-C2-00-00-03), is an
> + *   address that no conformant S-VLAN component, C-VLAN component, or MAC
> + *   Bridge can forward; however, this address is relayed by a TPMR component.
> + *   PDUs using this destination address, or any of the other addresses that
> + *   appear in both Table 8-1 and Table 8-2 but not in Table 8-3
> + *   (01-80-C2-00-00-[00,03,05,06,07,08,09,0A,0B,0C,0D,0F]), will be relayed by
> + *   any TPMRs but will propagate no further than the nearest S-VLAN component,
> + *   C-VLAN component, or MAC Bridge.
> + *
> + *   The Nearest Customer Bridge group address (01-80-C2-00-00-00) is an address
> + *   that no conformant C-VLAN component, MAC Bridge can forward; however, it is
> + *   relayed by TPMR components and S-VLAN components. PDUs using this
> + *   destination address, or any of the other addresses that appear in Table 8-1
> + *   but not in either Table 8-2 or Table 8-3 (01-80-C2-00-00-[00,0B,0C,0D,0F]),
> + *   will be relayed by TPMR components and S-VLAN components but will propagate
> + *   no further than the nearest C-VLAN component or MAC Bridge.
> + *
> + * Because the LLC Entity associated with each Bridge Port is provided via CPU
> + * port, we must not filter these frames but forward them to CPU port.
> + *
> + * In a Bridge, the transmission Port is majorly decided by ingress and egress
> + * rules, FDB, and spanning tree Port State functions of the Forwarding Process.
> + * For link-local frames, only CPU port should be designated as destination port
> + * in the FDB, and the other functions of the Forwarding Process must not
> + * interfere with the decision of the transmission Port. We call this process
> + * trapping frames to CPU port.
> + *
> + * Therefore, on the switch with CPU port architecture, link-local frames must
> + * be trapped to CPU port, and certain link-local frames received by a Port of a
> + * Bridge comprising a TPMR component or an S-VLAN component must be excluded
> + * from it.
> + *
> + * A Bridge of the switch with CPU architecture cannot comprise a non-Two-Port
> + * MAC Relay (TPMR) component as TPMR component is only supposed to support a
> + * subset of the functionality of a MAC Bridge. A Bridge comprising two ports
> + * (Management Port doesn't count) of this architecture will either function as
> + * a standard MAC Bridge or a standard VLAN Bridge.
> + *
> + * Therefore, a Bridge of this architecture can only comprise S-VLAN components,
> + * C-VLAN components, or MAC Bridge components. Since there's no TPMR component,
> + * we don't need to relay PDUs using the destination addresses specified on the
> + * Nearest non-TPMR section, and the proportion of the Nearest Customer Bridge
> + * section where they must be relayed by TPMR components.
> + *
> + * One option to trap link-local frames to CPU port is to add static FDB entries
> + * with CPU port designated as destination port. However, because that
> + * Independent VLAN Learning (IVL) is being used on every VID, each entry only
> + * applies to a single VLAN Identifier (VID). For a Bridge comprising a MAC
> + * Bridge component or a C-VLAN component, there would have to be 16 times 4096
> + * entries. This switch intellectual property can only hold a maximum of 2048
> + * entries. Using this option, there also isn't a mechanism to prevent
> + * link-local frames from being discarded when the spanning tree Port State of
> + * the reception Port is discarding.
> + *
> + * The remaining option is to utilise the BPC, RGAC1, RGAC2, RGAC3, and RGAC4
> + * registers. Whilst this applies to every VID, it doesn't contain all of the
> + * reserved MAC addresses without affecting the remaining Standard Group MAC
> + * Addresses. The REV_UN frame tag utilised using the RGAC4 register covers the
> + * remaining 01-80-C2-00-00-[04,05,06,07,08,09,0A,0B,0C,0D,0F] destination
> + * addresses. It also includes the 01-80-C2-00-00-22 to 01-80-C2-00-00-FF
> + * destination addresses which may be relayed by MAC Bridges or VLAN Bridges.
> + * The latter option provides better but not complete conformance.
> + *
> + * This switch intellectual property also does not provide a mechanism to trap
> + * link-local frames with specific destination addresses to CPU port by Bridge,
> + * to conform to the filtering rules for the distinct Bridge components.
> + *
> + * Therefore, regardless of the type of the Bridge component, link-local frames
> + * with these destination addresses will be trapped to CPU port:
> + *
> + * 01-80-C2-00-00-[00,01,02,03,0E]
> + *
> + * In a Bridge comprising a MAC Bridge component or a C-VLAN component:
> + *
> + *   Link-local frames with these destination addresses won't be trapped to CPU
> + *   port which won't conform to IEEE Std 802.1Q-2022:
> + *
> + *   01-80-C2-00-00-[04,05,06,07,08,09,0A,0B,0C,0D,0F]
> + *
> + * In a Bridge comprising an S-VLAN component:
> + *
> + *   Link-local frames with these destination addresses will be trapped to CPU
> + *   port which won't conform to IEEE Std 802.1Q-2022:
> + *
> + *   01-80-C2-00-00-00
> + *
> + *   Link-local frames with these destination addresses won't be trapped to CPU
> + *   port which won't conform to IEEE Std 802.1Q-2022:
> + *
> + *   01-80-C2-00-00-[04,05,06,07,08,09,0A]
> + *
> + * To trap link-local frames to CPU port as conformant as this switch
> + * intellectual property can allow, link-local frames are made to be regarded as
> + * BPDU. This is because this switch intellectual property only lets the frames
> + * regarded as BPDU bypass the spanning tree Port State function of the
> + * Forwarding Process.
> + *
> + * The only remaining interference left is the ingress rules. When the reception
> + * Port has no PVID assigned on software, VLAN-untagged frames won't be allowed
> + * in. There doesn't seem to be a mechanism on the switch intellectual property
> + * to have link-local frames bypass this function of the Forwarding Process.
>   */
>  static void
>  mt753x_trap_frames(struct mt7530_priv *priv)
> @@ -971,35 +1124,43 @@ mt753x_trap_frames(struct mt7530_priv *priv)
>  	/* Trap 802.1X PAE frames and BPDUs to the CPU port(s) and egress them
>  	 * VLAN-untagged.
>  	 */
> -	mt7530_rmw(priv, MT753X_BPC, MT753X_PAE_EG_TAG_MASK |
> -		   MT753X_PAE_PORT_FW_MASK | MT753X_BPDU_EG_TAG_MASK |
> -		   MT753X_BPDU_PORT_FW_MASK,
> -		   MT753X_PAE_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> -		   MT753X_PAE_PORT_FW(MT753X_BPDU_CPU_ONLY) |
> -		   MT753X_BPDU_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> -		   MT753X_BPDU_CPU_ONLY);
> +	mt7530_rmw(priv, MT753X_BPC,
> +		   MT753X_PAE_BPDU_FR | MT753X_PAE_EG_TAG_MASK |
> +			   MT753X_PAE_PORT_FW_MASK | MT753X_BPDU_EG_TAG_MASK |
> +			   MT753X_BPDU_PORT_FW_MASK,
> +		   MT753X_PAE_BPDU_FR |
> +			   MT753X_PAE_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> +			   MT753X_PAE_PORT_FW(MT753X_BPDU_CPU_ONLY) |
> +			   MT753X_BPDU_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> +			   MT753X_BPDU_CPU_ONLY);
>  
>  	/* Trap frames with :01 and :02 MAC DAs to the CPU port(s) and egress
>  	 * them VLAN-untagged.
>  	 */
> -	mt7530_rmw(priv, MT753X_RGAC1, MT753X_R02_EG_TAG_MASK |
> -		   MT753X_R02_PORT_FW_MASK | MT753X_R01_EG_TAG_MASK |
> -		   MT753X_R01_PORT_FW_MASK,
> -		   MT753X_R02_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> -		   MT753X_R02_PORT_FW(MT753X_BPDU_CPU_ONLY) |
> -		   MT753X_R01_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> -		   MT753X_BPDU_CPU_ONLY);
> +	mt7530_rmw(priv, MT753X_RGAC1,
> +		   MT753X_R02_BPDU_FR | MT753X_R02_EG_TAG_MASK |
> +			   MT753X_R02_PORT_FW_MASK | MT753X_R01_BPDU_FR |
> +			   MT753X_R01_EG_TAG_MASK | MT753X_R01_PORT_FW_MASK,
> +		   MT753X_R02_BPDU_FR |
> +			   MT753X_R02_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> +			   MT753X_R02_PORT_FW(MT753X_BPDU_CPU_ONLY) |
> +			   MT753X_R01_BPDU_FR |
> +			   MT753X_R01_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> +			   MT753X_BPDU_CPU_ONLY);
>  
>  	/* Trap frames with :03 and :0E MAC DAs to the CPU port(s) and egress
>  	 * them VLAN-untagged.
>  	 */
> -	mt7530_rmw(priv, MT753X_RGAC2, MT753X_R0E_EG_TAG_MASK |
> -		   MT753X_R0E_PORT_FW_MASK | MT753X_R03_EG_TAG_MASK |
> -		   MT753X_R03_PORT_FW_MASK,
> -		   MT753X_R0E_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> -		   MT753X_R0E_PORT_FW(MT753X_BPDU_CPU_ONLY) |
> -		   MT753X_R03_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> -		   MT753X_BPDU_CPU_ONLY);
> +	mt7530_rmw(priv, MT753X_RGAC2,
> +		   MT753X_R0E_BPDU_FR | MT753X_R0E_EG_TAG_MASK |
> +			   MT753X_R0E_PORT_FW_MASK | MT753X_R03_BPDU_FR |
> +			   MT753X_R03_EG_TAG_MASK | MT753X_R03_PORT_FW_MASK,
> +		   MT753X_R0E_BPDU_FR |
> +			   MT753X_R0E_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> +			   MT753X_R0E_PORT_FW(MT753X_BPDU_CPU_ONLY) |
> +			   MT753X_R03_BPDU_FR |
> +			   MT753X_R03_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
> +			   MT753X_BPDU_CPU_ONLY);
>  }
>  
>  static void
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index d17b318e6ee4..2deffe741484 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -65,6 +65,7 @@ enum mt753x_id {
>  
>  /* Registers for BPDU and PAE frame control*/
>  #define MT753X_BPC			0x24
> +#define  MT753X_PAE_BPDU_FR		BIT(25)
>  #define  MT753X_PAE_EG_TAG_MASK		GENMASK(24, 22)
>  #define  MT753X_PAE_EG_TAG(x)		FIELD_PREP(MT753X_PAE_EG_TAG_MASK, x)
>  #define  MT753X_PAE_PORT_FW_MASK	GENMASK(18, 16)
> @@ -75,20 +76,24 @@ enum mt753x_id {
>  
>  /* Register for :01 and :02 MAC DA frame control */
>  #define MT753X_RGAC1			0x28
> +#define  MT753X_R02_BPDU_FR		BIT(25)
>  #define  MT753X_R02_EG_TAG_MASK		GENMASK(24, 22)
>  #define  MT753X_R02_EG_TAG(x)		FIELD_PREP(MT753X_R02_EG_TAG_MASK, x)
>  #define  MT753X_R02_PORT_FW_MASK	GENMASK(18, 16)
>  #define  MT753X_R02_PORT_FW(x)		FIELD_PREP(MT753X_R02_PORT_FW_MASK, x)
> +#define  MT753X_R01_BPDU_FR		BIT(9)
>  #define  MT753X_R01_EG_TAG_MASK		GENMASK(8, 6)
>  #define  MT753X_R01_EG_TAG(x)		FIELD_PREP(MT753X_R01_EG_TAG_MASK, x)
>  #define  MT753X_R01_PORT_FW_MASK	GENMASK(2, 0)
>  
>  /* Register for :03 and :0E MAC DA frame control */
>  #define MT753X_RGAC2			0x2c
> +#define  MT753X_R0E_BPDU_FR		BIT(25)
>  #define  MT753X_R0E_EG_TAG_MASK		GENMASK(24, 22)
>  #define  MT753X_R0E_EG_TAG(x)		FIELD_PREP(MT753X_R0E_EG_TAG_MASK, x)
>  #define  MT753X_R0E_PORT_FW_MASK	GENMASK(18, 16)
>  #define  MT753X_R0E_PORT_FW(x)		FIELD_PREP(MT753X_R0E_PORT_FW_MASK, x)
> +#define  MT753X_R03_BPDU_FR		BIT(9)
>  #define  MT753X_R03_EG_TAG_MASK		GENMASK(8, 6)
>  #define  MT753X_R03_EG_TAG(x)		FIELD_PREP(MT753X_R03_EG_TAG_MASK, x)
>  #define  MT753X_R03_PORT_FW_MASK	GENMASK(2, 0)
> 
> ---
> base-commit: 365af7ace014ef3fc6f5d0a373c96cc7193db4ce
> change-id: 20240401-b4-for-net-mt7530-fix-link-local-when-stp-discarding-6e2a4e3e867a
> 
> Best regards,
> -- 
> Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v4 1/2] dt-bindings: phy: mediatek,mt7988-xfi-tphy: add new bindings
From: Daniel Golle @ 2024-04-09  2:23 UTC (permalink / raw)
  To: Vinod Koul, Bc-bocun Chen, Steven Liu, John Crispin, Chunfeng Yun,
	Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Daniel Golle, Qingfang Deng, SkyLake Huang, Philipp Zabel,
	linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
	linux-kernel, netdev

Add bindings for the MediaTek XFI Ethernet SerDes T-PHY found in the
MediaTek MT7988 SoC which can operate at various interfaces modes:

via USXGMII PCS:
 * USXGMII
 * 10GBase-R
 * 5GBase-R

via LynxI SGMII PCS:
 * 2500Base-X
 * 1000Base-X
 * Cisco SGMII (MAC side)

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
v4: no changes
v3: Add reference to MediaTek-internal "pextp" name, better explain reset as
    well as 10GBase-R tuning work-around.
v2: unify filename and compatible as requested

 .../phy/mediatek,mt7988-xfi-tphy.yaml         | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt7988-xfi-tphy.yaml

diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7988-xfi-tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt7988-xfi-tphy.yaml
new file mode 100644
index 00000000000000..cfb3ca97f87c68
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt7988-xfi-tphy.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mediatek,mt7988-xfi-tphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek MT7988 XFI T-PHY
+
+maintainers:
+  - Daniel Golle <daniel@makrotopia.org>
+
+description:
+  The MediaTek XFI SerDes T-PHY provides the physical SerDes lanes
+  used by the (10G/5G) USXGMII PCS and (1G/2.5G) LynxI PCS found in
+  MediaTek's 10G-capabale MT7988 SoC.
+  In MediaTek's SDK sources, this unit is referred to as "pextp".
+
+properties:
+  compatible:
+    const: mediatek,mt7988-xfi-tphy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: XFI PHY clock
+      - description: XFI register clock
+
+  clock-names:
+    items:
+      - const: xfipll
+      - const: topxtal
+
+  resets:
+    items:
+      - description: Reset controller corresponding to the phy instance.
+
+  mediatek,usxgmii-performance-errata:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      One instance of the T-PHY on MT7988 suffers from a performance
+      problem in 10GBase-R mode which needs a work-around in the driver.
+      This flag enables a work-around ajusting an analog phy setting and
+      is required for XFI Port0 of the MT7988 SoC to be in compliance with
+      the SFP specification.
+
+  "#phy-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mediatek,mt7988-clk.h>
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      phy@11f20000 {
+        compatible = "mediatek,mt7988-xfi-tphy";
+        reg = <0 0x11f20000 0 0x10000>;
+        clocks = <&xfi_pll CLK_XFIPLL_PLL_EN>,
+                 <&topckgen CLK_TOP_XFI_PHY_0_XTAL_SEL>;
+        clock-names = "xfipll", "topxtal";
+        resets = <&watchdog 14>;
+        mediatek,usxgmii-performance-errata;
+        #phy-cells = <0>;
+      };
+    };
+
+...
-- 
2.44.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v4 2/2] phy: add driver for MediaTek XFI T-PHY
From: Daniel Golle @ 2024-04-09  2:24 UTC (permalink / raw)
  To: Vinod Koul, Bc-bocun Chen, Steven Liu, John Crispin, Chunfeng Yun,
	Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Daniel Golle, Qingfang Deng, SkyLake Huang, Philipp Zabel,
	linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
	linux-kernel, netdev
In-Reply-To: <da5498096f71a40ca1eac4124b7bb601c82396fb.1712625857.git.daniel@makrotopia.org>

Add driver for MediaTek's XFI T-PHY which can be found in the MT7988
SoC. The XFI T-PHY is a 10 Gigabit/s Ethernet SerDes PHY with muxes on
the internal side to be used with either USXGMII PCS or LynxI PCS,
depending on the selected PHY interface mode.

The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
PHY_INTERFACE_MODE_* corresponding to the supported modes:

 * USXGMII                 \
 * 10GBase-R                }- USXGMII PCS - XGDM  \
 * 5GBase-R                /                        \
                                                     }- Ethernet MAC
 * 2500Base-X              \                        /
 * 1000Base-X               }- LynxI PCS - GDM     /
 * Cisco SGMII (MAC side)  /

I chose the name XFI T-PHY because names of functions dealing with the
phy in the vendor driver are prefixed "xfi_pextp_".

The register space used by the phy is called "pextp" in the vendor
sources, which could be read as "_P_CI _ex_press _T_-_P_hy", and that
is quite misleading as this phy isn't used for anything related to
PCIe, so I wanted to find a better name.

XFI is still somehow related (as in: you would find the relevant
places using grep in the vendor driver when looking for that) and the
term seemed to at least somehow be aligned with the function of that
phy: Dealing with (up to) 10 Gbit/s Ethernet serialized differential
signals.

In order to work-around a performance issue present on the first of
two XFI T-PHYs found in MT7988, special tuning is applied which can
be selected by adding the 'mediatek,usxgmii-performance-errata'
property to the device tree node, similar to how the vendor driver is
doing that too.

There is no documentation for most registers used for the
analog/tuning part, however, most of the registers have been partially
reverse-engineered from MediaTek's SDK implementation (see links, an
opaque sequence of 32-bit register writes) and descriptions for all
relevant digital registers and bits such as resets and muxes have been
supplied by MediaTek.

Link: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/b72d6cba92bf9e29fb035c03052fa1e86664a25b/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_sgmii.c
Link: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/dec96a1d9b82cdcda4a56453fd0b453d4cab4b85/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_eth_soc.c
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
v4:
 Functional changes:
  * Include fix for SGMII, found in vendor SDK[1].

 Non-functional/cosmetic changes:
  * Separate allocation and assignment of bools in mtk_xfi_tphy_setup().
  * Remove unnecessary parentheses.
  * Explain naming in commit message.
  * Drop unnecessary dependencies from Kconfig.
  * Provide URL references to source of magic values.
  * Add kernel-doc comments.

[1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/b72d6cba92bf9e29fb035c03052fa1e86664a25b

v3: no changes

v2:
 * use IO helpers from mtk-io.h instead of rolling my own
 * use devm_clk_bulk_get()
 * yse devm_platform_ioremap_resource()
 * unify name and description everywhere
 * invert bool is_xgmii into bool use_lynxi_pcs and add comments
   describing the meaning of each of the stack variables
 * not much we can do about remaining magic values unless MTK provides
   definitions for them

 MAINTAINERS                             |   1 +
 drivers/phy/mediatek/Kconfig            |  11 +
 drivers/phy/mediatek/Makefile           |   1 +
 drivers/phy/mediatek/phy-mtk-xfi-tphy.c | 451 ++++++++++++++++++++++++
 4 files changed, 464 insertions(+)
 create mode 100644 drivers/phy/mediatek/phy-mtk-xfi-tphy.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a4924fda31829b..98b4b391d224f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13870,6 +13870,7 @@ L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/phy/mediatek-ge-soc.c
 F:	drivers/net/phy/mediatek-ge.c
+F:	drivers/phy/mediatek/phy-mtk-xfi-tphy.c
 
 MEDIATEK I2C CONTROLLER DRIVER
 M:	Qii Wang <qii.wang@mediatek.com>
diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index 3849b7c87d2872..60e00057e8bc71 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -13,6 +13,17 @@ config PHY_MTK_PCIE
 	  callback for PCIe GEN3 port, it supports software efuse
 	  initialization.
 
+config PHY_MTK_XFI_TPHY
+	tristate "MediaTek 10GE SerDes XFI T-PHY driver"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	help
+	  Say 'Y' here to add support for MediaTek XFI T-PHY driver.
+	  The driver provides access to the Ethernet SerDes T-PHY supporting
+	  1GE and 2.5GE modes via the LynxI PCS, and 5GE and 10GE modes
+	  via the USXGMII PCS found in MediaTek SoCs with 10G Ethernet.
+
 config PHY_MTK_TPHY
 	tristate "MediaTek T-PHY Driver"
 	depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index f6e24a47e08153..1b8088df71e845 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PHY_MTK_PCIE)		+= phy-mtk-pcie.o
 obj-$(CONFIG_PHY_MTK_TPHY)		+= phy-mtk-tphy.o
 obj-$(CONFIG_PHY_MTK_UFS)		+= phy-mtk-ufs.o
 obj-$(CONFIG_PHY_MTK_XSPHY)		+= phy-mtk-xsphy.o
+obj-$(CONFIG_PHY_MTK_XFI_TPHY)		+= phy-mtk-xfi-tphy.o
 
 phy-mtk-hdmi-drv-y			:= phy-mtk-hdmi.o
 phy-mtk-hdmi-drv-y			+= phy-mtk-hdmi-mt2701.o
diff --git a/drivers/phy/mediatek/phy-mtk-xfi-tphy.c b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
new file mode 100644
index 00000000000000..3e3af7f7a3fdf5
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
@@ -0,0 +1,451 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MediaTek 10GE SerDes XFI T-PHY driver
+ *
+ * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
+ *                    Bc-bocun Chen <bc-bocun.chen@mediatek.com>
+ * based on mtk_usxgmii.c and mtk_sgmii.c found in MediaTek's SDK (GPL-2.0)
+ * Copyright (c) 2022 MediaTek Inc.
+ * Author: Henry Yen <henry.yen@mediatek.com>
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/reset.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+
+#include "phy-mtk-io.h"
+
+#define MTK_XFI_TPHY_NUM_CLOCKS		2
+
+#define REG_DIG_GLB_70			0x0070
+#define  XTP_PCS_RX_EQ_IN_PROGRESS(x)	FIELD_PREP(GENMASK(25, 24), (x))
+#define  XTP_PCS_MODE_MASK		GENMASK(17, 16)
+#define  XTP_PCS_MODE(x)		FIELD_PREP(GENMASK(17, 16), (x))
+#define  XTP_PCS_RST_B			BIT(15)
+#define  XTP_FRC_PCS_RST_B		BIT(14)
+#define  XTP_PCS_PWD_SYNC_MASK		GENMASK(13, 12)
+#define  XTP_PCS_PWD_SYNC(x)		FIELD_PREP(XTP_PCS_PWD_SYNC_MASK, (x))
+#define  XTP_PCS_PWD_ASYNC_MASK		GENMASK(11, 10)
+#define  XTP_PCS_PWD_ASYNC(x)		FIELD_PREP(XTP_PCS_PWD_ASYNC_MASK, (x))
+#define  XTP_FRC_PCS_PWD_ASYNC		BIT(8)
+#define  XTP_PCS_UPDT			BIT(4)
+#define  XTP_PCS_IN_FR_RG		BIT(0)
+
+#define REG_DIG_GLB_F4			0x00f4
+#define  XFI_DPHY_PCS_SEL		BIT(0)
+#define   XFI_DPHY_PCS_SEL_SGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 1)
+#define   XFI_DPHY_PCS_SEL_USXGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 0)
+#define  XFI_DPHY_AD_SGDT_FRC_EN	BIT(5)
+
+#define REG_DIG_LN_TRX_40		0x3040
+#define  XTP_LN_FRC_TX_DATA_EN		BIT(29)
+#define  XTP_LN_TX_DATA_EN		BIT(28)
+
+#define REG_DIG_LN_TRX_B0		0x30b0
+#define  XTP_LN_FRC_TX_MACCK_EN		BIT(5)
+#define  XTP_LN_TX_MACCK_EN		BIT(4)
+
+#define REG_ANA_GLB_D0			0x90d0
+#define  XTP_GLB_USXGMII_SEL_MASK	GENMASK(3, 1)
+#define  XTP_GLB_USXGMII_SEL(x)		FIELD_PREP(GENMASK(3, 1), (x))
+#define  XTP_GLB_USXGMII_EN		BIT(0)
+
+/**
+ * struct mtk_xfi_tphy - run-time data of the XFI phy instance
+ * @base: IO memory area to access phy registers.
+ * @dev: Kernel device used to output prefixed debug info.
+ * @reset: Reset control corresponding to the phy instance.
+ * @clocks: All clocks required for the phy to operate.
+ * @da_war: Enables work-around for 10GBase-R mode.
+ */
+struct mtk_xfi_tphy {
+	void __iomem		*base;
+	struct device		*dev;
+	struct reset_control	*reset;
+	struct clk_bulk_data	clocks[MTK_XFI_TPHY_NUM_CLOCKS];
+	bool			da_war;
+};
+
+/**
+ * mtk_xfi_tphy_setup() - Setup phy for specified interface mode.
+ * @xfi_tphy: XFI phy instance.
+ * @interface: Ethernet interface mode
+ *
+ * The setup function is the condensed result of combining the 5 functions which
+ * setup the phy in MediaTek's GPL licensed public SDK sources. They can be found
+ * in mtk_sgmii.c[1] as well as mtk_usxgmii.c[2].
+ *
+ * Many magic values have been replaced by register and bit definitions, however,
+ * that has not been possible in all cases. While the vendor driver uses a
+ * sequence of 32-bit writes, here we try to only modify the actually required
+ * bits.
+ *
+ * [1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/b72d6cba92bf9e29fb035c03052fa1e86664a25b/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_sgmii.c
+ *
+ * [2]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/dec96a1d9b82cdcda4a56453fd0b453d4cab4b85/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+ */
+static void mtk_xfi_tphy_setup(struct mtk_xfi_tphy *xfi_tphy,
+			       phy_interface_t interface)
+{
+	bool is_1g, is_2p5g, is_5g, is_10g, da_war, use_lynxi_pcs;
+
+	/* shorthands for specific clock speeds depending on interface mode */
+	is_1g = interface == PHY_INTERFACE_MODE_1000BASEX ||
+		interface == PHY_INTERFACE_MODE_SGMII;
+	is_2p5g = interface == PHY_INTERFACE_MODE_2500BASEX;
+	is_5g = interface == PHY_INTERFACE_MODE_5GBASER;
+	is_10g = interface == PHY_INTERFACE_MODE_10GBASER ||
+		 interface == PHY_INTERFACE_MODE_USXGMII;
+
+	/* Is overriding 10GBase-R tuning value required? */
+	da_war = xfi_tphy->da_war && (interface == PHY_INTERFACE_MODE_10GBASER);
+
+	/* configure input mux to either
+	 *  - USXGMII PCS (64b/66b coding) for 5G/10G
+	 *  - LynxI PCS (8b/10b coding) for 1G/2.5G
+	 */
+	use_lynxi_pcs = is_1g || is_2p5g;
+
+	dev_dbg(xfi_tphy->dev, "setting up for mode %s\n", phy_modes(interface));
+
+	/* Setup PLL setting */
+	mtk_phy_update_bits(xfi_tphy->base + 0x9024, 0x100000, is_10g ? 0x0 : 0x100000);
+	mtk_phy_update_bits(xfi_tphy->base + 0x2020, 0x202000, is_5g ? 0x202000 : 0x0);
+	mtk_phy_update_bits(xfi_tphy->base + 0x2030, 0x500, is_1g ? 0x0 : 0x500);
+	mtk_phy_update_bits(xfi_tphy->base + 0x2034, 0xa00, is_1g ? 0x0 : 0xa00);
+	mtk_phy_update_bits(xfi_tphy->base + 0x2040, 0x340000, is_1g ? 0x200000 : 0x140000);
+
+	/* Setup RXFE BW setting */
+	mtk_phy_update_bits(xfi_tphy->base + 0x50f0, 0xc10, is_1g ? 0x410 : is_5g ? 0x800 : 0x400);
+	mtk_phy_update_bits(xfi_tphy->base + 0x50e0, 0x4000, is_5g ? 0x0 : 0x4000);
+
+	/* Setup RX CDR setting */
+	mtk_phy_update_bits(xfi_tphy->base + 0x506c, 0x30000, is_5g ? 0x0 : 0x30000);
+	mtk_phy_update_bits(xfi_tphy->base + 0x5070, 0x670000, is_5g ? 0x620000 : 0x50000);
+	mtk_phy_update_bits(xfi_tphy->base + 0x5074, 0x180000, is_5g ? 0x180000 : 0x0);
+	mtk_phy_update_bits(xfi_tphy->base + 0x5078, 0xf000400, is_5g ? 0x8000000 :
+									0x7000400);
+	mtk_phy_update_bits(xfi_tphy->base + 0x507c, 0x5000500, is_5g ? 0x4000400 :
+									0x1000100);
+	mtk_phy_update_bits(xfi_tphy->base + 0x5080, 0x1410, is_1g ? 0x400 : is_5g ? 0x1010 : 0x0);
+	mtk_phy_update_bits(xfi_tphy->base + 0x5084, 0x30300, is_1g ? 0x30300 :
+							      is_5g ? 0x30100 :
+								      0x100);
+	mtk_phy_update_bits(xfi_tphy->base + 0x5088, 0x60200, is_1g ? 0x20200 :
+							      is_5g ? 0x40000 :
+								      0x20000);
+
+	/* Setting RXFE adaptation range setting */
+	mtk_phy_update_bits(xfi_tphy->base + 0x50e4, 0xc0000, is_5g ? 0x0 : 0xc0000);
+	mtk_phy_update_bits(xfi_tphy->base + 0x50e8, 0x40000, is_5g ? 0x0 : 0x40000);
+	mtk_phy_update_bits(xfi_tphy->base + 0x50ec, 0xa00, is_1g ? 0x200 : 0x800);
+	mtk_phy_update_bits(xfi_tphy->base + 0x50a8, 0xee0000, is_5g ? 0x800000 :
+								       0x6e0000);
+	mtk_phy_update_bits(xfi_tphy->base + 0x6004, 0x190000, is_5g ? 0x0 : 0x190000);
+
+	if (is_10g)
+		writel(0x01423342, xfi_tphy->base + 0x00f8);
+	else if (is_5g)
+		writel(0x00a132a1, xfi_tphy->base + 0x00f8);
+	else if (is_2p5g)
+		writel(0x009c329c, xfi_tphy->base + 0x00f8);
+	else
+		writel(0x00fa32fa, xfi_tphy->base + 0x00f8);
+
+	/* Force SGDT_OUT off and select PCS */
+	mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_F4,
+			    XFI_DPHY_AD_SGDT_FRC_EN | XFI_DPHY_PCS_SEL,
+			    XFI_DPHY_AD_SGDT_FRC_EN |
+			    (use_lynxi_pcs ? XFI_DPHY_PCS_SEL_SGMII :
+					     XFI_DPHY_PCS_SEL_USXGMII));
+
+	/* Force GLB_CKDET_OUT */
+	mtk_phy_set_bits(xfi_tphy->base + 0x0030, 0xc00);
+
+	/* Force AEQ on */
+	writel(XTP_PCS_RX_EQ_IN_PROGRESS(2) | XTP_PCS_PWD_SYNC(2) | XTP_PCS_PWD_ASYNC(2),
+	       xfi_tphy->base + REG_DIG_GLB_70);
+
+	usleep_range(1, 5);
+	writel(XTP_LN_FRC_TX_DATA_EN, xfi_tphy->base + REG_DIG_LN_TRX_40);
+
+	/* Setup TX DA default value */
+	mtk_phy_update_bits(xfi_tphy->base + 0x30b0, 0x30, 0x20);
+	writel(0x00008a01, xfi_tphy->base + 0x3028);
+	writel(0x0000a884, xfi_tphy->base + 0x302c);
+	writel(0x00083002, xfi_tphy->base + 0x3024);
+
+	/* Setup RG default value */
+	if (use_lynxi_pcs) {
+		writel(0x00011110, xfi_tphy->base + 0x3010);
+		writel(0x40704000, xfi_tphy->base + 0x3048);
+	} else {
+		writel(0x00022220, xfi_tphy->base + 0x3010);
+		writel(0x0f020a01, xfi_tphy->base + 0x5064);
+		writel(0x06100600, xfi_tphy->base + 0x50b4);
+		if (interface == PHY_INTERFACE_MODE_USXGMII)
+			writel(0x40704000, xfi_tphy->base + 0x3048);
+		else
+			writel(0x47684100, xfi_tphy->base + 0x3048);
+	}
+
+	if (is_1g)
+		writel(0x0000c000, xfi_tphy->base + 0x3064);
+
+	/* Setup RX EQ initial value */
+	mtk_phy_update_bits(xfi_tphy->base + 0x3050, 0xa8000000,
+			    (interface != PHY_INTERFACE_MODE_10GBASER) ? 0xa8000000 : 0x0);
+	mtk_phy_update_bits(xfi_tphy->base + 0x3054, 0xaa,
+			    (interface != PHY_INTERFACE_MODE_10GBASER) ? 0xaa : 0x0);
+
+	if (!use_lynxi_pcs)
+		writel(0x00000f00, xfi_tphy->base + 0x306c);
+	else if (is_2p5g)
+		writel(0x22000f00, xfi_tphy->base + 0x306c);
+	else
+		writel(0x20200f00, xfi_tphy->base + 0x306c);
+
+	mtk_phy_update_bits(xfi_tphy->base + 0xa008, 0x10000, da_war ? 0x10000 : 0x0);
+
+	mtk_phy_update_bits(xfi_tphy->base + 0xa060, 0x50000, use_lynxi_pcs ? 0x50000 : 0x40000);
+
+	/* Setup PHYA speed */
+	mtk_phy_update_bits(xfi_tphy->base + REG_ANA_GLB_D0,
+			    XTP_GLB_USXGMII_SEL_MASK | XTP_GLB_USXGMII_EN,
+			    is_10g ?  XTP_GLB_USXGMII_SEL(0) :
+			    is_5g ?   XTP_GLB_USXGMII_SEL(1) :
+			    is_2p5g ? XTP_GLB_USXGMII_SEL(2) :
+				      XTP_GLB_USXGMII_SEL(3));
+	mtk_phy_set_bits(xfi_tphy->base + REG_ANA_GLB_D0, XTP_GLB_USXGMII_EN);
+
+	/* Release reset */
+	mtk_phy_set_bits(xfi_tphy->base + REG_DIG_GLB_70,
+			 XTP_PCS_RST_B | XTP_FRC_PCS_RST_B);
+	usleep_range(150, 500);
+
+	/* Switch to P0 */
+	mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_70,
+			    XTP_PCS_IN_FR_RG |
+			    XTP_FRC_PCS_PWD_ASYNC |
+			    XTP_PCS_PWD_ASYNC_MASK |
+			    XTP_PCS_PWD_SYNC_MASK |
+			    XTP_PCS_UPDT,
+			    XTP_PCS_IN_FR_RG |
+			    XTP_FRC_PCS_PWD_ASYNC |
+			    XTP_PCS_UPDT);
+	usleep_range(1, 5);
+
+	mtk_phy_clear_bits(xfi_tphy->base + REG_DIG_GLB_70, XTP_PCS_UPDT);
+	usleep_range(15, 50);
+
+	if (use_lynxi_pcs) {
+		/* Switch to Gen2 */
+		mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_70,
+				    XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
+				    XTP_PCS_MODE(1) | XTP_PCS_UPDT);
+	} else {
+		/* Switch to Gen3 */
+		mtk_phy_update_bits(xfi_tphy->base + REG_DIG_GLB_70,
+				    XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
+				    XTP_PCS_MODE(2) | XTP_PCS_UPDT);
+	}
+	usleep_range(1, 5);
+
+	mtk_phy_clear_bits(xfi_tphy->base + REG_DIG_GLB_70, XTP_PCS_UPDT);
+
+	usleep_range(100, 500);
+
+	/* Enable MAC CK */
+	mtk_phy_set_bits(xfi_tphy->base + REG_DIG_LN_TRX_B0, XTP_LN_TX_MACCK_EN);
+	mtk_phy_clear_bits(xfi_tphy->base + REG_DIG_GLB_F4, XFI_DPHY_AD_SGDT_FRC_EN);
+
+	/* Enable TX data */
+	mtk_phy_set_bits(xfi_tphy->base + REG_DIG_LN_TRX_40,
+			 XTP_LN_FRC_TX_DATA_EN | XTP_LN_TX_DATA_EN);
+	usleep_range(400, 1000);
+}
+
+/**
+ * mtk_xfi_tphy_set_mode() - Setup phy for specified interface mode.
+ *
+ * @phy: Phy instance.
+ * @mode: Only PHY_MODE_ETHERNET is supported.
+ * @submode: An Ethernet interface mode.
+ *
+ * Validate selected mode and call function mtk_xfi_tphy_setup().
+ *
+ * Return:
+ * * %0 - OK
+ * * %-EINVAL - invalid mode
+ */
+static int mtk_xfi_tphy_set_mode(struct phy *phy, enum phy_mode mode, int
+				 submode)
+{
+	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
+
+	if (mode != PHY_MODE_ETHERNET)
+		return -EINVAL;
+
+	switch (submode) {
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_5GBASER:
+	case PHY_INTERFACE_MODE_10GBASER:
+	case PHY_INTERFACE_MODE_USXGMII:
+		mtk_xfi_tphy_setup(xfi_tphy, submode);
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+/**
+ * mtk_xfi_tphy_reset() - Reset the phy.
+ *
+ * @phy: Phy instance.
+ *
+ * Reset the phy using the external reset controller.
+ *
+ * Return:
+ * %0 - OK
+ */
+static int mtk_xfi_tphy_reset(struct phy *phy)
+{
+	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
+
+	reset_control_assert(xfi_tphy->reset);
+	usleep_range(100, 500);
+	reset_control_deassert(xfi_tphy->reset);
+	usleep_range(1, 10);
+
+	return 0;
+}
+
+/**
+ * mtk_xfi_tphy_power_on() - Power-on the phy.
+ *
+ * @phy: Phy instance.
+ *
+ * Prepare and enable all clocks required for the phy to operate.
+ *
+ * Return:
+ * See clk_bulk_prepare_enable().
+ */
+static int mtk_xfi_tphy_power_on(struct phy *phy)
+{
+	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
+
+	return clk_bulk_prepare_enable(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
+}
+
+/**
+ * mtk_xfi_tphy_power_off() - Power-off the phy.
+ *
+ * @phy: Phy instance.
+ *
+ * Disable and unprepare all clocks previously enabled.
+ *
+ * Return:
+ * See clk_bulk_prepare_disable().
+ */
+static int mtk_xfi_tphy_power_off(struct phy *phy)
+{
+	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
+
+	clk_bulk_disable_unprepare(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
+
+	return 0;
+}
+
+static const struct phy_ops mtk_xfi_tphy_ops = {
+	.power_on	= mtk_xfi_tphy_power_on,
+	.power_off	= mtk_xfi_tphy_power_off,
+	.set_mode	= mtk_xfi_tphy_set_mode,
+	.reset		= mtk_xfi_tphy_reset,
+	.owner		= THIS_MODULE,
+};
+
+/**
+ * mtk_xfi_tphy_probe() - Probe phy instance from Device Tree.
+ * @pdev: Matching platform device.
+ *
+ * The probe function gets IO resource, clocks, reset controller and
+ * whether the DA work-around for 10GBase-R is required from Device Tree and
+ * allocates memory for holding that information in a struct mtk_xfi_tphy.
+ *
+ * Return:
+ * * %0       - OK
+ * * %-ENODEV - Missing associated Device Tree node (should never happen).
+ * * %-ENOMEM - Out of memory.
+ * * Any error value which devm_platform_ioremap_resource(),
+ *   devm_clk_bulk_get(), devm_reset_control_get_exclusive(),
+ *   devm_phy_create() or devm_of_phy_provider_register() may return.
+ */
+static int mtk_xfi_tphy_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct phy_provider *phy_provider;
+	struct mtk_xfi_tphy *xfi_tphy;
+	struct phy *phy;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	xfi_tphy = devm_kzalloc(&pdev->dev, sizeof(*xfi_tphy), GFP_KERNEL);
+	if (!xfi_tphy)
+		return -ENOMEM;
+
+	xfi_tphy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(xfi_tphy->base))
+		return PTR_ERR(xfi_tphy->base);
+
+	xfi_tphy->dev = &pdev->dev;
+	xfi_tphy->clocks[0].id = "topxtal";
+	xfi_tphy->clocks[1].id = "xfipll";
+	ret = devm_clk_bulk_get(&pdev->dev, MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
+	if (ret)
+		return ret;
+
+	xfi_tphy->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(xfi_tphy->reset))
+		return PTR_ERR(xfi_tphy->reset);
+
+	xfi_tphy->da_war = of_property_read_bool(np, "mediatek,usxgmii-performance-errata");
+
+	phy = devm_phy_create(&pdev->dev, NULL, &mtk_xfi_tphy_ops);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	phy_set_drvdata(phy, xfi_tphy);
+	phy_provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id mtk_xfi_tphy_match[] = {
+	{ .compatible = "mediatek,mt7988-xfi-tphy", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_xfi_tphy_match);
+
+static struct platform_driver mtk_xfi_tphy_driver = {
+	.probe = mtk_xfi_tphy_probe,
+	.driver = {
+		.name = "mtk-xfi-tphy",
+		.of_match_table = mtk_xfi_tphy_match,
+	},
+};
+module_platform_driver(mtk_xfi_tphy_driver);
+
+MODULE_DESCRIPTION("MediaTek 10GE SerDes XFI T-PHY driver");
+MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
+MODULE_AUTHOR("Bc-bocun Chen <bc-bocun.chen@mediatek.com>");
+MODULE_LICENSE("GPL");
-- 
2.44.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v3 2/2] media: i2c: Add GC05A2 image sensor driver
From: Zhi Mao (毛智) @ 2024-04-09  2:56 UTC (permalink / raw)
  To: mchehab@kernel.org, sakari.ailus@linux.intel.com,
	robh+dt@kernel.org, kieran.bingham@ideasonboard.com,
	krzysztof.kozlowski+dt@linaro.org
  Cc: heiko@sntech.de, gerald.loacker@wolfvision.net,
	linux-kernel@vger.kernel.org, yunkec@chromium.org,
	linux-mediatek@lists.infradead.org, dan.scally@ideasonboard.com,
	linux-media@vger.kernel.org,
	Shengnan Wang (王圣男), hdegoede@redhat.com,
	linus.walleij@linaro.org, andy.shevchenko@gmail.com,
	Yaya Chang (張雅清), bingbu.cao@intel.com,
	jacopo.mondi@ideasonboard.com, jernej.skrabec@gmail.com,
	devicetree@vger.kernel.org, conor+dt@kernel.org,
	tomi.valkeinen@ideasonboard.com, 10572168@qq.com,
	hverkuil-cisco@xs4all.nl, linux-arm-kernel@lists.infradead.org,
	Project_Global_Chro.me_Upstream_Group@mediatek.com,
	matthias.bgg@gmail.com, laurent.pinchart@ideasonboard.com,
	angelogioacchino.delregno@collabora.com, macromorgan@hotmail.com
In-Reply-To: <171258299292.1623123.5498855680488410646@ping.linuxembedded.co.uk>

On Mon, 2024-04-08 at 14:29 +0100, Kieran Bingham wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Quoting Zhi Mao (毛智) (2024-04-08 12:50:21)
> > Hi Kieran,
> > 
> > Thanks for your review this patch.
> > 
> > It seems that there are some difficult for us(Mediatek) to explain
> > these register setting comments.
> > As these settings are released by GC sensor vendor, and we have not
> > detailed datasheet described them.
> > And even if send the letter to ask sensor vendor, I am afraid there
> may
> > be not a clear response.
> > 
> > Can we just focus on the driver code function and control flow
> part?
> > 
> 
> As I said - You can take my comments with a pinch of salt ... but I
> wanted to know your position on it ;-)
> 
I agree with your opinion and have sent mail to vendor for these
comments.
If I got the feedback, I will sync information with you.

> --
> Kieran
> 
> > On Sun, 2024-04-07 at 10:08 +0100, Kieran Bingham wrote:
> > >        
> > > External email : Please do not click links or open attachments
> until
> > > you have verified the sender or the content.
> > >  Hello,
> > > 
> > > Thanks for helping extending the kernels sensor driver support.
> > > 
> > > My comments below can likely be taken with a pinch of salt, as
> they
> > > are
> > > mostly around the tabled register values ... but we have many
> drivers
> > > which are binary blobs of sensor register values and I think it
> would
> > > be
> > > far more beneficial to clean these up where possible...
> > > 
> > > So the first question is ... Can we ?
> > > 
> > > 
> > > 
> > > Quoting Zhi Mao (2024-04-03 04:38:25)
> > > > Add a V4L2 sub-device driver for Galaxycore GC05A2 image
> sensor.
> > > > 
> > > > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com>
> > > > ---
> > > >  drivers/media/i2c/Kconfig  |   10 +
> > > >  drivers/media/i2c/Makefile |    1 +
> > > >  drivers/media/i2c/gc05a2.c | 1383
> > > ++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 1394 insertions(+)
> > > >  create mode 100644 drivers/media/i2c/gc05a2.c
> > > > 
> > > > diff --git a/drivers/media/i2c/Kconfig
> b/drivers/media/i2c/Kconfig
> > > > index 56f276b920ab..97993bf160f9 100644
> > > > --- a/drivers/media/i2c/Kconfig
> > > > +++ b/drivers/media/i2c/Kconfig
> > > > @@ -70,6 +70,16 @@ config VIDEO_GC0308
> > > >           To compile this driver as a module, choose M here:
> the
> > > >           module will be called gc0308.
> > > >  
> > > > +config VIDEO_GC05A2
> > > > +       tristate "GalaxyCore gc05a2 sensor support"
> > > > +       select V4L2_CCI_I2C
> > > > +       help
> > > > +         This is a Video4Linux2 sensor driver for the
> GalaxyCore
> > > gc05a2
> > > > +         camera.
> > > > +
> > > > +         To compile this driver as a module, choose M here:
> the
> > > > +         module will be called gc05a2.
> > > > +
> > > >  config VIDEO_GC2145
> > > >         select V4L2_CCI_I2C
> > > >         tristate "GalaxyCore GC2145 sensor support"
> > > > diff --git a/drivers/media/i2c/Makefile
> > > b/drivers/media/i2c/Makefile
> > > > index dfbe6448b549..8ed6faf0f854 100644
> > > > --- a/drivers/media/i2c/Makefile
> > > > +++ b/drivers/media/i2c/Makefile
> > > > @@ -38,6 +38,7 @@ obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> > > >  obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> > > >  obj-$(CONFIG_VIDEO_ET8EK8) += et8ek8/
> > > >  obj-$(CONFIG_VIDEO_GC0308) += gc0308.o
> > > > +obj-$(CONFIG_VIDEO_GC05A2) += gc05a2.o
> > > >  obj-$(CONFIG_VIDEO_GC2145) += gc2145.o
> > > >  obj-$(CONFIG_VIDEO_HI556) += hi556.o
> > > >  obj-$(CONFIG_VIDEO_HI846) += hi846.o
> > > > diff --git a/drivers/media/i2c/gc05a2.c
> > > b/drivers/media/i2c/gc05a2.c
> > > > new file mode 100644
> > > > index 000000000000..461d33055a3b
> > > > --- /dev/null
> > > > +++ b/drivers/media/i2c/gc05a2.c
> > > > @@ -0,0 +1,1383 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Driver for GalaxyCore gc05a2 image sensor
> > > > + *
> > > > + * Copyright 2024 MediaTek
> > > > + *
> > > > + * Zhi Mao <zhi.mao@mediatek.com>
> > > > + */
> > > > +#include <linux/array_size.h>
> > > > +#include <linux/bits.h>
> > > > +#include <linux/clk.h>
> > > > +#include <linux/container_of.h>
> > > > +#include <linux/delay.h>
> > > > +#include <linux/device.h>
> > > > +#include <linux/err.h>
> > > > +#include <linux/gpio/consumer.h>
> > > > +#include <linux/math64.h>
> > > > +#include <linux/mod_devicetable.h>
> > > > +#include <linux/pm_runtime.h>
> > > > +#include <linux/property.h>
> > > > +#include <linux/regulator/consumer.h>
> > > > +#include <linux/types.h>
> > > > +#include <linux/units.h>
> > > > +
> > > > +#include <media/v4l2-cci.h>
> > > > +#include <media/v4l2-ctrls.h>
> > > > +#include <media/v4l2-event.h>
> > > > +#include <media/v4l2-fwnode.h>
> > > > +#include <media/v4l2-subdev.h>
> > > > +
> > > > +#define GC05A2_REG_TEST_PATTERN_EN CCI_REG8(0x008c)
> > > > +#define GC05A2_REG_TEST_PATTERN_IDX CCI_REG8(0x008d)
> > > > +#define GC05A2_TEST_PATTERN_EN 0x01
> > > > +
> > > > +#define GC05A2_STREAMING_REG CCI_REG8(0x0100)
> > > > +
> > > > +#define GC05A2_FLIP_REG CCI_REG8(0x0101)
> > > > +#define GC05A2_FLIP_H_MASK BIT(0)
> > > > +#define GC05A2_FLIP_V_MASK BIT(1)
> > > > +
> > > > +#define GC05A2_EXP_REG CCI_REG16(0x0202)
> > > > +#define GC05A2_EXP_MARGIN 16
> > > > +#define GC05A2_EXP_MIN 4
> > > > +#define GC05A2_EXP_STEP 1
> > > > +
> > > > +#define GC05A2_AGAIN_REG CCI_REG16(0x0204)
> > > > +#define GC05A2_AGAIN_MIN 1024
> > > > +#define GC05A2_AGAIN_MAX (1024 * 16)
> > > > +#define GC05A2_AGAIN_STEP 1
> > > > +
> > > > +#define GC05A2_FRAME_LENGTH_REG CCI_REG16(0x0340)
> > > > +#define GC05A2_VTS_MAX 0xffff
> > > > +
> > > > +#define GC05A2_REG_CHIP_ID CCI_REG16(0x03f0)
> > > > +#define GC05A2_CHIP_ID 0x05a2
> > > > +
> > > > +#define GC05A2_NATIVE_WIDTH 2592
> > > > +#define GC05A2_NATIVE_HEIGHT 1944
> > > > +
> > > > +#define GC05A2_DEFAULT_CLK_FREQ (24 * HZ_PER_MHZ)
> > > > +#define GC05A2_MBUS_CODE MEDIA_BUS_FMT_SGRBG10_1X10
> > > > +#define GC05A2_DATA_LANES 2
> > > > +#define GC05A2_RGB_DEPTH 10
> > > > +#define GC05A2_SLEEP_US  (2 * USEC_PER_MSEC)
> > > > +
> > > > +static const char *const gc05a2_test_pattern_menu[] = {
> > > > +       "No Pattern",  "Fade_to_gray_Color Bar", "Color Bar",
> > > > +       "PN9",         "Horizental_gradient",    "Checkboard
> > > Pattern",
> > > > +       "Slant",       "Resolution",             "Solid Black",
> > > > +       "Solid White",
> > > > +};
> > > > +
> > > > +static const s64 gc05a2_link_freq_menu_items[] = {
> > > > +       (448 * HZ_PER_MHZ),
> > > > +       (224 * HZ_PER_MHZ),
> > > > +};
> > > > +
> > > > +static const char *const gc05a2_supply_name[] = {
> > > > +       "avdd",
> > > > +       "dvdd",
> > > > +       "dovdd",
> > > > +};
> > > > +
> > > > +struct gc05a2 {
> > > > +       struct device *dev;
> > > > +       struct v4l2_subdev sd;
> > > > +       struct media_pad pad;
> > > > +
> > > > +       struct clk *xclk;
> > > > +       struct regulator_bulk_data
> > > supplies[ARRAY_SIZE(gc05a2_supply_name)];
> > > > +       struct gpio_desc *reset_gpio;
> > > > +
> > > > +       struct v4l2_ctrl_handler ctrls;
> > > > +       struct v4l2_ctrl *pixel_rate;
> > > > +       struct v4l2_ctrl *link_freq;
> > > > +       struct v4l2_ctrl *exposure;
> > > > +       struct v4l2_ctrl *vblank;
> > > > +       struct v4l2_ctrl *hblank;
> > > > +       struct v4l2_ctrl *hflip;
> > > > +       struct v4l2_ctrl *vflip;
> > > > +
> > > > +       struct regmap *regmap;
> > > > +       unsigned long link_freq_bitmap;
> > > > +
> > > > +       /* True if the device has been identified */
> > > > +       bool identified;
> > > > +       const struct gc05a2_mode *cur_mode;
> > > > +};
> > > > +
> > > > +struct gc05a2_reg_list {
> > > > +       u32 num_of_regs;
> > > > +       const struct cci_reg_sequence *regs;
> > > > +};
> > > > +
> > > > +static const struct cci_reg_sequence mode_2592x1944[] = {
> > > > +       /* system */
> > > > +       { CCI_REG8(0x0135), 0x01 },
> > > > +
> > > > +       /* pre_setting */
> > > > +       { CCI_REG8(0x0084), 0x21 },
> > > > +       { CCI_REG8(0x0d05), 0xcc },
> > > > +       { CCI_REG8(0x0218), 0x00 },
> > > > +       { CCI_REG8(0x005e), 0x48 },
> > > > +       { CCI_REG8(0x0d06), 0x01 },
> > > > +       { CCI_REG8(0x0007), 0x16 },
> > > > +       { CCI_REG8(0x0101), 0x00 },
> > > > +
> > > > +       /* analog */
> > > > +       { CCI_REG8(0x0342), 0x07 },
> > > > +       { CCI_REG8(0x0343), 0x28 },
> > > > +       { CCI_REG8(0x0220), 0x07 },
> > > > +       { CCI_REG8(0x0221), 0xd0 },
> > > > +       { CCI_REG8(0x0202), 0x07 },
> > > > +       { CCI_REG8(0x0203), 0x32 },
> > > > +       { CCI_REG8(0x0340), 0x07 },
> > > > +       { CCI_REG8(0x0341), 0xf0 },
> > > > +       { CCI_REG8(0x0219), 0x00 },
> > > > +       { CCI_REG8(0x0346), 0x00 },
> > > > +       { CCI_REG8(0x0347), 0x04 },
> > > > +       { CCI_REG8(0x0d14), 0x00 },
> > > > +       { CCI_REG8(0x0d13), 0x05 },
> > > > +       { CCI_REG8(0x0d16), 0x05 },
> > > > +       { CCI_REG8(0x0d15), 0x1d },
> > > > +       { CCI_REG8(0x00c0), 0x0a },
> > > > +       { CCI_REG8(0x00c1), 0x30 },
> > > > +       { CCI_REG8(0x034a), 0x07 },
> > > > +       { CCI_REG8(0x034b), 0xa8 },
> > > > +       { CCI_REG8(0x0e0a), 0x00 },
> > > > +       { CCI_REG8(0x0e0b), 0x00 },
> > > > +       { CCI_REG8(0x0e0e), 0x03 },
> > > > +       { CCI_REG8(0x0e0f), 0x00 },
> > > > +       { CCI_REG8(0x0e06), 0x0a },
> > > > +       { CCI_REG8(0x0e23), 0x15 },
> > > > +       { CCI_REG8(0x0e24), 0x15 },
> > > > +       { CCI_REG8(0x0e2a), 0x10 },
> > > > +       { CCI_REG8(0x0e2b), 0x10 },
> > > > +       { CCI_REG8(0x0e17), 0x49 },
> > > > +       { CCI_REG8(0x0e1b), 0x1c },
> > > > +       { CCI_REG8(0x0e3a), 0x36 },
> > > > +       { CCI_REG8(0x0d11), 0x84 },
> > > > +       { CCI_REG8(0x0e52), 0x14 },
> > > > +       { CCI_REG8(0x000b), 0x10 },
> > > > +       { CCI_REG8(0x0008), 0x08 },
> > > > +       { CCI_REG8(0x0223), 0x17 },
> > > > +       { CCI_REG8(0x0d27), 0x39 },
> > > > +       { CCI_REG8(0x0d22), 0x00 },
> > > > +       { CCI_REG8(0x03f6), 0x0d },
> > > > +       { CCI_REG8(0x0d04), 0x07 },
> > > > +       { CCI_REG8(0x03f3), 0x72 },
> > > > +       { CCI_REG8(0x03f4), 0xb8 },
> > > > +       { CCI_REG8(0x03f5), 0xbc },
> > > > +       { CCI_REG8(0x0d02), 0x73 },
> > > > +
> > > > +       /* auto load start */
> > > > +       { CCI_REG8(0x00cb), 0x00 },
> > > > +
> > > > +       /* OUT 2592*1944 */
> > > > +       { CCI_REG8(0x0350), 0x01 },
> > > > +       { CCI_REG8(0x0353), 0x00 },
> > > > +       { CCI_REG8(0x0354), 0x08 },
> > > 
> > > > +       { CCI_REG8(0x034c), 0x0a },
> > > > +       { CCI_REG8(0x034d), 0x20 },
> > > 
> > > Should/Could this be
> > >         { CCI_REG16(0x034c), 2592 }, /* Width */
> > > 
> > > 
> > > > +       { CCI_REG8(0x021f), 0x14 },
> > > > +
> > > > +       /* MIPI */
> > > > +       { CCI_REG8(0x0107), 0x05 },
> > > > +       { CCI_REG8(0x0117), 0x01 },
> > > > +       { CCI_REG8(0x0d81), 0x00 },
> > > > +       { CCI_REG8(0x0d84), 0x0c },
> > > > +       { CCI_REG8(0x0d85), 0xa8 },
> > > > +       { CCI_REG8(0x0d86), 0x06 },
> > > > +       { CCI_REG8(0x0d87), 0x55 },
> > > > +       { CCI_REG8(0x0db3), 0x06 },
> > > > +       { CCI_REG8(0x0db4), 0x08 },
> > > > +       { CCI_REG8(0x0db5), 0x1e },
> > > > +       { CCI_REG8(0x0db6), 0x02 },
> > > > +       { CCI_REG8(0x0db8), 0x12 },
> > > > +       { CCI_REG8(0x0db9), 0x0a },
> > > > +       { CCI_REG8(0x0d93), 0x06 },
> > > > +       { CCI_REG8(0x0d94), 0x09 },
> > > > +       { CCI_REG8(0x0d95), 0x0d },
> > > > +       { CCI_REG8(0x0d99), 0x0b },
> > > > +       { CCI_REG8(0x0084), 0x01 },
> > > > +
> > > > +       /* OUT */
> > > > +       { CCI_REG8(0x0110), 0x01 },
> > > > +};
> > > > +
> > > > +static const struct cci_reg_sequence mode_1280x720[] = {
> > > > +       /* system */
> > > > +       { CCI_REG8(0x0135), 0x05 },
> > > 
> > > In 2592x1944 this is 0x01. Do you have a datasheet? Can you
> explain
> > > why
> > > they are different? Can you add register definitions that have
> names
> > > to
> > > make this more maintainable or extendable in the future?
> > > 
> > > There's discussion in the recent series improving the IMX258
> which
> > > makes
> > > me wonder if we should try harder to have sensor drivers with
> clearer
> > > definitions.
> > > 
> > > 
> > > > +
> > > > +       /*pre_setting*/
> > > 
> > > /* pre_setting */ ?
> > > 
> > > > +       { CCI_REG8(0x0084), 0x21 },
> > > > +       { CCI_REG8(0x0d05), 0xcc },
> > > > +       { CCI_REG8(0x0218), 0x80 },
> > > > +       { CCI_REG8(0x005e), 0x49 },
> > > > +       { CCI_REG8(0x0d06), 0x81 },
> > > > +       { CCI_REG8(0x0007), 0x16 },
> > > > +       { CCI_REG8(0x0101), 0x00 },
> > > 
> > > In 2592x1944, only register 0x0218 differs. Why? What is that?
> Can it
> > > be
> > > broken out to a function that applies the correct configuration
> at
> > > startuup based on a parameter instead of duplicating this table
> set?
> > > 
> > > > +
> > > > +       /* analog */
> > > > +       { CCI_REG8(0x0342), 0x07 },
> > > > +       { CCI_REG8(0x0343), 0x10 },
> > > > +       { CCI_REG8(0x0220), 0x07 },
> > > > +       { CCI_REG8(0x0221), 0xd0 },
> > > > +       { CCI_REG8(0x0202), 0x03 },
> > > > +       { CCI_REG8(0x0203), 0x32 },
> > > > +       { CCI_REG8(0x0340), 0x04 },
> > > > +       { CCI_REG8(0x0341), 0x08 },
> > > > +       { CCI_REG8(0x0219), 0x00 },
> > > > +       { CCI_REG8(0x0346), 0x01 },
> > > > +       { CCI_REG8(0x0347), 0x00 },
> > > > +       { CCI_REG8(0x0d14), 0x00 },
> > > > +       { CCI_REG8(0x0d13), 0x05 },
> > > > +       { CCI_REG8(0x0d16), 0x05 },
> > > > +       { CCI_REG8(0x0d15), 0x1d },
> > > > +       { CCI_REG8(0x00c0), 0x0a },
> > > > +       { CCI_REG8(0x00c1), 0x30 },
> > > > +       { CCI_REG8(0x034a), 0x05 },
> > > > +       { CCI_REG8(0x034b), 0xb0 },
> > > > +       { CCI_REG8(0x0e0a), 0x00 },
> > > > +       { CCI_REG8(0x0e0b), 0x00 },
> > > > +       { CCI_REG8(0x0e0e), 0x03 },
> > > > +       { CCI_REG8(0x0e0f), 0x00 },
> > > > +       { CCI_REG8(0x0e06), 0x0a },
> > > > +       { CCI_REG8(0x0e23), 0x15 },
> > > > +       { CCI_REG8(0x0e24), 0x15 },
> > > > +       { CCI_REG8(0x0e2a), 0x10 },
> > > > +       { CCI_REG8(0x0e2b), 0x10 },
> > > > +       { CCI_REG8(0x0e17), 0x49 },
> > > > +       { CCI_REG8(0x0e1b), 0x1c },
> > > > +       { CCI_REG8(0x0e3a), 0x36 },
> > > > +       { CCI_REG8(0x0d11), 0x84 },
> > > > +       { CCI_REG8(0x0e52), 0x14 },
> > > > +       { CCI_REG8(0x000b), 0x0e },
> > > > +       { CCI_REG8(0x0008), 0x03 },
> > > > +       { CCI_REG8(0x0223), 0x16 },
> > > > +       { CCI_REG8(0x0d27), 0x39 },
> > > > +       { CCI_REG8(0x0d22), 0x00 },
> > > > +       { CCI_REG8(0x03f6), 0x0d },
> > > > +       { CCI_REG8(0x0d04), 0x07 },
> > > > +       { CCI_REG8(0x03f3), 0x72 },
> > > > +       { CCI_REG8(0x03f4), 0xb8 },
> > > > +       { CCI_REG8(0x03f5), 0xbc },
> > > > +       { CCI_REG8(0x0d02), 0x73 },
> > > > +
> > > 
> > > Are any of those able to be broken out to named register to be
> more
> > > clear in their intent?
> > > 
> > > > +       /* auto load start */
> > > > +       { CCI_REG8(0x00cb), 0xfc },
> > > > +
> > > 
> > > Why is this auto load start so different to the other modes 'auto
> > > load
> > > start'? What do the bits refer to ?
> > > 
> > > > +       /* OUT 1280x720 */
> > > > +       { CCI_REG8(0x0350), 0x01 },
> > > > +       { CCI_REG8(0x0353), 0x00 },
> > > > +       { CCI_REG8(0x0354), 0x0c },
> > > 
> > > > +       { CCI_REG8(0x034c), 0x05 },
> > > > +       { CCI_REG8(0x034d), 0x00 },
> > > 
> > > Should/Could this be 
> > >          { CCI_REG16(0x034c), 1280 },
> > > 
> > > Are there any other register settings that would make more sense
> to
> > > be
> > > in decimal units that match their actual context?
> > > 
> > > 
> > > > +       { CCI_REG8(0x021f), 0x14 },
> > > 
> > > I don't see a setting for 720/0x2d0. Do these registers only set
> the
> > > width?
> > > 
> > > > +
> > > > +       /* MIPI */
> > > > +       { CCI_REG8(0x0107), 0x05 },
> > > > +       { CCI_REG8(0x0117), 0x01 },
> > > > +       { CCI_REG8(0x0d81), 0x00 },
> > > > +       { CCI_REG8(0x0d84), 0x06 },
> > > > +       { CCI_REG8(0x0d85), 0x40 },
> > > > +       { CCI_REG8(0x0d86), 0x03 },
> > > > +       { CCI_REG8(0x0d87), 0x21 },
> > > > +       { CCI_REG8(0x0db3), 0x03 },
> > > > +       { CCI_REG8(0x0db4), 0x04 },
> > > > +       { CCI_REG8(0x0db5), 0x0d },
> > > > +       { CCI_REG8(0x0db6), 0x01 },
> > > > +       { CCI_REG8(0x0db8), 0x04 },
> > > > +       { CCI_REG8(0x0db9), 0x06 },
> > > > +       { CCI_REG8(0x0d93), 0x03 },
> > > > +       { CCI_REG8(0x0d94), 0x04 },
> > > > +       { CCI_REG8(0x0d95), 0x05 },
> > > > +       { CCI_REG8(0x0d99), 0x06 },
> > > > +       { CCI_REG8(0x0084), 0x01 },
> > > > +
> > > > +       /* OUT */
> > > 
> > > Out where? What is out?
> > > 
> > > > +       { CCI_REG8(0x0110), 0x01 },
> > > > +};
> > > > +
> > > > +static const struct cci_reg_sequence mode_table_common[] = {
> > > > +       { GC05A2_STREAMING_REG, 0x00 },
> > > > +       /* system */
> > > > +       { CCI_REG8(0x0315), 0xd4 },
> > > > +       { CCI_REG8(0x0d06), 0x01 },
> > > > +       { CCI_REG8(0x0a70), 0x80 },
> > > > +       { CCI_REG8(0x031a), 0x00 },
> > > > +       { CCI_REG8(0x0314), 0x00 },
> > > > +       { CCI_REG8(0x0130), 0x08 },
> > > > +       { CCI_REG8(0x0132), 0x01 },
> > > > +       { CCI_REG8(0x0136), 0x38 },
> > > > +       { CCI_REG8(0x0137), 0x03 },
> > > > +       { CCI_REG8(0x0134), 0x5b },
> > > > +       { CCI_REG8(0x031c), 0xe0 },
> > > > +       { CCI_REG8(0x0d82), 0x14 },
> > > > +       { CCI_REG8(0x0dd1), 0x56 },
> > > > +
> > > > +       /* gate_mode */
> > > > +       { CCI_REG8(0x0af4), 0x01 },
> > > > +       { CCI_REG8(0x0002), 0x10 },
> > > > +       { CCI_REG8(0x00c3), 0x34 },
> > > > +
> > > > +       /* auto load start */
> > > 
> > > The previous 'auto load start' referenced 0x00cb ?
> > > 
> > > > +       { CCI_REG8(0x00c4), 0x00 },
> > > > +       { CCI_REG8(0x00c5), 0x01 },
> > > > +       { CCI_REG8(0x0af6), 0x00 },
> > > > +       { CCI_REG8(0x0ba0), 0x17 },
> > > > +       { CCI_REG8(0x0ba1), 0x00 },
> > > > +       { CCI_REG8(0x0ba2), 0x00 },
> > > > +       { CCI_REG8(0x0ba3), 0x00 },
> > > > +       { CCI_REG8(0x0ba4), 0x03 },
> > > > +       { CCI_REG8(0x0ba5), 0x00 },
> > > > +       { CCI_REG8(0x0ba6), 0x00 },
> > > > +       { CCI_REG8(0x0ba7), 0x00 },
> > > > +       { CCI_REG8(0x0ba8), 0x40 },
> > > > +       { CCI_REG8(0x0ba9), 0x00 },
> > > > +       { CCI_REG8(0x0baa), 0x00 },
> > > > +       { CCI_REG8(0x0bab), 0x00 },
> > > > +       { CCI_REG8(0x0bac), 0x40 },
> > > > +       { CCI_REG8(0x0bad), 0x00 },
> > > > +       { CCI_REG8(0x0bae), 0x00 },
> > > > +       { CCI_REG8(0x0baf), 0x00 },
> > > > +       { CCI_REG8(0x0bb0), 0x02 },
> > > > +       { CCI_REG8(0x0bb1), 0x00 },
> > > > +       { CCI_REG8(0x0bb2), 0x00 },
> > > > +       { CCI_REG8(0x0bb3), 0x00 },
> > > > +       { CCI_REG8(0x0bb8), 0x02 },
> > > > +       { CCI_REG8(0x0bb9), 0x00 },
> > > > +       { CCI_REG8(0x0bba), 0x00 },
> > > > +       { CCI_REG8(0x0bbb), 0x00 },
> > > > +       { CCI_REG8(0x0a70), 0x80 },
> > > > +       { CCI_REG8(0x0a71), 0x00 },
> > > > +       { CCI_REG8(0x0a72), 0x00 },
> > > > +       { CCI_REG8(0x0a66), 0x00 },
> > > > +       { CCI_REG8(0x0a67), 0x80 },
> > > > +       { CCI_REG8(0x0a4d), 0x4e },
> > > > +       { CCI_REG8(0x0a50), 0x00 },
> > > > +       { CCI_REG8(0x0a4f), 0x0c },
> > > > +       { CCI_REG8(0x0a66), 0x00 },
> > > > +       { CCI_REG8(0x00ca), 0x00 },
> > > > +       { CCI_REG8(0x00cc), 0x00 },
> > > > +       { CCI_REG8(0x00cd), 0x00 },
> > > > +       { CCI_REG8(0x0aa1), 0x00 },
> > > > +       { CCI_REG8(0x0aa2), 0xe0 },
> > > > +       { CCI_REG8(0x0aa3), 0x00 },
> > > > +       { CCI_REG8(0x0aa4), 0x40 },
> > > > +       { CCI_REG8(0x0a90), 0x03 },
> > > > +       { CCI_REG8(0x0a91), 0x0e },
> > > > +       { CCI_REG8(0x0a94), 0x80 },
> > > > +
> > > > +       /* standby */
> > > > +       { CCI_REG8(0x0af6), 0x20 },
> > > > +       { CCI_REG8(0x0b00), 0x91 },
> > > > +       { CCI_REG8(0x0b01), 0x17 },
> > > > +       { CCI_REG8(0x0b02), 0x01 },
> > > > +       { CCI_REG8(0x0b03), 0x00 },
> > > > +       { CCI_REG8(0x0b04), 0x01 },
> > > > +       { CCI_REG8(0x0b05), 0x17 },
> > > > +       { CCI_REG8(0x0b06), 0x01 },
> > > > +       { CCI_REG8(0x0b07), 0x00 },
> > > > +       { CCI_REG8(0x0ae9), 0x01 },
> > > > +       { CCI_REG8(0x0aea), 0x02 },
> > > > +       { CCI_REG8(0x0ae8), 0x53 },
> > > > +       { CCI_REG8(0x0ae8), 0x43 },
> > > > +
> > > > +       /* gain_partition */
> > > > +       { CCI_REG8(0x0af6), 0x30 },
> > > > +       { CCI_REG8(0x0b00), 0x08 },
> > > > +       { CCI_REG8(0x0b01), 0x0f },
> > > > +       { CCI_REG8(0x0b02), 0x00 },
> > > > +       { CCI_REG8(0x0b04), 0x1c },
> > > > +       { CCI_REG8(0x0b05), 0x24 },
> > > > +       { CCI_REG8(0x0b06), 0x00 },
> > > > +       { CCI_REG8(0x0b08), 0x30 },
> > > > +       { CCI_REG8(0x0b09), 0x40 },
> > > > +       { CCI_REG8(0x0b0a), 0x00 },
> > > > +       { CCI_REG8(0x0b0c), 0x0e },
> > > > +       { CCI_REG8(0x0b0d), 0x2a },
> > > > +       { CCI_REG8(0x0b0e), 0x00 },
> > > > +       { CCI_REG8(0x0b10), 0x0e },
> > > > +       { CCI_REG8(0x0b11), 0x2b },
> > > > +       { CCI_REG8(0x0b12), 0x00 },
> > > > +       { CCI_REG8(0x0b14), 0x0e },
> > > > +       { CCI_REG8(0x0b15), 0x23 },
> > > > +       { CCI_REG8(0x0b16), 0x00 },
> > > > +       { CCI_REG8(0x0b18), 0x0e },
> > > > +       { CCI_REG8(0x0b19), 0x24 },
> > > > +       { CCI_REG8(0x0b1a), 0x00 },
> > > > +       { CCI_REG8(0x0b1c), 0x0c },
> > > > +       { CCI_REG8(0x0b1d), 0x0c },
> > > > +       { CCI_REG8(0x0b1e), 0x00 },
> > > > +       { CCI_REG8(0x0b20), 0x03 },
> > > > +       { CCI_REG8(0x0b21), 0x03 },
> > > > +       { CCI_REG8(0x0b22), 0x00 },
> > > > +       { CCI_REG8(0x0b24), 0x0e },
> > > > +       { CCI_REG8(0x0b25), 0x0e },
> > > > +       { CCI_REG8(0x0b26), 0x00 },
> > > > +       { CCI_REG8(0x0b28), 0x03 },
> > > > +       { CCI_REG8(0x0b29), 0x03 },
> > > > +       { CCI_REG8(0x0b2a), 0x00 },
> > > > +       { CCI_REG8(0x0b2c), 0x12 },
> > > > +       { CCI_REG8(0x0b2d), 0x12 },
> > > > +       { CCI_REG8(0x0b2e), 0x00 },
> > > > +       { CCI_REG8(0x0b30), 0x08 },
> > > > +       { CCI_REG8(0x0b31), 0x08 },
> > > > +       { CCI_REG8(0x0b32), 0x00 },
> > > > +       { CCI_REG8(0x0b34), 0x14 },
> > > > +       { CCI_REG8(0x0b35), 0x14 },
> > > > +       { CCI_REG8(0x0b36), 0x00 },
> > > > +       { CCI_REG8(0x0b38), 0x10 },
> > > > +       { CCI_REG8(0x0b39), 0x10 },
> > > > +       { CCI_REG8(0x0b3a), 0x00 },
> > > > +       { CCI_REG8(0x0b3c), 0x16 },
> > > > +       { CCI_REG8(0x0b3d), 0x16 },
> > > > +       { CCI_REG8(0x0b3e), 0x00 },
> > > > +       { CCI_REG8(0x0b40), 0x10 },
> > > > +       { CCI_REG8(0x0b41), 0x10 },
> > > > +       { CCI_REG8(0x0b42), 0x00 },
> > > > +       { CCI_REG8(0x0b44), 0x19 },
> > > > +       { CCI_REG8(0x0b45), 0x19 },
> > > > +       { CCI_REG8(0x0b46), 0x00 },
> > > > +       { CCI_REG8(0x0b48), 0x16 },
> > > > +       { CCI_REG8(0x0b49), 0x16 },
> > > > +       { CCI_REG8(0x0b4a), 0x00 },
> > > > +       { CCI_REG8(0x0b4c), 0x19 },
> > > > +       { CCI_REG8(0x0b4d), 0x19 },
> > > > +       { CCI_REG8(0x0b4e), 0x00 },
> > > > +       { CCI_REG8(0x0b50), 0x16 },
> > > > +       { CCI_REG8(0x0b51), 0x16 },
> > > > +       { CCI_REG8(0x0b52), 0x00 },
> > > > +       { CCI_REG8(0x0b80), 0x01 },
> > > > +       { CCI_REG8(0x0b81), 0x00 },
> > > > +       { CCI_REG8(0x0b82), 0x00 },
> > > > +       { CCI_REG8(0x0b84), 0x00 },
> > > > +       { CCI_REG8(0x0b85), 0x00 },
> > > > +       { CCI_REG8(0x0b86), 0x00 },
> > > > +       { CCI_REG8(0x0b88), 0x01 },
> > > > +       { CCI_REG8(0x0b89), 0x6a },
> > > > +       { CCI_REG8(0x0b8a), 0x00 },
> > > > +       { CCI_REG8(0x0b8c), 0x00 },
> > > > +       { CCI_REG8(0x0b8d), 0x01 },
> > > > +       { CCI_REG8(0x0b8e), 0x00 },
> > > > +       { CCI_REG8(0x0b90), 0x01 },
> > > > +       { CCI_REG8(0x0b91), 0xf6 },
> > > > +       { CCI_REG8(0x0b92), 0x00 },
> > > > +       { CCI_REG8(0x0b94), 0x00 },
> > > > +       { CCI_REG8(0x0b95), 0x02 },
> > > > +       { CCI_REG8(0x0b96), 0x00 },
> > > > +       { CCI_REG8(0x0b98), 0x02 },
> > > > +       { CCI_REG8(0x0b99), 0xc4 },
> > > > +       { CCI_REG8(0x0b9a), 0x00 },
> > > > +       { CCI_REG8(0x0b9c), 0x00 },
> > > > +       { CCI_REG8(0x0b9d), 0x03 },
> > > > +       { CCI_REG8(0x0b9e), 0x00 },
> > > > +       { CCI_REG8(0x0ba0), 0x03 },
> > > > +       { CCI_REG8(0x0ba1), 0xd8 },
> > > > +       { CCI_REG8(0x0ba2), 0x00 },
> > > > +       { CCI_REG8(0x0ba4), 0x00 },
> > > > +       { CCI_REG8(0x0ba5), 0x04 },
> > > > +       { CCI_REG8(0x0ba6), 0x00 },
> > > > +       { CCI_REG8(0x0ba8), 0x05 },
> > > > +       { CCI_REG8(0x0ba9), 0x4d },
> > > > +       { CCI_REG8(0x0baa), 0x00 },
> > > > +       { CCI_REG8(0x0bac), 0x00 },
> > > > +       { CCI_REG8(0x0bad), 0x05 },
> > > > +       { CCI_REG8(0x0bae), 0x00 },
> > > > +       { CCI_REG8(0x0bb0), 0x07 },
> > > > +       { CCI_REG8(0x0bb1), 0x3e },
> > > > +       { CCI_REG8(0x0bb2), 0x00 },
> > > > +       { CCI_REG8(0x0bb4), 0x00 },
> > > > +       { CCI_REG8(0x0bb5), 0x06 },
> > > > +       { CCI_REG8(0x0bb6), 0x00 },
> > > > +       { CCI_REG8(0x0bb8), 0x0a },
> > > > +       { CCI_REG8(0x0bb9), 0x1a },
> > > > +       { CCI_REG8(0x0bba), 0x00 },
> > > > +       { CCI_REG8(0x0bbc), 0x09 },
> > > > +       { CCI_REG8(0x0bbd), 0x36 },
> > > > +       { CCI_REG8(0x0bbe), 0x00 },
> > > > +       { CCI_REG8(0x0bc0), 0x0e },
> > > > +       { CCI_REG8(0x0bc1), 0x66 },
> > > > +       { CCI_REG8(0x0bc2), 0x00 },
> > > > +       { CCI_REG8(0x0bc4), 0x10 },
> > > > +       { CCI_REG8(0x0bc5), 0x06 },
> > > > +       { CCI_REG8(0x0bc6), 0x00 },
> > > > +       { CCI_REG8(0x02c1), 0xe0 },
> > > > +       { CCI_REG8(0x0207), 0x04 },
> > > > +       { CCI_REG8(0x02c2), 0x10 },
> > > > +       { CCI_REG8(0x02c3), 0x74 },
> > > > +       { CCI_REG8(0x02c5), 0x09 },
> > > > +       { CCI_REG8(0x02c1), 0xe0 },
> > > > +       { CCI_REG8(0x0207), 0x04 },
> > > > +       { CCI_REG8(0x02c2), 0x10 },
> > > > +       { CCI_REG8(0x02c5), 0x09 },
> > > > +       { CCI_REG8(0x02c1), 0xe0 },
> > > > +       { CCI_REG8(0x0207), 0x04 },
> > > > +       { CCI_REG8(0x02c2), 0x10 },
> > > > +       { CCI_REG8(0x02c5), 0x09 },
> > > > +
> > > > +       /* auto load CH_GAIN */
> > > > +       { CCI_REG8(0x0aa1), 0x15 },
> > > > +       { CCI_REG8(0x0aa2), 0x50 },
> > > > +       { CCI_REG8(0x0aa3), 0x00 },
> > > > +       { CCI_REG8(0x0aa4), 0x09 },
> > > > +       { CCI_REG8(0x0a90), 0x25 },
> > > > +       { CCI_REG8(0x0a91), 0x0e },
> > > > +       { CCI_REG8(0x0a94), 0x80 },
> > > > +
> > > > +       /* ISP */
> > > > +       { CCI_REG8(0x0050), 0x00 },
> > > > +       { CCI_REG8(0x0089), 0x83 },
> > > > +       { CCI_REG8(0x005a), 0x40 },
> > > > +       { CCI_REG8(0x00c3), 0x35 },
> > > > +       { CCI_REG8(0x00c4), 0x80 },
> > > > +       { CCI_REG8(0x0080), 0x10 },
> > > > +       { CCI_REG8(0x0040), 0x12 },
> > > > +       { CCI_REG8(0x0053), 0x0a },
> > > > +       { CCI_REG8(0x0054), 0x44 },
> > > > +       { CCI_REG8(0x0055), 0x32 },
> > > > +       { CCI_REG8(0x0058), 0x89 },
> > > > +       { CCI_REG8(0x004a), 0x03 },
> > > > +       { CCI_REG8(0x0048), 0xf0 },
> > > > +       { CCI_REG8(0x0049), 0x0f },
> > > > +       { CCI_REG8(0x0041), 0x20 },
> > > > +       { CCI_REG8(0x0043), 0x0a },
> > > > +       { CCI_REG8(0x009d), 0x08 },
> > > > +       { CCI_REG8(0x0236), 0x40 },
> > > > +
> > > > +       /* gain */
> > > 
> > > Is the gain configurable? Is this analogue gain? digital gain? or
> > > colour
> > > balanace gains ?
> > > 
> > > 
> > > > +       { CCI_REG8(0x0204), 0x04 },
> > > > +       { CCI_REG8(0x0205), 0x00 },
> > > > +       { CCI_REG8(0x02b3), 0x00 },
> > > > +       { CCI_REG8(0x02b4), 0x00 },
> > > > +       { CCI_REG8(0x009e), 0x01 },
> > > > +       { CCI_REG8(0x009f), 0x94 },
> > > > +
> > > > +       /* auto load REG */
> > > > +       { CCI_REG8(0x0aa1), 0x10 },
> > > > +       { CCI_REG8(0x0aa2), 0xf8 },
> > > > +       { CCI_REG8(0x0aa3), 0x00 },
> > > > +       { CCI_REG8(0x0aa4), 0x1f },
> > > > +       { CCI_REG8(0x0a90), 0x11 },
> > > > +       { CCI_REG8(0x0a91), 0x0e },
> > > > +       { CCI_REG8(0x0a94), 0x80 },
> > > > +       { CCI_REG8(0x03fe), 0x00 },
> > > > +       { CCI_REG8(0x0a90), 0x00 },
> > > > +       { CCI_REG8(0x0a70), 0x00 },
> > > > +       { CCI_REG8(0x0a67), 0x00 },
> > > > +       { CCI_REG8(0x0af4), 0x29 },
> > > > +
> > > > +       /* DPHY */
> > > > +       { CCI_REG8(0x0d80), 0x07 },
> > > > +       { CCI_REG8(0x0dd3), 0x18 },
> > > > +
> > > > +       /* CISCTL_Reset */
> > > > +       { CCI_REG8(0x031c), 0x80 },
> > > > +       { CCI_REG8(0x03fe), 0x30 },
> > > > +       { CCI_REG8(0x0d17), 0x06 },
> > > > +       { CCI_REG8(0x03fe), 0x00 },
> > > > +       { CCI_REG8(0x0d17), 0x00 },
> > > > +       { CCI_REG8(0x031c), 0x93 },
> > > > +       { CCI_REG8(0x03fe), 0x00 },
> > > > +       { CCI_REG8(0x031c), 0x80 },
> > > > +       { CCI_REG8(0x03fe), 0x30 },
> > > > +       { CCI_REG8(0x0d17), 0x06 },
> > > > +       { CCI_REG8(0x03fe), 0x00 },
> > > > +       { CCI_REG8(0x0d17), 0x00 },
> > > > +       { CCI_REG8(0x031c), 0x93 },
> > > > +};
> > > > +
> > > > +struct gc05a2_mode {
> > > > +       u32 width;
> > > > +       u32 height;
> > > > +       const struct gc05a2_reg_list reg_list;
> > > > +
> > > > +       u32 hts; /* Horizontal timining size */
> > > > +       u32 vts_def; /* Default vertical timining size */
> > > > +       u32 vts_min; /* Min vertical timining size */
> > > > +};
> > > > +
> > > > +/* Declare modes in order, from biggest to smallest height. */
> > > > +static const struct gc05a2_mode gc05a2_modes[] = {
> > > > +       {
> > > > +               /* 2592*1944@30fps */
> > > > +               .width = GC05A2_NATIVE_WIDTH,
> > > > +               .height = GC05A2_NATIVE_HEIGHT,
> > > > +               .reg_list = {
> > > > +                       .num_of_regs =
> ARRAY_SIZE(mode_2592x1944),
> > > > +                       .regs = mode_2592x1944,
> > > > +               },
> > > > +               .hts = 3664,
> > > > +               .vts_def = 2032,
> > > > +               .vts_min = 2032,
> > > > +       },
> > > > +       {
> > > > +               /* 1280*720@60fps */
> > > > +               .width = 1280,
> > > > +               .height = 720,
> > > > +               .reg_list = {
> > > > +                       .num_of_regs =
> ARRAY_SIZE(mode_1280x720),
> > > > +                       .regs = mode_1280x720,
> > > > +               },
> > > > +               .hts = 3616,
> > > > +               .vts_def = 1032,
> > > > +               .vts_min = 1032,
> > > > +       },
> > > > +};
> > > > +
> > > > +static inline struct gc05a2 *to_gc05a2(struct v4l2_subdev *sd)
> > > > +{
> > > > +       return container_of(sd, struct gc05a2, sd);
> > > > +}
> > > > +
> > > > +static int gc05a2_power_on(struct device *dev)
> > > > +{
> > > > +       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > > > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > > +       int ret;
> > > > +
> > > > +       ret =
> regulator_bulk_enable(ARRAY_SIZE(gc05a2_supply_name),
> > > > +                                   gc05a2->supplies);
> > > > +       if (ret < 0) {
> > > > +               dev_err(gc05a2->dev, "failed to enable
> regulators:
> > > %d\n", ret);
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       ret = clk_prepare_enable(gc05a2->xclk);
> > > > +       if (ret < 0) {
> > > >
> > >
> +               regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name)
> > > ,
> > > > +                                      gc05a2->supplies);
> > > > +               dev_err(gc05a2->dev, "clk prepare enable
> > > failed\n");
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       fsleep(GC05A2_SLEEP_US);
> > > > +
> > > > +       gpiod_set_value_cansleep(gc05a2->reset_gpio, 0);
> > > > +       fsleep(GC05A2_SLEEP_US);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int gc05a2_power_off(struct device *dev)
> > > > +{
> > > > +       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > > > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > > +
> > > > +       clk_disable_unprepare(gc05a2->xclk);
> > > > +       gpiod_set_value_cansleep(gc05a2->reset_gpio, 1);
> > > > +       regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name),
> > > > +                              gc05a2->supplies);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int gc05a2_enum_mbus_code(struct v4l2_subdev *sd,
> > > > +                                struct v4l2_subdev_state
> > > *sd_state,
> > > > +                                struct
> v4l2_subdev_mbus_code_enum
> > > *code)
> > > > +{
> > > > +       if (code->index > 0)
> > > > +               return -EINVAL;
> > > > +
> > > > +       code->code = GC05A2_MBUS_CODE;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int gc05a2_enum_frame_size(struct v4l2_subdev *subdev,
> > > > +                                 struct v4l2_subdev_state
> > > *sd_state,
> > > > +                                 struct
> > > v4l2_subdev_frame_size_enum *fse)
> > > > +{
> > > > +       if (fse->code != GC05A2_MBUS_CODE)
> > > > +               return -EINVAL;
> > > > +
> > > > +       if (fse->index >= ARRAY_SIZE(gc05a2_modes))
> > > > +               return -EINVAL;
> > > > +
> > > > +       fse->min_width = gc05a2_modes[fse->index].width;
> > > > +       fse->max_width = gc05a2_modes[fse->index].width;
> > > > +       fse->min_height = gc05a2_modes[fse->index].height;
> > > > +       fse->max_height = gc05a2_modes[fse->index].height;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int gc05a2_update_cur_mode_controls(struct gc05a2
> *gc05a2,
> > > > +                                          const struct
> gc05a2_mode
> > > *mode)
> > > > +{
> > > > +       s64 exposure_max, h_blank;
> > > > +       int ret;
> > > > +
> > > > +       ret = __v4l2_ctrl_modify_range(gc05a2->vblank,
> > > > +                                      mode->vts_min - mode-
> > > >height,
> > > > +                                      GC05A2_VTS_MAX - mode-
> > > >height, 1,
> > > > +                                      mode->vts_def - mode-
> > > >height);
> > > > +       if (ret) {
> > > > +               dev_err(gc05a2->dev, "VB ctrl range update
> > > failed\n");
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       h_blank = mode->hts - mode->width;
> > > > +       ret = __v4l2_ctrl_modify_range(gc05a2->hblank, h_blank,
> > > h_blank, 1,
> > > > +                                      h_blank);
> > > > +       if (ret) {
> > > > +               dev_err(gc05a2->dev, "HB ctrl range update
> > > failed\n");
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > > > +       ret = __v4l2_ctrl_modify_range(gc05a2->exposure,
> > > GC05A2_EXP_MIN,
> > > > +                                      exposure_max,
> > > GC05A2_EXP_STEP,
> > > > +                                      exposure_max);
> > > > +       if (ret) {
> > > > +               dev_err(gc05a2->dev, "exposure ctrl range
> update
> > > failed\n");
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static void gc05a2_update_pad_format(struct gc05a2 *gc08a3,
> > > > +                                    const struct gc05a2_mode
> > > *mode,
> > > > +                                    struct v4l2_mbus_framefmt
> > > *fmt)
> > > > +{
> > > > +       fmt->width = mode->width;
> > > > +       fmt->height = mode->height;
> > > > +       fmt->code = GC05A2_MBUS_CODE;
> > > > +       fmt->field = V4L2_FIELD_NONE;
> > > > +       fmt->colorspace = V4L2_COLORSPACE_RAW;
> > > > +       fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt-
> > > >colorspace);
> > > > +       fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> > > > +       fmt->xfer_func = V4L2_XFER_FUNC_NONE;
> > > > +}
> > > > +
> > > > +static int gc05a2_set_format(struct v4l2_subdev *sd,
> > > > +                            struct v4l2_subdev_state *state,
> > > > +                            struct v4l2_subdev_format *fmt)
> > > > +{
> > > > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > > +       struct v4l2_mbus_framefmt *mbus_fmt;
> > > > +       struct v4l2_rect *crop;
> > > > +       const struct gc05a2_mode *mode;
> > > > +
> > > > +       mode = v4l2_find_nearest_size(gc05a2_modes,
> > > ARRAY_SIZE(gc05a2_modes),
> > > > +                                     width, height, fmt-
> > > >format.width,
> > > > +                                     fmt->format.height);
> > > > +
> > > > +       /* update crop info to subdev state */
> > > > +       crop = v4l2_subdev_state_get_crop(state, 0);
> > > > +       crop->width = mode->width;
> > > > +       crop->height = mode->height;
> > > > +
> > > > +       /* update fmt info to subdev state */
> > > > +       gc05a2_update_pad_format(gc05a2, mode, &fmt->format);
> > > > +       mbus_fmt = v4l2_subdev_state_get_format(state, 0);
> > > > +       *mbus_fmt = fmt->format;
> > > > +
> > > > +       if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > > > +               return 0;
> > > > +       gc05a2->cur_mode = mode;
> > > > +       gc05a2_update_cur_mode_controls(gc05a2, mode);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int gc05a2_get_selection(struct v4l2_subdev *sd,
> > > > +                               struct v4l2_subdev_state
> *state,
> > > > +                               struct v4l2_subdev_selection
> *sel)
> > > > +{
> > > > +       switch (sel->target) {
> > > > +       case V4L2_SEL_TGT_CROP_DEFAULT:
> > > > +       case V4L2_SEL_TGT_CROP:
> > > > +               sel->r = *v4l2_subdev_state_get_crop(state, 0);
> > > > +               break;
> > > > +       case V4L2_SEL_TGT_CROP_BOUNDS:
> > > > +               sel->r.top = 0;
> > > > +               sel->r.left = 0;
> > > > +               sel->r.width = GC05A2_NATIVE_WIDTH;
> > > > +               sel->r.height = GC05A2_NATIVE_HEIGHT;
> > > > +               break;
> > > > +       default:
> > > > +               return -EINVAL;
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int gc05a2_init_state(struct v4l2_subdev *sd,
> > > > +                            struct v4l2_subdev_state *state)
> > > > +{
> > > > +       struct v4l2_subdev_format fmt = {
> > > > +               .which = V4L2_SUBDEV_FORMAT_TRY,
> > > > +               .pad = 0,
> > > > +               .format = {
> > > > +                       .code = GC05A2_MBUS_CODE,
> > > > +                       .width = gc05a2_modes[0].width,
> > > > +                       .height = gc05a2_modes[0].height,
> > > > +               },
> > > > +       };
> > > > +
> > > > +       gc05a2_set_format(sd, state, &fmt);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int gc05a2_set_ctrl_hflip(struct gc05a2 *gc05a2, u32
> > > ctrl_val)
> > > > +{
> > > > +       int ret;
> > > > +       u64 val;
> > > > +
> > > > +       ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> > > NULL);
> > > > +       if (ret) {
> > > > +               dev_err(gc05a2->dev, "read hflip register
> failed:
> > > %d\n", ret);
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > > > +                              GC05A2_FLIP_H_MASK,
> > > > +                              ctrl_val ? GC05A2_FLIP_H_MASK :
> 0,
> > > NULL);
> > > > +}
> > > > +
> > > > +static int gc05a2_set_ctrl_vflip(struct gc05a2 *gc05a2, u32
> > > ctrl_val)
> > > > +{
> > > > +       int ret;
> > > > +       u64 val;
> > > > +
> > > > +       ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> > > NULL);
> > > > +       if (ret) {
> > > > +               dev_err(gc05a2->dev, "read vflip register
> failed:
> > > %d\n", ret);
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > > > +                              GC05A2_FLIP_V_MASK,
> > > > +                              ctrl_val ? GC05A2_FLIP_V_MASK :
> 0,
> > > NULL);
> > > > +}
> > > > +
> > > > +static int gc05a2_test_pattern(struct gc05a2 *gc05a2, u32
> > > pattern_menu)
> > > > +{
> > > > +       u32 pattern;
> > > > +       int ret;
> > > > +
> > > > +       if (pattern_menu) {
> > > > +               switch (pattern_menu) {
> > > > +               case 1:
> > > > +               case 2:
> > > > +               case 3:
> > > > +               case 4:
> > > > +               case 5:
> > > > +               case 6:
> > > > +               case 7:
> > > > +                       pattern = pattern_menu << 4;
> > > > +                       break;
> > > > +
> > > > +               case 8:
> > > > +                       pattern = 0;
> > > > +                       break;
> > > > +
> > > > +               case 9:
> > > > +                       pattern = 4;
> > > > +                       break;
> > > > +
> > > > +               default:
> > > > +                       pattern = 0x00;
> > > > +                       break;
> > > > +               }
> > > 
> > > This is fairly terse. Can we add comments, or definitions for the
> > > types
> > > or such so that the above is easier to interpret?
> > > 
> > > > +
> > > > +               ret = cci_write(gc05a2->regmap,
> > > GC05A2_REG_TEST_PATTERN_IDX,
> > > > +                               pattern, NULL);
> > > > +               if (ret)
> > > > +                       return ret;
> > > > +
> > > > +               return cci_write(gc05a2->regmap,
> > > GC05A2_REG_TEST_PATTERN_EN,
> > > > +                                GC05A2_TEST_PATTERN_EN, NULL);
> > > > +       } else {
> > > > +               return cci_write(gc05a2->regmap,
> > > GC05A2_REG_TEST_PATTERN_EN,
> > > > +                                0x00, NULL);
> > > > +       }
> > > > +}
> > > > +
> > > > +static int gc05a2_set_ctrl(struct v4l2_ctrl *ctrl)
> > > > +{
> > > > +       struct gc05a2 *gc05a2 =
> > > > +               container_of(ctrl->handler, struct gc05a2,
> ctrls);
> > > > +       int ret = 0;
> > > > +       s64 exposure_max;
> > > > +       struct v4l2_subdev_state *state;
> > > > +       const struct v4l2_mbus_framefmt *format;
> > > > +
> > > > +       state = v4l2_subdev_get_locked_active_state(&gc05a2-
> >sd);
> > > > +       format = v4l2_subdev_state_get_format(state, 0);
> > > > +
> > > > +       if (ctrl->id == V4L2_CID_VBLANK) {
> > > > +               /* Update max exposure while meeting expected
> > > vblanking */
> > > > +               exposure_max = format->height + ctrl->val -
> > > GC05A2_EXP_MARGIN;
> > > > +               __v4l2_ctrl_modify_range(gc05a2->exposure,
> > > > +                                        gc05a2->exposure-
> >minimum,
> > > > +                                        exposure_max, gc05a2-
> > > >exposure->step,
> > > > +                                        exposure_max);
> > > > +       }
> > > > +
> > > > +       /*
> > > > +        * Applying V4L2 control value only happens
> > > > +        * when power is on for streaming.
> > > > +        */
> > > > +       if (!pm_runtime_get_if_active(gc05a2->dev))
> > > > +               return 0;
> > > > +
> > > > +       switch (ctrl->id) {
> > > > +       case V4L2_CID_EXPOSURE:
> > > > +               ret = cci_write(gc05a2->regmap, GC05A2_EXP_REG,
> > > > +                               ctrl->val, NULL);
> > > > +               break;
> > > > +
> > > > +       case V4L2_CID_ANALOGUE_GAIN:
> > > > +               ret = cci_write(gc05a2->regmap,
> GC05A2_AGAIN_REG,
> > > > +                               ctrl->val, NULL);
> > > > +               break;
> > > > +
> > > > +       case V4L2_CID_VBLANK:
> > > > +               ret = cci_write(gc05a2->regmap,
> > > GC05A2_FRAME_LENGTH_REG,
> > > > +                               gc05a2->cur_mode->height +
> ctrl-
> > > >val, NULL);
> > > > +               break;
> > > > +
> > > > +       case V4L2_CID_HFLIP:
> > > > +               ret = gc05a2_set_ctrl_hflip(gc05a2, ctrl->val);
> > > > +               break;
> > > > +
> > > > +       case V4L2_CID_VFLIP:
> > > > +               ret = gc05a2_set_ctrl_vflip(gc05a2, ctrl->val);
> > > > +               break;
> > > > +
> > > > +       case V4L2_CID_TEST_PATTERN:
> > > > +               ret = gc05a2_test_pattern(gc05a2, ctrl->val);
> > > > +               break;
> > > > +
> > > > +       default:
> > > > +               break;
> > > > +       }
> > > > +
> > > > +       pm_runtime_put(gc05a2->dev);
> > > > +
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static const struct v4l2_ctrl_ops gc05a2_ctrl_ops = {
> > > > +       .s_ctrl = gc05a2_set_ctrl,
> > > > +};
> > > > +
> > > > +static int gc05a2_identify_module(struct gc05a2 *gc05a2)
> > > > +{
> > > > +       u64 val;
> > > > +       int ret;
> > > > +
> > > > +       if (gc05a2->identified)
> > > > +               return 0;
> > > > +
> > > > +       ret = cci_read(gc05a2->regmap, GC05A2_REG_CHIP_ID,
> &val,
> > > NULL);
> > > > +       if (ret)
> > > > +               return ret;
> > > > +
> > > > +       if (val != GC05A2_CHIP_ID) {
> > > > +               dev_err(gc05a2->dev, "chip id mismatch:
> > > 0x%x!=0x%llx",
> > > > +                       GC05A2_CHIP_ID, val);
> > > > +               return -ENXIO;
> > > > +       }
> > > > +
> > > > +       gc05a2->identified = true;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int gc05a2_start_streaming(struct gc05a2 *gc05a2)
> > > > +{
> > > > +       const struct gc05a2_mode *mode;
> > > > +       const struct gc05a2_reg_list *reg_list;
> > > > +       int ret;
> > > > +
> > > > +       ret = pm_runtime_resume_and_get(gc05a2->dev);
> > > > +       if (ret < 0)
> > > > +               return ret;
> > > > +
> > > > +       ret = gc05a2_identify_module(gc05a2);
> > > > +       if (ret)
> > > > +               goto err_rpm_put;
> > > > +
> > > > +       ret = cci_multi_reg_write(gc05a2->regmap,
> > > > +                                 mode_table_common,
> > > >
> +                                 ARRAY_SIZE(mode_table_common),
> > > NULL);
> > > > +       if (ret)
> > > > +               goto err_rpm_put;
> > > > +
> > > > +       mode = gc05a2->cur_mode;
> > > > +       reg_list = &mode->reg_list;
> > > > +
> > > > +       ret = cci_multi_reg_write(gc05a2->regmap,
> > > > +                                 reg_list->regs, reg_list-
> > > >num_of_regs, NULL);
> > > > +       if (ret < 0)
> > > > +               goto err_rpm_put;
> > > > +
> > > > +       ret = __v4l2_ctrl_handler_setup(&gc05a2->ctrls);
> > > > +       if (ret < 0) {
> > > > +               dev_err(gc05a2->dev, "could not sync v4l2
> > > controls\n");
> > > > +               goto err_rpm_put;
> > > > +       }
> > > > +
> > > > +       ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG,
> 1,
> > > NULL);
> > > > +       if (ret < 0) {
> > > > +               dev_err(gc05a2->dev, "write STREAMING_REG
> failed:
> > > %d\n", ret);
> > > > +               goto err_rpm_put;
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +
> > > > +err_rpm_put:
> > > > +       pm_runtime_put(gc05a2->dev);
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static int gc05a2_stop_streaming(struct gc05a2 *gc05a2)
> > > > +{
> > > > +       int ret;
> > > > +
> > > > +       ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG,
> 0,
> > > NULL);
> > > > +       if (ret < 0)
> > > > +               dev_err(gc05a2->dev, "could not sent stop
> streaming
> > > %d\n", ret);
> > > > +
> > > > +       pm_runtime_put(gc05a2->dev);
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static int gc05a2_s_stream(struct v4l2_subdev *subdev, int
> enable)
> > > > +{
> > > > +       struct gc05a2 *gc05a2 = to_gc05a2(subdev);
> > > > +       struct v4l2_subdev_state *state;
> > > > +       int ret;
> > > > +
> > > > +       state = v4l2_subdev_lock_and_get_active_state(subdev);
> > > > +
> > > > +       if (enable)
> > > > +               ret = gc05a2_start_streaming(gc05a2);
> > > > +       else
> > > > +               ret = gc05a2_stop_streaming(gc05a2);
> > > > +
> > > > +       v4l2_subdev_unlock_state(state);
> > > > +
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static const struct v4l2_subdev_video_ops gc05a2_video_ops = {
> > > > +       .s_stream = gc05a2_s_stream,
> > > > +};
> > > > +
> > > > +static const struct v4l2_subdev_pad_ops gc05a2_subdev_pad_ops
> = {
> > > > +       .enum_mbus_code = gc05a2_enum_mbus_code,
> > > > +       .enum_frame_size = gc05a2_enum_frame_size,
> > > > +       .get_fmt = v4l2_subdev_get_fmt,
> > > > +       .set_fmt = gc05a2_set_format,
> > > > +       .get_selection = gc05a2_get_selection,
> > > > +};
> > > > +
> > > > +static const struct v4l2_subdev_core_ops gc05a2_core_ops = {
> > > > +       .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> > > > +       .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> > > > +};
> > > > +
> > > > +static const struct v4l2_subdev_ops gc05a2_subdev_ops = {
> > > > +       .core = &gc05a2_core_ops,
> > > > +       .video = &gc05a2_video_ops,
> > > > +       .pad = &gc05a2_subdev_pad_ops,
> > > > +};
> > > > +
> > > > +static const struct v4l2_subdev_internal_ops
> gc05a2_internal_ops =
> > > {
> > > > +       .init_state = gc05a2_init_state,
> > > > +};
> > > > +
> > > > +static int gc05a2_get_regulators(struct device *dev, struct
> gc05a2
> > > *gc05a2)
> > > > +{
> > > > +       unsigned int i;
> > > > +
> > > > +       for (i = 0; i < ARRAY_SIZE(gc05a2_supply_name); i++)
> > > > +               gc05a2->supplies[i].supply =
> gc05a2_supply_name[i];
> > > > +
> > > > +       return devm_regulator_bulk_get(dev,
> > > ARRAY_SIZE(gc05a2_supply_name),
> > > > +                                      gc05a2->supplies);
> > > > +}
> > > > +
> > > > +static int gc05a2_parse_fwnode(struct gc05a2 *gc05a2)
> > > > +{
> > > > +       struct fwnode_handle *endpoint;
> > > > +       struct v4l2_fwnode_endpoint bus_cfg = {
> > > > +               .bus_type = V4L2_MBUS_CSI2_DPHY,
> > > > +       };
> > > > +       int ret;
> > > > +       struct device *dev = gc05a2->dev;
> > > > +
> > > > +       endpoint =
> > > >
> +               fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0,
> > > 0,
> > > >
> > >
> +                                               FWNODE_GRAPH_ENDPOINT
> > > _NEXT);
> > > > +       if (!endpoint) {
> > > > +               dev_err(dev, "endpoint node not found\n");
> > > > +               return -EINVAL;
> > > > +       }
> > > > +
> > > > +       ret = v4l2_fwnode_endpoint_alloc_parse(endpoint,
> &bus_cfg);
> > > > +       if (ret) {
> > > > +               dev_err(dev, "parsing endpoint node failed\n");
> > > > +               goto done;
> > > > +       }
> > > > +
> > > > +       ret = v4l2_link_freq_to_bitmap(dev,
> > > bus_cfg.link_frequencies,
> > > >
> > >
> +                                      bus_cfg.nr_of_link_frequencies
> > > ,
> > > >
> +                                      gc05a2_link_freq_menu_items,
> > > >
> > >
> +                                      ARRAY_SIZE(gc05a2_link_freq_me
> > > nu_items),
> > > > +                                      &gc05a2-
> >link_freq_bitmap);
> > > > +       if (ret)
> > > > +               goto done;
> > > > +
> > > > +done:
> > > > +       v4l2_fwnode_endpoint_free(&bus_cfg);
> > > > +       fwnode_handle_put(endpoint);
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static u64 gc05a2_to_pixel_rate(u32 f_index)
> > > > +{
> > > > +       u64 pixel_rate =
> > > > +               gc05a2_link_freq_menu_items[f_index] * 2 *
> > > GC05A2_DATA_LANES;
> > > > +
> > > > +       return div_u64(pixel_rate, GC05A2_RGB_DEPTH);
> > > > +}
> > > > +
> > > > +static int gc05a2_init_controls(struct gc05a2 *gc05a2)
> > > > +{
> > > > +       struct i2c_client *client =
> v4l2_get_subdevdata(&gc05a2-
> > > >sd);
> > > > +       const struct gc05a2_mode *mode = &gc05a2_modes[0];
> > > > +       const struct v4l2_ctrl_ops *ops = &gc05a2_ctrl_ops;
> > > > +       struct v4l2_fwnode_device_properties props;
> > > > +       struct v4l2_ctrl_handler *ctrl_hdlr;
> > > > +       s64 exposure_max, h_blank;
> > > > +       int ret;
> > > > +
> > > > +       ctrl_hdlr = &gc05a2->ctrls;
> > > > +       ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
> > > > +       if (ret)
> > > > +               return ret;
> > > > +
> > > > +       gc05a2->hflip = v4l2_ctrl_new_std(ctrl_hdlr,
> > > &gc05a2_ctrl_ops,
> > > > +                                         V4L2_CID_HFLIP, 0, 1,
> 1,
> > > 0);
> > > > +       gc05a2->vflip = v4l2_ctrl_new_std(ctrl_hdlr,
> > > &gc05a2_ctrl_ops,
> > > > +                                         V4L2_CID_VFLIP, 0, 1,
> 1,
> > > 0);
> > > > +       v4l2_ctrl_cluster(2, &gc05a2->hflip);
> > > > +
> > > > +       gc05a2->link_freq =
> > > > +       v4l2_ctrl_new_int_menu(ctrl_hdlr,
> > > > +                              &gc05a2_ctrl_ops,
> > > > +                              V4L2_CID_LINK_FREQ,
> > > >
> > >
> +                              ARRAY_SIZE(gc05a2_link_freq_menu_items
> > > ) - 1,
> > > > +                              0,
> > > > +                              gc05a2_link_freq_menu_items);
> > > > +       if (gc05a2->link_freq)
> > > > +               gc05a2->link_freq->flags |=
> > > V4L2_CTRL_FLAG_READ_ONLY;
> > > > +
> > > > +       gc05a2->pixel_rate =
> > > > +               v4l2_ctrl_new_std(ctrl_hdlr,
> > > > +                                 &gc05a2_ctrl_ops,
> > > > +                                 V4L2_CID_PIXEL_RATE, 0,
> > > > +                                 gc05a2_to_pixel_rate(0),
> > > > +                                 1,
> > > > +                                 gc05a2_to_pixel_rate(0));
> > > > +
> > > > +       gc05a2->vblank =
> > > > +               v4l2_ctrl_new_std(ctrl_hdlr,
> > > > +                                 &gc05a2_ctrl_ops,
> > > V4L2_CID_VBLANK,
> > > > +                                 mode->vts_min - mode->height,
> > > > +                                 GC05A2_VTS_MAX - mode-
> >height, 1,
> > > > +                                 mode->vts_def - mode-
> >height);
> > > > +
> > > > +       h_blank = mode->hts - mode->width;
> > > > +       gc05a2->hblank = v4l2_ctrl_new_std(ctrl_hdlr,
> > > &gc05a2_ctrl_ops,
> > > > +                                          V4L2_CID_HBLANK,
> > > h_blank, h_blank, 1,
> > > > +                                          h_blank);
> > > > +       if (gc05a2->hblank)
> > > > +               gc05a2->hblank->flags |=
> V4L2_CTRL_FLAG_READ_ONLY;
> > > > +
> > > > +       v4l2_ctrl_new_std(ctrl_hdlr, &gc05a2_ctrl_ops,
> > > > +                         V4L2_CID_ANALOGUE_GAIN,
> GC05A2_AGAIN_MIN,
> > > > +                         GC05A2_AGAIN_MAX, GC05A2_AGAIN_STEP,
> > > > +                         GC05A2_AGAIN_MIN);
> > > > +
> > > > +       exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > > > +       gc05a2->exposure = v4l2_ctrl_new_std(ctrl_hdlr,
> > > &gc05a2_ctrl_ops,
> > > > +                                            V4L2_CID_EXPOSURE,
> > > GC05A2_EXP_MIN,
> > > > +                                            exposure_max,
> > > GC05A2_EXP_STEP,
> > > > +                                            exposure_max);
> > > > +
> > > > +       v4l2_ctrl_new_std_menu_items(ctrl_hdlr,
> &gc05a2_ctrl_ops,
> > > > +                                    V4L2_CID_TEST_PATTERN,
> > > >
> > >
> +                                    ARRAY_SIZE(gc05a2_test_pattern_m
> > > enu) - 1,
> > > > +                                    0, 0,
> > > gc05a2_test_pattern_menu);
> > > > +
> > > > +       /* register properties to fwnode (e.g. rotation,
> > > orientation) */
> > > > +       ret = v4l2_fwnode_device_parse(&client->dev, &props);
> > > > +       if (ret)
> > > > +               goto error_ctrls;
> > > > +
> > > > +       ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, ops,
> > > &props);
> > > > +       if (ret)
> > > > +               goto error_ctrls;
> > > > +
> > > > +       if (ctrl_hdlr->error) {
> > > > +               ret = ctrl_hdlr->error;
> > > > +               goto error_ctrls;
> > > > +       }
> > > > +
> > > > +       gc05a2->sd.ctrl_handler = ctrl_hdlr;
> > > > +
> > > > +       return 0;
> > > > +
> > > > +error_ctrls:
> > > > +       v4l2_ctrl_handler_free(ctrl_hdlr);
> > > > +
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static int gc05a2_probe(struct i2c_client *client)
> > > > +{
> > > > +       struct device *dev = &client->dev;
> > > > +       struct gc05a2 *gc05a2;
> > > > +       int ret;
> > > > +
> > > > +       gc05a2 = devm_kzalloc(dev, sizeof(*gc05a2),
> GFP_KERNEL);
> > > > +       if (!gc05a2)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       gc05a2->dev = dev;
> > > > +
> > > > +       ret = gc05a2_parse_fwnode(gc05a2);
> > > > +       if (ret)
> > > > +               return ret;
> > > > +
> > > > +       gc05a2->regmap = devm_cci_regmap_init_i2c(client, 16);
> > > > +       if (IS_ERR(gc05a2->regmap))
> > > > +               return dev_err_probe(dev, PTR_ERR(gc05a2-
> >regmap),
> > > > +                                    "failed to init CCI\n");
> > > > +
> > > > +       gc05a2->xclk = devm_clk_get(dev, NULL);
> > > > +       if (IS_ERR(gc05a2->xclk))
> > > > +               return dev_err_probe(dev, PTR_ERR(gc05a2-
> >xclk),
> > > > +                                    "failed to get xclk\n");
> > > > +
> > > > +       ret = clk_set_rate(gc05a2->xclk,
> GC05A2_DEFAULT_CLK_FREQ);
> > > > +       if (ret)
> > > > +               return dev_err_probe(dev, ret,
> > > > +                                    "failed to set xclk
> > > frequency\n");
> > > > +
> > > > +       ret = gc05a2_get_regulators(dev, gc05a2);
> > > > +       if (ret < 0)
> > > > +               return dev_err_probe(dev, ret,
> > > > +                                    "failed to get
> regulators\n");
> > > > +
> > > > +       gc05a2->reset_gpio = devm_gpiod_get(dev, "reset",
> > > GPIOD_OUT_LOW);
> > > > +       if (IS_ERR(gc05a2->reset_gpio))
> > > > +               return dev_err_probe(dev, PTR_ERR(gc05a2-
> > > >reset_gpio),
> > > > +                                    "failed to get gpio\n");
> > > > +
> > > > +       v4l2_i2c_subdev_init(&gc05a2->sd, client,
> > > &gc05a2_subdev_ops);
> > > > +       gc05a2->sd.internal_ops = &gc05a2_internal_ops;
> > > > +       gc05a2->cur_mode = &gc05a2_modes[0];
> > > > +
> > > > +       ret = gc05a2_init_controls(gc05a2);
> > > > +       if (ret)
> > > > +               return dev_err_probe(dev, ret,
> > > > +                                    "failed to init
> controls\n");
> > > > +
> > > > +       gc05a2->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > > > +                           V4L2_SUBDEV_FL_HAS_EVENTS;
> > > > +       gc05a2->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > > +       gc05a2->sd.dev = &client->dev;
> > > > +       gc05a2->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > > +
> > > > +       ret = media_entity_pads_init(&gc05a2->sd.entity, 1,
> > > &gc05a2->pad);
> > > > +       if (ret < 0) {
> > > > +               dev_err(dev, "could not register media
> entity\n");
> > > > +               goto err_v4l2_ctrl_handler_free;
> > > > +       }
> > > > +
> > > > +       gc05a2->sd.state_lock = gc05a2->ctrls.lock;
> > > > +       ret = v4l2_subdev_init_finalize(&gc05a2->sd);
> > > > +       if (ret < 0) {
> > > > +               dev_err(dev, "v4l2 subdev init error: %d\n",
> ret);
> > > > +               goto err_media_entity_cleanup;
> > > > +       }
> > > > +
> > > > +       pm_runtime_set_active(gc05a2->dev);
> > > > +       pm_runtime_enable(gc05a2->dev);
> > > > +       pm_runtime_set_autosuspend_delay(gc05a2->dev, 1000);
> > > > +       pm_runtime_use_autosuspend(gc05a2->dev);
> > > > +       pm_runtime_idle(gc05a2->dev);
> > > > +
> > > > +       ret = v4l2_async_register_subdev_sensor(&gc05a2->sd);
> > > > +       if (ret < 0) {
> > > > +               dev_err(dev, "could not register v4l2
> device\n");
> > > > +               goto err_rpm;
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +
> > > > +err_rpm:
> > > > +       pm_runtime_disable(gc05a2->dev);
> > > > +       v4l2_subdev_cleanup(&gc05a2->sd);
> > > > +
> > > > +err_media_entity_cleanup:
> > > > +       media_entity_cleanup(&gc05a2->sd.entity);
> > > > +
> > > > +err_v4l2_ctrl_handler_free:
> > > > +       v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > > > +
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static void gc05a2_remove(struct i2c_client *client)
> > > > +{
> > > > +       struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > > +
> > > > +       v4l2_async_unregister_subdev(&gc05a2->sd);
> > > > +       v4l2_subdev_cleanup(sd);
> > > > +       media_entity_cleanup(&gc05a2->sd.entity);
> > > > +       v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > > > +
> > > > +       pm_runtime_disable(&client->dev);
> > > > +       if (!pm_runtime_status_suspended(&client->dev))
> > > > +               gc05a2_power_off(gc05a2->dev);
> > > > +       pm_runtime_set_suspended(&client->dev);
> > > > +}
> > > > +
> > > > +static const struct of_device_id gc05a2_of_match[] = {
> > > > +       { .compatible = "galaxycore,gc05a2" },
> > > > +       {}
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, gc05a2_of_match);
> > > > +
> > > > +static DEFINE_RUNTIME_DEV_PM_OPS(gc05a2_pm_ops,
> > > > +                                gc05a2_power_off,
> > > > +                                gc05a2_power_on,
> > > > +                                NULL);
> > > > +
> > > > +static struct i2c_driver gc05a2_i2c_driver = {
> > > > +       .driver = {
> > > > +               .of_match_table = gc05a2_of_match,
> > > > +               .pm = pm_ptr(&gc05a2_pm_ops),
> > > > +               .name  = "gc05a2",
> > > > +       },
> > > > +       .probe = gc05a2_probe,
> > > > +       .remove = gc05a2_remove,
> > > > +};
> > > > +module_i2c_driver(gc05a2_i2c_driver);
> > > > +
> > > > +MODULE_DESCRIPTION("GalaxyCore gc05a2 Camera driver");
> > > > +MODULE_AUTHOR("Zhi Mao <zhi.mao@mediatek.com>");
> > > > +MODULE_LICENSE("GPL");
> > > > -- 
> > > > 2.25.1
> > > >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v7 1/3] KVM: selftests: aarch64: Add helper function for the vpmu vcpu creation
From: Shaoqin Huang @ 2024-04-09  3:03 UTC (permalink / raw)
  To: Oliver Upton, Marc Zyngier, kvmarm
  Cc: Shaoqin Huang, Eric Auger, James Morse, Suzuki K Poulose,
	Zenghui Yu, Paolo Bonzini, Shuah Khan, linux-kernel,
	linux-arm-kernel, kvm, linux-kselftest
In-Reply-To: <20240409030320.182591-1-shahuang@redhat.com>

Create a vcpu with vpmu would be a common requirement for the vpmu test,
so add the helper function for the vpmu vcpu creation. And use those
helper function in the vpmu_counter_access.c test.

Use this chance to delete the meaningless ASSERT about the pmuver,
because KVM does not advertise an IMP_DEF PMU to guests.

No functional changes intended.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
---
 .../kvm/aarch64/vpmu_counter_access.c         | 33 ++++---------------
 .../selftests/kvm/include/aarch64/vpmu.h      | 28 ++++++++++++++++
 2 files changed, 35 insertions(+), 26 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/vpmu.h

diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
index f2fb0e3f14bc..33358797e7ec 100644
--- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
+++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
@@ -16,6 +16,7 @@
 #include <processor.h>
 #include <test_util.h>
 #include <vgic.h>
+#include <vpmu.h>
 #include <perf/arm_pmuv3.h>
 #include <linux/bitfield.h>
 
@@ -410,18 +411,8 @@ static void guest_code(uint64_t expected_pmcr_n)
 /* Create a VM that has one vCPU with PMUv3 configured. */
 static void create_vpmu_vm(void *guest_code)
 {
-	struct kvm_vcpu_init init;
-	uint8_t pmuver, ec;
-	uint64_t dfr0, irq = 23;
-	struct kvm_device_attr irq_attr = {
-		.group = KVM_ARM_VCPU_PMU_V3_CTRL,
-		.attr = KVM_ARM_VCPU_PMU_V3_IRQ,
-		.addr = (uint64_t)&irq,
-	};
-	struct kvm_device_attr init_attr = {
-		.group = KVM_ARM_VCPU_PMU_V3_CTRL,
-		.attr = KVM_ARM_VCPU_PMU_V3_INIT,
-	};
+	uint8_t ec;
+	uint64_t irq = 23;
 
 	/* The test creates the vpmu_vm multiple times. Ensure a clean state */
 	memset(&vpmu_vm, 0, sizeof(vpmu_vm));
@@ -433,27 +424,17 @@ static void create_vpmu_vm(void *guest_code)
 					guest_sync_handler);
 	}
 
-	/* Create vCPU with PMUv3 */
-	vm_ioctl(vpmu_vm.vm, KVM_ARM_PREFERRED_TARGET, &init);
-	init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
-	vpmu_vm.vcpu = aarch64_vcpu_add(vpmu_vm.vm, 0, &init, guest_code);
+	vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
 	vcpu_init_descriptor_tables(vpmu_vm.vcpu);
+
 	vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
 					GICD_BASE_GPA, GICR_BASE_GPA);
 	__TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
 		       "Failed to create vgic-v3, skipping");
 
-	/* Make sure that PMUv3 support is indicated in the ID register */
-	vcpu_get_reg(vpmu_vm.vcpu,
-		     KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), &dfr0);
-	pmuver = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), dfr0);
-	TEST_ASSERT(pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF &&
-		    pmuver >= ID_AA64DFR0_EL1_PMUVer_IMP,
-		    "Unexpected PMUVER (0x%x) on the vCPU with PMUv3", pmuver);
-
 	/* Initialize vPMU */
-	vcpu_ioctl(vpmu_vm.vcpu, KVM_SET_DEVICE_ATTR, &irq_attr);
-	vcpu_ioctl(vpmu_vm.vcpu, KVM_SET_DEVICE_ATTR, &init_attr);
+	vpmu_set_irq(vpmu_vm.vcpu, irq);
+	vpmu_init(vpmu_vm.vcpu);
 }
 
 static void destroy_vpmu_vm(void)
diff --git a/tools/testing/selftests/kvm/include/aarch64/vpmu.h b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
new file mode 100644
index 000000000000..5ef6cb011e41
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <kvm_util.h>
+
+static inline struct kvm_vcpu *vm_vcpu_add_with_vpmu(struct kvm_vm *vm,
+						     uint32_t vcpu_id,
+						     void *guest_code)
+{
+	struct kvm_vcpu_init init;
+
+	/* Create vCPU with PMUv3 */
+	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
+	init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
+
+	return aarch64_vcpu_add(vm, 0, &init, guest_code);
+}
+
+static void vpmu_set_irq(struct kvm_vcpu *vcpu, int irq)
+{
+	kvm_device_attr_set(vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
+			    KVM_ARM_VCPU_PMU_V3_IRQ, &irq);
+}
+
+static void vpmu_init(struct kvm_vcpu *vcpu)
+{
+	kvm_device_attr_set(vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
+			    KVM_ARM_VCPU_PMU_V3_INIT, NULL);
+}
-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v7 0/3] KVM: selftests: aarch64: Introduce pmu_event_filter_test
From: Shaoqin Huang @ 2024-04-09  3:03 UTC (permalink / raw)
  To: Oliver Upton, Marc Zyngier, kvmarm
  Cc: Shaoqin Huang, James Morse, kvm, linux-arm-kernel, linux-kernel,
	linux-kselftest, Paolo Bonzini, Shuah Khan, Suzuki K Poulose,
	Zenghui Yu

The test is inspired by the pmu_event_filter_test which implemented by x86. On
the arm64 platform, there is the same ability to set the pmu_event_filter
through the KVM_ARM_VCPU_PMU_V3_FILTER attribute. So add the test for arm64.

The series first create the helper function which can be used
for the vpmu related tests. Then, it implement the test.

Changelog:
----------
v6->v7:
  - Rebased to v6.9-rc3.

v5->v6:
  - Rebased to v6.9-rc1.
  - Collect RB.
  - Add multiple filter test.

v4->v5:
  - Rebased to v6.8-rc6.
  - Refactor the helper function, make it fine-grained and easy to be used.
  - Namimg improvements.
  - Use the kvm_device_attr_set() helper.
  - Make the test descriptor array readable and clean.
  - Delete the patch which moves the pmu related helper to vpmu.h.
  - Remove the kvm_supports_pmu_event_filter() function since nobody will run
  this on a old kernel.

v3->v4:
  - Rebased to the v6.8-rc2.

v2->v3:
  - Check the pmceid in guest code instead of pmu event count since different
  hardware may have different event count result, check pmceid makes it stable
  on different platform.                        [Eric]
  - Some typo fixed and commit message improved.

v1->v2:
  - Improve the commit message.                 [Eric]
  - Fix the bug in [enable|disable]_counter.    [Raghavendra & Marc]
  - Add the check if kvm has attr KVM_ARM_VCPU_PMU_V3_FILTER.
  - Add if host pmu support the test event throught pmceid0.
  - Split the test_invalid_filter() to another patch. [Eric]

v1: https://lore.kernel.org/all/20231123063750.2176250-1-shahuang@redhat.com/
v2: https://lore.kernel.org/all/20231129072712.2667337-1-shahuang@redhat.com/
v3: https://lore.kernel.org/all/20240116060129.55473-1-shahuang@redhat.com/
v4: https://lore.kernel.org/all/20240202025659.5065-1-shahuang@redhat.com/
v5: https://lore.kernel.org/all/20240229065625.114207-1-shahuang@redhat.com/
v6: https://lore.kernel.org/all/20240326033706.117189-1-shahuang@redhat.com/

Shaoqin Huang (3):
  KVM: selftests: aarch64: Add helper function for the vpmu vcpu
    creation
  KVM: selftests: aarch64: Introduce pmu_event_filter_test
  KVM: selftests: aarch64: Add invalid filter test in
    pmu_event_filter_test

 tools/testing/selftests/kvm/Makefile          |   1 +
 .../kvm/aarch64/pmu_event_filter_test.c       | 336 ++++++++++++++++++
 .../kvm/aarch64/vpmu_counter_access.c         |  33 +-
 .../selftests/kvm/include/aarch64/vpmu.h      |  28 ++
 4 files changed, 372 insertions(+), 26 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/vpmu.h

-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v7 3/3] KVM: selftests: aarch64: Add invalid filter test in pmu_event_filter_test
From: Shaoqin Huang @ 2024-04-09  3:03 UTC (permalink / raw)
  To: Oliver Upton, Marc Zyngier, kvmarm
  Cc: Shaoqin Huang, Eric Auger, James Morse, Suzuki K Poulose,
	Zenghui Yu, Paolo Bonzini, Shuah Khan, linux-arm-kernel, kvm,
	linux-kselftest, linux-kernel
In-Reply-To: <20240409030320.182591-1-shahuang@redhat.com>

Add the invalid filter test which sets the filter beyond the event
space and sets the invalid action to double check if the
KVM_ARM_VCPU_PMU_V3_FILTER will return the expected error.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
---
 .../kvm/aarch64/pmu_event_filter_test.c       | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
index 972384e81067..1abcd9ab325e 100644
--- a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
+++ b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
@@ -8,6 +8,7 @@
  * This test checks if the guest only see the limited pmu event that userspace
  * sets, if the guest can use those events which user allow, and if the guest
  * can't use those events which user deny.
+ * It also checks that setting invalid filter ranges return the expected error.
  * This test runs only when KVM_CAP_ARM_PMU_V3, KVM_ARM_VCPU_PMU_V3_FILTER
  * is supported on the host.
  */
@@ -273,6 +274,41 @@ static void run_tests(void)
 		run_test(t);
 }
 
+static void test_invalid_filter(void)
+{
+	struct kvm_pmu_event_filter invalid;
+	int ret;
+
+	pr_info("Test: test_invalid_filter\n");
+
+	memset(&vpmu_vm, 0, sizeof(vpmu_vm));
+
+	vpmu_vm.vm = vm_create(1);
+	vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
+	vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
+					GICD_BASE_GPA, GICR_BASE_GPA);
+	__TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
+		       "Failed to create vgic-v3, skipping");
+
+	/* The max event number is (1 << 16), set a range largeer than it. */
+	invalid = __DEFINE_FILTER(BIT(15), BIT(15) + 1, 0);
+	ret = __kvm_device_attr_set(vpmu_vm.vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
+				    KVM_ARM_VCPU_PMU_V3_FILTER, &invalid);
+	TEST_ASSERT(ret && errno == EINVAL, "Set Invalid filter range "
+		    "ret = %d, errno = %d (expected ret = -1, errno = EINVAL)",
+		    ret, errno);
+
+	/* Set the Invalid action. */
+	invalid = __DEFINE_FILTER(0, 1, 3);
+	ret = __kvm_device_attr_set(vpmu_vm.vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
+				    KVM_ARM_VCPU_PMU_V3_FILTER, &invalid);
+	TEST_ASSERT(ret && errno == EINVAL, "Set Invalid filter action "
+		    "ret = %d, errno = %d (expected ret = -1, errno = EINVAL)",
+		    ret, errno);
+
+	destroy_vpmu_vm();
+}
+
 static bool kvm_pmu_support_events(void)
 {
 	create_vpmu_vm(guest_get_pmceid);
@@ -295,4 +331,6 @@ int main(void)
 	TEST_REQUIRE(kvm_pmu_support_events());
 
 	run_tests();
+
+	test_invalid_filter();
 }
-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v7 2/3] KVM: selftests: aarch64: Introduce pmu_event_filter_test
From: Shaoqin Huang @ 2024-04-09  3:03 UTC (permalink / raw)
  To: Oliver Upton, Marc Zyngier, kvmarm
  Cc: Shaoqin Huang, Paolo Bonzini, Shuah Khan, James Morse,
	Suzuki K Poulose, Zenghui Yu, linux-kernel, kvm, linux-kselftest,
	linux-arm-kernel
In-Reply-To: <20240409030320.182591-1-shahuang@redhat.com>

Introduce pmu_event_filter_test for arm64 platforms. The test configures
PMUv3 for a vCPU, and sets different pmu event filters for the vCPU, and
check if the guest can see those events which user allow and can't use
those events which use deny.

This test refactor the create_vpmu_vm() and make it a wrapper for
__create_vpmu_vm(), which allows some extra init code before
KVM_ARM_VCPU_PMU_V3_INIT.

And this test use the KVM_ARM_VCPU_PMU_V3_FILTER attribute to set the
pmu event filter in KVM. And choose to filter two common event
branches_retired and instructions_retired, and let the guest to check if
it see the right pmceid register.

Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../kvm/aarch64/pmu_event_filter_test.c       | 298 ++++++++++++++++++
 2 files changed, 299 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 741c7dc16afc..9745be534df3 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -151,6 +151,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/aarch32_id_regs
 TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
 TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
 TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
+TEST_GEN_PROGS_aarch64 += aarch64/pmu_event_filter_test
 TEST_GEN_PROGS_aarch64 += aarch64/psci_test
 TEST_GEN_PROGS_aarch64 += aarch64/set_id_regs
 TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
diff --git a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
new file mode 100644
index 000000000000..972384e81067
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
@@ -0,0 +1,298 @@
+
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * pmu_event_filter_test - Test user limit pmu event for guest.
+ *
+ * Copyright (c) 2023 Red Hat, Inc.
+ *
+ * This test checks if the guest only see the limited pmu event that userspace
+ * sets, if the guest can use those events which user allow, and if the guest
+ * can't use those events which user deny.
+ * This test runs only when KVM_CAP_ARM_PMU_V3, KVM_ARM_VCPU_PMU_V3_FILTER
+ * is supported on the host.
+ */
+#include <kvm_util.h>
+#include <processor.h>
+#include <vgic.h>
+#include <vpmu.h>
+#include <test_util.h>
+#include <perf/arm_pmuv3.h>
+
+struct pmu_common_event_ids {
+	uint64_t pmceid0;
+	uint64_t pmceid1;
+} max_pmce, expected_pmce;
+
+struct vpmu_vm {
+	struct kvm_vm *vm;
+	struct kvm_vcpu *vcpu;
+	int gic_fd;
+};
+
+static struct vpmu_vm vpmu_vm;
+
+#define FILTER_NR 10
+
+struct test_desc {
+	const char *name;
+	struct kvm_pmu_event_filter filter[FILTER_NR];
+};
+
+#define __DEFINE_FILTER(base, num, act)		\
+	((struct kvm_pmu_event_filter) {	\
+		.base_event	= base,		\
+		.nevents	= num,		\
+		.action		= act,		\
+	})
+
+#define DEFINE_FILTER(base, act) __DEFINE_FILTER(base, 1, act)
+
+static void guest_code(void)
+{
+	uint64_t pmceid0 = read_sysreg(pmceid0_el0);
+	uint64_t pmceid1 = read_sysreg(pmceid1_el0);
+
+	GUEST_ASSERT_EQ(expected_pmce.pmceid0, pmceid0);
+	GUEST_ASSERT_EQ(expected_pmce.pmceid1, pmceid1);
+
+	GUEST_DONE();
+}
+
+static void guest_get_pmceid(void)
+{
+	max_pmce.pmceid0 = read_sysreg(pmceid0_el0);
+	max_pmce.pmceid1 = read_sysreg(pmceid1_el0);
+
+	GUEST_DONE();
+}
+
+static void run_vcpu(struct kvm_vcpu *vcpu)
+{
+	struct ucall uc;
+
+	while (1) {
+		vcpu_run(vcpu);
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_DONE:
+			return;
+		case UCALL_ABORT:
+			REPORT_GUEST_ASSERT(uc);
+			break;
+		default:
+			TEST_FAIL("Unknown ucall %lu", uc.cmd);
+		}
+	}
+}
+
+static void set_pmce(struct pmu_common_event_ids *pmce, int action, int event)
+{
+	int base = 0;
+	uint64_t *pmceid = NULL;
+
+	if (event >= 0x4000) {
+		event -= 0x4000;
+		base = 32;
+	}
+
+	if (event >= 0 && event <= 0x1F) {
+		pmceid = &pmce->pmceid0;
+	} else if (event >= 0x20 && event <= 0x3F) {
+		event -= 0x20;
+		pmceid = &pmce->pmceid1;
+	} else {
+		return;
+	}
+
+	event += base;
+	if (action == KVM_PMU_EVENT_ALLOW)
+		*pmceid |= BIT(event);
+	else
+		*pmceid &= ~BIT(event);
+}
+
+static void prepare_expected_pmce(struct kvm_pmu_event_filter *filter)
+{
+	struct pmu_common_event_ids pmce_mask = { ~0, ~0 };
+	bool first_filter = true;
+	int i;
+
+	while (filter && filter->nevents != 0) {
+		if (first_filter) {
+			if (filter->action == KVM_PMU_EVENT_ALLOW)
+				memset(&pmce_mask, 0, sizeof(pmce_mask));
+			first_filter = false;
+		}
+
+		for (i = 0; i < filter->nevents; i++)
+			set_pmce(&pmce_mask, filter->action,
+				 filter->base_event + i);
+
+		filter++;
+	}
+
+	expected_pmce.pmceid0 = max_pmce.pmceid0 & pmce_mask.pmceid0;
+	expected_pmce.pmceid1 = max_pmce.pmceid1 & pmce_mask.pmceid1;
+}
+
+static void pmu_event_filter_init(struct kvm_pmu_event_filter *filter)
+{
+	while (filter && filter->nevents != 0) {
+		kvm_device_attr_set(vpmu_vm.vcpu->fd,
+				    KVM_ARM_VCPU_PMU_V3_CTRL,
+				    KVM_ARM_VCPU_PMU_V3_FILTER,
+				    filter);
+		filter++;
+	}
+}
+
+#define GICD_BASE_GPA	0x8000000ULL
+#define GICR_BASE_GPA	0x80A0000ULL
+
+/* Create a VM that has one vCPU with PMUv3 configured. */
+static void create_vpmu_vm_with_filter(void *guest_code,
+				       struct kvm_pmu_event_filter *filter)
+{
+	uint64_t irq = 23;
+
+	/* The test creates the vpmu_vm multiple times. Ensure a clean state */
+	memset(&vpmu_vm, 0, sizeof(vpmu_vm));
+
+	vpmu_vm.vm = vm_create(1);
+	vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
+	vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
+					GICD_BASE_GPA, GICR_BASE_GPA);
+	__TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
+		       "Failed to create vgic-v3, skipping");
+
+	pmu_event_filter_init(filter);
+
+	/* Initialize vPMU */
+	vpmu_set_irq(vpmu_vm.vcpu, irq);
+	vpmu_init(vpmu_vm.vcpu);
+}
+
+static void create_vpmu_vm(void *guest_code)
+{
+	create_vpmu_vm_with_filter(guest_code, NULL);
+}
+
+static void destroy_vpmu_vm(void)
+{
+	close(vpmu_vm.gic_fd);
+	kvm_vm_free(vpmu_vm.vm);
+}
+
+static void run_test(struct test_desc *t)
+{
+	pr_info("Test: %s\n", t->name);
+
+	create_vpmu_vm_with_filter(guest_code, t->filter);
+	prepare_expected_pmce(t->filter);
+	sync_global_to_guest(vpmu_vm.vm, expected_pmce);
+
+	run_vcpu(vpmu_vm.vcpu);
+
+	destroy_vpmu_vm();
+}
+
+static struct test_desc tests[] = {
+	{
+		.name = "without_filter",
+		.filter = {
+			{ 0 }
+		},
+	},
+	{
+		.name = "member_allow_filter",
+		.filter = {
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 0),
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 0),
+			{ 0 },
+		},
+	},
+	{
+		.name = "member_deny_filter",
+		.filter = {
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 1),
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 1),
+			{ 0 },
+		},
+	},
+	{
+		.name = "not_member_deny_filter",
+		.filter = {
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
+			{ 0 },
+		},
+	},
+	{
+		.name = "not_member_allow_filter",
+		.filter = {
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
+			{ 0 },
+		},
+	},
+	{
+		.name = "deny_chain_filter",
+		.filter = {
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CHAIN, 1),
+			{ 0 },
+		},
+	},
+	{
+		.name = "deny_cpu_cycles_filter",
+		.filter = {
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
+			{ 0 },
+		},
+	},
+	{
+		.name = "cancel_filter",
+		.filter = {
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 0),
+			DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
+		},
+	},
+	{
+		.name = "multiple_filter",
+		.filter = {
+			__DEFINE_FILTER(0x0, 0x10, 0),
+			__DEFINE_FILTER(0x6, 0x3, 1),
+		},
+	},
+	{ 0 }
+};
+
+static void run_tests(void)
+{
+	struct test_desc *t;
+
+	for (t = &tests[0]; t->name; t++)
+		run_test(t);
+}
+
+static bool kvm_pmu_support_events(void)
+{
+	create_vpmu_vm(guest_get_pmceid);
+
+	memset(&max_pmce, 0, sizeof(max_pmce));
+	sync_global_to_guest(vpmu_vm.vm, max_pmce);
+	run_vcpu(vpmu_vm.vcpu);
+	sync_global_from_guest(vpmu_vm.vm, max_pmce);
+	destroy_vpmu_vm();
+
+	return max_pmce.pmceid0 &
+	       (ARMV8_PMUV3_PERFCTR_BR_RETIRED |
+	       ARMV8_PMUV3_PERFCTR_INST_RETIRED |
+	       ARMV8_PMUV3_PERFCTR_CHAIN);
+}
+
+int main(void)
+{
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PMU_V3));
+	TEST_REQUIRE(kvm_pmu_support_events());
+
+	run_tests();
+}
-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] soc: fsl: qe: Add check for platform_driver_register
From: Chen Ni @ 2024-04-09  3:08 UTC (permalink / raw)
  To: qiang.zhao, leoyang.li, saravanak, fido_max
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Chen Ni

Return platform_driver_register() in order to transfer the error if
it fails.

Fixes: be7ecbd240b2 ("soc: fsl: qe: convert QE interrupt controller to platform_device")
Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
---
 drivers/soc/fsl/qe/qe_ic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index bbae3d39c7be..f17de6000ff2 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -481,7 +481,6 @@ static struct platform_driver qe_ic_driver =
 
 static int __init qe_ic_of_init(void)
 {
-	platform_driver_register(&qe_ic_driver);
-	return 0;
+	return platform_driver_register(&qe_ic_driver);
 }
 subsys_initcall(qe_ic_of_init);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 1/1] arm64: dts: rockchip: enable onboard spi flash for rock-3a
From: Chukun Pan @ 2024-04-09  3:28 UTC (permalink / raw)
  To: jonas
  Cc: amadeus, conor+dt, devicetree, heiko, krzk+dt, linux-arm-kernel,
	linux-kernel, linux-rockchip, robh
In-Reply-To: <fb57172e-2517-498c-9412-76f62929d24a@kwiboo.se>

> I think the on-board spi nor flash support 104/108/133 mhz
> so 50 mhz is wrong in any case.

Thanks, I tested 104MHz on linux and it works fine. As for
133 MHz, only MX25U12835FZNI-08G supports it. And I didn't
find it support 108MHz in the datasheet of MX25U12835F.

Thanks,
Chukun

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path
From: Chen-Yu Tsai @ 2024-04-09  3:39 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: chunkuang.hu, robh, krzysztof.kozlowski+dt, conor+dt, p.zabel,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	matthias.bgg, shawn.sung, yu-chang.lee, ck.hu, jitao.shi,
	devicetree, linux-kernel, dri-devel, linux-mediatek,
	linux-arm-kernel, kernel
In-Reply-To: <5e385c2e-6509-4347-96a5-4606b32d20ff@collabora.com>

On Mon, Apr 8, 2024 at 6:16 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 08/04/24 05:20, Chen-Yu Tsai ha scritto:
> > On Thu, Apr 4, 2024 at 4:16 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Document OF graph on MMSYS/VDOSYS: this supports up to three DDP paths
> >> per HW instance (so potentially up to six displays for multi-vdo SoCs).
> >>
> >> The MMSYS or VDOSYS is always the first component in the DDP pipeline,
> >> so it only supports an output port with multiple endpoints - where each
> >> endpoint defines the starting point for one of the (currently three)
> >> possible hardware paths.
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >> ---
> >>   .../bindings/arm/mediatek/mediatek,mmsys.yaml | 23 +++++++++++++++++++
> >>   1 file changed, 23 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> >> index b3c6888c1457..90758bb5bcb1 100644
> >> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> >> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> >> @@ -93,6 +93,29 @@ properties:
> >>     '#reset-cells':
> >>       const: 1
> >>
> >> +  port:
> >> +    $ref: /schemas/graph.yaml#/properties/port
> >> +    description:
> >> +      Output port node. This port connects the MMSYS/VDOSYS output to
> >> +      the first component of one display pipeline, for example one of
> >> +      the available OVL or RDMA blocks.
> >> +      Some MediaTek SoCs support up to three display outputs per MMSYS.
> >> +    properties:
> >> +      endpoint@0:
> >> +        $ref: /schemas/graph.yaml#/properties/endpoint
> >> +        description: Output to the primary display pipeline
> >> +
> >> +      endpoint@1:
> >> +        $ref: /schemas/graph.yaml#/properties/endpoint
> >> +        description: Output to the secondary display pipeline
> >> +
> >> +      endpoint@2:
> >> +        $ref: /schemas/graph.yaml#/properties/endpoint
> >> +        description: Output to the tertiary display pipeline
> >> +
> >> +      required:
> >> +        - endpoint@0
> >> +
> >
> > Technically the mmsys device serves as an glue layer for the display
> > pipeline, providing things like clock control and signal routing; the
> > device itself is not part of the pipeline, and probably shouldn't be
> > part of the graph?
> >
>
> That is (only) partially true: in the case of older SoCs, the MMSYS can only
> connect to a single first IP of the pipeline, but in the case of newer ones,
> and especially (but not limited to) MT8195 onwards having multiple instances
> of VDOSYS, that really becomes part of the pipeline.
>
> This is not because of the possible different first IP in the pipeline, but
> because of support for dual-interface (DSI and DP) that, in even newer SoCs,
> can be done with cross-mmsys (cross-vdosys, actually...) as some of those do
> have the two in different VDOs.
>
> So yes, this can be done without the graph in MMSYS *in this precise moment in
> time*, but we'll anyway end up adding it sooner than later - and I'm doing this
> right now, instead of later, because it's also simplifying the implementation
> so like that I'm "catching two birds with one stone" :-)

I see. Thanks for sorting it out. We had something similar on Allwinner
platforms but it was never as complex or flexible as this.

ChenYu

> Cheers,
> Angelo
>
> > ChenYu
> >
> >>   required:
> >>     - compatible
> >>     - reg
> >> --
> >> 2.44.0
> >>
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] soc: mediatek: mtk-socinfo: Correct the marketing name for MT8188GV
From: Jason-ch Chen @ 2024-04-09  3:53 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Matthias Brugger, AngeloGioacchino Del Regno
  Cc: Project_Global_Chrome_Upstream_Group, linux-watchdog,
	linux-arm-kernel, linux-kernel, linux-mediatek, Jason-ch Chen

From: Jason-ch Chen <Jason-ch.Chen@mediatek.com>

Change 'Kompanio 830' to 'Kompanio 838'.

Signed-off-by: Jason-ch Chen <Jason-ch.Chen@mediatek.com>
---
 drivers/soc/mediatek/mtk-socinfo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-socinfo.c b/drivers/soc/mediatek/mtk-socinfo.c
index 6943ab29f095..74672a9d6d13 100644
--- a/drivers/soc/mediatek/mtk-socinfo.c
+++ b/drivers/soc/mediatek/mtk-socinfo.c
@@ -48,8 +48,8 @@ static struct socinfo_data socinfo_data_table[] = {
 	MTK_SOCINFO_ENTRY("MT8183", "MT8183V/AZA", "Kompanio 500", 0x00010043, 0x00000940),
 	MTK_SOCINFO_ENTRY("MT8186", "MT8186GV/AZA", "Kompanio 520", 0x81861001, CELL_NOT_USED),
 	MTK_SOCINFO_ENTRY("MT8186T", "MT8186TV/AZA", "Kompanio 528", 0x81862001, CELL_NOT_USED),
-	MTK_SOCINFO_ENTRY("MT8188", "MT8188GV/AZA", "Kompanio 830", 0x81880000, 0x00000010),
-	MTK_SOCINFO_ENTRY("MT8188", "MT8188GV/HZA", "Kompanio 830", 0x81880000, 0x00000011),
+	MTK_SOCINFO_ENTRY("MT8188", "MT8188GV/AZA", "Kompanio 838", 0x81880000, 0x00000010),
+	MTK_SOCINFO_ENTRY("MT8188", "MT8188GV/HZA", "Kompanio 838", 0x81880000, 0x00000011),
 	MTK_SOCINFO_ENTRY("MT8192", "MT8192V/AZA", "Kompanio 820", 0x00001100, 0x00040080),
 	MTK_SOCINFO_ENTRY("MT8192T", "MT8192V/ATZA", "Kompanio 828", 0x00000100, 0x000400C0),
 	MTK_SOCINFO_ENTRY("MT8195", "MT8195GV/EZA", "Kompanio 1200", 0x81950300, CELL_NOT_USED),
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [WIP 0/3] Memory model and atomic API in Rust
From: Paul E. McKenney @ 2024-04-09  4:47 UTC (permalink / raw)
  To: Kent Overstreet
  Cc: Matthew Wilcox, Linus Torvalds, Philipp Stanner, Boqun Feng,
	rust-for-linux, linux-kernel, linux-arch, llvm, Miguel Ojeda,
	Alex Gaynor, Wedson Almeida Filho, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Alan Stern,
	Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
	David Howells, Jade Alglave, Luc Maranget, Akira Yokosawa,
	Daniel Lustig, Joel Fernandes, Nathan Chancellor,
	Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
	Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Catalin Marinas,
	linux-arm-kernel, linux-fsdevel
In-Reply-To: <bmbsx3zfgedqo5ef6yzzvpnwx2ukhzhm33ovb6zyhq4g6vutnn@b7qlnf2pyxvj>

On Mon, Apr 08, 2024 at 08:58:18PM -0400, Kent Overstreet wrote:
> On Mon, Apr 08, 2024 at 06:03:11PM +0100, Matthew Wilcox wrote:
> > On Mon, Apr 08, 2024 at 09:55:23AM -0700, Paul E. McKenney wrote:
> > > On Mon, Apr 08, 2024 at 05:02:37PM +0100, Matthew Wilcox wrote:
> > > > In my ideal world, the compiler would turn this into:
> > > > 
> > > > 	newfolio->flags |= folio->flags & MIGRATE_MASK;
> > > 
> > > Why not accumulate the changes in a mask, and then apply the mask the
> > > one time?  (In situations where __folio_set_foo() need not apply.)
> > 
> > But it irks me that we can't tell the compiler this is a safe
> > transformation for it to make. There are a number of places where
> > similar things happen.
> 
> Same thing comes up with bignum code - you really want to be able to
> tell the compiler "you can apply x/y/z optimizations for these
> functions", e.g. replace add(mul(a, b), c) with fma(a, b, c).
> 
> Compiler optimizations are just algebraic transformations, we just need
> a way to tell the compiler what the algebraic properties of our
> functions are.

That might indeed be more straightforward than doing this on a per-type
basis.  But the C++ guys would likely just start shouting "template
metaprogramming!!!"  ;-)

							Thanx, Paul

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v1 2/2] of: dynamic: Fix probing of overlay devices
From: Saravana Kannan @ 2024-04-09  5:13 UTC (permalink / raw)
  To: Herve Codina, Geert Uytterhoeven
  Cc: Rob Herring, kernel-team, linux-kernel, imx, linux-arm-kernel,
	linux-i2c, devicetree, linux-spi, linux-acpi
In-Reply-To: <CAGETcx8gGcq01CwTZyn=Q0fSQkzMf6tshSmPr5YWYZuhhJVdXg@mail.gmail.com>

On Mon, Apr 8, 2024 at 6:40 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Mon, Apr 8, 2024 at 4:13 PM Saravana Kannan <saravanak@google.com> wrote:
> >
> > Get fw_devlink to work well with overlay devices.
> >
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> >  drivers/base/core.c    | 10 ++++++++++
> >  drivers/of/dynamic.c   |  8 ++++++++
> >  include/linux/fwnode.h |  2 ++
> >  3 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 5f4e03336e68..d856f9c5d601 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -237,6 +237,16 @@ static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
> >                 __fw_devlink_pickup_dangling_consumers(child, new_sup);
> >  }
> >
> > +
> > +void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
> > +                                                struct fwnode_handle *parent)
> > +{
> > +       mutex_lock(&fwnode_link_lock);
> > +       __fw_devlink_pickup_dangling_consumers(child, parent);
> > +       __fw_devlink_link_to_consumers(parent->dev);
> > +       mutex_unlock(&fwnode_link_lock);
> > +}
> > +
> >  static DEFINE_MUTEX(device_links_lock);
> >  DEFINE_STATIC_SRCU(device_links_srcu);
> >
> > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> > index 19a1a38554f2..0a936f46820e 100644
> > --- a/drivers/of/dynamic.c
> > +++ b/drivers/of/dynamic.c
> > @@ -237,6 +237,7 @@ static void __of_attach_node(struct device_node *np)
> >  int of_attach_node(struct device_node *np)
> >  {
> >         struct of_reconfig_data rd;
> > +       struct fwnode_handle *fwnode, *parent;
> >
> >         memset(&rd, 0, sizeof(rd));
> >         rd.dn = np;
> > @@ -246,6 +247,13 @@ int of_attach_node(struct device_node *np)
> >         mutex_unlock(&of_mutex);
> >
> >         of_reconfig_notify(OF_RECONFIG_ATTACH_NODE, &rd);
> > +       fwnode = of_fwnode_handle(np);
> > +       fwnode_for_each_parent_node(fwnode, parent)
> > +               if (parent->dev) {
> > +                       fw_devlink_pickup_dangling_consumers(fwnode, parent);
> > +                       fwnode_handle_put(parent);
> > +                       break;
> > +               }
>
> I'm clearly calling this in the wrong location. Please move this logic
> to __of_changeset_entry_notify() and for the case
> OF_RECONFIG_ATTACH_NODE. Haven't fully thought through the DETACH
> case, but it should work correctly for that case too. If not, I'll
> take care of that next.
>

I'll send out a RFC v2 with the code fixed up in a few minutes.

> -Saravana
>
> >
> >         return 0;
> >  }
> > diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> > index 0d79070c5a70..4b3f697a90e8 100644
> > --- a/include/linux/fwnode.h
> > +++ b/include/linux/fwnode.h
> > @@ -220,6 +220,8 @@ int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,
> >                     u8 flags);
> >  void fwnode_links_purge(struct fwnode_handle *fwnode);
> >  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
> > +void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
> > +                                         struct fwnode_handle *parent);
> >  bool fw_devlink_is_strict(void);
> >
> >  #endif
> > --
> > 2.44.0.478.gd926399ef9-goog
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 11/25] media: i2c: imx258: Add get_selection for pixel array information
From: Luis Garcia @ 2024-04-09  5:23 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
	linux-kernel, phone-devel
In-Reply-To: <Zg2kHYc6kdiZEAFD@duo.ucw.cz>

On 4/3/24 12:46, Pavel Machek wrote:
> Hi!
> 
>> Libcamera requires the cropping information for each mode, so
>> add this information to the driver.
> 
>> @@ -116,6 +124,9 @@ struct imx258_mode {
>>  	u32 link_freq_index;
>>  	/* Default register values */
>>  	struct imx258_reg_list reg_list;
>> +
>> +	/* Analog crop rectangle. */
> 
> No need for "." at the end, as it is not above.
> 
>> +	struct v4l2_rect crop;
>>  };
> 
> If the crop is same in all modes, should we have it in common place?
> 
> Best regards,
> 								Pavel

I gave this a try similar to what was done on imx219 but its having
issues and makes a bunch of changes to the following patches so we
might need to hold off on that change for now.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"
From: Alexey Dobriyan @ 2024-04-09  5:32 UTC (permalink / raw)
  To: Chaitanya S Prakash
  Cc: linux-perf-users, anshuman.khandual, james.clark, Josh Poimboeuf,
	Peter Zijlstra, Suzuki K Poulose, Mike Leach, John Garry,
	Will Deacon, Leo Yan, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Chenyuan Mi, Masami Hiramatsu,
	Ravi Bangoria, Ahelenia Ziemiańska, Colin Ian King,
	Changbin Du, Kan Liang, Athira Rajeev, Tiezhu Yang,
	Georg Müller, Liam Howlett, bpf, coresight, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20240408062230.1949882-1-ChaitanyaS.Prakash@arm.com>

On Mon, Apr 08, 2024 at 11:52:22AM +0530, Chaitanya S Prakash wrote:
> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
> - Delete ends_with() and replace its usage with str_has_suffix()

> - Delete strstarts() from tools/include/linux/string.h and replace its
>   usage with str_has_prefix()

It should be the other way: starts_with is normal in userspace.
C++, Python, Java, C# all have it. JavaScript too!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [RFC PATCH v2 0/2] fw_devlink overlay fix
From: Saravana Kannan @ 2024-04-09  5:37 UTC (permalink / raw)
  To: Herve Codina, Geert Uytterhoeven
  Cc: Saravana Kannan, Rob Herring, kernel-team, linux-kernel, imx,
	linux-arm-kernel, linux-i2c, devicetree, linux-spi, linux-acpi

Don't bother reviewing this patch. It needs to be tested and possibly
refactored first.

Geert and Herve,

This patch serious should hopefully fix both of your use cases
[1][2][3]. Can you please check to make sure the device links created
to/from the overlay devices are to/from the right ones?

I've only compile tested it. If I made some obvious mistake, feel free
to fix it and give it a shot.

Cc: Rob Herring <robh@kernel.org>

[1] - https://lore.kernel.org/lkml/CAMuHMdXEnSD4rRJ-o90x4OprUacN_rJgyo8x6=9F9rZ+-KzjOg@mail.gmail.com/
[2] - https://lore.kernel.org/all/20240221095137.616d2aaa@bootlin.com/
[3] - https://lore.kernel.org/lkml/20240312151835.29ef62a0@bootlin.com/

Thanks,
Saravana


Saravana Kannan (2):
  Revert "treewide: Fix probing of devices in DT overlays"
  of: dynamic: Fix probing of overlay devices

 drivers/base/core.c       | 23 +++++++++++++++++++++++
 drivers/bus/imx-weim.c    |  6 ------
 drivers/i2c/i2c-core-of.c |  5 -----
 drivers/of/dynamic.c      |  2 +-
 drivers/of/platform.c     |  5 -----
 drivers/spi/spi.c         |  5 -----
 include/linux/fwnode.h    |  1 +
 7 files changed, 25 insertions(+), 22 deletions(-)

-- 
2.44.0.478.gd926399ef9-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [RFC PATCH v2 2/2] of: dynamic: Fix probing of overlay devices
From: Saravana Kannan @ 2024-04-09  5:37 UTC (permalink / raw)
  To: Herve Codina, Geert Uytterhoeven
  Cc: Saravana Kannan, Rob Herring, kernel-team, linux-kernel, imx,
	linux-arm-kernel, linux-i2c, devicetree, linux-spi, linux-acpi
In-Reply-To: <20240409053704.428336-1-saravanak@google.com>

Get fw_devlink to work well with overlay devices.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/base/core.c    | 23 +++++++++++++++++++++++
 drivers/of/dynamic.c   |  1 +
 include/linux/fwnode.h |  1 +
 3 files changed, 25 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5f4e03336e68..5ce33dbd7560 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -237,6 +237,29 @@ static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
 		__fw_devlink_pickup_dangling_consumers(child, new_sup);
 }
 
+
+void fw_devlink_fixup_new_fwnode(struct fwnode_handle *new_fwnode)
+{
+	struct fwnode_handle *parent;
+
+	if (new_fwnode->dev && new_fwnode->dev->bus)
+		return;
+
+	fwnode_for_each_parent_node(new_fwnode, parent)
+		if (parent->dev && parent->dev->bus)
+			break;
+
+	if (!parent)
+		return;
+
+	mutex_lock(&fwnode_link_lock);
+	__fw_devlink_pickup_dangling_consumers(new_fwnode, parent);
+	__fw_devlink_link_to_consumers(parent->dev);
+	mutex_unlock(&fwnode_link_lock);
+
+	fwnode_handle_put(parent);
+}
+
 static DEFINE_MUTEX(device_links_lock);
 DEFINE_STATIC_SRCU(device_links_srcu);
 
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 19a1a38554f2..a2ad79ea9a06 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -581,6 +581,7 @@ static int __of_changeset_entry_notify(struct of_changeset_entry *ce,
 		memset(&rd, 0, sizeof(rd));
 		rd.dn = ce->np;
 		ret = of_reconfig_notify(ce->action, &rd);
+		fw_devlink_fixup_new_fwnode(of_fwnode_handle(ce->np));
 		break;
 	case OF_RECONFIG_ADD_PROPERTY:
 	case OF_RECONFIG_REMOVE_PROPERTY:
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 0d79070c5a70..230aeccef6f3 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -220,6 +220,7 @@ int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,
 		    u8 flags);
 void fwnode_links_purge(struct fwnode_handle *fwnode);
 void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
+void fw_devlink_fixup_new_fwnode(struct fwnode_handle *new_fwnode);
 bool fw_devlink_is_strict(void);
 
 #endif
-- 
2.44.0.478.gd926399ef9-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [RFC PATCH v2 1/2] Revert "treewide: Fix probing of devices in DT overlays"
From: Saravana Kannan @ 2024-04-09  5:37 UTC (permalink / raw)
  To: Herve Codina, Geert Uytterhoeven
  Cc: Saravana Kannan, Rob Herring, kernel-team, linux-kernel, imx,
	linux-arm-kernel, linux-i2c, devicetree, linux-spi, linux-acpi
In-Reply-To: <20240409053704.428336-1-saravanak@google.com>

This reverts commit 1a50d9403fb90cbe4dea0ec9fd0351d2ecbd8924.
---
 drivers/bus/imx-weim.c    | 6 ------
 drivers/i2c/i2c-core-of.c | 5 -----
 drivers/of/dynamic.c      | 1 -
 drivers/of/platform.c     | 5 -----
 drivers/spi/spi.c         | 5 -----
 5 files changed, 22 deletions(-)

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 837bf9d51c6e..caaf887e0ccc 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -331,12 +331,6 @@ static int of_weim_notify(struct notifier_block *nb, unsigned long action,
 				 "Failed to setup timing for '%pOF'\n", rd->dn);
 
 		if (!of_node_check_flag(rd->dn, OF_POPULATED)) {
-			/*
-			 * Clear the flag before adding the device so that
-			 * fw_devlink doesn't skip adding consumers to this
-			 * device.
-			 */
-			rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 			if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) {
 				dev_err(&pdev->dev,
 					"Failed to create child device '%pOF'\n",
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index a6c407d36800..a250921bbce0 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -178,11 +178,6 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
 			return NOTIFY_OK;
 		}
 
-		/*
-		 * Clear the flag before adding the device so that fw_devlink
-		 * doesn't skip adding consumers to this device.
-		 */
-		rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 		client = of_i2c_register_device(adap, rd->dn);
 		if (IS_ERR(client)) {
 			dev_err(&adap->dev, "failed to create client for '%pOF'\n",
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 4d57a4e34105..19a1a38554f2 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -224,7 +224,6 @@ static void __of_attach_node(struct device_node *np)
 	np->sibling = np->parent->child;
 	np->parent->child = np;
 	of_node_clear_flag(np, OF_DETACHED);
-	np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;
 
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 389d4ea6bfc1..efd861fa254f 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -743,11 +743,6 @@ static int of_platform_notify(struct notifier_block *nb,
 		if (of_node_check_flag(rd->dn, OF_POPULATED))
 			return NOTIFY_OK;
 
-		/*
-		 * Clear the flag before adding the device so that fw_devlink
-		 * doesn't skip adding consumers to this device.
-		 */
-		rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 		/* pdev_parent may be NULL when no bus platform device */
 		pdev_parent = of_find_device_by_node(rd->dn->parent);
 		pdev = of_platform_device_create(rd->dn, NULL,
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ff75838c1b5d..17cd417f7681 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -4761,11 +4761,6 @@ static int of_spi_notify(struct notifier_block *nb, unsigned long action,
 			return NOTIFY_OK;
 		}
 
-		/*
-		 * Clear the flag before adding the device so that fw_devlink
-		 * doesn't skip adding consumers to this device.
-		 */
-		rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 		spi = of_register_spi_device(ctlr, rd->dn);
 		put_device(&ctlr->dev);
 
-- 
2.44.0.478.gd926399ef9-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox