Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] test: mm/arm: pgtable: remove young bit check for pte_valid_user
From: Brian Ruley @ 2026-04-10 11:43 UTC (permalink / raw)
  To: Russell King
  Cc: Russell King, Will Deacon, Steve Capper, linux-arm-kernel,
	linux-kernel, Brian Ruley
In-Reply-To: <adjcc7QvquXI3G0Q@shell.armlinux.org.uk>

Instrumentation to print recently migrated pages in undefined
instruction handler. This was used to determine if the faulting address
was migrated earlier.

Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
---
Not intended for integration. This is just to share the testing details.
---
 arch/arm/kernel/traps.c |  5 ++++
 mm/migrate.c            | 53 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index afbd2ebe5c39..64ef872f1555 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -28,6 +28,8 @@
 #include <linux/irq.h>
 #include <linux/vmalloc.h>
 
+void migrate_exec_log_dump(unsigned long fault_addr);
+
 #include <linux/atomic.h>
 #include <asm/cacheflush.h>
 #include <asm/exception.h>
@@ -490,6 +492,9 @@ asmlinkage void do_undefinstr(struct pt_regs *regs)
 		dump_instr(KERN_INFO, regs);
 	}
 #endif
+	if (user_mode(regs))
+		migrate_exec_log_dump((unsigned long)pc);
+
 	arm_notify_die("Oops - undefined instruction", regs,
 		       SIGILL, ILL_ILLOPC, pc, 0, 6);
 }
diff --git a/mm/migrate.c b/mm/migrate.c
index 2c3d489ecf51..987d0376b433 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -48,6 +48,54 @@
 
 #include <trace/events/migrate.h>
 
+
+/* Debug: track recent exec page migrations */
+#define MIGRATE_EXEC_LOG_SIZE 8
+struct migrate_exec_entry {
+	unsigned long addr;
+	unsigned long old_pfn;
+	unsigned long new_pfn;
+	unsigned int pid;
+	bool flushed;
+};
+static DEFINE_PER_CPU(struct migrate_exec_entry[MIGRATE_EXEC_LOG_SIZE], migrate_exec_log);
+static DEFINE_PER_CPU(unsigned int, migrate_exec_idx);
+
+void migrate_exec_log_add(unsigned long addr, unsigned long old_pfn,
+			  unsigned long new_pfn, bool flushed)
+{
+	unsigned int idx = __this_cpu_read(migrate_exec_idx);
+	struct migrate_exec_entry *log = this_cpu_ptr(migrate_exec_log);
+	struct migrate_exec_entry *e = &log[idx];
+
+	e->addr = addr;
+	e->old_pfn = old_pfn;
+	e->new_pfn = new_pfn;
+	e->pid = current->pid;
+	e->flushed = flushed;
+	__this_cpu_write(migrate_exec_idx, (idx + 1) % MIGRATE_EXEC_LOG_SIZE);
+}
+
+void migrate_exec_log_dump(unsigned long fault_addr)
+{
+	int cpu;
+
+	pr_err("SIGILL at %lx pid %d, recent exec migrations:\n",
+	       fault_addr, current->pid);
+	for_each_online_cpu(cpu) {
+		struct migrate_exec_entry *log = per_cpu(migrate_exec_log, cpu);
+		int i;
+		for (i = 0; i < MIGRATE_EXEC_LOG_SIZE; i++) {
+			if (log[i].addr == 0)
+				continue;
+			pr_err("  cpu%d: addr=%lx old_pfn=%lx new_pfn=%lx pid=%d flushed=%d%s\n",
+			       cpu, log[i].addr, log[i].old_pfn, log[i].new_pfn,
+			       log[i].pid, log[i].flushed,
+			       (PAGE_ALIGN(fault_addr) == PAGE_ALIGN(log[i].addr)) ?
+			       " *** MATCH ***" : "");
+		}
+	}
+}
 #include "internal.h"
 #include "swap.h"
 
@@ -434,6 +482,11 @@ static bool remove_migration_pte(struct folio *folio,
 			else
 				folio_add_file_rmap_pte(folio, new, vma);
 			set_pte_at(vma->vm_mm, pvmw.address, pvmw.pte, pte);
+
+			if (vma->vm_flags & VM_EXEC)
+				migrate_exec_log_add(pvmw.address,
+					swp_offset(entry), page_to_pfn(new),
+					pte_young(pte));
 		}
 		if (READ_ONCE(vma->vm_flags) & VM_LOCKED)
 			mlock_drain_local();
-- 
2.47.3



^ permalink raw reply related

* Re: [PATCH net-next v3 04/12] net: airoha: Rely on net_device pointer in HTB callbacks
From: Benjamin Larsson @ 2026-04-10 11:29 UTC (permalink / raw)
  To: Jakub Kicinski, lorenzo
  Cc: andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
	ansuelsmth, linux-arm-kernel, linux-mediatek, netdev, devicetree
In-Reply-To: <20260410025722.1809973-1-kuba@kernel.org>

On 10/04/2026 04:57, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email says "For your consideration."
> ---
> net: airoha: Introduce WAN device flag
> 
> This commit introduces a WAN flag to specify whether a device is used to
> transmit or receive WAN or LAN traffic. It enables specifying multiple LAN
> devices but limits the configuration to a single WAN device.
> 

[...]

>> +		fallthrough;
>> +	}
>> +	case AIROHA_GDM2_IDX:
>> +		/* GDM2 is always used as wan */
>> +		dev->flags |= PRIV_FLAG_WAN;
>>   		break;
>>   	default:
>>   		break;
>>   	}
>>   
>> -	for (i = 0; i < eth->soc->num_ppe; i++)
>> -		airoha_ppe_set_cpu_port(dev, i, airoha_get_fe_port(dev));
>> +	airoha_dev_set_qdma(dev);
>> +	airoha_set_macaddr(dev, netdev->dev_addr);
> 
> Since the commit message notes it is possible to specify multiple LAN
> devices, does unconditionally calling airoha_set_macaddr() for each
> device overwrite the single shared hardware LAN MAC register?
> 
> If so, could this cause only the last initialized LAN device to have its MAC
> correctly programmed into the hardware, leading to dropped unicast
> packets for the other LAN interfaces?
> 
>> +
>> +	if (!airoha_is_lan_gdm_dev(dev) &&
>> +	    (port->id == AIROHA_GDM3_IDX || port->id == AIROHA_GDM4_IDX)) {
>> +		int err;
> 
> [ ... ]

Hi, the frame engine can be configured with a range of wan mac addresses 
and a range of lan mac addresses via registers that set the top 24 bits 
and then 2 registers that form a range of the low 24 bits.

Documentation says that packets that fall into this mac address range 
either on the lan side or the wan side are to be treated as layer 3 
packages and if a packet is not then it will be handled as a layer 2 packet.

The exact implication of this and if it actually matters is unknown. But 
traffic that comes in on an interface that is not matched by an 
acceleration flow is usually forwarded to the cpu for further processing.

MvH
Benjamin Larsson


^ permalink raw reply

* Aw: Re: [PATCH] pinctrl: mediatek: moore: implement gpio_chip::get_direction()
From: Frank Wunderlich @ 2026-04-10 11:23 UTC (permalink / raw)
  To: brgl
  Cc: bartosz.golaszewski, linux, sean.wang, linusw, matthias.bgg,
	angelogioacchino.delregno, linux-mediatek, linux-gpio,
	linux-kernel, linux-arm-kernel
In-Reply-To: <CAMRc=Md1pC_a8zSQqWWcubNG-1ret8Lf9sajVDnU8nw2gnXZiA@mail.gmail.com>


> > > Reported-by: Frank Wunderlich <linux@fw-web.de>
> >
> > please use the email i used for SoB in my linked patch (closes link below), the other email i use only for sending patches due to mail provider limitation.
> >
> 
> Linus: Can you fix this when applying, please?
> 
> Frank: Can you also leave your Tested-by under the patch?

if my testcase is enough (just accessing /sys/kernel/debug/gpio via cat)

Tested-By: Frank Wunderlich <frank-w@public-files.de>

> Thanks,
> Bartosz


^ permalink raw reply

* Re: [PATCH] mm/arm: pgtable: remove young bit check for pte_valid_user
From: Russell King (Oracle) @ 2026-04-10 11:18 UTC (permalink / raw)
  To: Brian Ruley; +Cc: Will Deacon, Steve Capper, linux-arm-kernel, linux-kernel
In-Reply-To: <adjYlUk8_JjPivNi@zoo11.fihel.lab.ge-healthcare.net>

On Fri, Apr 10, 2026 at 02:01:41PM +0300, Brian Ruley wrote:
> Thank you for the clarification, this is very educational for me.
> I understand your scepticism, and I can't explain what's going on based
> on what you replied. However, I do honestly believe there is a problem
> here. I'll share the exact testing details and the instrumentation
> we added that convinced us to reach out at the end. One idea we also
> had was that could cache aliasing be happening here.
> 
> To clarify any potential misunderstanding, we've observed the
> following:
> 
> - Sporadic SIGILL and SIGSEGV under memory pressure
> - Scales with core count, i.e., quad core more likely to reproduce
>   than dual core. We haven't observed an issue on single core.
> - Coredumps show valid instructions at the faulting PC.
>   The CPU executed something different from what's in memory.
>   This pointed us to stale I-cache.
> - Instrumentation indicates a correlation.
>   A per-CPU ring buffer tracking exec page migrations was dumped on
>   SIGILL. The faulting PC matched a recently migrated pages.
> - We started seeing this after upgrade 6.1->6.12->6.18. We bisected
>   two commits which had an impact, but we weren't convinced that
>   either was the root cause: 5dfab109d5193e6c224d96cabf90e9cc2c039884
>   and 6faea3422e3b4e8de44a55aa3e6e843320da66d2.
> - Failed processes include systemd, tar, bash, ...
> - Debug options, e.g., page poisoning, seems to hide the bug
> 
> 
> > So you're saying that stress-ng doesn't reproduce this bug but
> triggers the OOM-killer... confused.
> 
> Apologies for the confusion. I meant that with `stress-ng' we created
> the memory pressure and OOM might have played a role in exposing the
> "bug" as we (at the time) believed that anything that would trigger
> memory free/reclaims and page migration was the key. One note I'll add
> is that in our test we invoked stress-ng for 2 minutes (--timeout 2m)
> and after each we would reboot the device. We had observed that reboots
> seemed to have a discernible effect on the occurence in earlier testing
> so we kept that in. I'm beginning to doubt if it had an effect now,
> and unfortunately it's all anecdotal.
> 
> One more thing, even if you don't accept the patch, is this patch
> harmful in any way or is it just sub-optimal?
> 
> I'll send the instrumentation patch as a follow-up, migh be there's a
> flaw in it.

I'll try it - I have Cortex A9 systems (some which I rely on...)

Please can you also try to track the history of what happens for
the PTEs corresponding to the old and new PFN?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply

* Re: [RFC PATCH] arm64: mm: support set_memory_encrypted/decrypted for vmalloc addresses
From: Catalin Marinas @ 2026-04-10 11:06 UTC (permalink / raw)
  To: Kameron Carr
  Cc: will, suzuki.poulose, steven.price, ryan.roberts, dev.jain, yang,
	shijie, kevin.brodsky, linux-arm-kernel, linux-kernel
In-Reply-To: <20260406213317.216171-1-kameroncarr@linux.microsoft.com>

On Mon, Apr 06, 2026 at 02:33:17PM -0700, Kameron Carr wrote:
> Currently __set_memory_enc_dec() only handles linear map (lm) addresses
> and returns -EINVAL for anything else. This means callers using
> vmalloc'd buffers cannot mark memory as shared/protected with the RMM
> via set_memory_decrypted()/set_memory_encrypted().
> 
> Extend the implementation to handle vmalloc (non-linear-map) addresses
> by introducing __set_va_addr_enc_dec(). For vmalloc addresses, the page
> table entries are not contiguous in the physical address space, so the
> function walks the vm_area's pages array and issues per-page RSI calls
> to transition each page between shared and protected states.
> 
> The original linear-map path is factored out into __set_lm_addr_enc_dec(),
> and __set_memory_enc_dec() now dispatches to the appropriate helper based
> on whether the address is a linear map address.

Could you give more details about the user of set_memory_decrypted() on
vmalloc()'ed addresses? I think this came up in the past and I wondered
whether something like GFP_DECRYPTED would be simpler to implement (even
posted a hack but without vmalloc() support). If it is known upfront
that the memory will be decrypted, it's easier/cheaper to do this on the
page allocation time to change the linear map and just use
pgprot_decrypted() for vmap(). No need to rewrite the page table after
mapping the pages.

-- 
Catalin


^ permalink raw reply

* [PATCH] perf arm-spe: Don't warn about the discard bit if it doesn't exist
From: James Clark @ 2026-04-10 11:05 UTC (permalink / raw)
  To: John Garry, Will Deacon, Mike Leach, Leo Yan, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter
  Cc: linux-arm-kernel, linux-perf-users, linux-kernel, James Clark

Opening an SPE event shows a warning that doesn't concern the user:

  $ perf record -e arm_spe
  Unknown/empty format name: discard

Perf only wants to know if the discard bit is set for configuring the
event, not in response to anything the user has done. Fix it by adding
another helper that returns if a config bit exists without warning.

We should probably keep the warning in evsel__get_config_val() to avoid
having every caller having to do it, and most format bits should never
be missing.

Add a test for the new helper. Rename the parent test function to be
more generic rather than adding a new one as it requires a lot of
boilerplate.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/arch/arm64/util/arm-spe.c |  3 ++-
 tools/perf/tests/pmu.c               | 11 +++++++++--
 tools/perf/util/evsel.c              |  6 ++++++
 tools/perf/util/evsel.h              |  1 +
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index f00d72d087fc..91bb28cad79a 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -428,7 +428,8 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 	evlist__for_each_entry_safe(evlist, tmp, evsel) {
 		if (evsel__is_aux_event(evsel)) {
 			arm_spe_setup_evsel(evsel, cpus);
-			if (!evsel__get_config_val(evsel, "discard", &discard_bit))
+			if (evsel__config_exists(evsel, "discard") &&
+			    !evsel__get_config_val(evsel, "discard", &discard_bit))
 				discard = !!discard_bit;
 		}
 	}
diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index 0ebf2d7b2cb4..d7be9d1c6f52 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -201,7 +201,8 @@ static int test__pmu_format(struct test_suite *test __maybe_unused, int subtest
 	return ret;
 }
 
-static int test__pmu_usr_chgs(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
+static int test__pmu_config_helpers(struct test_suite *test __maybe_unused,
+				    int subtest __maybe_unused)
 {
 	const char *event = "perf-pmu-test/config=15,config1=4,krava02=170,"
 			    "krava03=1,krava11=27,krava12=1/";
@@ -236,6 +237,12 @@ static int test__pmu_usr_chgs(struct test_suite *test __maybe_unused, int subtes
 	}
 	evsel = evlist__first(evlist);
 
+	/* Test evsel__config_exists() */
+	TEST_ASSERT_EQUAL("krava01 should exist",
+			  evsel__config_exists(evsel, "krava01"), true);
+	TEST_ASSERT_EQUAL("krava99 should not exist",
+			  evsel__config_exists(evsel, "krava99"), false);
+
 	/*
 	 * Set via config=15, krava01 bits 0-1
 	 * Set via config1=4, krava11 bit 1
@@ -629,7 +636,7 @@ static struct test_case tests__pmu[] = {
 	TEST_CASE("PMU name combining", name_len),
 	TEST_CASE("PMU name comparison", name_cmp),
 	TEST_CASE("PMU cmdline match", pmu_match),
-	TEST_CASE("PMU user config changes", pmu_usr_chgs),
+	TEST_CASE("PMU config helpers", pmu_config_helpers),
 	{	.name = NULL, }
 };
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 2ee87fd84d3e..685c4118aef3 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1396,6 +1396,12 @@ void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
 	perf_pmu__format_pack(format->bits, val, vp, /*zero=*/true);
 }
 
+bool evsel__config_exists(const struct evsel *evsel, const char *config_name)
+{
+	struct perf_pmu_format *format = pmu_find_format(&evsel->pmu->format, config_name);
+
+	return format && !bitmap_empty(format->bits, PERF_PMU_FORMAT_BITS);
+}
 
 int evsel__get_config_val(const struct evsel *evsel, const char *config_name,
 			  u64 *val)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 339b5c08a33d..a255ae2b1f8e 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -578,6 +578,7 @@ void evsel__uniquify_counter(struct evsel *counter);
 	((((src) >> (pos)) & ((1ull << (size)) - 1)) << (63 - ((pos) + (size) - 1)))
 
 u64 evsel__bitfield_swap_branch_flags(u64 value);
+bool evsel__config_exists(const struct evsel *evsel, const char *config_name);
 int evsel__get_config_val(const struct evsel *evsel, const char *config_name,
 			  u64 *val);
 void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,

---
base-commit: 4cf1f549bbcdfea9c20df52994bb342677472dcd
change-id: 20260410-james-spe-discard-warning-049b27a53aac

Best regards,
-- 
James Clark <james.clark@linaro.org>



^ permalink raw reply related

* Re: [PATCH v3 0/4] mm: improve large folio readahead and alignment for exec memory
From: Usama Arif @ 2026-04-10 11:03 UTC (permalink / raw)
  To: Andrew Morton, david, willy, ryan.roberts, linux-mm
  Cc: r, jack, ajd, apopple, baohua, baolin.wang, brauner,
	catalin.marinas, dev.jain, kees, kevin.brodsky, lance.yang,
	Liam.Howlett, linux-arm-kernel, linux-fsdevel, linux-kernel,
	Lorenzo Stoakes, mhocko, npache, pasha.tatashin, rmclure, rppt,
	surenb, vbabka, Al Viro, ziy, hannes, kas, shakeel.butt, leitao,
	kernel-team
In-Reply-To: <20260402181326.3107102-1-usama.arif@linux.dev>



On 02/04/2026 19:08, Usama Arif wrote:
> v2 -> v3: https://lore.kernel.org/all/20260320140315.979307-1-usama.arif@linux.dev/
> - Take into account READ_ONLY_THP_FOR_FS for elf alignment by aligning
>   to HPAGE_PMD_SIZE limited to 2M (Rui)
> - Reviewed-by tags for patch 1 from Kiryl and Jan
> - Remove preferred_exec_order() (Jan)
> - Change ra->order to HPAGE_PMD_ORDER if vma_pages(vma) >= HPAGE_PMD_NR
>   otherwise use exec_folio_order() with gfp &= ~__GFP_RECLAIM for
>   do_sync_mmap_readahead().
> - Change exec_folio_order() to return 2M (cont-pte size) for 64K base
>   page size for arm64.
> - remove bprm->file NULL check (Matthew)
> - Change filp to file (Matthew)
> - Improve checking of p_vaddr and p_vaddr (Rui and Matthew)
> 

Hello!

Just wanted to check if there was any feedback/review on the latest
revision? 

Thanks!



^ permalink raw reply

* Re: [PATCH] mm/arm: pgtable: remove young bit check for pte_valid_user
From: Brian Ruley @ 2026-04-10 11:01 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Will Deacon, Steve Capper, linux-arm-kernel, linux-kernel
In-Reply-To: <adfNN33QIOP3VfDm@shell.armlinux.org.uk>

On Apr 09, Russell King (Oracle) wrote:
> 
> On Thu, Apr 09, 2026 at 06:17:36PM +0300, Brian Ruley wrote:
> > However, in the case I describe, if VA_B is mapped immediately to pfn_q
> > after it been has unmapped and freed for VA_A, then it's quite possible
> > that the page is still indexed in the cache.
> 
> True.
> 
> > The hypothesis is that if
> > VA_A and VA_B land in the same I-cache set and VA_A old cache entry
> > still exists (tagged with pfn_q), then the CPU can fetch stale
> > instructions because the tag will match. That's one reason why we need
> > to invalidate the cache, but that will be skipped in the path:
> >
> >     migrate_pages
> >      migrate_pages_batch
> >       migrate_folio_move
> >        remove_migration_ptes
> >         remove_migration_pte
> >          set_pte_at
> >           set_ptes
> >            __sync_icache_dcache  (skipped if !young)
> >             set_pte_ext
> 
> In this case, if the old PTE was marked !young, then the new PTE will
> have:
>         pte = pte_mkold(pte);
> 
> on it, which marks it !young. As you say, __sync_icache_dcache() will
> be skipped. While a PTE entry will be set for the kernel, the code in
> set_pte_ext() will *not* establish a hardware PTE entry. For the
> 2-level pte code:
> 
>         tst     r1, #L_PTE_YOUNG        @ <- results in Z being set
>         tstne   r1, #L_PTE_VALID        @ <- not executed
>         eorne   r1, r1, #L_PTE_NONE     @ <- not executed
>         tstne   r1, #L_PTE_NONE         @ <- not executed
>         moveq   r3, #0                  @ <- hardware PTE value
>  ARM(   str     r3, [r0, #2048]! )      @ <- writes hardware PTE
> 
> So, for a !young PTE, the hardware PTE entry is written as zero,
> which means accesses should fault, which will then cause the PTE to
> be marked young.
> 
> For the 3-level case, the L_PTE_YOUNG bit corresponds with the AF bit
> in the PTE, and there aren't split Linux / hardware PTE entries. AF
> being clear should result in a page fault being generated for the
> kernel to handle making the PTE young.
> 
> In both of these cases, set_ptes() will need to be called with the
> updated PTE which will now be marked young, and that will result in
> the I-cache being flushed.

Hi Russell,

Thank you for the clarification, this is very educational for me.
I understand your scepticism, and I can't explain what's going on based
on what you replied. However, I do honestly believe there is a problem
here. I'll share the exact testing details and the instrumentation
we added that convinced us to reach out at the end. One idea we also
had was that could cache aliasing be happening here.

To clarify any potential misunderstanding, we've observed the
following:

- Sporadic SIGILL and SIGSEGV under memory pressure
- Scales with core count, i.e., quad core more likely to reproduce
  than dual core. We haven't observed an issue on single core.
- Coredumps show valid instructions at the faulting PC.
  The CPU executed something different from what's in memory.
  This pointed us to stale I-cache.
- Instrumentation indicates a correlation.
  A per-CPU ring buffer tracking exec page migrations was dumped on
  SIGILL. The faulting PC matched a recently migrated pages.
- We started seeing this after upgrade 6.1->6.12->6.18. We bisected
  two commits which had an impact, but we weren't convinced that
  either was the root cause: 5dfab109d5193e6c224d96cabf90e9cc2c039884
  and 6faea3422e3b4e8de44a55aa3e6e843320da66d2.
- Failed processes include systemd, tar, bash, ...
- Debug options, e.g., page poisoning, seems to hide the bug


> So you're saying that stress-ng doesn't reproduce this bug but
triggers the OOM-killer... confused.

Apologies for the confusion. I meant that with `stress-ng' we created
the memory pressure and OOM might have played a role in exposing the
"bug" as we (at the time) believed that anything that would trigger
memory free/reclaims and page migration was the key. One note I'll add
is that in our test we invoked stress-ng for 2 minutes (--timeout 2m)
and after each we would reboot the device. We had observed that reboots
seemed to have a discernible effect on the occurence in earlier testing
so we kept that in. I'm beginning to doubt if it had an effect now,
and unfortunately it's all anecdotal.

One more thing, even if you don't accept the patch, is this patch
harmful in any way or is it just sub-optimal?

I'll send the instrumentation patch as a follow-up, migh be there's a
flaw in it.

Best regards,
Brian

###TESTING###

1. stress-ng --vm 4 --vm-bytes 2G --vm-method zero-one --verify \
             --timeout 2m
2. reboot
3. repeat

Cleaned up logs of instrumentation:
```
kernel: [  104.610248] SIGILL at b6e6f1c0 pid 896, recent exec migrations:
kernel: [  104.610313]   cpu0: addr=b6e6f000 old_pfn=467d3 new_pfn=577fe pid=34 flushed=0
[...]
kernel: [  456.066661] SIGILL at b6d99f40 pid 455, recent exec migrations:
[...]
kernel: [  456.066963]   cpu0: addr=b6d99000 old_pfn=44270 new_pfn=7c9ea pid=34 flushed=0
[...] 
```


^ permalink raw reply

* Re: [PATCH v1 1/7] dt-bindings: arm: fsl: Add verdin imx8m[mp] and imx95 zinnia board
From: Krzysztof Kozlowski @ 2026-04-10 10:56 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frank Li,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Shawn Guo,
	Francesco Dolcini, devicetree, linux-kernel, imx,
	linux-arm-kernel
In-Reply-To: <20260409095855.61252-2-francesco@dolcini.it>

On Thu, Apr 09, 2026 at 11:58:47AM +0200, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Add Toradex Verdin Zinnia carrier board mated with Verdin
> iMX8M Plus, Verdin iMX8M Mini and Verdin iMX95.
> 
> Link: https://www.toradex.com/products/carrier-board/zinnia-carrier-board
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof



^ permalink raw reply

* Re: [PATCH 2/3] pwm: rp1: Add RP1 PWM controller driver
From: Uwe Kleine-König @ 2026-04-10 10:47 UTC (permalink / raw)
  To: Andrea della Porta
  Cc: linux-pwm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Broadcom internal kernel review list,
	devicetree, linux-rpi-kernel, linux-arm-kernel, linux-kernel,
	Naushir Patuck, Stanimir Varbanov
In-Reply-To: <adjQl37-6a--_y3Y@apocalypse>

[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]

Hello Andrea,

On Fri, Apr 10, 2026 at 12:27:35PM +0200, Andrea della Porta wrote:
> On 08:27 Fri 10 Apr     , Uwe Kleine-König wrote:
> > On Thu, Apr 09, 2026 at 06:16:41PM +0200, Andrea della Porta wrote:
> > > On 23:45 Sun 05 Apr     , Uwe Kleine-König wrote:
> > > > On Fri, Apr 03, 2026 at 04:31:55PM +0200, Andrea della Porta wrote:
> > > > > +static void rp1_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> > > > > +{
> > > > > +	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > > > > +	u32 value;
> > > > > +
> > > > > +	value = readl(rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> > > > > +	value &= ~PWM_MODE_MASK;
> > > > > +	writel(value, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> > > > > +
> > > > > +	rp1_pwm_apply_config(chip, pwm);
> > > > 
> > > > What is the purpose of this call?
> > > 
> > > To update the configuration on the next PWM strobe in order to avoid
> > > glitches. I'll add a short comment in the code.
> > 
> > .pwm_free() should not touch the hardware configuration. Changing the
> > pinmuxing (which I guess is the purpose of clearing PWM_MODE_MASK) is
> > somewhat a grey area. If that saves energy, that's okish. Otherwise
> > not interfering with the operation of the PWM (e.g. to keep a display on
> > during kexec or so) is preferred.
> 
> Sorry I should've been more clear on this. The pinmux/conf is not changed
> at all by this mask, only the PWM output mode is. The controller can output
> several type of waveforms and clearing PWM_MODE_MASK is just setting the
> controller to output a 0, which is the reset default i.e. the same value
> as just before exporting the channel.
> I guess this is the expected behaviour in case of a fan, it should stop
> spinning in case you unexport the pwm channel, but I see it could be
> different with displays.
> Honestly I don't have a strong opinion about that, please just let me
> know if I should drop that pwm_free entirely.

Yes, in this case drop the function completely. It's the responsibility
of the consumer to stop the PWM before releasing it.

> > > > > +static int rp1_pwm_resume(struct device *dev)
> > > > > +{
> > > > > +	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> > > > > +
> > > > > +	return clk_prepare_enable(rp1->clk);
> > > > 
> > > > Hmm, if this fails and then the driver is unbound, the clk operations
> > > > are not balanced.
> > > 
> > > I'll add some flags to check if the clock is really enabled or not.
> > 
> > To be honest, I guess that is a problem of several drivers, not only in
> > drivers/pwm. If this complicates the driver, I guess addressing this
> > isn't very critical.
> 
> I'll come up with something, we can always drop this check if deemed
> too 'noisy'. 

Great, thanks
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH v2 8/9] pmdomain: renesas: rcar-sysc: Drop GENPD_FLAG_NO_STAY_ON
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Due to the new fine grained sync_state support for onecell genpd provider
drivers, we should no longer need use the legacy behaviour. Therefore,
let's drop GENPD_FLAG_NO_STAY_ON.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/renesas/rcar-sysc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pmdomain/renesas/rcar-sysc.c b/drivers/pmdomain/renesas/rcar-sysc.c
index bd7bb9cbd9da..e4608c657629 100644
--- a/drivers/pmdomain/renesas/rcar-sysc.c
+++ b/drivers/pmdomain/renesas/rcar-sysc.c
@@ -241,7 +241,6 @@ static int __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd)
 		}
 	}
 
-	genpd->flags |= GENPD_FLAG_NO_STAY_ON;
 	genpd->power_off = rcar_sysc_pd_power_off;
 	genpd->power_on = rcar_sysc_pd_power_on;
 
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 6/9] pmdomain: core: Export a common function for ->queue_sync_state()
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Along with of_genpd_sync_state() that genpd provider drivers may use to
manage sync_state, let's add and export of_genpd_queue_sync_state() for
those that may need it. It's expected that the genpd provider driver
assigns it's own ->queue_sync_state() callback and invoke the new helper
from there.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/core.c   | 14 +++++++++-----
 include/linux/pm_domain.h |  2 ++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index f11dc2110737..49e60cb67b3e 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2764,7 +2764,7 @@ static void genpd_parse_for_consumer(struct device_node *sup,
 	}
 }
 
-static void _genpd_queue_sync_state(struct device_node *np)
+static void genpd_queue_sync_state(struct device_node *np)
 {
 	struct generic_pm_domain *genpd;
 
@@ -2782,11 +2782,14 @@ static void _genpd_queue_sync_state(struct device_node *np)
 	mutex_unlock(&gpd_list_lock);
 }
 
-static void genpd_queue_sync_state(struct device *dev)
+void of_genpd_queue_sync_state(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
 	struct device_link *link;
 
+	if (!np)
+		return;
+
 	if (!genpd_should_wait_for_consumer(np))
 		return;
 
@@ -2810,8 +2813,9 @@ static void genpd_queue_sync_state(struct device *dev)
 		genpd_parse_for_consumer(np, consumer->of_node);
 	}
 
-	_genpd_queue_sync_state(np);
+	genpd_queue_sync_state(np);
 }
+EXPORT_SYMBOL_GPL(of_genpd_queue_sync_state);
 
 static void genpd_sync_state(struct device *dev)
 {
@@ -2922,7 +2926,7 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 		sync_state = true;
 	} else if (!dev_has_sync_state(dev)) {
 		dev_set_drv_sync_state(dev, genpd_sync_state);
-		dev_set_drv_queue_sync_state(dev, genpd_queue_sync_state);
+		dev_set_drv_queue_sync_state(dev, of_genpd_queue_sync_state);
 	}
 
 	put_device(dev);
@@ -3654,7 +3658,7 @@ static void genpd_provider_queue_sync_state(struct device *dev)
 	if (genpd->sync_state != GENPD_SYNC_STATE_ONECELL)
 		return;
 
-	genpd_queue_sync_state(dev);
+	of_genpd_queue_sync_state(dev);
 }
 
 static void genpd_provider_sync_state(struct device *dev)
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 7aa49721cde5..d428dd805c46 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -467,6 +467,7 @@ int of_genpd_remove_subdomain(const struct of_phandle_args *parent_spec,
 struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
 int of_genpd_parse_idle_states(struct device_node *dn,
 			       struct genpd_power_state **states, int *n);
+void of_genpd_queue_sync_state(struct device *dev);
 void of_genpd_sync_state(struct device_node *np);
 
 int genpd_dev_pm_attach(struct device *dev);
@@ -513,6 +514,7 @@ static inline int of_genpd_parse_idle_states(struct device_node *dn,
 	return -ENODEV;
 }
 
+static inline void of_genpd_queue_sync_state(struct device *dev) {}
 static inline void of_genpd_sync_state(struct device_node *np) {}
 
 static inline int genpd_dev_pm_attach(struct device *dev)
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 9/9] pmdomain: renesas: rmobile-sysc: Drop GENPD_FLAG_NO_STAY_ON
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Rmobile-sysc is not a onecell provider and didn't really needed
the GENPD_FLAG_NO_STAY_ON flag in the first place. Let's drop it.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/renesas/rmobile-sysc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pmdomain/renesas/rmobile-sysc.c b/drivers/pmdomain/renesas/rmobile-sysc.c
index 93103ff33d6e..e36f5d763c91 100644
--- a/drivers/pmdomain/renesas/rmobile-sysc.c
+++ b/drivers/pmdomain/renesas/rmobile-sysc.c
@@ -100,8 +100,7 @@ static void rmobile_init_pm_domain(struct rmobile_pm_domain *rmobile_pd)
 	struct generic_pm_domain *genpd = &rmobile_pd->genpd;
 	struct dev_power_governor *gov = rmobile_pd->gov;
 
-	genpd->flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP |
-		GENPD_FLAG_NO_STAY_ON;
+	genpd->flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
 	genpd->attach_dev = cpg_mstp_attach_dev;
 	genpd->detach_dev = cpg_mstp_detach_dev;
 
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 7/9] pmdomain: renesas: rcar-gen4-sysc: Drop GENPD_FLAG_NO_STAY_ON
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Due to the new fine grained sync_state support for onecell genpd provider
drivers, we should no longer need use the legacy behaviour. Therefore,
let's drop GENPD_FLAG_NO_STAY_ON.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/renesas/rcar-gen4-sysc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pmdomain/renesas/rcar-gen4-sysc.c b/drivers/pmdomain/renesas/rcar-gen4-sysc.c
index 0c6c639a91d0..81b154da725f 100644
--- a/drivers/pmdomain/renesas/rcar-gen4-sysc.c
+++ b/drivers/pmdomain/renesas/rcar-gen4-sysc.c
@@ -251,7 +251,6 @@ static int __init rcar_gen4_sysc_pd_setup(struct rcar_gen4_sysc_pd *pd)
 		genpd->detach_dev = cpg_mssr_detach_dev;
 	}
 
-	genpd->flags |= GENPD_FLAG_NO_STAY_ON;
 	genpd->power_off = rcar_gen4_sysc_pd_power_off;
 	genpd->power_on = rcar_gen4_sysc_pd_power_on;
 
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 5/9] pmdomain: core: Extend fine grained sync_state to more onecell providers
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

A onecell power domain provider driver that we can assign a common
->sync_state() callback for, should be able to benefit from the improved
fine grained sync_state support in genpd. Therefore, let's also assign the
->queue_sync_state() callback for these types of provider drivers.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 783d6f981708..f11dc2110737 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2918,10 +2918,12 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 
 	fwnode = of_fwnode_handle(np);
 	dev = get_dev_from_fwnode(fwnode);
-	if (!dev)
+	if (!dev) {
 		sync_state = true;
-	else
+	} else if (!dev_has_sync_state(dev)) {
 		dev_set_drv_sync_state(dev, genpd_sync_state);
+		dev_set_drv_queue_sync_state(dev, genpd_queue_sync_state);
+	}
 
 	put_device(dev);
 
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 4/9] pmdomain: core: Add initial fine grained sync_state support
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

A onecell (#power-domain-cells = <1 or 2>; in DT) power domain provider
typically provides multiple independent power domains, each with their own
corresponding consumers. In these cases we have to wait for all consumers
for all the provided power domains before the ->sync_state() callback gets
called for the supplier.

In a first step to improve this, let's implement support for fine grained
sync_state support a per genpd basis by using the ->queue_sync_state()
callback. To take step by step, let's initially limit the improvement to
the internal genpd provider driver and to its corresponding genpd devices
for onecell providers.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/core.c   | 125 ++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h |   1 +
 2 files changed, 126 insertions(+)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index ad57846f02a3..783d6f981708 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2699,6 +2699,120 @@ static struct generic_pm_domain *genpd_get_from_provider(
 	return genpd;
 }
 
+static bool genpd_should_wait_for_consumer(struct device_node *np)
+{
+	struct generic_pm_domain *genpd;
+	bool should_wait = false;
+
+	mutex_lock(&gpd_list_lock);
+	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+		if (genpd->provider == of_fwnode_handle(np)) {
+			genpd_lock(genpd);
+
+			/* Clear the previous state before reevaluating. */
+			genpd->wait_for_consumer = false;
+
+			/*
+			 * Unless there is at least one genpd for the provider
+			 * that is being kept powered-on, we don't have to care
+			 * about waiting for consumers.
+			 */
+			if (genpd->stay_on)
+				should_wait = true;
+
+			genpd_unlock(genpd);
+		}
+	}
+	mutex_unlock(&gpd_list_lock);
+
+	return should_wait;
+}
+
+static void genpd_parse_for_consumer(struct device_node *sup,
+				     struct device_node *con)
+{
+	struct generic_pm_domain *genpd;
+	int i;
+
+	for (i = 0; ; i++) {
+		struct of_phandle_args pd_args;
+
+		if (of_parse_phandle_with_args(con, "power-domains",
+					       "#power-domain-cells",
+					        i, &pd_args))
+			break;
+
+		/*
+		 * The phandle must correspond to the supplier's genpd provider
+		 * to be relevant else let's move to the next index.
+		 */
+		if (sup != pd_args.np) {
+			of_node_put(pd_args.np);
+			continue;
+		}
+
+		mutex_lock(&gpd_list_lock);
+		genpd = genpd_get_from_provider(&pd_args);
+		if (!IS_ERR(genpd)) {
+			genpd_lock(genpd);
+			genpd->wait_for_consumer = true;
+			genpd_unlock(genpd);
+		}
+		mutex_unlock(&gpd_list_lock);
+
+		of_node_put(pd_args.np);
+	}
+}
+
+static void _genpd_queue_sync_state(struct device_node *np)
+{
+	struct generic_pm_domain *genpd;
+
+	mutex_lock(&gpd_list_lock);
+	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+		if (genpd->provider == of_fwnode_handle(np)) {
+			genpd_lock(genpd);
+			if (genpd->stay_on && !genpd->wait_for_consumer) {
+				genpd->stay_on = false;
+				genpd_queue_power_off_work(genpd);
+			}
+			genpd_unlock(genpd);
+		}
+	}
+	mutex_unlock(&gpd_list_lock);
+}
+
+static void genpd_queue_sync_state(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_link *link;
+
+	if (!genpd_should_wait_for_consumer(np))
+		return;
+
+	list_for_each_entry(link, &dev->links.consumers, s_node) {
+		struct device *consumer = link->consumer;
+
+		if (!device_link_test(link, DL_FLAG_MANAGED))
+			continue;
+
+		if (link->status == DL_STATE_ACTIVE)
+			continue;
+
+		if (!consumer->of_node)
+			continue;
+
+		/*
+		 * A consumer device has not been probed yet. Let's parse its
+		 * device node for the power-domains property, to find out the
+		 * genpds it may belong to and then prevent sync state for them.
+		 */
+		genpd_parse_for_consumer(np, consumer->of_node);
+	}
+
+	_genpd_queue_sync_state(np);
+}
+
 static void genpd_sync_state(struct device *dev)
 {
 	return of_genpd_sync_state(dev->of_node);
@@ -3531,6 +3645,16 @@ static int genpd_provider_probe(struct device *dev)
 	return 0;
 }
 
+static void genpd_provider_queue_sync_state(struct device *dev)
+{
+	struct generic_pm_domain *genpd = container_of(dev, struct generic_pm_domain, dev);
+
+	if (genpd->sync_state != GENPD_SYNC_STATE_ONECELL)
+		return;
+
+	genpd_queue_sync_state(dev);
+}
+
 static void genpd_provider_sync_state(struct device *dev)
 {
 	struct generic_pm_domain *genpd = container_of(dev, struct generic_pm_domain, dev);
@@ -3559,6 +3683,7 @@ static struct device_driver genpd_provider_drv = {
 	.name = "genpd_provider",
 	.bus = &genpd_provider_bus_type,
 	.probe = genpd_provider_probe,
+	.queue_sync_state = genpd_provider_queue_sync_state,
 	.sync_state = genpd_provider_sync_state,
 	.suppress_bind_attrs = true,
 };
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index b299dc0128d6..7aa49721cde5 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -215,6 +215,7 @@ struct generic_pm_domain {
 	cpumask_var_t cpus;		/* A cpumask of the attached CPUs */
 	bool synced_poweroff;		/* A consumer needs a synced poweroff */
 	bool stay_on;			/* Stay powered-on during boot. */
+	bool wait_for_consumer;		/* Consumers awaits to be probed. */
 	enum genpd_sync_state sync_state; /* How sync_state is managed. */
 	int (*power_off)(struct generic_pm_domain *domain);
 	int (*power_on)(struct generic_pm_domain *domain);
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 3/9] pmdomain: core: Move genpd_get_from_provider()
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

To prepare for subsequent changes and to avoid an unnecessary function
declaration, let's move genpd_get_from_provider() a bit earlier in the
code.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/pmdomain/core.c | 70 ++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 4d32fc676aaf..ad57846f02a3 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2664,6 +2664,41 @@ static bool genpd_present(const struct generic_pm_domain *genpd)
 	return ret;
 }
 
+/**
+ * genpd_get_from_provider() - Look-up PM domain
+ * @genpdspec: OF phandle args to use for look-up
+ *
+ * Looks for a PM domain provider under the node specified by @genpdspec and if
+ * found, uses xlate function of the provider to map phandle args to a PM
+ * domain.
+ *
+ * Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
+ * on failure.
+ */
+static struct generic_pm_domain *genpd_get_from_provider(
+					const struct of_phandle_args *genpdspec)
+{
+	struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
+	struct of_genpd_provider *provider;
+
+	if (!genpdspec)
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&of_genpd_mutex);
+
+	/* Check if we have such a provider in our array */
+	list_for_each_entry(provider, &of_genpd_providers, link) {
+		if (provider->node == genpdspec->np)
+			genpd = provider->xlate(genpdspec, provider->data);
+		if (!IS_ERR(genpd))
+			break;
+	}
+
+	mutex_unlock(&of_genpd_mutex);
+
+	return genpd;
+}
+
 static void genpd_sync_state(struct device *dev)
 {
 	return of_genpd_sync_state(dev->of_node);
@@ -2889,41 +2924,6 @@ void of_genpd_del_provider(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
-/**
- * genpd_get_from_provider() - Look-up PM domain
- * @genpdspec: OF phandle args to use for look-up
- *
- * Looks for a PM domain provider under the node specified by @genpdspec and if
- * found, uses xlate function of the provider to map phandle args to a PM
- * domain.
- *
- * Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
- * on failure.
- */
-static struct generic_pm_domain *genpd_get_from_provider(
-					const struct of_phandle_args *genpdspec)
-{
-	struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
-	struct of_genpd_provider *provider;
-
-	if (!genpdspec)
-		return ERR_PTR(-EINVAL);
-
-	mutex_lock(&of_genpd_mutex);
-
-	/* Check if we have such a provider in our array */
-	list_for_each_entry(provider, &of_genpd_providers, link) {
-		if (provider->node == genpdspec->np)
-			genpd = provider->xlate(genpdspec, provider->data);
-		if (!IS_ERR(genpd))
-			break;
-	}
-
-	mutex_unlock(&of_genpd_mutex);
-
-	return genpd;
-}
-
 /**
  * of_genpd_add_device() - Add a device to an I/O PM domain
  * @genpdspec: OF phandle args to use for look-up PM domain
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 1/9] driver core: Enable suppliers to implement fine grained sync_state support
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

The common sync_state support isn't fine grained enough for some types of
suppliers, like power domains for example. Especially when a supplier
provides multiple independent power domains, each with their own set of
consumers. In these cases we need to wait for all consumers for all the
provided power domains before invoking the supplier's ->sync_state().

To allow a more fine grained sync_state support to be implemented on per
supplier's driver basis, let's add a new optional callback. As soon as
there is an update worth to consider in regards to managing sync_state for
a supplier device, __device_links_queue_sync_state() invokes the callback.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/core.c           | 7 ++++++-
 include/linux/device/driver.h | 7 +++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 09b98f02f559..4085a011d8ca 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1106,7 +1106,9 @@ int device_links_check_suppliers(struct device *dev)
  * Queues a device for a sync_state() callback when the device links write lock
  * isn't held. This allows the sync_state() execution flow to use device links
  * APIs.  The caller must ensure this function is called with
- * device_links_write_lock() held.
+ * device_links_write_lock() held.  Note, if the optional queue_sync_state()
+ * callback has been assigned too, it gets called for every update to allowing a
+ * more fine grained support to be implemented on per supplier basis.
  *
  * This function does a get_device() to make sure the device is not freed while
  * on this list.
@@ -1126,6 +1128,9 @@ static void __device_links_queue_sync_state(struct device *dev,
 	if (dev->state_synced)
 		return;
 
+	if (dev->driver && dev->driver->queue_sync_state)
+		dev->driver->queue_sync_state(dev);
+
 	list_for_each_entry(link, &dev->links.consumers, s_node) {
 		if (!device_link_test(link, DL_FLAG_MANAGED))
 			continue;
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index bbc67ec513ed..bc9ae1cbe03c 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -68,6 +68,12 @@ enum probe_type {
  *		be called at late_initcall_sync level. If the device has
  *		consumers that are never bound to a driver, this function
  *		will never get called until they do.
+ * @queue_sync_state: Similar to the ->sync_state() callback, but called to
+ *		allow syncing device state to software state in a more fine
+ *		grained way. It is called when there is an updated state that
+ *		may be worth to consider for any of the consumers linked to
+ *		this device. If implemented, the ->sync_state() callback is
+ *		required too.
  * @remove:	Called when the device is removed from the system to
  *		unbind a device from this driver.
  * @shutdown:	Called at shut-down time to quiesce the device.
@@ -110,6 +116,7 @@ struct device_driver {
 
 	int (*probe) (struct device *dev);
 	void (*sync_state)(struct device *dev);
+	void (*queue_sync_state)(struct device *dev);
 	int (*remove) (struct device *dev);
 	void (*shutdown) (struct device *dev);
 	int (*suspend) (struct device *dev, pm_message_t state);
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 2/9] driver core: Add dev_set_drv_queue_sync_state()
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>

Similar to the dev_set_drv_sync_state() helper, let's add another one to
allow subsystem level code to set the ->queue_sync_state() callback for a
driver that has not already set it.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 include/linux/device.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index e65d564f01cd..f812e70bdf22 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -994,6 +994,18 @@ static inline int dev_set_drv_sync_state(struct device *dev,
 	return 0;
 }
 
+static inline int dev_set_drv_queue_sync_state(struct device *dev,
+					       void (*fn)(struct device *dev))
+{
+	if (!dev || !dev->driver)
+		return 0;
+	if (dev->driver->queue_sync_state && dev->driver->queue_sync_state != fn)
+		return -EBUSY;
+	if (!dev->driver->queue_sync_state)
+		dev->driver->queue_sync_state = fn;
+	return 0;
+}
+
 static inline void dev_set_removable(struct device *dev,
 				     enum device_removable removable)
 {
-- 
2.43.0



^ permalink raw reply related

* [PATCH v2 0/9] driver core / pmdomain: Add support for fined grained sync_state
From: Ulf Hansson @ 2026-04-10 10:40 UTC (permalink / raw)
  To: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm
  Cc: Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Geert Uytterhoeven, Dmitry Baryshkov,
	Ulf Hansson, linux-arm-kernel, linux-kernel

Since the introduction [1] of the common sync_state support for pmdomains
(genpd), we have encountered a lot of various interesting problems. In most
cases the new behaviour of genpd triggered some weird platform specific bugs.

That said, in LPC in Tokyo me and Saravana hosted a session to walk through the
remaining limitations that we have found for genpd's sync state support. In
particular, we discussed the problems we have for the so-called onecell power
domain providers, where a single provider typically provides multiple
independent power domains, all with their own set of consumers.

Note that, onecell power domain providers are very common. It's being used by
many SoCs/platforms/technologies. To name a few:
SCMI, Qualcomm, NXP, Mediatek, Renesas, TI, etc.

Anyway, in these cases, the generic sync_state mechanism with fw_devlink isn't
fine grained enough, as we end up waiting for all consumers for all power
domains before the ->sync_callback gets called for the supplier/provider. In
other words, we may end up keeping unused power domains powered-on, for no good
reasons.

The series intends to fix this problem. Please have a look at the commit
messages for more details and help review/test!

Kind regards
Ulf Hansson

[1]
https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/

Ulf Hansson (9):
  driver core: Enable suppliers to implement fine grained sync_state
    support
  driver core: Add dev_set_drv_queue_sync_state()
  pmdomain: core: Move genpd_get_from_provider()
  pmdomain: core: Add initial fine grained sync_state support
  pmdomain: core: Extend fine grained sync_state to more onecell
    providers
  pmdomain: core: Export a common function for ->queue_sync_state()
  pmdomain: renesas: rcar-gen4-sysc: Drop GENPD_FLAG_NO_STAY_ON
  pmdomain: renesas: rcar-sysc: Drop GENPD_FLAG_NO_STAY_ON
  pmdomain: renesas: rmobile-sysc: Drop GENPD_FLAG_NO_STAY_ON

 drivers/base/core.c                       |   7 +-
 drivers/pmdomain/core.c                   | 205 ++++++++++++++++++----
 drivers/pmdomain/renesas/rcar-gen4-sysc.c |   1 -
 drivers/pmdomain/renesas/rcar-sysc.c      |   1 -
 drivers/pmdomain/renesas/rmobile-sysc.c   |   3 +-
 include/linux/device.h                    |  12 ++
 include/linux/device/driver.h             |   7 +
 include/linux/pm_domain.h                 |   3 +
 8 files changed, 197 insertions(+), 42 deletions(-)

-- 
2.43.0



^ permalink raw reply

* Re: [PATCH net-next v3 00/12] net: airoha: Support multiple net_devices connected to the same GDM port
From: Lorenzo Bianconi @ 2026-04-10 10:39 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Christian Marangi,
	Benjamin Larsson, linux-arm-kernel, linux-mediatek, netdev,
	devicetree, Xuegang Lu
In-Reply-To: <20260409195645.16c68979@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

On Apr 09, Jakub Kicinski wrote:
> On Mon, 06 Apr 2026 12:34:05 +0200 Lorenzo Bianconi wrote:
> > EN7581 or AN7583 SoCs support connecting multiple external SerDes (e.g.
> > Ethernet or USB SerDes) to GDM3 or GDM4 ports via a hw arbiter that
> > manages the traffic in a TDM manner. As a result multiple net_devices can
> > connect to the same GDM{3,4} port and there is a theoretical "1:n"
> > relation between GDM ports and net_devices.
> 
> Still waiting for the device tree review. I'm going to blindly send out
> the Sashiko review, please comment if any of it makes sense?

ack, I will do.

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v3 10/12] net: airoha: Do not stop GDM port if it is shared
From: Lorenzo Bianconi @ 2026-04-10 10:35 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
	ansuelsmth, benjamin.larsson, linux-arm-kernel, linux-mediatek,
	netdev, devicetree, Xuegang Lu
In-Reply-To: <20260410025726.1810041-1-kuba@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 4665 bytes --]

> This is an AI-generated review of your patch. The human sending this
> email says "For your consideration."
> ---
> net: airoha: Do not stop GDM port if it is shared
> 
> Theoretically, in the current codebase, two independent net_devices can
> be connected to the same GDM port so we need to check the GDM port is not
> used by any other running net_device before setting the forward
> configuration to FE_PSE_PORT_DROP.
> 
> Tested-by: Xuegang Lu <xuegang.lu@airoha.com>
> 
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > index fd27ba13029bd..5b0cd37b155e4 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> 
> [ ... ]
> 
> > @@ -1688,18 +1693,18 @@ static int airoha_dev_stop(struct net_device *netdev)
> >  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> >  	struct airoha_gdm_port *port = dev->port;
> >  	struct airoha_qdma *qdma = dev->qdma;
> > -	int i, err;
> > +	int i;
> >  
> >  	netif_tx_disable(netdev);
> > -	err = airoha_set_vip_for_gdm_port(dev, false);
> > -	if (err)
> > -		return err;
> > -
> >  	for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++)
> >  		netdev_tx_reset_subqueue(netdev, i);
> >  
> > -	airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
> > -				    FE_PSE_PORT_DROP);
> > +	if (atomic_dec_and_test(&port->users)) {
> > +		airoha_set_vip_for_gdm_port(dev, false);
> > +		airoha_set_gdm_port_fwd_cfg(qdma->eth,
> > +					    REG_GDM_FWD_CFG(port->id),
> > +					    FE_PSE_PORT_DROP);
> > +	}
> >  
> >  	if (atomic_dec_and_test(&qdma->users)) {
> >  		airoha_qdma_clear(qdma, REG_QDMA_GLOBAL_CFG,
> > 				  GLOBAL_CFG_TX_DMA_EN_MASK |
> > 				  GLOBAL_CFG_RX_DMA_EN_MASK);
> > 
> > 		for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++) {
> > 			if (!qdma->q_tx[i].ndesc)
> > 				continue;
> > 
> > 			airoha_qdma_cleanup_tx_queue(&qdma->q_tx[i]);
> > 		}
> > 	}
> 
> This problem wasn't introduced by this patch, but does the QDMA teardown
> path have a DMA corruption vulnerability here?
> 
> When GLOBAL_CFG_TX_DMA_EN_MASK is cleared, it fails to poll
> GLOBAL_CFG_TX_DMA_BUSY_MASK, proceeding to unmap memory while the hardware
> DMA engine may still be processing in-flight descriptors.
> 
> Furthermore, airoha_qdma_cleanup_tx_queue() unmaps the DMA addresses and
> frees the SKBs, but it does not appear to clear the physical hardware
> descriptors or reset the hardware ring pointers (REG_TX_CPU_IDX /
> REG_TX_DMA_IDX).
> 
> When the device is later reopened in airoha_dev_open(),
> GLOBAL_CFG_TX_DMA_EN_MASK is set again without re-initializing the rings.
> Will the hardware DMA engine resume fetching from its previous index,
> processing stale descriptors whose backing memory was already unmapped
> and freed?

ack, I will work on it with a dedicated patch.

> 
> [ ... ]
> 
> > @@ -1851,10 +1856,22 @@ static void airoha_dev_get_stats64(struct net_device *netdev,
> >  static int airoha_dev_change_mtu(struct net_device *netdev, int mtu)
> >  {
> >  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> > +	u32 cur_len, len = ETH_HLEN + mtu + ETH_FCS_LEN;
> >  	struct airoha_gdm_port *port = dev->port;
> > -	u32 len = ETH_HLEN + mtu + ETH_FCS_LEN;
> >  	struct airoha_eth *eth = dev->eth;
> >  
> > +	cur_len = FIELD_GET(GDM_LONG_LEN_MASK,
> > +			    airoha_fe_rr(eth, REG_GDM_LEN_CFG(port->id)));
> > +	if (len < cur_len) {
> > +		u8 port_refcnt = atomic_read(&port->users);
> > +
> > +		/* We can decrease the device MTU just if the GDM port is
> > +		 * not shared or if the other device is not running.
> > +		 */
> > +		if (port_refcnt > 1 || (port_refcnt && !netif_running(netdev)))
> > +			return -EBUSY;
> > +	}
> > +
> 
> Does this MTU decrease validation create a state trap on shared ports?
> 
> If both eth0 and eth1 (sharing a port) are up, and a user increases eth0 MTU
> to 9000, the hardware MTU updates to 9000. If the user then tries to lower
> eth0 MTU back to 1500, len < cur_len evaluates to true. Because eth1 is also
> up (port_refcnt > 1), this returns -EBUSY.
> 
> This would prevent either interface from decreasing its MTU without fully
> bringing down the sibling interface.
> 
> Should the software netdev->mtu be decoupled from the hardware validation,
> allowing valid MTU changes in software while dynamically programming the
> hardware MTU to the maximum of all currently up interfaces on the shared
> port?

It seems more correct to me avoid receiving in the stack packet bigger than the
configured 'SW MTU".

Regards,
Lorenzo


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3] pwm: rp1: Add RP1 PWM controller driver
From: Andrea della Porta @ 2026-04-10 10:27 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Andrea della Porta, linux-pwm, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli,
	Broadcom internal kernel review list, devicetree,
	linux-rpi-kernel, linux-arm-kernel, linux-kernel, Naushir Patuck,
	Stanimir Varbanov
In-Reply-To: <adiW1tBC8Imd14LD@monoceros>

Hi Uwe,

On 08:27 Fri 10 Apr     , Uwe Kleine-König wrote:
> Hello Andrea,
> 
> On Thu, Apr 09, 2026 at 06:16:41PM +0200, Andrea della Porta wrote:
> > On 23:45 Sun 05 Apr     , Uwe Kleine-König wrote:
> > > On Fri, Apr 03, 2026 at 04:31:55PM +0200, Andrea della Porta wrote:
> > > > +static void rp1_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> > > > +{
> > > > +	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > > > +	u32 value;
> > > > +
> > > > +	value = readl(rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> > > > +	value &= ~PWM_MODE_MASK;
> > > > +	writel(value, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> > > > +
> > > > +	rp1_pwm_apply_config(chip, pwm);
> > > 
> > > What is the purpose of this call?
> > 
> > To update the configuration on the next PWM strobe in order to avoid
> > glitches. I'll add a short comment in the code.
> 
> .pwm_free() should not touch the hardware configuration. Changing the
> pinmuxing (which I guess is the purpose of clearing PWM_MODE_MASK) is
> somewhat a grey area. If that saves energy, that's okish. Otherwise
> not interfering with the operation of the PWM (e.g. to keep a display on
> during kexec or so) is preferred.

Sorry I should've been more clear on this. The pinmux/conf is not changed
at all by this mask, only the PWM output mode is. The controller can output
several type of waveforms and clearing PWM_MODE_MASK is just setting the
controller to output a 0, which is the reset default i.e. the same value
as just before exporting the channel.
I guess this is the expected behaviour in case of a fan, it should stop
spinning in case you unexport the pwm channel, but I see it could be
different with displays.
Honestly I don't have a strong opinion about that, please just let me
know if I should drop that pwm_free entirely.

> 
> > > > +static int rp1_pwm_resume(struct device *dev)
> > > > +{
> > > > +	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> > > > +
> > > > +	return clk_prepare_enable(rp1->clk);
> > > 
> > > Hmm, if this fails and then the driver is unbound, the clk operations
> > > are not balanced.
> > 
> > I'll add some flags to check if the clock is really enabled or not.
> 
> To be honest, I guess that is a problem of several drivers, not only in
> drivers/pwm. If this complicates the driver, I guess addressing this
> isn't very critical.

I'll come up with something, we can always drop this check if deemed
too 'noisy'. 

Many thanks,
Andrea

> 
> Best regards
> Uwe




^ permalink raw reply

* Re: [PATCH net-next v3 09/12] net: airoha: Support multiple net_devices for a single FE GDM port
From: Lorenzo Bianconi @ 2026-04-10 10:19 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
	ansuelsmth, benjamin.larsson, linux-arm-kernel, linux-mediatek,
	netdev, devicetree
In-Reply-To: <20260410025724.1810007-1-kuba@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 8640 bytes --]

On Apr 09, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email says "For your consideration."
> ---
> net: airoha: Support multiple net_devices for a single FE GDM port
> 
> This commit introduces support for mapping multiple net_device interfaces
> to a single Frame Engine (FE) GDM port via an external hardware arbiter.
> It updates the rx/tx paths, port allocation, and registration logic to
> handle the new net_device array.
> 
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> [ ... ]
> > @@ -615,9 +617,8 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
> >  		u32 hash, reason, msg1 = le32_to_cpu(desc->msg1);
> >  		struct page *page = virt_to_head_page(e->buf);
> >  		u32 desc_ctrl = le32_to_cpu(desc->ctrl);
> > -		struct airoha_gdm_port *port;
> > -		struct net_device *netdev;
> > -		int data_len, len, p;
> > +		struct airoha_gdm_dev *dev;
> > +		int data_len, len;
> >  
> >  		if (!(desc_ctrl & QDMA_DESC_DONE_MASK))
> >  			break;
> 
> While looking at this code, I noticed an issue that predates this patch.
> Can the lack of a memory barrier after checking the descriptor ownership bit
> cause stale reads?
> 
> The driver checks QDMA_DESC_DONE_MASK to verify that the hardware has finished
> writing the descriptor. However, it reads desc->msg1 before this check and
> without a dma_rmb():
> 
> 		u32 hash, reason, msg1 = le32_to_cpu(desc->msg1);
> 		struct page *page = virt_to_head_page(e->buf);
> 		u32 desc_ctrl = le32_to_cpu(desc->ctrl);
> 
> 		if (!(desc_ctrl & QDMA_DESC_DONE_MASK))
> 			break;
> 
> Since weakly ordered architectures can reorder memory reads, could the CPU
> prefetch msg1 before the DMA engine writes the new descriptor values, leading
> to routing the packet to the wrong gdm_dev?

This issue has been already fixed with the following commit:

commit 4ae0604a0673e11e2075b178387151fcad5111b5
Author: Lorenzo Bianconi <lorenzo@kernel.org>
Date:   Tue Apr 7 08:48:04 2026 +0200

    net: airoha: Add dma_rmb() and READ_ONCE() in airoha_qdma_rx_process()
    
    Add missing dma_rmb() in airoha_qdma_rx_process routine to make sure the
    DMA read operations are completed when the NIC reports the processing on
    the current descriptor is done. Moreover, add missing READ_ONCE() in
    airoha_qdma_rx_process() for DMA descriptor control fields in order to
    avoid any compiler reordering.

> 
> [ ... ]
> > @@ -1764,9 +1765,7 @@ static int airoha_set_gdm2_loopback(struct airoha_gdm_dev *dev)
> >  	airoha_fe_clear(eth, REG_FE_VIP_PORT_EN, BIT(AIROHA_GDM2_IDX));
> >  	airoha_fe_clear(eth, REG_FE_IFC_PORT_EN, BIT(AIROHA_GDM2_IDX));
> >  
> > -	/* XXX: handle XSI_USB_PORT and XSI_PCE1_PORT */
> > -	nbq = port->id == AIROHA_GDM3_IDX && airoha_is_7581(eth) ? 4 : 0;
> > -	src_port = eth->soc->ops.get_src_port_id(port, nbq);
> > +	src_port = eth->soc->ops.get_src_port_id(port, dev->nbq);
> >  	if (src_port < 0)
> >  		return src_port;
> 
> Can configuring the loopback for one device break connectivity for other
> devices sharing the same port?
> 
> When multiple net_devices (like PCIE0 and PCIE1 on GDM3) open and call
> airoha_set_gdm2_loopback(), they modify global hardware registers such as
> REG_FE_WAN_PORT.
> 
> Since WAN0_MASK is unconditionally updated to track the current device's
> src_port:
> 
> 	airoha_fe_rmw(eth, REG_FE_WAN_PORT,
> 		      WAN1_EN_MASK | WAN1_MASK | WAN0_MASK,
> 		      FIELD_PREP(WAN0_MASK, src_port));
> 
> won't the last configured device clobber the hardware state for the others?

This issue is fixed with the following patch in the series:

commit 008995bd8f39c5fdf37a933fa3c962e7ef52e8c6
Author: Lorenzo Bianconi <lorenzo@kernel.org>
Date:   Fri Apr 3 12:07:27 2026 +0200

    net: airoha: Introduce WAN device flag
    
    Introduce WAN flag to specify if a given device is used to transmit/receive
    WAN or LAN traffic. Current codebase supports specifying LAN/WAN device
    configuration in ndo_init() callback during device bootstrap.
    Please note it is possible to specify multiple LAN devices but just a
    single WAN one.

> 
> [ ... ]
> > @@ -1987,7 +1986,8 @@ static netdev_tx_t airoha_dev_xmit(struct sk_buff *skb,
> >  	}
> >  
> >  	fport = airoha_get_fe_port(dev);
> > -	msg1 = FIELD_PREP(QDMA_ETH_TXMSG_FPORT_MASK, fport) |
> > +	msg1 = FIELD_PREP(QDMA_ETH_TXMSG_NBOQ_MASK, dev->nbq) |
> > +	       FIELD_PREP(QDMA_ETH_TXMSG_FPORT_MASK, fport) |
> >  	       FIELD_PREP(QDMA_ETH_TXMSG_METER_MASK, 0x7f);
> >  
> >  	q = &qdma->q_tx[qid];
> 
> Does sharing the hardware QDMA TX queues among multiple net_device instances
> introduce a permanent transmit stall?
> 
> In airoha_dev_xmit(), if a shared hardware queue is full, the driver stops the
> netdev_queue of the specific netdev making the call:
> 
> 	if (q->queued + nr_frags >= q->ndesc) {
> 		/* not enough space in the queue */
> 		netif_tx_stop_queue(txq);
> 		spin_unlock_bh(&q->lock);
> 		return NETDEV_TX_BUSY;
> 	}
> 
> However, the TX completion handler (airoha_tx_irq_process()) only wakes the
> queue of the netdev attached to the completed packet. If an interface's TX
> queue is stopped but it has no pending packets in the hardware queue (because
> another interface filled it), will it ever get woken up?

This issue is orthogonal to this series and it will be fixed with a dedicated
patch.

> 
> [ ... ]
> > @@ -2955,11 +2958,25 @@ static int airoha_alloc_gdm_device(struct airoha_eth *eth,
> >  			 netdev->dev_addr);
> >  	}
> >  
> > +	/* Allowed nbq for EN7581 on GDM3 port are 4 and 5 for PCIE0
> > +	 * and PCIE1 respectively.
> > +	 */
> > +	index = nbq;
> > +	if (index && airoha_is_7581(eth) && port->id == AIROHA_GDM3_IDX)
> > +		index -= 4;
> > +
> > +	if (index >= ARRAY_SIZE(port->devs) || port->devs[index]) {
> > +		dev_err(eth->dev, "invalid nbq id: %d\n", nbq);
> > +		return -EINVAL;
> > +	}
> > +
> > +	netdev->dev.of_node = of_node_get(np);
> >  	dev = netdev_priv(netdev);
> 
> Does this code leak the of_node reference?
> 
> The node reference is explicitly incremented via of_node_get(np). The networking
> core does not automatically drop explicit of_node references on free_netdev().
> Without a matching of_node_put() in the error paths or the driver's teardown
> function (airoha_remove()), will this reference permanently leak when the device
> is destroyed?

ack, I will fix it in v4.

> 
> [ ... ]
> > @@ -3003,7 +3021,43 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth,
> >  	if (err)
> >  		return err;
> >  
> > -	return airoha_alloc_gdm_device(eth, port, np);
> > +	for_each_child_of_node(np, node) {
> > +		/* Multiple external serdes connected to the FE GDM port via an
> > +		 * external arbiter.
> > +		 */
> > +		const __be32 *nbq_ptr;
> > +		int nbq;
> > +
> > +		if (!of_device_is_compatible(node, "airoha,eth-port"))
> > +			continue;
> > +
> > +		d++;
> > +		if (!of_device_is_available(node))
> > +			continue;
> > +
> > +		nbq_ptr = of_get_property(node, "reg", NULL);
> > +		if (!nbq_ptr) {
> > +			dev_err(eth->dev, "missing nbq id\n");
> > +			of_node_put(node);
> > +			return -EINVAL;
> > +		}
> > +
> > +		/* Verify the provided nbq parameter is valid */
> > +		nbq = be32_to_cpup(nbq_ptr);
> > +		err = eth->soc->ops.get_src_port_id(port, nbq);
> > +		if (err < 0) {
> > +			of_node_put(node);
> > +			return err;
> > +		}
> > +
> > +		err = airoha_alloc_gdm_device(eth, port, nbq, node);
> > +		if (err) {
> > +			of_node_put(node);
> > +			return err;
> > +		}
> > +	}
> > +
> > +	return !d ? airoha_alloc_gdm_device(eth, port, 0, np) : 0;
> >  }
> 
> Does passing 0 for nbq in the fallback path break older device trees on the
> EN7581 SoC?
> 
> Later, when the interface is brought up, airoha_set_gdm2_loopback() calls
> airoha_en7581_get_src_port_id(), which returns -EINVAL for GDM3 if nbq is
> not 4 or 5:
> 
> 	switch (port->id) {
> 	case AIROHA_GDM3_IDX:
> 		/* 7581 SoC supports PCIe serdes on GDM3 port */
> 		if (nbq == 4)
> 			return HSGMII_LAN_7581_PCIE0_SRCPORT;
> 		if (nbq == 5)
> 			return HSGMII_LAN_7581_PCIE1_SRCPORT;
> 		break;
> 
> If nbq is hardcoded to 0 for legacy DTs, won't this cause interface
> initialization to fail unconditionally for GDM3?

ack, I will fix it in v4.

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* [PATCH 4/4] arm64: dts: amlogic: t7: Add clk measure support
From: Jian Hu @ 2026-04-10 10:03 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Michael Turquette,
	Martin Blumenstingl, robh+dt, Rob Herring, Krzysztof Kozlowski
  Cc: Jian Hu, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20260410100329.3167482-1-jian.hu@amlogic.com>

Add the clock measure device to the T7 SoC family.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
index 7fe72c94ed62..cec2ea74850d 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
@@ -701,6 +701,11 @@ pwm_ao_cd: pwm@60000 {
 				status = "disabled";
 			};
 
+			clock-measurer@48000 {
+				compatible = "amlogic,t7-clk-measure";
+				reg = <0x0 0x48000 0x0 0x1c>;
+			};
+
 			sd_emmc_a: mmc@88000 {
 				compatible = "amlogic,t7-mmc", "amlogic,meson-axg-mmc";
 				reg = <0x0 0x88000 0x0 0x800>;
-- 
2.47.1



^ 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