From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 70B6A175A7F; Sun, 27 Sep 2026 00:31:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790469082; cv=none; b=og/Qo1PW6C30s1XL+SIy3+WyVaUe/EEbsxbTx1SJr6Re6/LCXk/Yi2ll8SPJH4d0AH9XQpguwXgGw/YRwIHcT5JX/T3Jx8Y6EqQpfEXIqNvppYozNd1KtUjQcoxpBXs2l3Crmnw0tv5+NGq9rsiUNH7Z9ZmGpyzuoStboPHiFDg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790469082; c=relaxed/simple; bh=YORTb5yutXDV5H673UOiPG5EgMUGQfBx5ymK0UccKgo=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=KlcvdOPprjm0xvyUBWAfKNTONJCIaSI/Chk4V8Ifm9gNIZdk1oWIFpABDlJMcMNtbjaM1sy8R/VmBWWZUMJY68r+0WxitgsrBCR9mkZ7VmxZc7RvP9D6UOhlqXcpeff4VR0I/PO6stzulJFO6/Mzrs3qLI5ucPkJWxx8iXIhnH4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Ojuo7u4T; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Ojuo7u4T" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 14C351F000FF; Sun, 27 Sep 2026 00:31:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1790469081; bh=H7qpkZKC54dnRx7tycWTlHDfjrGOf3huD2YCeKG11p0=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=Ojuo7u4TzbCKATVhMD6MPv4gyamm7IVNC1jmwaX7yZA4mzkvcduEvwhgObKJQ+nXZ AZrj3F9ED0BNbxsLI1bpiii+tYJdls7DbzwZLmmI3FQ+PjUPCQX+as2G2Wam96AmL9 XYBNiM4KHI9CKK92XhyKqPR0ycNvh0lLjhc+6FmjO6lz1yW+jDJ2qRyZZBlTzuzKgm F0ahPOn/g9Aj617CooC0L5P8JmEx2EfJifmiVnhyTGt3fdbrIMEsOWN2iPuPPbaiL1 mU6+VBQ4UzQbUvR8aQAwMvP3kTmx+/7Z3O3WFi5mNkPQDC1IKh+EWXYrorqv4D64Fa EDulYk0aEdhuA== From: sashiko-bot@kernel.org Subject: Re: [PATCH v2 3/7] mailbox: sun55i: add KUnit test suite for sun55i msgbox driver Reply-To: sashiko-reviews@lists.linux.dev To: "Tim Michals" Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org, linux-sunxi@lists.linux.dev In-Reply-To: <20260927002021.797069-4-tcmichals@gmail.com> References: <20260922034711.190253-1-tcmichals@gmail.com> <20260927002021.797069-1-tcmichals@gmail.com> <20260927002021.797069-4-tcmichals@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Sun, 27 Sep 2026 00:31:20 +0000 Message-Id: <20260927003121.14C351F000FF@smtp.kernel.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 3 potential issue(= s) to consider: - [Medium] test: MMIO endianness bug in mock registers causes test failures= on BE - [Low] build: missing SUN55I_MSGBOX dependency for KUnit test config - [Low] test: mock bypass causes startup() flush test to silently succeed v= ia failsafe -- commit cd5e63e730e04c965eb8f6c0be2bc107e7e91214 Author: Tim Michals mailbox: sun55i: add KUnit test suite for sun55i msgbox driver Adds a KUnit test suite for the Allwinner sun55i hardware Message Box driver, validating channel routing, mock MMIO message sending, and interrupt handling. > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index 9067f5161276a..af51794d260df 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -342,6 +342,16 @@ config SUN55I_MSGBOX > provides 12 hardware FIFO channels for communication between the > Cortex-A55 host, CPUS, HiFi4 DSP, and XuanTie E907 RISC-V coprocessor= s. > =20 > +config SUN55I_MSGBOX_KUNIT_TEST > + tristate "KUnit tests for Allwinner sun55i msgbox" if !KUNIT_ALL_TESTS > + depends on KUNIT > + default KUNIT_ALL_TESTS [Severity: Low] Should this test configuration also depend on SUN55I_MSGBOX? If the kernel is built with SUN55I_MSGBOX as a module and the test is built-in, or if the driver is disabled completely, could this cause undefined reference linker errors since the test links against symbols exported by the driver? > diff --git a/drivers/mailbox/sun55i_msgbox_test.c b/drivers/mailbox/sun55= i_msgbox_test.c > new file mode 100644 > --- /dev/null > +++ b/drivers/mailbox/sun55i_msgbox_test.c [ ... ] > +static struct mock_msgbox_fixture *create_mock_fixture(struct kunit *tes= t) > +{ > + struct mock_msgbox_fixture *fix; > + int i; [ ... ] > + for (i =3D 0; i < SUN55I_MAX_PROCESSORS; i++) > + fix->mbox.regs[i] =3D (void __iomem *)fix->regs[i]; [Severity: Medium] Will this cause the KUnit tests to fail unconditionally on big-endian architectures? The test fixture uses a host-endian u32 array for mock memory, but the driver uses readl() and writel(). On a big-endian host, the device accessors will perform a byte-swap, resulting in scrambled values when reading from or writing to the mock registers. [ ... ] > +static void mock_rx_cb(struct mbox_client *cl, void *data) > +{ [ ... ] > + /* > + * Emulate hardware FIFO pop behavior: reading a word from the > + * message FIFO decrements MSG_STATUS in real hardware. > + */ > + if (sink->status_reg && (*sink->status_reg & MSG_NUM_MASK) > 0) > + (*sink->status_reg)--; > +} [ ... ] > +static void test_functional_startup_flushes_stale_fifo(struct kunit *tes= t) > +{ > + struct mock_msgbox_fixture *fix =3D create_mock_fixture(test); > + u32 en_idx =3D SUNXI_MSGBOX_READ_IRQ_ENABLE(2) / 4; > + u32 stat_idx =3D SUNXI_MSGBOX_READ_IRQ_STATUS(2) / 4; > + u32 msg_stat_idx =3D SUNXI_MSGBOX_MSG_STATUS(2, 0) / 4; > + u32 fifo_idx =3D SUNXI_MSGBOX_MSG_FIFO(2, 0) / 4; > + int ret; > + > + /* Simulate stale messages present in hardware FIFO before channel open= */ > + fix->regs[0][msg_stat_idx] =3D 4; > + fix->regs[0][fifo_idx] =3D 0xDEADBEEF; > + > + ret =3D sun55i_msgbox_chan_ops.startup(&fix->chans[8]); > + KUNIT_EXPECT_EQ(test, ret, 0); [Severity: Low] Does test_functional_startup_flushes_stale_fifo() pass here solely due to the driver's hardcoded failsafe boundary? Since the readl() calls in the driver's startup() function do not trigger the mock IRQ callback mock_rx_cb() (the only place where the mock MSG_STATUS count is decremented), the status count never drops. The test intends to simulate a 4-message flush, but it appears the driver loop will spin until it hits the SUN55I_FIFO_MAX boundary cap instead. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260927002021.7970= 69-1-tcmichals@gmail.com?part=3D3